feat: add logic to not download layers if already exists localy (#2056)

Author: vjripoll

agent-control/src/package/manager.rs

@@ -6,7 +6,7 @@ use oci_client::Reference;
 use std::path::PathBuf;
 
 /// Information required to reference and install a package
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct PackageData {
     pub id: String, // same type as the packages map on an agent type definition
     pub package_type: PackageType,

agent-control/src/package/oci/package_manager.rs

@@ -225,6 +225,19 @@ where
             &package_data.id,
             &package_data.oci_reference,
         )?;
+
+        if package_path.exists() {
+            debug!(
+                "Package already installed at {}. Skipping download and extraction.",
+                package_path.display()
+            );
+
+            return Ok(InstalledPackageData {
+                id: package_data.id,
+                installation_path: package_path,
+            });
+        }
+
         let temp_package_path = get_temp_package_path(
             &self.remote_dir,
             agent_id,
@@ -372,9 +385,12 @@ mod tests {
         let mut directory_manager = MockDirectoryManager::new();
 
         let agent_id = AgentID::try_from("agent-id").unwrap();
-        let root_dir = PathBuf::from("/tmp/base");
+
+        let temp_dir = tempdir().unwrap();
+        let remote_dir = temp_dir.path().to_path_buf();
+
         let download_dir = get_temp_package_path(
-            &root_dir,
+            &remote_dir,
             &agent_id,
             &TEST_PACKAGE_ID.to_string(),
             &test_reference(),
@@ -396,7 +412,7 @@ mod tests {
         let pm = OCIPackageManager {
             downloader,
             directory_manager,
-            remote_dir: PathBuf::from("/tmp/base"),
+            remote_dir,
         };
         let package_data = PackageData {
             id: TEST_PACKAGE_ID.to_string(),
@@ -414,9 +430,12 @@ mod tests {
         let mut directory_manager = MockDirectoryManager::new();
 
         let agent_id = AgentID::try_from("agent-id").unwrap();
-        let root_dir = PathBuf::from("/tmp/base");
+
+        let temp_dir = tempdir().unwrap();
+        let remote_dir = temp_dir.path().to_path_buf();
+
         let download_dir = get_temp_package_path(
-            &root_dir,
+            &remote_dir,
             &agent_id,
             &TEST_PACKAGE_ID.to_string(),
             &test_reference(),
@@ -448,7 +467,7 @@ mod tests {
         let pm = OCIPackageManager {
             downloader,
             directory_manager,
-            remote_dir: PathBuf::from("/tmp/base"),
+            remote_dir,
         };
         let package_data = PackageData {
             id: TEST_PACKAGE_ID.to_string(),
@@ -466,9 +485,12 @@ mod tests {
         let mut directory_manager = MockDirectoryManager::new();
 
         let agent_id = AgentID::try_from("agent-id").unwrap();
-        let root_dir = PathBuf::from("/tmp/base");
+
+        let temp_dir = tempdir().unwrap();
+        let remote_dir = temp_dir.path().to_path_buf();
+
         let download_dir = get_temp_package_path(
-            &root_dir,
+            &remote_dir,
             &agent_id,
             &TEST_PACKAGE_ID.to_string(),
             &test_reference(),
@@ -496,7 +518,7 @@ mod tests {
         let pm = OCIPackageManager {
             downloader,
             directory_manager,
-            remote_dir: PathBuf::from("/tmp/base"),
+            remote_dir,
         };
         let package_data = PackageData {
             id: TEST_PACKAGE_ID.to_string(),
@@ -517,9 +539,12 @@ mod tests {
         let mut directory_manager = MockDirectoryManager::new();
 
         let agent_id = AgentID::try_from("agent-id").unwrap();
-        let root_dir = PathBuf::from("/tmp/base");
+
+        let temp_dir = tempdir().unwrap();
+        let remote_dir = temp_dir.path().to_path_buf();
+
         let download_dir = get_temp_package_path(
-            &root_dir,
+            &remote_dir,
             &agent_id,
             &TEST_PACKAGE_ID.to_string(),
             &test_reference(),
@@ -547,7 +572,7 @@ mod tests {
         let pm = OCIPackageManager {
             downloader,
             directory_manager,
-            remote_dir: PathBuf::from("/tmp/base"),
+            remote_dir,
         };
         let package_data = PackageData {
             id: TEST_PACKAGE_ID.to_string(),
@@ -568,9 +593,12 @@ mod tests {
         let mut directory_manager = MockDirectoryManager::new();
 
         let agent_id = AgentID::try_from("agent-id").unwrap();
-        let root_dir = PathBuf::from("/tmp/base");
+
+        let temp_dir = tempdir().unwrap();
+        let remote_dir = temp_dir.path().to_path_buf();
+
         let download_dir = get_temp_package_path(
-            &root_dir,
+            &remote_dir,
             &agent_id,
             &TEST_PACKAGE_ID.to_string(),
             &test_reference(),
@@ -579,7 +607,7 @@ mod tests {
 
         let downloaded_file = download_dir.join("layer_digest.tar.gz");
         let install_dir = get_package_path(
-            &root_dir,
+            &remote_dir,
             &agent_id,
             &TEST_PACKAGE_ID.to_string(),
             &test_reference(),
@@ -613,7 +641,7 @@ mod tests {
         let pm = OCIPackageManager {
             downloader,
             directory_manager,
-            remote_dir: PathBuf::from("/tmp/base"),
+            remote_dir,
         };
         let package_data = PackageData {
             id: TEST_PACKAGE_ID.to_string(),
@@ -631,17 +659,20 @@ mod tests {
         let mut directory_manager = MockDirectoryManager::new();
 
         let agent_id = AgentID::try_from("agent-id").unwrap();
-        let root_dir = PathBuf::from("/tmp/base");
+
+        let temp_dir = tempdir().unwrap();
+        let remote_dir = temp_dir.path().to_path_buf();
+
         let download_dir = get_temp_package_path(
-            &root_dir,
+            &remote_dir,
             &agent_id,
             &TEST_PACKAGE_ID.to_string(),
             &test_reference(),
         )
         .unwrap();
 
         let install_dir = get_package_path(
-            &root_dir,
+            &remote_dir,
             &agent_id,
             &TEST_PACKAGE_ID.to_string(),
             &test_reference(),
@@ -687,7 +718,7 @@ mod tests {
         let pm = OCIPackageManager {
             downloader,
             directory_manager,
-            remote_dir: PathBuf::from("/tmp/base"),
+            remote_dir,
         };
         let package_data = PackageData {
             id: TEST_PACKAGE_ID.to_string(),
@@ -741,7 +772,10 @@ mod tests {
             .once()
             .returning(|_| Err(io::Error::other("error deleting directory")));
 
-        let pm = OCIPackageManager::new(downloader, directory_manager, PathBuf::from("/tmp/base"));
+        let temp_dir = tempdir().unwrap();
+
+        let pm =
+            OCIPackageManager::new(downloader, directory_manager, temp_dir.path().to_path_buf());
         let installed_package = InstalledPackageData {
             id: TEST_PACKAGE_ID.to_string(),
             installation_path: package_path,
@@ -750,4 +784,43 @@ mod tests {
 
         assert!(matches!(result, Err(OCIPackageManagerError::Uninstall(_))));
     }
+
+    #[test]
+    fn test_install_skips_download_if_already_installed() {
+        let mut downloader = MockOCIDownloader::new();
+        let agent_id = AgentID::try_from("agent-id").unwrap();
+        let remote_dir = tempdir().unwrap();
+
+        let install_dir = get_package_path(
+            remote_dir.path(),
+            &agent_id,
+            &TEST_PACKAGE_ID.to_string(),
+            &test_reference(),
+        )
+        .unwrap();
+
+        std::fs::create_dir_all(&install_dir).expect("Failed to create dir");
+
+        downloader.expect_download().times(0);
+
+        let pm = OCIPackageManager {
+            downloader,
+            directory_manager: DirectoryManagerFs {},
+            remote_dir: PathBuf::from(remote_dir.path()),
+        };
+
+        let package_data = PackageData {
+            id: TEST_PACKAGE_ID.to_string(),
+            package_type: PackageType::Tar,
+            oci_reference: test_reference(),
+        };
+
+        let result = pm.install(&agent_id, package_data);
+
+        assert!(result.is_ok());
+        let installed = result.unwrap();
+
+        assert_eq!(installed.installation_path, install_dir);
+        assert_eq!(installed.id, TEST_PACKAGE_ID);
+    }
 }

agent-control/tests/on_host/oci_package_management.rs

@@ -59,3 +59,56 @@ fn test_install_and_uninstall_with_oci_registry() {
         .unwrap();
     assert!(!installation_path.exists());
 }
+#[test]
+#[ignore = "needs oci registry, needs elevated privileges on Windows"]
+fn test_install_skips_download_if_exists_with_oci_registry() {
+    let dir = tempdir().unwrap();
+    let content_dir = tempdir().unwrap();
+
+    let payload_file_source = content_dir.path().join("payload.txt");
+    std::fs::write(&payload_file_source, "ORIGINAL_CONTENT").unwrap();
+
+    let file_to_push = dir.path().join("layer_digest.tar.gz");
+
+    TestDataHelper::compress_tar_gz(content_dir.path(), file_to_push.as_path());
+
+    let (_artifact_digest, reference) = push_artifact(&file_to_push, REGISTRY_URL);
+
+    let temp_dir = tempdir().unwrap();
+    let base_path = temp_dir.path().to_path_buf();
+    let package_manager = new_testing_oci_package_manager(base_path.clone());
+
+    let agent_id = AgentID::try_from("test-agent").unwrap();
+    let pkg_id = "test-package-idempotency";
+
+    let package_data = PackageData {
+        id: pkg_id.to_string(),
+        package_type: Tar,
+        oci_reference: reference.clone(),
+    };
+
+    let installed_1 = package_manager
+        .install(&agent_id, package_data.clone())
+        .expect("First install failed");
+
+    let installed_file_path = installed_1.installation_path.join("payload.txt");
+    assert!(
+        installed_file_path.exists(),
+        "Payload file should exist after install"
+    );
+
+    let content_1 = std::fs::read_to_string(&installed_file_path).expect("Failed to read payload");
+    assert_eq!(content_1, "ORIGINAL_CONTENT");
+
+    std::fs::write(&installed_file_path, "MODIFIED_CONTENT_BY_USER").unwrap();
+
+    let result_2 = package_manager.install(&agent_id, package_data);
+    assert!(result_2.is_ok());
+
+    let content_2 = std::fs::read_to_string(&installed_file_path).unwrap();
+
+    assert_eq!(
+        content_2, "MODIFIED_CONTENT_BY_USER",
+        "The package manager overwrote the existing files! It should have skipped download/extraction."
+    );
+}