From 988ca9a32ab17fcca59c64e5780a2b2bb0c9b514 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala 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 de0cb34d63e..21a0e1f15dd 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 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 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 1bb954da410..47bcb5ff667 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 java.util.concurrent.ExecutionException; 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 @@ public class TemplateServiceImpl implements TemplateService { } protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) { - Long zoneId = destStore.getScope().getScopeId(); - List storesInZone = _storeMgr.getImageStoresByZoneIds(zoneId); - for (DataStore sourceStore : storesInZone) { + Long destZoneId = destStore.getScope().getScopeId(); + + List 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 allZoneIds = _dcDao.listAllIds(); + for (Long otherZoneId : allZoneIds) { + if (otherZoneId.equals(destZoneId)) { + continue; + } + + List 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 stores) { + for (DataStore sourceStore : stores) { Map 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 stores) { + for (DataStore store : stores) { + Map 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 copyTemplateToImageStore(DataObject source, DataStore destStore) { TemplateObject sourceTmpl = (TemplateObject) source; @@ -681,7 +764,7 @@ public class TemplateServiceImpl implements TemplateService { } 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 276581e2e48..2f59490ddbb 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 templatesInSourceStore = new HashMap<>(); @Before @@ -167,6 +178,11 @@ public class TemplateServiceImplTest { 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 class TemplateServiceImplTest { 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 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 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 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 d23856552ee..ece70ccefc9 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 class StorageManagerImpl extends ManagerBase implements StorageManager, C DataStoreDownloadFollowRedirects, AllowVolumeReSizeBeyondAllocation, StoragePoolHostConnectWorkers, - COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES + COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES }; }