From 1c448d32b57b2cacb1afe4866b9aece4e0b68cf6 Mon Sep 17 00:00:00 2001
From: Harikrishna Patnala <harikrishna.patnala@gmail.com>
Date: Thu, 18 Dec 2025 15:33:24 +0530
Subject: [PATCH] Allow copy of templates from secondary storages of other zone
 when adding a new secondary storage

---
 .../com/cloud/storage/StorageManager.java     |   5 +-
 .../storage/image/TemplateServiceImpl.java    | 101 +++++++++++++++--
 .../image/TemplateServiceImplTest.java        | 104 ++++++++++++++++++
 .../com/cloud/storage/StorageManagerImpl.java |   2 +-
 4 files changed, 200 insertions(+), 12 deletions(-)

diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java
index de0cb34d63ee..21a0e1f15ddc 100644
--- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java
+++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java
@@ -220,8 +220,9 @@ public interface StorageManager extends StorageService {
             "storage.pool.host.connect.workers", "1",
             "Number of worker threads to be used to connect hosts to a primary storage", true);
 
-    ConfigKey<Boolean> COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES = new ConfigKey<>(Boolean.class, "copy.public.templates.from.other.storages",
-            "Storage", "true", "Allow SSVMs to try copying public templates from one secondary storage to another instead of downloading them from the source.",
+    ConfigKey<Boolean> COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES = new ConfigKey<>(Boolean.class, "copy.templates.from.other.secondary.storages",
+            "Storage", "true", "Allow templates to be copied from existing Secondary Storage servers (within the same zone or across zones) " +
+            "when adding a new Secondary Storage, instead of downloading them from the source URL.",
             true, ConfigKey.Scope.Zone, null);
 
     /**
diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java
index fd723b8bf34f..c2cbd2e46c95 100644
--- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java
+++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java
@@ -31,6 +31,7 @@
 
 import javax.inject.Inject;
 
+import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
 import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
@@ -615,28 +616,110 @@ protected void tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore
     }
 
     protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) {
-        Long zoneId = destStore.getScope().getScopeId();
-        List<DataStore> storesInZone = _storeMgr.getImageStoresByZoneIds(zoneId);
-        for (DataStore sourceStore : storesInZone) {
+        Long destZoneId = destStore.getScope().getScopeId();
+
+        List<DataStore> storesInSameZone = _storeMgr.getImageStoresByZoneIds(destZoneId);
+        if (searchAndCopyWithinZone(tmplt, destStore, storesInSameZone)) {
+            return true;
+        }
+
+        logger.debug("Template [{}] not found in any image store of zone [{}]. Checking other zones",
+                tmplt.getUniqueName(), destZoneId);
+
+        return searchAndCopyAcrossZones(tmplt, destStore, destZoneId);
+    }
+
+    private boolean searchAndCopyAcrossZones(VMTemplateVO tmplt, DataStore destStore, Long destZoneId) {
+        List<Long> allZoneIds = _dcDao.listAllIds();
+        for (Long otherZoneId : allZoneIds) {
+            if (otherZoneId.equals(destZoneId)) {
+                continue;
+            }
+
+            List<DataStore> storesInOtherZone = _storeMgr.getImageStoresByZoneIds(otherZoneId);
+            logger.debug("Checking zone [{}] for template [{}]...", otherZoneId, tmplt.getUniqueName());
+
+            if (storesInOtherZone == null || storesInOtherZone.isEmpty()) {
+                logger.debug("Zone [{}] has no image stores. Skipping.", otherZoneId);
+                continue;
+            }
+
+            DataStore sourceStore = findTemplateInStores(tmplt, storesInOtherZone);
+            if (sourceStore == null) {
+                logger.debug("Template [{}] not found in any image store of zone [{}].",
+                        tmplt.getUniqueName(), otherZoneId);
+                continue;
+            }
+
+            logger.info("Template [{}] found in zone [{}]. Initiating cross-zone copy to zone [{}].",
+                    tmplt.getUniqueName(), otherZoneId, destZoneId);
+
+            return copyTemplateAcrossZones(sourceStore, destStore, tmplt);
+        }
+
+        logger.debug("Template [{}] was not found in any zone. Cannot perform zone-to-zone copy.",
+                tmplt.getUniqueName());
+        return false;
+    }
+
+    private boolean searchAndCopyWithinZone(VMTemplateVO tmplt, DataStore destStore, List<DataStore> stores) {
+        for (DataStore sourceStore : stores) {
             Map<String, TemplateProp> existingTemplatesInSourceStore = listTemplate(sourceStore);
-            if (existingTemplatesInSourceStore == null || !existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) {
-                logger.debug("Template [{}] does not exist on image store [{}]; searching on another one.",
+            if (existingTemplatesInSourceStore == null ||
+                    !existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) {
+                logger.debug("Template [{}] does not exist on image store [{}]; searching another.",
                         tmplt.getUniqueName(), sourceStore.getName());
                 continue;
             }
+
             TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore);
             if (sourceTmpl.getInstallPath() == null) {
-                logger.warn("Can not copy template [{}] from image store [{}], as it returned a null install path.", tmplt.getUniqueName(),
-                        sourceStore.getName());
+                logger.warn("Cannot copy template [{}] from image store [{}]; install path is null.",
+                        tmplt.getUniqueName(), sourceStore.getName());
                 continue;
             }
+
             storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore);
             return true;
         }
-        logger.debug("Can't copy template [{}] from another image store.", tmplt.getUniqueName());
         return false;
     }
 
+    private DataStore findTemplateInStores(VMTemplateVO tmplt, List<DataStore> stores) {
+        for (DataStore store : stores) {
+            Map<String, TemplateProp> templates = listTemplate(store);
+            if (templates != null && templates.containsKey(tmplt.getUniqueName())) {
+                return store;
+            }
+        }
+        return null;
+    }
+
+    private boolean copyTemplateAcrossZones(DataStore sourceStore,
+                                            DataStore destStore,
+                                            VMTemplateVO tmplt) {
+        Long dstZoneId = destStore.getScope().getScopeId();
+        DataCenterVO dstZone = _dcDao.findById(dstZoneId);
+
+        if (dstZone == null) {
+            logger.warn("Destination zone [{}] not found for template [{}]",
+                    dstZoneId, tmplt.getUniqueName());
+            return false;
+        }
+
+        try {
+            Long userId = CallContext.current().getCallingUserId();
+            return _tmpltMgr.copy(userId, tmplt, sourceStore, dstZone);
+        } catch (Exception e) {
+            logger.error("Failed to copy template [{}] from zone [{}] to zone [{}]",
+                    tmplt.getUniqueName(),
+                    sourceStore.getScope().getScopeId(),
+                    dstZoneId,
+                    e);
+            return false;
+        }
+    }
+
     @Override
     public AsyncCallFuture<TemplateApiResult> copyTemplateToImageStore(DataObject source, DataStore destStore) {
         TemplateObject sourceTmpl = (TemplateObject) source;
@@ -681,7 +764,7 @@ protected Void copyTemplateToImageStoreCallback(AsyncCallbackDispatcher<Template
     }
 
     protected boolean isCopyFromOtherStoragesEnabled(Long zoneId) {
-        return StorageManager.COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES.valueIn(zoneId);
+        return StorageManager.COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES.valueIn(zoneId);
     }
 
     protected void publishTemplateCreation(TemplateInfo tmplt) {
diff --git a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java
index 276581e2e482..2f59490ddbb3 100644
--- a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java
+++ b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java
@@ -18,7 +18,12 @@
  */
 package org.apache.cloudstack.storage.image;
 
+import com.cloud.dc.DataCenterVO;
+import com.cloud.dc.dao.DataCenterDao;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.StorageUnavailableException;
 import com.cloud.storage.template.TemplateProp;
+import com.cloud.template.TemplateManager;
 import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -82,6 +87,12 @@ public class TemplateServiceImplTest {
     @Mock
     StorageOrchestrationService storageOrchestrator;
 
+    @Mock
+    DataCenterDao _dcDao;
+
+    @Mock
+    TemplateManager _tmpltMgr;
+
     Map<String, TemplateProp> templatesInSourceStore = new HashMap<>();
 
     @Before
@@ -167,6 +178,11 @@ public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenInstallPathIsNull(
         templatesInSourceStore.put(tmpltMock.getUniqueName(), tmpltPropMock);
         Mockito.doReturn(null).when(templateInfoMock).getInstallPath();
 
+        Scope scopeMock = Mockito.mock(Scope.class);
+        Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
+        Mockito.doReturn(1L).when(scopeMock).getScopeId();
+        Mockito.doReturn(List.of(1L)).when(_dcDao).listAllIds();
+
         boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
 
         Assert.assertFalse(result);
@@ -183,4 +199,92 @@ public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAno
         Assert.assertTrue(result);
         Mockito.verify(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any());
     }
+
+    @Test
+    public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() throws StorageUnavailableException, ResourceAllocationException {
+        Scope scopeMock = Mockito.mock(Scope.class);
+        Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
+        Mockito.doReturn(1L).when(scopeMock).getScopeId();
+        Mockito.doReturn(List.of(sourceStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
+        Mockito.doReturn(null).when(templateService).listTemplate(sourceStoreMock);
+        Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
+
+        DataStore otherZoneStoreMock = Mockito.mock(DataStore.class);
+        Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L);
+
+        Map<String, TemplateProp> templatesInOtherZone = new HashMap<>();
+        templatesInOtherZone.put(tmpltMock.getUniqueName(), tmpltPropMock);
+        Mockito.doReturn(templatesInOtherZone).when(templateService).listTemplate(otherZoneStoreMock);
+
+        DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
+        Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);
+        Mockito.doReturn(true).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.eq(tmpltMock), Mockito.eq(otherZoneStoreMock), Mockito.eq(dstZoneMock));
+
+        boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
+
+        Assert.assertTrue(result);
+    }
+
+    @Test
+    public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenDestinationZoneIsMissing() {
+        Scope scopeMock = Mockito.mock(Scope.class);
+        Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
+        Mockito.doReturn(1L).when(scopeMock).getScopeId();
+        Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
+        Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
+
+        DataStore otherZoneStoreMock = Mockito.mock(DataStore.class);
+        Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L);
+
+        Map<String, TemplateProp> templates = new HashMap<>();
+        templates.put(tmpltMock.getUniqueName(), tmpltPropMock);
+        Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock);
+        Mockito.doReturn(null).when(_dcDao).findById(1L);
+
+        boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
+
+        Assert.assertFalse(result);
+    }
+
+    @Test
+    public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenCrossZoneCopyThrowsException() throws StorageUnavailableException, ResourceAllocationException {
+        Scope scopeMock = Mockito.mock(Scope.class);
+        Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
+        Mockito.doReturn(1L).when(scopeMock).getScopeId();
+        Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
+        Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
+
+        DataStore otherZoneStoreMock = Mockito.mock(DataStore.class);
+        Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L);
+
+        Map<String, TemplateProp> templates = new HashMap<>();
+        templates.put(tmpltMock.getUniqueName(), tmpltPropMock);
+        Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock);
+
+        DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
+        Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);
+        Mockito.doThrow(new RuntimeException("copy failed")).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
+
+        Scope sourceScopeMock = Mockito.mock(Scope.class);
+        Mockito.doReturn(sourceScopeMock).when(otherZoneStoreMock).getScope();
+        Mockito.doReturn(2L).when(sourceScopeMock).getScopeId();
+
+        boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
+
+        Assert.assertFalse(result);
+    }
+
+    @Test
+    public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenTemplateNotFoundInAnyZone() {
+        Scope scopeMock = Mockito.mock(Scope.class);
+        Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
+        Mockito.doReturn(1L).when(scopeMock).getScopeId();
+        Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
+        Mockito.doReturn(List.of(sourceStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(Mockito.anyLong());
+        Mockito.doReturn(null).when(templateService).listTemplate(Mockito.any());
+
+        boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
+
+        Assert.assertFalse(result);
+    }
 }
diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
index 19da1425dc0d..a790f248ea97 100644
--- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
@@ -4195,7 +4195,7 @@ public ConfigKey<?>[] getConfigKeys() {
                 DataStoreDownloadFollowRedirects,
                 AllowVolumeReSizeBeyondAllocation,
                 StoragePoolHostConnectWorkers,
-                COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES
+                COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES
         };
     }