From 988ca9a32ab17fcca59c64e5780a2b2bb0c9b514 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Thu, 18 Dec 2025 15:33:24 +0530 Subject: [PATCH 01/17] 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 }; } From 9a60a9287b8444cb8011ea82a6b36543918613a4 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Mon, 22 Dec 2025 20:11:34 +0530 Subject: [PATCH 02/17] Add API param and UI changes on add secondary storage page --- .../admin/host/AddSecondaryStorageCmd.java | 24 ++++- .../storage/image/TemplateServiceImpl.java | 6 +- .../cloud/storage/ImageStoreDetailsUtil.java | 12 +++ ui/public/locales/en.json | 1 + ui/src/views/infra/AddSecondaryStorage.vue | 87 ++++++++++++++++++- 5 files changed, 121 insertions(+), 9 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java index 9a7eff7e2e5..585fd1b87a8 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java @@ -29,6 +29,11 @@ import org.apache.cloudstack.api.response.ZoneResponse; import com.cloud.exception.DiscoveryException; import com.cloud.storage.ImageStore; import com.cloud.user.Account; +import org.apache.commons.collections.MapUtils; + +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; @APICommand(name = "addSecondaryStorage", description = "Adds secondary storage.", responseObject = ImageStoreResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) @@ -44,6 +49,9 @@ public class AddSecondaryStorageCmd extends BaseCmd { @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, description = "The Zone ID for the secondary storage") protected Long zoneId; + @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "Details in key/value pairs using format details[i].keyname=keyvalue. Example: details[0].copytemplatesfromothersecondarystorages=true") + protected Map details; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -56,6 +64,20 @@ public class AddSecondaryStorageCmd extends BaseCmd { return zoneId; } + public Map getDetails() { + Map detailsMap = new HashMap<>(); + if (MapUtils.isNotEmpty(details)) { + Collection props = details.values(); + for (Object prop : props) { + HashMap detail = (HashMap) prop; + for (Map.Entry entry: detail.entrySet()) { + detailsMap.put(entry.getKey(),entry.getValue()); + } + } + } + return detailsMap; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// @@ -68,7 +90,7 @@ public class AddSecondaryStorageCmd extends BaseCmd { @Override public void execute(){ try{ - ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), null); + ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), getDetails()); ImageStoreResponse storeResponse = null; if (result != null ) { storeResponse = _responseGenerator.createImageStoreResponse(result); 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 47bcb5ff667..5daa256cb04 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 @@ -549,7 +549,7 @@ public class TemplateServiceImpl implements TemplateService { } if (availHypers.contains(tmplt.getHypervisorType())) { - boolean copied = isCopyFromOtherStoragesEnabled(zoneId) && tryCopyingTemplateToImageStore(tmplt, store); + boolean copied = imageStoreDetailsUtil.isCopyTemplatesFromOtherStoragesEnabled(storeId, zoneId) && tryCopyingTemplateToImageStore(tmplt, store); if (!copied) { tryDownloadingTemplateToImageStore(tmplt, store); } @@ -763,10 +763,6 @@ public class TemplateServiceImpl implements TemplateService { return null; } - protected boolean isCopyFromOtherStoragesEnabled(Long zoneId) { - return StorageManager.COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES.valueIn(zoneId); - } - protected void publishTemplateCreation(TemplateInfo tmplt) { VMTemplateVO tmpltVo = _templateDao.findById(tmplt.getId()); diff --git a/server/src/main/java/com/cloud/storage/ImageStoreDetailsUtil.java b/server/src/main/java/com/cloud/storage/ImageStoreDetailsUtil.java index baf5ef8902d..7e0fcc550b4 100755 --- a/server/src/main/java/com/cloud/storage/ImageStoreDetailsUtil.java +++ b/server/src/main/java/com/cloud/storage/ImageStoreDetailsUtil.java @@ -78,4 +78,16 @@ public class ImageStoreDetailsUtil { return getGlobalDefaultNfsVersion(); } + public boolean isCopyTemplatesFromOtherStoragesEnabled(Long storeId, Long zoneId) { + + final Map storeDetails = imageStoreDetailsDao.getDetails(storeId); + final String keyWithoutDots = StorageManager.COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES.key() + .replace(".", ""); + + if (storeDetails != null && storeDetails.containsKey(keyWithoutDots)) { + return Boolean.parseBoolean(storeDetails.get(keyWithoutDots)); + } + + return StorageManager.COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES.valueIn(zoneId); + } } diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 624a13d1e21..efa06c60e3c 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -589,6 +589,7 @@ "label.copy.consoleurl": "Copy console URL to clipboard", "label.copyid": "Copy ID", "label.copy.password": "Copy password", +"label.copy.templates.from.other.secondary.storages": "Copy templates from other storages instead of fetching from URLs", "label.core": "Core", "label.core.zone.type": "Core Zone type", "label.counter": "Counter", diff --git a/ui/src/views/infra/AddSecondaryStorage.vue b/ui/src/views/infra/AddSecondaryStorage.vue index 746af5b959d..fa8cb2194ed 100644 --- a/ui/src/views/infra/AddSecondaryStorage.vue +++ b/ui/src/views/infra/AddSecondaryStorage.vue @@ -48,6 +48,10 @@ +
+ + + +
{{ $t('label.cancel') }} {{ $t('label.ok') }} @@ -191,7 +206,9 @@ export default { providers: ['NFS', 'SMB/CIFS', 'S3', 'Swift'], zones: [], loading: false, - secondaryStorageNFSStaging: false + secondaryStorageNFSStaging: false, + showCopyTemplatesToggle: false, + copyTemplatesTouched: false } }, created () { @@ -203,7 +220,8 @@ export default { this.formRef = ref() this.form = reactive({ provider: 'NFS', - secondaryStorageHttps: true + secondaryStorageHttps: true, + copyTemplatesFromOtherSecondaryStorages: true }) this.rules = reactive({ zone: [{ required: true, message: this.$t('label.required') }], @@ -229,16 +247,57 @@ export default { closeModal () { this.$emit('close-action') }, + fetchCopyTemplatesConfig () { + if (!this.form.zone) { + return + } + + api('listConfigurations', { + name: 'copy.templates.from.other.secondary.storages', + zoneid: this.form.zone + }).then(json => { + const items = + json?.listconfigurationsresponse?.configuration || [] + + items.forEach(item => { + if (item.name === 'copy.templates.from.other.secondary.storages') { + this.form.copyTemplatesFromOtherSecondaryStorages = + item.value === 'true' + } + }) + }) + }, listZones () { api('listZones', { showicon: true }).then(json => { - if (json && json.listzonesresponse && json.listzonesresponse.zone) { + if (json?.listzonesresponse?.zone) { this.zones = json.listzonesresponse.zone + if (this.zones.length > 0) { this.form.zone = this.zones[0].id || '' + this.fetchCopyTemplatesConfig() + this.checkOtherSecondaryStorages() } } }) }, + checkOtherSecondaryStorages () { + api('listImageStores', { + listall: true + }).then(json => { + const stores = json?.listimagestoresresponse?.imagestore || [] + + this.showCopyTemplatesToggle = stores.some(store => { + if (store.providername !== 'NFS') { + return false + } + + return store.zoneid !== this.form.zone || store.zoneid === this.form.zone + }) + }) + }, + onCopyTemplatesToggleChanged (val) { + this.copyTemplatesTouched = true + }, nfsURL (server, path) { var url if (path.substring(0, 1) !== '/') { @@ -362,6 +421,23 @@ export default { nfsParams.url = nfsUrl } + if ( + provider === 'NFS' && + this.showCopyTemplatesToggle && + this.copyTemplatesTouched + ) { + const copyTemplatesKey = 'copytemplatesfromothersecondarystorages' + + const detailIdx = Object.keys(data) + .filter(k => k.startsWith('details[')) + .map(k => parseInt(k.match(/details\[(\d+)\]/)[1])) + .reduce((a, b) => Math.max(a, b), -1) + 1 + + data[`details[${detailIdx}].key`] = copyTemplatesKey + data[`details[${detailIdx}].value`] = + values.copyTemplatesFromOtherSecondaryStorages.toString() + } + this.loading = true try { @@ -402,6 +478,11 @@ export default { reject(error) }) }) + }, + watch: { + 'form.zone' () { + this.copyTemplatesTouched = false + } } } } From 19b6f5206f408695ccb310f5b44f17e294fcfac3 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 26 Dec 2025 14:25:47 +0530 Subject: [PATCH 03/17] Make copy template across zones non blocking --- .../service/StorageOrchestrationService.java | 2 + .../orchestration/StorageOrchestrator.java | 68 +++++++++++++++++++ .../storage/image/TemplateServiceImpl.java | 10 +-- .../image/TemplateServiceImplTest.java | 24 ++++--- 4 files changed, 88 insertions(+), 16 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java index 8be2015bfef..aa8d93d7ae7 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java @@ -32,4 +32,6 @@ public interface StorageOrchestrationService { MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreId, List templateIdList, List snapshotIdList); Future orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore); + + Future orchestrateTemplateCopyAcrossZones(TemplateInfo source, DataStore sourceStore, DataStore destStore); } diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java index 37a1f8dc196..931f358997c 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java @@ -36,7 +36,13 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.template.TemplateManager; import org.apache.cloudstack.api.response.MigrationResponse; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -103,6 +109,13 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra VolumeDataStoreDao volumeDataStoreDao; @Inject DataMigrationUtility migrationHelper; + @Inject + TemplateManager templateManager; + @Inject + VMTemplateDao templateDao; + @Inject + DataCenterDao dcDao; + ConfigKey ImageStoreImbalanceThreshold = new ConfigKey<>("Advanced", Double.class, "image.store.imbalance.threshold", @@ -308,6 +321,14 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra return submit(destStore.getScope().getScopeId(), new CopyTemplateTask(source, destStore)); } + @Override + public Future orchestrateTemplateCopyAcrossZones(TemplateInfo templateInfo, DataStore sourceStore, DataStore destStore) { + Long dstZoneId = destStore.getScope().getScopeId(); + DataCenterVO dstZone = dcDao.findById(dstZoneId); + Long userId = CallContext.current().getCallingUserId(); + return submit(dstZoneId, new CrossZoneCopyTemplateTask(userId, templateInfo, sourceStore, dstZone)); + } + protected Pair migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List files, MigrationPolicy migrationPolicy, int skipped) { String message = ""; boolean success = true; @@ -653,4 +674,51 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra return result; } } + + private class CrossZoneCopyTemplateTask implements Callable { + private final long userId; + private final TemplateInfo sourceTmpl; + private final DataStore sourceStore; + private final DataCenterVO dstZone; + private final String logid; + + CrossZoneCopyTemplateTask(long userId, + TemplateInfo sourceTmpl, + DataStore sourceStore, + DataCenterVO dstZone) { + this.userId = userId; + this.sourceTmpl = sourceTmpl; + this.sourceStore = sourceStore; + this.dstZone = dstZone; + this.logid = ThreadContext.get(LOGCONTEXTID); + } + + @Override + public TemplateApiResult call() { + ThreadContext.put(LOGCONTEXTID, logid); + TemplateApiResult result; + VMTemplateVO template = templateDao.findById(sourceTmpl.getId()); + try { + boolean success = templateManager.copy(userId, template, sourceStore, dstZone); + + result = new TemplateApiResult(sourceTmpl); + if (!success) { + result.setResult("Cross-zone template copy failed"); + } + } catch (Exception e) { + logger.error("Exception while copying template [{}] from zone [{}] to zone [{}]", + template, + sourceStore.getScope().getScopeId(), + dstZone.getId(), + e); + result = new TemplateApiResult(sourceTmpl); + result.setResult(e.getMessage()); + } finally { + tryCleaningUpExecutor(dstZone.getId()); + ThreadContext.clearAll(); + } + + return result; + } + } } 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 5daa256cb04..2708825fcef 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,7 +31,6 @@ 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; @@ -644,7 +643,7 @@ public class TemplateServiceImpl implements TemplateService { continue; } - DataStore sourceStore = findTemplateInStores(tmplt, storesInOtherZone); + DataStore sourceStore = findImageStorageHavingTemplate(tmplt, storesInOtherZone); if (sourceStore == null) { logger.debug("Template [{}] not found in any image store of zone [{}].", tmplt.getUniqueName(), otherZoneId); @@ -685,7 +684,7 @@ public class TemplateServiceImpl implements TemplateService { return false; } - private DataStore findTemplateInStores(VMTemplateVO tmplt, List stores) { + private DataStore findImageStorageHavingTemplate(VMTemplateVO tmplt, List stores) { for (DataStore store : stores) { Map templates = listTemplate(store); if (templates != null && templates.containsKey(tmplt.getUniqueName())) { @@ -707,9 +706,10 @@ public class TemplateServiceImpl implements TemplateService { return false; } + TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore); try { - Long userId = CallContext.current().getCallingUserId(); - return _tmpltMgr.copy(userId, tmplt, sourceStore, dstZone); + storageOrchestrator.orchestrateTemplateCopyAcrossZones(sourceTmpl, sourceStore, destStore); + return true; } catch (Exception e) { logger.error("Failed to copy template [{}] from zone [{}] to zone [{}]", tmplt.getUniqueName(), 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 2f59490ddbb..272b51a1b6b 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 @@ -218,7 +218,6 @@ public class TemplateServiceImplTest { 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); @@ -247,31 +246,34 @@ public class TemplateServiceImplTest { } @Test - public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenCrossZoneCopyThrowsException() throws StorageUnavailableException, ResourceAllocationException { + public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenCrossZoneCopyTaskIsScheduled() { 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); + + Mockito.doReturn(List.of()) + .when(dataStoreManagerMock) + .getImageStoresByZoneIds(1L); DataStore otherZoneStoreMock = Mockito.mock(DataStore.class); - Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L); + 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(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); + Assert.assertTrue(result); } @Test From 1e0feca4be01e3ee1ecce4e15e16aa8a9f8d2e69 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 26 Dec 2025 14:46:42 +0530 Subject: [PATCH 04/17] Code fixes --- .../orchestration/StorageOrchestrator.java | 2 +- .../storage/image/TemplateServiceImpl.java | 13 ++++++-- .../image/TemplateServiceImplTest.java | 31 ++++++++++++------- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java index 931f358997c..d2a5dd88848 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java @@ -325,7 +325,7 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra public Future orchestrateTemplateCopyAcrossZones(TemplateInfo templateInfo, DataStore sourceStore, DataStore destStore) { Long dstZoneId = destStore.getScope().getScopeId(); DataCenterVO dstZone = dcDao.findById(dstZoneId); - Long userId = CallContext.current().getCallingUserId(); + long userId = CallContext.current().getCallingUserId(); return submit(dstZoneId, new CrossZoneCopyTemplateTask(userId, templateInfo, sourceStore, dstZone)); } 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 2708825fcef..00b7bef9a5c 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 @@ -622,7 +622,7 @@ public class TemplateServiceImpl implements TemplateService { return true; } - logger.debug("Template [{}] not found in any image store of zone [{}]. Checking other zones", + logger.debug("Template [{}] not found in any image store of zone [{}]. Checking other zones.", tmplt.getUniqueName(), destZoneId); return searchAndCopyAcrossZones(tmplt, destStore, destZoneId); @@ -650,6 +650,13 @@ public class TemplateServiceImpl implements TemplateService { continue; } + TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore); + if (sourceTmpl.getInstallPath() == null) { + logger.warn("Cannot copy template [{}] from image store [{}]; install path is null.", + tmplt.getUniqueName(), sourceStore.getName()); + continue; + } + logger.info("Template [{}] found in zone [{}]. Initiating cross-zone copy to zone [{}].", tmplt.getUniqueName(), otherZoneId, destZoneId); @@ -701,7 +708,7 @@ public class TemplateServiceImpl implements TemplateService { DataCenterVO dstZone = _dcDao.findById(dstZoneId); if (dstZone == null) { - logger.warn("Destination zone [{}] not found for template [{}]", + logger.warn("Destination zone [{}] not found for template [{}].", dstZoneId, tmplt.getUniqueName()); return false; } @@ -711,7 +718,7 @@ public class TemplateServiceImpl implements TemplateService { storageOrchestrator.orchestrateTemplateCopyAcrossZones(sourceTmpl, sourceStore, destStore); return true; } catch (Exception e) { - logger.error("Failed to copy template [{}] from zone [{}] to zone [{}]", + logger.error("Failed to copy template [{}] from zone [{}] to zone [{}].", tmplt.getUniqueName(), sourceStore.getScope().getScopeId(), dstZoneId, 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 272b51a1b6b..b6ed9e805ed 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 @@ -201,10 +201,11 @@ public class TemplateServiceImplTest { } @Test - public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() throws StorageUnavailableException, ResourceAllocationException { + public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() { Scope scopeMock = Mockito.mock(Scope.class); Mockito.doReturn(scopeMock).when(destStoreMock).getScope(); Mockito.doReturn(1L).when(scopeMock).getScopeId(); + Mockito.doReturn(100L).when(tmpltMock).getId(); 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(); @@ -216,6 +217,10 @@ public class TemplateServiceImplTest { templatesInOtherZone.put(tmpltMock.getUniqueName(), tmpltPropMock); Mockito.doReturn(templatesInOtherZone).when(templateService).listTemplate(otherZoneStoreMock); + TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); + Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); + Mockito.doReturn("/mnt/secondary/template.qcow2").when(sourceTmplMock).getInstallPath(); + DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class); Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L); @@ -229,6 +234,7 @@ public class TemplateServiceImplTest { Scope scopeMock = Mockito.mock(Scope.class); Mockito.doReturn(scopeMock).when(destStoreMock).getScope(); Mockito.doReturn(1L).when(scopeMock).getScopeId(); + Mockito.doReturn(100L).when(tmpltMock).getId(); Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds(); Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L); @@ -238,6 +244,10 @@ public class TemplateServiceImplTest { Map templates = new HashMap<>(); templates.put(tmpltMock.getUniqueName(), tmpltPropMock); Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock); + + TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); + Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); + Mockito.doReturn("/mnt/secondary/template.qcow2").when(sourceTmplMock).getInstallPath(); Mockito.doReturn(null).when(_dcDao).findById(1L); boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); @@ -250,23 +260,20 @@ public class TemplateServiceImplTest { Scope scopeMock = Mockito.mock(Scope.class); Mockito.doReturn(scopeMock).when(destStoreMock).getScope(); Mockito.doReturn(1L).when(scopeMock).getScopeId(); - + Mockito.doReturn(100L).when(tmpltMock).getId(); Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds(); - - Mockito.doReturn(List.of()) - .when(dataStoreManagerMock) - .getImageStoresByZoneIds(1L); + Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L); DataStore otherZoneStoreMock = Mockito.mock(DataStore.class); - Mockito.doReturn(List.of(otherZoneStoreMock)) - .when(dataStoreManagerMock) - .getImageStoresByZoneIds(2L); + 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(templates).when(templateService).listTemplate(otherZoneStoreMock); + + TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); + Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); + Mockito.doReturn("/mnt/secondary/template.qcow2").when(sourceTmplMock).getInstallPath(); DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class); Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L); From 8edf96fab833a0e2224ddfbdc53dac7d183adab9 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 26 Dec 2025 16:56:42 +0530 Subject: [PATCH 05/17] unused imports --- .../cloudstack/storage/image/TemplateServiceImplTest.java | 2 -- 1 file changed, 2 deletions(-) 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 b6ed9e805ed..ed370fde701 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 @@ -20,8 +20,6 @@ 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; From 555e6b36cab54ac20d625c5b471178015dd7d681 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Mon, 29 Dec 2025 14:44:01 +0530 Subject: [PATCH 06/17] Add copy template flag in zone wizard and remove NFS checks --- ui/public/locales/en.json | 3 ++- ui/src/views/infra/AddSecondaryStorage.vue | 23 ++++------------- .../infra/zone/ZoneWizardAddResources.vue | 25 ++++++++++++++++++- .../views/infra/zone/ZoneWizardLaunchZone.vue | 5 ++++ 4 files changed, 36 insertions(+), 20 deletions(-) diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index efa06c60e3c..aad2f6c7d43 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -590,6 +590,7 @@ "label.copyid": "Copy ID", "label.copy.password": "Copy password", "label.copy.templates.from.other.secondary.storages": "Copy templates from other storages instead of fetching from URLs", +"label.copy.templates.from.other.secondary.storages.add.zone": "Copy templates from other storages", "label.core": "Core", "label.core.zone.type": "Core Zone type", "label.counter": "Counter", @@ -3017,7 +3018,7 @@ "message.desc.importmigratefromvmwarewizard": "By selecting an existing or external VMware Datacenter and an instance to import, CloudStack migrates the selected instance from VMware to KVM on a conversion host using virt-v2v and imports it into a KVM Cluster", "message.desc.primary.storage": "Each Cluster must contain one or more primary storage servers. We will add the first one now. Primary storage contains the disk volumes for all the Instances running on hosts in the cluster. Use any standards-compliant protocol that is supported by the underlying hypervisor.", "message.desc.reset.ssh.key.pair": "Please specify a ssh key pair that you would like to add to this Instance.", -"message.desc.secondary.storage": "Each Zone must have at least one NFS or secondary storage server. We will add the first one now. Secondary storage stores Instance Templates, ISO images, and Instance disk volume Snapshots. This server must be available to all hosts in the zone.

Provide the IP address and exported path.", +"message.desc.secondary.storage": "Each Zone must have at least one NFS or secondary storage server. We will add the first one now. Secondary storage stores Instance Templates, ISO images, and Instance disk volume Snapshots. This server must be available to all hosts in the zone.

Provide the IP address and exported path.

\"Copy templates from other secondary storages\" checkbox can be used to automatically copy existing templates from secondary storages in other zones instead of fetching from their URLs.", "message.desc.register.user.data": "Please fill in the following to register new User Data.", "message.desc.registered.user.data": "Registered a User Data.", "message.desc.zone": "A Zone is the largest organizational unit in CloudStack, and it typically corresponds to a single datacenter. Zones provide physical isolation and redundancy. A zone consists of one or more Pods (each of which contains hosts and primary storage servers) and a secondary storage server which is shared by all pods in the zone.", diff --git a/ui/src/views/infra/AddSecondaryStorage.vue b/ui/src/views/infra/AddSecondaryStorage.vue index fa8cb2194ed..fcfb35c9d27 100644 --- a/ui/src/views/infra/AddSecondaryStorage.vue +++ b/ui/src/views/infra/AddSecondaryStorage.vue @@ -48,10 +48,6 @@
-
+
0) { this.form.zone = this.zones[0].id || '' - this.fetchCopyTemplatesConfig() - this.checkOtherSecondaryStorages() } } }) }, checkOtherSecondaryStorages () { - api('listImageStores', { - listall: true - }).then(json => { + api('listImageStores', { listall: true }).then(json => { const stores = json?.listimagestoresresponse?.imagestore || [] - this.showCopyTemplatesToggle = stores.some(store => { - if (store.providername !== 'NFS') { - return false - } - - return store.zoneid !== this.form.zone || store.zoneid === this.form.zone - }) + this.showCopyTemplatesToggle = stores.length > 0 }) }, onCopyTemplatesToggleChanged (val) { @@ -422,7 +410,6 @@ export default { } if ( - provider === 'NFS' && this.showCopyTemplatesToggle && this.copyTemplatesTouched ) { diff --git a/ui/src/views/infra/zone/ZoneWizardAddResources.vue b/ui/src/views/infra/zone/ZoneWizardAddResources.vue index 4bd602f0aca..24c0238bd2e 100644 --- a/ui/src/views/infra/zone/ZoneWizardAddResources.vue +++ b/ui/src/views/infra/zone/ZoneWizardAddResources.vue @@ -840,6 +840,13 @@ export default { display: { secondaryStorageProvider: ['Swift'] } + }, + { + title: 'label.copy.templates.from.other.secondary.storages.add.zone', + key: 'copyTemplatesFromOtherSecondaryStorages', + required: false, + switch: true, + checked: this.copytemplate, } ] } @@ -860,7 +867,8 @@ export default { }], storageProviders: [], currentStep: null, - options: ['primaryStorageScope', 'primaryStorageProtocol', 'provider', 'primaryStorageProvider'] + options: ['primaryStorageScope', 'primaryStorageProtocol', 'provider', 'primaryStorageProvider'], + copytemplate: true } }, created () { @@ -885,6 +893,7 @@ export default { primaryStorageScope: null }) } + this.applyCopyTemplatesOptionFromGlobalSettingDuringSecondaryStorageAddition() } }, watch: { @@ -1108,6 +1117,20 @@ export default { this.storageProviders = storageProviders }) }, + applyCopyTemplatesOptionFromGlobalSettingDuringSecondaryStorageAddition () { + api('listConfigurations', { + name: 'copy.templates.from.other.secondary.storages' + }).then(json => { + const config = json?.listconfigurationsresponse?.configuration?.[0] + + if (!config || config.value === undefined) { + return + } + + const value = String(config.value).toLowerCase() === 'true' + this.copytemplate = value + }) + }, fetchPrimaryStorageProvider () { this.primaryStorageProviders = [] api('listStorageProviders', { type: 'primary' }).then(json => { diff --git a/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue b/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue index a787ad839cd..fbf5e6f5c20 100644 --- a/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue +++ b/ui/src/views/infra/zone/ZoneWizardLaunchZone.vue @@ -1580,6 +1580,11 @@ export default { params.provider = this.prefillContent.secondaryStorageProvider params.zoneid = this.stepData.zoneReturned.id params.url = url + if (this.prefillContent.copyTemplatesFromOtherSecondaryStorages !== undefined) { + params['details[0].key'] = 'copytemplatesfromothersecondarystorages' + params['details[0].value'] = + this.prefillContent.copyTemplatesFromOtherSecondaryStorages + } } else if (this.prefillContent.secondaryStorageProvider === 'SMB') { const nfsServer = this.prefillContent.secondaryStorageServer const path = this.prefillContent.secondaryStoragePath From b7c67d04333aabe7ce8da2a3f8f8240cdfe42192 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Mon, 29 Dec 2025 15:02:19 +0530 Subject: [PATCH 07/17] Fix UI --- ui/public/locales/en.json | 2 +- ui/src/views/infra/zone/ZoneWizardAddResources.vue | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index aad2f6c7d43..6c29ec65cbe 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -3018,7 +3018,7 @@ "message.desc.importmigratefromvmwarewizard": "By selecting an existing or external VMware Datacenter and an instance to import, CloudStack migrates the selected instance from VMware to KVM on a conversion host using virt-v2v and imports it into a KVM Cluster", "message.desc.primary.storage": "Each Cluster must contain one or more primary storage servers. We will add the first one now. Primary storage contains the disk volumes for all the Instances running on hosts in the cluster. Use any standards-compliant protocol that is supported by the underlying hypervisor.", "message.desc.reset.ssh.key.pair": "Please specify a ssh key pair that you would like to add to this Instance.", -"message.desc.secondary.storage": "Each Zone must have at least one NFS or secondary storage server. We will add the first one now. Secondary storage stores Instance Templates, ISO images, and Instance disk volume Snapshots. This server must be available to all hosts in the zone.

Provide the IP address and exported path.

\"Copy templates from other secondary storages\" checkbox can be used to automatically copy existing templates from secondary storages in other zones instead of fetching from their URLs.", +"message.desc.secondary.storage": "Each Zone must have at least one NFS or secondary storage server. We will add the first one now. Secondary storage stores Instance Templates, ISO images, and Instance disk volume Snapshots. This server must be available to all hosts in the zone.

Provide the IP address and exported path.

\"Copy templates from other secondary storages\" switch can be used to automatically copy existing templates from secondary storages in other zones instead of fetching from their URLs.", "message.desc.register.user.data": "Please fill in the following to register new User Data.", "message.desc.registered.user.data": "Registered a User Data.", "message.desc.zone": "A Zone is the largest organizational unit in CloudStack, and it typically corresponds to a single datacenter. Zones provide physical isolation and redundancy. A zone consists of one or more Pods (each of which contains hosts and primary storage servers) and a secondary storage server which is shared by all pods in the zone.", diff --git a/ui/src/views/infra/zone/ZoneWizardAddResources.vue b/ui/src/views/infra/zone/ZoneWizardAddResources.vue index 24c0238bd2e..298cc7fec9d 100644 --- a/ui/src/views/infra/zone/ZoneWizardAddResources.vue +++ b/ui/src/views/infra/zone/ZoneWizardAddResources.vue @@ -846,7 +846,7 @@ export default { key: 'copyTemplatesFromOtherSecondaryStorages', required: false, switch: true, - checked: this.copytemplate, + checked: this.copytemplate } ] } From a3d1c9464f23394c30bc84930c26efc6e82dc66e Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Mon, 29 Dec 2025 15:18:33 +0530 Subject: [PATCH 08/17] Label fixes --- ui/public/locales/en.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 6c29ec65cbe..f995d1b6332 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -589,8 +589,8 @@ "label.copy.consoleurl": "Copy console URL to clipboard", "label.copyid": "Copy ID", "label.copy.password": "Copy password", -"label.copy.templates.from.other.secondary.storages": "Copy templates from other storages instead of fetching from URLs", -"label.copy.templates.from.other.secondary.storages.add.zone": "Copy templates from other storages", +"label.copy.templates.from.other.secondary.storages": "Copy Templates from other storages instead of fetching from URLs", +"label.copy.templates.from.other.secondary.storages.add.zone": "Copy Templates from other storages", "label.core": "Core", "label.core.zone.type": "Core Zone type", "label.counter": "Counter", From 4928d9bd15d987290fff1eb97ca4d2fd65135b99 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 30 Dec 2025 11:25:20 +0530 Subject: [PATCH 09/17] code optimizations --- .../service/StorageOrchestrationService.java | 2 +- .../orchestration/StorageOrchestrator.java | 18 ++--- .../storage/image/TemplateServiceImpl.java | 77 +++++++++---------- .../image/TemplateServiceImplTest.java | 30 +++++++- .../cloud/storage/ImageStoreDetailsUtil.java | 1 - 5 files changed, 71 insertions(+), 57 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java index aa8d93d7ae7..018f5c7b61e 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java @@ -33,5 +33,5 @@ public interface StorageOrchestrationService { Future orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore); - Future orchestrateTemplateCopyAcrossZones(TemplateInfo source, DataStore sourceStore, DataStore destStore); + Future orchestrateTemplateCopyAcrossZones(TemplateInfo source, DataStore destStore); } diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java index d2a5dd88848..26990cecb49 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java @@ -38,6 +38,8 @@ import javax.naming.ConfigurationException; 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.VMTemplateVO; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.template.TemplateManager; @@ -322,11 +324,11 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra } @Override - public Future orchestrateTemplateCopyAcrossZones(TemplateInfo templateInfo, DataStore sourceStore, DataStore destStore) { + public Future orchestrateTemplateCopyAcrossZones(TemplateInfo templateInfo, DataStore destStore) { Long dstZoneId = destStore.getScope().getScopeId(); DataCenterVO dstZone = dcDao.findById(dstZoneId); long userId = CallContext.current().getCallingUserId(); - return submit(dstZoneId, new CrossZoneCopyTemplateTask(userId, templateInfo, sourceStore, dstZone)); + return submit(dstZoneId, new CrossZoneCopyTemplateTask(userId, templateInfo, dstZone)); } protected Pair migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List files, MigrationPolicy migrationPolicy, int skipped) { @@ -678,17 +680,12 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra private class CrossZoneCopyTemplateTask implements Callable { private final long userId; private final TemplateInfo sourceTmpl; - private final DataStore sourceStore; private final DataCenterVO dstZone; private final String logid; - CrossZoneCopyTemplateTask(long userId, - TemplateInfo sourceTmpl, - DataStore sourceStore, - DataCenterVO dstZone) { + CrossZoneCopyTemplateTask(long userId, TemplateInfo sourceTmpl, DataCenterVO dstZone) { this.userId = userId; this.sourceTmpl = sourceTmpl; - this.sourceStore = sourceStore; this.dstZone = dstZone; this.logid = ThreadContext.get(LOGCONTEXTID); } @@ -699,16 +696,17 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra TemplateApiResult result; VMTemplateVO template = templateDao.findById(sourceTmpl.getId()); try { + DataStore sourceStore = sourceTmpl.getDataStore(); boolean success = templateManager.copy(userId, template, sourceStore, dstZone); result = new TemplateApiResult(sourceTmpl); if (!success) { result.setResult("Cross-zone template copy failed"); } - } catch (Exception e) { + } catch (StorageUnavailableException | ResourceAllocationException e) { logger.error("Exception while copying template [{}] from zone [{}] to zone [{}]", template, - sourceStore.getScope().getScopeId(), + sourceTmpl.getDataStore().getScope().getScopeId(), dstZone.getId(), e); result = new TemplateApiResult(sourceTmpl); 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 00b7bef9a5c..d32a5240e20 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 @@ -615,13 +615,11 @@ public class TemplateServiceImpl implements TemplateService { } protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) { - Long destZoneId = destStore.getScope().getScopeId(); - - List storesInSameZone = _storeMgr.getImageStoresByZoneIds(destZoneId); - if (searchAndCopyWithinZone(tmplt, destStore, storesInSameZone)) { + if (searchAndCopyWithinZone(tmplt, destStore)) { return true; } + Long destZoneId = destStore.getScope().getScopeId(); logger.debug("Template [{}] not found in any image store of zone [{}]. Checking other zones.", tmplt.getUniqueName(), destZoneId); @@ -643,45 +641,54 @@ public class TemplateServiceImpl implements TemplateService { continue; } - DataStore sourceStore = findImageStorageHavingTemplate(tmplt, storesInOtherZone); - if (sourceStore == null) { - logger.debug("Template [{}] not found in any image store of zone [{}].", + TemplateObject sourceTmpl = findUsableTemplate(tmplt, storesInOtherZone); + if (sourceTmpl == null) { + logger.debug("Template [{}] not found with a valid install path in any image store of zone [{}].", tmplt.getUniqueName(), otherZoneId); continue; } - TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore); - if (sourceTmpl.getInstallPath() == null) { - logger.warn("Cannot copy template [{}] from image store [{}]; install path is null.", - tmplt.getUniqueName(), sourceStore.getName()); - continue; - } - logger.info("Template [{}] found in zone [{}]. Initiating cross-zone copy to zone [{}].", tmplt.getUniqueName(), otherZoneId, destZoneId); - return copyTemplateAcrossZones(sourceStore, destStore, tmplt); + return copyTemplateAcrossZones(destStore, sourceTmpl); } - logger.debug("Template [{}] was not found in any zone. Cannot perform zone-to-zone copy.", - tmplt.getUniqueName()); + 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) { + protected TemplateObject findUsableTemplate(VMTemplateVO tmplt, List imageStores) { + for (DataStore store : imageStores) { + TemplateObject tmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), store); + if (tmpl == null) { + continue; + } + + if (tmpl.getInstallPath() == null) { + logger.debug("Template [{}] found in image store [{}] but install path is null. Skipping.", + tmplt.getUniqueName(), store.getName()); + continue; + } + return tmpl; + } + return null; + } + + private boolean searchAndCopyWithinZone(VMTemplateVO tmplt, DataStore destStore) { + Long destZoneId = destStore.getScope().getScopeId(); + List storesInSameZone = _storeMgr.getImageStoresByZoneIds(destZoneId); + for (DataStore sourceStore : storesInSameZone) { Map existingTemplatesInSourceStore = listTemplate(sourceStore); if (existingTemplatesInSourceStore == null || !existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) { - logger.debug("Template [{}] does not exist on image store [{}]; searching another.", - tmplt.getUniqueName(), sourceStore.getName()); + 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("Cannot copy template [{}] from image store [{}]; install path is null.", - tmplt.getUniqueName(), sourceStore.getName()); + logger.warn("Cannot copy template [{}] from image store [{}]; install path is null.", tmplt.getUniqueName(), sourceStore.getName()); continue; } @@ -691,36 +698,22 @@ public class TemplateServiceImpl implements TemplateService { return false; } - private DataStore findImageStorageHavingTemplate(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) { + private boolean copyTemplateAcrossZones(DataStore destStore, TemplateObject sourceTmpl) { Long dstZoneId = destStore.getScope().getScopeId(); DataCenterVO dstZone = _dcDao.findById(dstZoneId); if (dstZone == null) { - logger.warn("Destination zone [{}] not found for template [{}].", - dstZoneId, tmplt.getUniqueName()); + logger.warn("Destination zone [{}] not found for template [{}].", dstZoneId, sourceTmpl.getUniqueName()); return false; } - TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore); try { - storageOrchestrator.orchestrateTemplateCopyAcrossZones(sourceTmpl, sourceStore, destStore); + storageOrchestrator.orchestrateTemplateCopyAcrossZones(sourceTmpl, destStore); return true; } catch (Exception e) { logger.error("Failed to copy template [{}] from zone [{}] to zone [{}].", - tmplt.getUniqueName(), - sourceStore.getScope().getScopeId(), + sourceTmpl.getUniqueName(), + sourceTmpl.getDataStore().getScope().getScopeId(), dstZoneId, e); return false; 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 ed370fde701..faf08de0061 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 @@ -213,7 +213,6 @@ public class TemplateServiceImplTest { Map templatesInOtherZone = new HashMap<>(); templatesInOtherZone.put(tmpltMock.getUniqueName(), tmpltPropMock); - Mockito.doReturn(templatesInOtherZone).when(templateService).listTemplate(otherZoneStoreMock); TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); @@ -241,7 +240,6 @@ public class TemplateServiceImplTest { Map templates = new HashMap<>(); templates.put(tmpltMock.getUniqueName(), tmpltPropMock); - Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock); TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); @@ -267,7 +265,6 @@ public class TemplateServiceImplTest { Map templates = new HashMap<>(); templates.put(tmpltMock.getUniqueName(), tmpltPropMock); - Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock); TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); @@ -294,4 +291,31 @@ public class TemplateServiceImplTest { Assert.assertFalse(result); } + + @Test + public void testFindUsableTemplateReturnsTemplateWithNonNullInstallPath() { + VMTemplateVO template = Mockito.mock(VMTemplateVO.class); + Mockito.when(template.getId()).thenReturn(10L); + Mockito.when(template.getUniqueName()).thenReturn("test-template"); + + DataStore storeWithNullPath = Mockito.mock(DataStore.class); + Mockito.when(storeWithNullPath.getName()).thenReturn("store-null"); + + DataStore storeWithValidPath = Mockito.mock(DataStore.class); + TemplateObject tmplWithNullPath = Mockito.mock(TemplateObject.class); + Mockito.when(tmplWithNullPath.getInstallPath()).thenReturn(null); + + TemplateObject tmplWithValidPath = Mockito.mock(TemplateObject.class); + Mockito.when(tmplWithValidPath.getInstallPath()).thenReturn("/mnt/secondary/template.qcow2"); + + Mockito.doReturn(tmplWithNullPath).when(templateDataFactoryMock).getTemplate(10L, storeWithNullPath); + Mockito.doReturn(tmplWithValidPath).when(templateDataFactoryMock).getTemplate(10L, storeWithValidPath); + + List imageStores = List.of(storeWithNullPath, storeWithValidPath); + + TemplateObject result = templateService.findUsableTemplate(template, imageStores); + + Assert.assertNotNull(result); + Assert.assertEquals(tmplWithValidPath, result); + } } diff --git a/server/src/main/java/com/cloud/storage/ImageStoreDetailsUtil.java b/server/src/main/java/com/cloud/storage/ImageStoreDetailsUtil.java index 7e0fcc550b4..9f5aa660f4f 100755 --- a/server/src/main/java/com/cloud/storage/ImageStoreDetailsUtil.java +++ b/server/src/main/java/com/cloud/storage/ImageStoreDetailsUtil.java @@ -79,7 +79,6 @@ public class ImageStoreDetailsUtil { } public boolean isCopyTemplatesFromOtherStoragesEnabled(Long storeId, Long zoneId) { - final Map storeDetails = imageStoreDetailsDao.getDetails(storeId); final String keyWithoutDots = StorageManager.COPY_TEMPLATES_FROM_OTHER_SECONDARY_STORAGES.key() .replace(".", ""); From fb053ca7a12393f602e5a7a47404e5964d7a0b89 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Mon, 5 Jan 2026 13:32:46 +0530 Subject: [PATCH 10/17] code refactoring --- .../storage/image/TemplateServiceImpl.java | 27 ++++++++----------- .../image/TemplateServiceImplTest.java | 8 ++++++ 2 files changed, 19 insertions(+), 16 deletions(-) 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 d32a5240e20..141b094f512 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 @@ -665,6 +665,11 @@ public class TemplateServiceImpl implements TemplateService { continue; } + Map templates = listTemplate(store); + if (templates == null || !templates.containsKey(tmplt.getUniqueName())) { + continue; + } + if (tmpl.getInstallPath() == null) { logger.debug("Template [{}] found in image store [{}] but install path is null. Skipping.", tmplt.getUniqueName(), store.getName()); @@ -678,24 +683,14 @@ public class TemplateServiceImpl implements TemplateService { private boolean searchAndCopyWithinZone(VMTemplateVO tmplt, DataStore destStore) { Long destZoneId = destStore.getScope().getScopeId(); List storesInSameZone = _storeMgr.getImageStoresByZoneIds(destZoneId); - for (DataStore sourceStore : storesInSameZone) { - Map existingTemplatesInSourceStore = listTemplate(sourceStore); - 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("Cannot copy template [{}] from image store [{}]; install path is null.", tmplt.getUniqueName(), sourceStore.getName()); - continue; - } - - storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore); - return true; + TemplateObject sourceTmpl = findUsableTemplate(tmplt, storesInSameZone); + if (sourceTmpl == null) { + return false; } - return false; + + storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore); + return true; } private boolean copyTemplateAcrossZones(DataStore destStore, TemplateObject sourceTmpl) { 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 faf08de0061..2ce219bca1e 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 @@ -302,6 +302,8 @@ public class TemplateServiceImplTest { Mockito.when(storeWithNullPath.getName()).thenReturn("store-null"); DataStore storeWithValidPath = Mockito.mock(DataStore.class); + Mockito.when(storeWithValidPath.getName()).thenReturn("store-valid"); + TemplateObject tmplWithNullPath = Mockito.mock(TemplateObject.class); Mockito.when(tmplWithNullPath.getInstallPath()).thenReturn(null); @@ -311,6 +313,12 @@ public class TemplateServiceImplTest { Mockito.doReturn(tmplWithNullPath).when(templateDataFactoryMock).getTemplate(10L, storeWithNullPath); Mockito.doReturn(tmplWithValidPath).when(templateDataFactoryMock).getTemplate(10L, storeWithValidPath); + Map templates = new HashMap<>(); + templates.put("test-template", Mockito.mock(TemplateProp.class)); + + Mockito.doReturn(templates).when(templateService).listTemplate(storeWithNullPath); + Mockito.doReturn(templates).when(templateService).listTemplate(storeWithValidPath); + List imageStores = List.of(storeWithNullPath, storeWithValidPath); TemplateObject result = templateService.findUsableTemplate(template, imageStores); From 7ccaf4e0bc75ac3c25c3f23d428c33cc3514ae5a Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Mon, 5 Jan 2026 17:05:46 +0530 Subject: [PATCH 11/17] missing changes --- .../storage/image/TemplateServiceImpl.java | 5 +---- .../storage/image/TemplateServiceImplTest.java | 15 ++++++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) 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 141b094f512..c6a04f73981 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 @@ -660,16 +660,13 @@ public class TemplateServiceImpl implements TemplateService { protected TemplateObject findUsableTemplate(VMTemplateVO tmplt, List imageStores) { for (DataStore store : imageStores) { - TemplateObject tmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), store); - if (tmpl == null) { - continue; - } Map templates = listTemplate(store); if (templates == null || !templates.containsKey(tmplt.getUniqueName())) { continue; } + TemplateObject tmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), store); if (tmpl.getInstallPath() == null) { logger.debug("Template [{}] found in image store [{}] but install path is null. Skipping.", tmplt.getUniqueName(), store.getName()); 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 2ce219bca1e..726c354b270 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 @@ -204,6 +204,7 @@ public class TemplateServiceImplTest { Mockito.doReturn(scopeMock).when(destStoreMock).getScope(); Mockito.doReturn(1L).when(scopeMock).getScopeId(); Mockito.doReturn(100L).when(tmpltMock).getId(); + Mockito.doReturn("unique-name").when(tmpltMock).getUniqueName(); 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(); @@ -212,7 +213,8 @@ public class TemplateServiceImplTest { Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L); Map templatesInOtherZone = new HashMap<>(); - templatesInOtherZone.put(tmpltMock.getUniqueName(), tmpltPropMock); + templatesInOtherZone.put("unique-name", tmpltPropMock); + Mockito.doReturn(templatesInOtherZone).when(templateService).listTemplate(otherZoneStoreMock); TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); @@ -232,6 +234,7 @@ public class TemplateServiceImplTest { Mockito.doReturn(scopeMock).when(destStoreMock).getScope(); Mockito.doReturn(1L).when(scopeMock).getScopeId(); Mockito.doReturn(100L).when(tmpltMock).getId(); + Mockito.doReturn("unique-name").when(tmpltMock).getUniqueName(); Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds(); Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L); @@ -239,7 +242,8 @@ public class TemplateServiceImplTest { Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L); Map templates = new HashMap<>(); - templates.put(tmpltMock.getUniqueName(), tmpltPropMock); + templates.put("unique-name", tmpltPropMock); + Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock); TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); @@ -257,6 +261,7 @@ public class TemplateServiceImplTest { Mockito.doReturn(scopeMock).when(destStoreMock).getScope(); Mockito.doReturn(1L).when(scopeMock).getScopeId(); Mockito.doReturn(100L).when(tmpltMock).getId(); + Mockito.doReturn("unique-name").when(tmpltMock).getUniqueName(); Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds(); Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L); @@ -264,7 +269,9 @@ public class TemplateServiceImplTest { Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L); Map templates = new HashMap<>(); - templates.put(tmpltMock.getUniqueName(), tmpltPropMock); + templates.put("unique-name", tmpltPropMock); + + Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock); TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); @@ -302,8 +309,6 @@ public class TemplateServiceImplTest { Mockito.when(storeWithNullPath.getName()).thenReturn("store-null"); DataStore storeWithValidPath = Mockito.mock(DataStore.class); - Mockito.when(storeWithValidPath.getName()).thenReturn("store-valid"); - TemplateObject tmplWithNullPath = Mockito.mock(TemplateObject.class); Mockito.when(tmplWithNullPath.getInstallPath()).thenReturn(null); From 55e211c3c5c8b341f001cbd29e9a6699dea499ce Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 9 Jan 2026 13:22:59 +0530 Subject: [PATCH 12/17] Combine template copy and download into a single asynchronous operation --- .../service/StorageOrchestrationService.java | 4 +- .../api/storage/TemplateService.java | 4 +- .../orchestration/StorageOrchestrator.java | 44 +++++--------- .../storage/image/TemplateServiceImpl.java | 57 ++++++++++++++++--- .../image/TemplateServiceImplTest.java | 47 +++++++++------ .../cloud/template/TemplateManagerImpl.java | 14 ++++- 6 files changed, 108 insertions(+), 62 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java index 018f5c7b61e..140478dea73 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java @@ -31,7 +31,5 @@ public interface StorageOrchestrationService { MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreId, List templateIdList, List snapshotIdList); - Future orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore); - - Future orchestrateTemplateCopyAcrossZones(TemplateInfo source, DataStore destStore); + Future orchestrateTemplateCopyFromSecondaryStores(long templateId, DataStore destStore); } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java index a8861d5acc6..269eb4f1c21 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java @@ -80,4 +80,6 @@ public interface TemplateService { List getTemplateDatadisksOnImageStore(TemplateInfo templateInfo, String configurationId); AsyncCallFuture copyTemplateToImageStore(DataObject source, DataStore destStore); -} + + void handleTemplateCopyFromSecondaryStores(long templateId, DataStore destStore); + } diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java index 26990cecb49..49ded1ba136 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java @@ -44,7 +44,6 @@ import com.cloud.storage.VMTemplateVO; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.template.TemplateManager; import org.apache.cloudstack.api.response.MigrationResponse; -import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -53,6 +52,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SecondaryStorageServic import org.apache.cloudstack.engine.subsystem.api.storage.SecondaryStorageService.DataObjectResult; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; @@ -116,6 +116,8 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra @Inject VMTemplateDao templateDao; @Inject + TemplateDataFactory templateDataFactory; + @Inject DataCenterDao dcDao; @@ -324,11 +326,9 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra } @Override - public Future orchestrateTemplateCopyAcrossZones(TemplateInfo templateInfo, DataStore destStore) { + public Future orchestrateTemplateCopyFromSecondaryStores(long srcTemplateId, DataStore destStore) { Long dstZoneId = destStore.getScope().getScopeId(); - DataCenterVO dstZone = dcDao.findById(dstZoneId); - long userId = CallContext.current().getCallingUserId(); - return submit(dstZoneId, new CrossZoneCopyTemplateTask(userId, templateInfo, dstZone)); + return submit(dstZoneId, new CopyTemplateFromSecondaryStorageTask(srcTemplateId, destStore)); } protected Pair migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List files, MigrationPolicy migrationPolicy, int skipped) { @@ -677,16 +677,14 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra } } - private class CrossZoneCopyTemplateTask implements Callable { - private final long userId; - private final TemplateInfo sourceTmpl; - private final DataCenterVO dstZone; + private class CopyTemplateFromSecondaryStorageTask implements Callable { + private final long srcTemplateId; + private final DataStore destStore; private final String logid; - CrossZoneCopyTemplateTask(long userId, TemplateInfo sourceTmpl, DataCenterVO dstZone) { - this.userId = userId; - this.sourceTmpl = sourceTmpl; - this.dstZone = dstZone; + CopyTemplateFromSecondaryStorageTask(long srcTemplateId, DataStore destStore) { + this.srcTemplateId = srcTemplateId; + this.destStore = destStore; this.logid = ThreadContext.get(LOGCONTEXTID); } @@ -694,25 +692,13 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra public TemplateApiResult call() { ThreadContext.put(LOGCONTEXTID, logid); TemplateApiResult result; - VMTemplateVO template = templateDao.findById(sourceTmpl.getId()); + long destZoneId = destStore.getScope().getScopeId(); + TemplateInfo sourceTmpl = templateDataFactory.getTemplate(srcTemplateId, DataStoreRole.Image); try { - DataStore sourceStore = sourceTmpl.getDataStore(); - boolean success = templateManager.copy(userId, template, sourceStore, dstZone); - + templateService.handleTemplateCopyFromSecondaryStores(srcTemplateId, destStore); result = new TemplateApiResult(sourceTmpl); - if (!success) { - result.setResult("Cross-zone template copy failed"); - } - } catch (StorageUnavailableException | ResourceAllocationException e) { - logger.error("Exception while copying template [{}] from zone [{}] to zone [{}]", - template, - sourceTmpl.getDataStore().getScope().getScopeId(), - dstZone.getId(), - e); - result = new TemplateApiResult(sourceTmpl); - result.setResult(e.getMessage()); } finally { - tryCleaningUpExecutor(dstZone.getId()); + tryCleaningUpExecutor(destZoneId); ThreadContext.clearAll(); } 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 c6a04f73981..ce31e853d78 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,8 @@ import java.util.concurrent.ExecutionException; import javax.inject.Inject; +import com.cloud.exception.StorageUnavailableException; +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; @@ -70,6 +72,7 @@ import org.apache.cloudstack.storage.to.TemplateObjectTO; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.ThreadContext; import org.springframework.stereotype.Component; import com.cloud.agent.api.Answer; @@ -548,10 +551,7 @@ public class TemplateServiceImpl implements TemplateService { } if (availHypers.contains(tmplt.getHypervisorType())) { - boolean copied = imageStoreDetailsUtil.isCopyTemplatesFromOtherStoragesEnabled(storeId, zoneId) && tryCopyingTemplateToImageStore(tmplt, store); - if (!copied) { - tryDownloadingTemplateToImageStore(tmplt, store); - } + storageOrchestrator.orchestrateTemplateCopyFromSecondaryStores(tmplt.getId(), store); } else { logger.info("Skip downloading template {} since current data center does not have hypervisor {}", tmplt, tmplt.getHypervisorType()); } @@ -598,6 +598,16 @@ public class TemplateServiceImpl implements TemplateService { } + @Override + public void handleTemplateCopyFromSecondaryStores(long templateId, DataStore destStore) { + VMTemplateVO template = _templateDao.findById(templateId); + long zoneId = destStore.getScope().getScopeId(); + boolean copied = imageStoreDetailsUtil.isCopyTemplatesFromOtherStoragesEnabled(destStore.getId(), zoneId) && tryCopyingTemplateToImageStore(template, destStore); + if (!copied) { + tryDownloadingTemplateToImageStore(template, destStore); + } + } + protected void tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) { if (tmplt.getUrl() == null) { logger.info("Not downloading template [{}] to image store [{}], as it has no URL.", tmplt.getUniqueName(), @@ -686,8 +696,17 @@ public class TemplateServiceImpl implements TemplateService { return false; } - storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore); - return true; + TemplateApiResult result; + AsyncCallFuture future = copyTemplateToImageStore(sourceTmpl, destStore); + try { + result = future.get(); + } catch (ExecutionException | InterruptedException e) { + logger.warn("Exception while copying template [{}] from image store [{}] to image store [{}]: {}", + sourceTmpl.getUniqueName(), sourceTmpl.getDataStore().getName(), destStore.getName(), e.toString()); + result = new TemplateApiResult(sourceTmpl); + result.setResult(e.getMessage()); + } + return result.isSuccess(); } private boolean copyTemplateAcrossZones(DataStore destStore, TemplateObject sourceTmpl) { @@ -699,9 +718,29 @@ public class TemplateServiceImpl implements TemplateService { return false; } + TemplateApiResult result; try { - storageOrchestrator.orchestrateTemplateCopyAcrossZones(sourceTmpl, destStore); - return true; + VMTemplateVO template = _templateDao.findById(sourceTmpl.getId()); + try { + DataStore sourceStore = sourceTmpl.getDataStore(); + long userId = CallContext.current().getCallingUserId(); + boolean success = _tmpltMgr.copy(userId, template, sourceStore, dstZone); + + result = new TemplateApiResult(sourceTmpl); + if (!success) { + result.setResult("Cross-zone template copy failed"); + } + } catch (StorageUnavailableException | ResourceAllocationException e) { + logger.error("Exception while copying template [{}] from zone [{}] to zone [{}]", + template, + sourceTmpl.getDataStore().getScope().getScopeId(), + dstZone.getId(), + e); + result = new TemplateApiResult(sourceTmpl); + result.setResult(e.getMessage()); + } finally { + ThreadContext.clearAll(); + } } catch (Exception e) { logger.error("Failed to copy template [{}] from zone [{}] to zone [{}].", sourceTmpl.getUniqueName(), @@ -710,6 +749,8 @@ public class TemplateServiceImpl implements TemplateService { e); return false; } + + return result.isSuccess(); } @Override 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 726c354b270..fc2a0e925cf 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 @@ -20,13 +20,18 @@ 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.dao.VMTemplateDao; import com.cloud.storage.template.TemplateProp; import com.cloud.template.TemplateManager; +import com.cloud.user.Account; +import com.cloud.user.User; +import org.apache.cloudstack.context.CallContext; 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; import org.apache.cloudstack.engine.subsystem.api.storage.Scope; -import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; import org.apache.cloudstack.storage.image.store.TemplateObject; @@ -48,6 +53,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import static org.mockito.Mockito.mock; + @RunWith(MockitoJUnitRunner.class) public class TemplateServiceImplTest { @@ -88,6 +95,9 @@ public class TemplateServiceImplTest { @Mock DataCenterDao _dcDao; + @Mock + VMTemplateDao templateDao; + @Mock TemplateManager _tmpltMgr; @@ -103,7 +113,6 @@ public class TemplateServiceImplTest { Mockito.doReturn(List.of(sourceStoreMock, destStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(zoneId); Mockito.doReturn(templatesInSourceStore).when(templateService).listTemplate(sourceStoreMock); Mockito.doReturn(null).when(templateService).listTemplate(destStoreMock); - Mockito.doReturn("install-path").when(templateInfoMock).getInstallPath(); Mockito.doReturn(templateInfoMock).when(templateDataFactoryMock).getTemplate(2L, sourceStoreMock); } @@ -168,7 +177,7 @@ public class TemplateServiceImplTest { boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); Assert.assertFalse(result); - Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyFromSecondaryStores(Mockito.anyLong(), Mockito.any()); } @Test @@ -184,22 +193,11 @@ public class TemplateServiceImplTest { boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); Assert.assertFalse(result); - Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyFromSecondaryStores(Mockito.anyLong(), Mockito.any()); } @Test - public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherStorageAndTaskWasScheduled() { - templatesInSourceStore.put(tmpltMock.getUniqueName(), tmpltPropMock); - Mockito.doReturn(new AsyncCallFuture<>()).when(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); - - boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); - - Assert.assertTrue(result); - Mockito.verify(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); - } - - @Test - public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() { + public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() throws StorageUnavailableException, ResourceAllocationException { Scope scopeMock = Mockito.mock(Scope.class); Mockito.doReturn(scopeMock).when(destStoreMock).getScope(); Mockito.doReturn(1L).when(scopeMock).getScopeId(); @@ -222,6 +220,7 @@ public class TemplateServiceImplTest { DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class); Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L); + Mockito.doReturn(true).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); @@ -256,7 +255,7 @@ public class TemplateServiceImplTest { } @Test - public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenCrossZoneCopyTaskIsScheduled() { + public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenCrossZoneCopyTaskIsScheduled() throws StorageUnavailableException, ResourceAllocationException { Scope scopeMock = Mockito.mock(Scope.class); Mockito.doReturn(scopeMock).when(destStoreMock).getScope(); Mockito.doReturn(1L).when(scopeMock).getScopeId(); @@ -270,15 +269,27 @@ public class TemplateServiceImplTest { Map templates = new HashMap<>(); templates.put("unique-name", tmpltPropMock); - Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock); TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class); Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock); Mockito.doReturn("/mnt/secondary/template.qcow2").when(sourceTmplMock).getInstallPath(); + Mockito.doReturn(100L).when(sourceTmplMock).getId(); + + DataStore sourceStoreMock = Mockito.mock(DataStore.class); + Scope sourceScopeMock = Mockito.mock(Scope.class); + Mockito.doReturn(sourceStoreMock).when(sourceTmplMock).getDataStore(); DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class); Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L); + VMTemplateVO templateVoMock = Mockito.mock(VMTemplateVO.class); + Mockito.doReturn(templateVoMock).when(templateDao).findById(100L); + + Mockito.doReturn(true).when(_tmpltMgr).copy(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); + + Account account = mock(Account.class); + User user = mock(User.class); + CallContext callContext = mock(CallContext.class); boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index ba8e5714180..3e5e0e65edb 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -838,6 +838,9 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, // Copy will just find one eligible image store for the destination zone // and copy template there, not propagate to all image stores // for that zone + + boolean copied = false; + for (DataStore dstSecStore : dstSecStores) { TemplateDataStoreVO dstTmpltStore = _tmplStoreDao.findByStoreTemplate(dstSecStore.getId(), tmpltId); if (dstTmpltStore != null && dstTmpltStore.getDownloadState() == Status.DOWNLOADED) { @@ -852,9 +855,12 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, TemplateApiResult result = future.get(); if (result.isFailed()) { logger.debug("Copy Template failed for image store {}: {}", dstSecStore, result.getResult()); + _tmplStoreDao.removeByTemplateStore(tmpltId, dstSecStore.getId()); continue; // try next image store } + copied = true; + _tmpltDao.addTemplateToZone(template, dstZoneId); if (account.getId() != Account.ACCOUNT_ID_SYSTEM) { @@ -882,12 +888,14 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, } } } + + return true; + } catch (Exception ex) { - logger.debug("Failed to copy Template to image store:{} ,will try next one", dstSecStore); + logger.debug("Failed to copy Template to image store:{} ,will try next one", dstSecStore, ex); } } - return true; - + return copied; } @Override From d17dabe433c8c1eeda2d7acfe477575599ce80fc Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 9 Jan 2026 13:37:04 +0530 Subject: [PATCH 13/17] unused import and fixed conflicts --- .../orchestration/service/StorageOrchestrationService.java | 1 - 1 file changed, 1 deletion(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java index 140478dea73..4af0c806060 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java @@ -22,7 +22,6 @@ import java.util.concurrent.Future; import org.apache.cloudstack.api.response.MigrationResponse; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; -import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; import org.apache.cloudstack.storage.ImageStoreService.MigrationPolicy; From e472e3acfb135d557abe6076e40aa6ed0a02ea64 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 9 Jan 2026 13:47:11 +0530 Subject: [PATCH 14/17] unused code --- .../engine/orchestration/StorageOrchestrator.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java index 49ded1ba136..93832468c4a 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java @@ -36,11 +36,7 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; -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.VMTemplateVO; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.template.TemplateManager; import org.apache.cloudstack.api.response.MigrationResponse; @@ -320,11 +316,6 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra return handleResponse(futures, null, message, success); } - @Override - public Future orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore) { - return submit(destStore.getScope().getScopeId(), new CopyTemplateTask(source, destStore)); - } - @Override public Future orchestrateTemplateCopyFromSecondaryStores(long srcTemplateId, DataStore destStore) { Long dstZoneId = destStore.getScope().getScopeId(); From f8e5a6c6ae1fdf9b7cfbe46e1c781d2340a92141 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Fri, 9 Jan 2026 14:48:39 +0530 Subject: [PATCH 15/17] update config message --- .../src/main/java/com/cloud/storage/StorageManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 21a0e1f15dd..4ce1f4a9638 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 @@ -221,8 +221,8 @@ public interface StorageManager extends StorageService { "Number of worker threads to be used to connect hosts to a primary storage", true); 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.", + "Storage", "true", "When enabled, this feature allows templates to be copied from existing Secondary Storage servers (within the same zone or across zones) " + + "while adding a new Secondary Storage. If the copy operation fails, the system falls back to downloading the template from the source URL.", true, ConfigKey.Scope.Zone, null); /** From 8ec9270f3cfd53ad43d1f9f0c1d6b04100a869b5 Mon Sep 17 00:00:00 2001 From: Harikrishna Patnala Date: Tue, 20 Jan 2026 18:03:20 +0530 Subject: [PATCH 16/17] Fix configuration setting value on add secondary storage page --- ui/src/views/infra/AddSecondaryStorage.vue | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/ui/src/views/infra/AddSecondaryStorage.vue b/ui/src/views/infra/AddSecondaryStorage.vue index fcfb35c9d27..db4893115a6 100644 --- a/ui/src/views/infra/AddSecondaryStorage.vue +++ b/ui/src/views/infra/AddSecondaryStorage.vue @@ -48,6 +48,7 @@ { private final long srcTemplateId; private final DataStore destStore; 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 ce31e853d78..31f9c947778 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 @@ -69,6 +69,7 @@ import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; import org.apache.cloudstack.storage.image.store.TemplateObject; import org.apache.cloudstack.storage.to.TemplateObjectTO; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -646,7 +647,7 @@ public class TemplateServiceImpl implements TemplateService { List storesInOtherZone = _storeMgr.getImageStoresByZoneIds(otherZoneId); logger.debug("Checking zone [{}] for template [{}]...", otherZoneId, tmplt.getUniqueName()); - if (storesInOtherZone == null || storesInOtherZone.isEmpty()) { + if (CollectionUtils.isEmpty(storesInOtherZone)) { logger.debug("Zone [{}] has no image stores. Skipping.", otherZoneId); continue; }