From 40b9e4daa643127cb291e154e04529ab8ba3a611 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 23 Mar 2026 12:09:05 -0400 Subject: [PATCH 1/3] Add config keys for controlling public/private template secondary storage replica counts --- .../java/com/cloud/template/TemplateManager.java | 14 ++++++++++++++ .../com/cloud/template/TemplateManagerImpl.java | 4 +++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java index f1891c774edd..77e010072fa4 100644 --- a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java +++ b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java @@ -45,6 +45,8 @@ public interface TemplateManager { static final String AllowPublicUserTemplatesCK = "allow.public.user.templates"; static final String TemplatePreloaderPoolSizeCK = "template.preloader.pool.size"; + static final String PublicTemplateSecStorageCopyCK = "public.template.secstorage.copy"; + static final String PrivateTemplateSecStorageCopyCK = "private.template.secstorage.copy"; static final ConfigKey AllowPublicUserTemplates = new ConfigKey("Advanced", Boolean.class, AllowPublicUserTemplatesCK, "true", "If false, users will not be able to create public Templates.", true, ConfigKey.Scope.Account); @@ -64,6 +66,18 @@ public interface TemplateManager { true, ConfigKey.Scope.Global); + ConfigKey PublicTemplateSecStorageCopy = new ConfigKey("Advanced", Integer.class, + PublicTemplateSecStorageCopyCK, "0", + "Maximum number of secondary storage pools to which a public template is copied. " + + "0 means copy to all secondary storage pools (default behavior).", + true, ConfigKey.Scope.Zone); + + ConfigKey PrivateTemplateSecStorageCopy = new ConfigKey("Advanced", Integer.class, + PrivateTemplateSecStorageCopyCK, "1", + "Maximum number of secondary storage pools to which a private template is copied. " + + "Default is 1 to preserve existing behavior.", + true, ConfigKey.Scope.Zone); + static final String VMWARE_TOOLS_ISO = "vmware-tools.iso"; static final String XS_TOOLS_ISO = "xs-tools.iso"; diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 67f7128e864d..3ad26e09f117 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -2493,7 +2493,9 @@ public ConfigKey[] getConfigKeys() { return new ConfigKey[] {AllowPublicUserTemplates, TemplatePreloaderPoolSize, ValidateUrlIsResolvableBeforeRegisteringTemplate, - TemplateDeleteFromPrimaryStorage}; + TemplateDeleteFromPrimaryStorage, + PublicTemplateSecStorageCopy, + PrivateTemplateSecStorageCopy}; } public List getTemplateAdapters() { From f99744491929640a14c765d55ce6fbca9f5bd9db Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 23 Mar 2026 12:36:13 -0400 Subject: [PATCH 2/3] Allow configurable secondary storage replica count for public and private templates --- .../template/HypervisorTemplateAdapter.java | 19 ++++++++------ .../cloud/template/TemplateAdapterBase.java | 25 ++++++++----------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index a7a6cea7d907..bc29a63fe6b5 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -19,10 +19,10 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Date; -import java.util.HashSet; +import java.util.HashMap; import java.util.LinkedList; import java.util.List; -import java.util.Set; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.stream.Collectors; @@ -294,7 +294,10 @@ protected void createTemplateWithinZones(TemplateProfile profile, VMTemplateVO t List imageStores = getImageStoresThrowsExceptionIfNotFound(zoneId, profile); standardImageStoreAllocation(imageStores, template); } else { - validateSecondaryStorageAndCreateTemplate(List.of(imageStore), template, null); + int replicaLimit = isPrivateTemplate(template) + ? TemplateManager.PrivateTemplateSecStorageCopy.value() + : TemplateManager.PublicTemplateSecStorageCopy.value(); + validateSecondaryStorageAndCreateTemplate(List.of(imageStore), template, new HashMap<>(), replicaLimit); } } } @@ -308,16 +311,18 @@ protected List getImageStoresThrowsExceptionIfNotFound(long zoneId, T } protected void standardImageStoreAllocation(List imageStores, VMTemplateVO template) { - Set zoneSet = new HashSet(); + int replicaLimit = isPrivateTemplate(template) + ? TemplateManager.PrivateTemplateSecStorageCopy.value() + : TemplateManager.PublicTemplateSecStorageCopy.value(); Collections.shuffle(imageStores); - validateSecondaryStorageAndCreateTemplate(imageStores, template, zoneSet); + validateSecondaryStorageAndCreateTemplate(imageStores, template, new HashMap<>(), replicaLimit); } - protected void validateSecondaryStorageAndCreateTemplate(List imageStores, VMTemplateVO template, Set zoneSet) { + protected void validateSecondaryStorageAndCreateTemplate(List imageStores, VMTemplateVO template, Map zoneCopyCount, int replicaLimit) { for (DataStore imageStore : imageStores) { Long zoneId = imageStore.getScope().getScopeId(); - if (!isZoneAndImageStoreAvailable(imageStore, zoneId, zoneSet, isPrivateTemplate(template))) { + if (!isZoneAndImageStoreAvailable(imageStore, zoneId, zoneCopyCount, replicaLimit)) { continue; } diff --git a/server/src/main/java/com/cloud/template/TemplateAdapterBase.java b/server/src/main/java/com/cloud/template/TemplateAdapterBase.java index 97c6e5903592..e32d2eba7ddd 100644 --- a/server/src/main/java/com/cloud/template/TemplateAdapterBase.java +++ b/server/src/main/java/com/cloud/template/TemplateAdapterBase.java @@ -20,11 +20,9 @@ import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.Set; import javax.inject.Inject; @@ -169,7 +167,7 @@ protected DataStore verifyHeuristicRulesForZone(VMTemplateVO template, Long zone return heuristicRuleHelper.getImageStoreIfThereIsHeuristicRule(zoneId, heuristicType, template); } - protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId, Set zoneSet, boolean isTemplatePrivate) { + protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId, Map zoneCopyCount, int replicaLimit) { if (zoneId == null) { logger.warn(String.format("Zone ID is null, cannot allocate ISO/template in image store [%s].", imageStore)); return false; @@ -191,19 +189,13 @@ protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId return false; } - if (zoneSet == null) { - logger.info(String.format("Zone set is null; therefore, the ISO/template should be allocated in every secondary storage of zone [%s].", zone)); - return true; - } - - if (isTemplatePrivate && zoneSet.contains(zoneId)) { - logger.info(String.format("The template is private and it is already allocated in a secondary storage in zone [%s]; therefore, image store [%s] will be skipped.", - zone, imageStore)); + int currentCount = zoneCopyCount.getOrDefault(zoneId, 0); + if (replicaLimit > 0 && currentCount >= replicaLimit) { + logger.info("Replica limit of {} reached for zone [{}]; skipping image store [{}].", replicaLimit, zone, imageStore); return false; } - logger.info(String.format("Private template will be allocated in image store [%s] in zone [%s].", imageStore, zone)); - zoneSet.add(zoneId); + zoneCopyCount.put(zoneId, currentCount + 1); return true; } @@ -212,12 +204,15 @@ protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId * {@link TemplateProfile#getZoneIdList()}. */ protected void postUploadAllocation(List imageStores, VMTemplateVO template, List payloads) { - Set zoneSet = new HashSet<>(); + int replicaLimit = isPrivateTemplate(template) + ? TemplateManager.PrivateTemplateSecStorageCopy.value() + : TemplateManager.PublicTemplateSecStorageCopy.value(); + Map zoneCopyCount = new HashMap<>(); Collections.shuffle(imageStores); for (DataStore imageStore : imageStores) { Long zoneId_is = imageStore.getScope().getScopeId(); - if (!isZoneAndImageStoreAvailable(imageStore, zoneId_is, zoneSet, isPrivateTemplate(template))) { + if (!isZoneAndImageStoreAvailable(imageStore, zoneId_is, zoneCopyCount, replicaLimit)) { continue; } From 0708236786f611243a9383dedf71f0684e0d7930 Mon Sep 17 00:00:00 2001 From: Daman Arora Date: Mon, 23 Mar 2026 12:43:59 -0400 Subject: [PATCH 3/3] Add unit tests --- .../HypervisorTemplateAdapterTest.java | 74 +++++++++---------- 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java index e2a97be469ff..a4b0f28e33c9 100644 --- a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java +++ b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java @@ -32,10 +32,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.ExecutionException; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -371,11 +369,11 @@ public void createTemplateWithinZonesTestZoneWithHeuristicRuleShouldCallValidate Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds); Mockito.doReturn(dataStoreMock).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong()); - Mockito.doNothing().when(_adapter).validateSecondaryStorageAndCreateTemplate(Mockito.any(List.class), Mockito.any(VMTemplateVO.class), Mockito.isNull()); + Mockito.doNothing().when(_adapter).validateSecondaryStorageAndCreateTemplate(Mockito.any(List.class), Mockito.any(VMTemplateVO.class), Mockito.any(Map.class), Mockito.anyInt()); _adapter.createTemplateWithinZones(templateProfileMock, vmTemplateVOMock); - Mockito.verify(_adapter, Mockito.times(1)).validateSecondaryStorageAndCreateTemplate(Mockito.any(List.class), Mockito.any(VMTemplateVO.class), Mockito.isNull()); + Mockito.verify(_adapter, Mockito.times(1)).validateSecondaryStorageAndCreateTemplate(Mockito.any(List.class), Mockito.any(VMTemplateVO.class), Mockito.any(Map.class), Mockito.anyInt()); } @Test(expected = CloudRuntimeException.class) @@ -411,11 +409,8 @@ public void getImageStoresThrowsExceptionIfNotFoundTestNonEmptyImageStoreShouldN @Test public void isZoneAndImageStoreAvailableTestZoneIdIsNullShouldReturnFalse() { DataStore dataStoreMock = Mockito.mock(DataStore.class); - Long zoneId = null; - Set zoneSet = null; - boolean isTemplatePrivate = false; - boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneSet, isTemplatePrivate); + boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, null, new HashMap<>(), 0); Mockito.verify(loggerMock, Mockito.times(1)).warn(String.format("Zone ID is null, cannot allocate ISO/template in image store [%s].", dataStoreMock)); Assert.assertFalse(result); @@ -425,13 +420,10 @@ public void isZoneAndImageStoreAvailableTestZoneIdIsNullShouldReturnFalse() { public void isZoneAndImageStoreAvailableTestZoneIsNullShouldReturnFalse() { DataStore dataStoreMock = Mockito.mock(DataStore.class); Long zoneId = 1L; - Set zoneSet = null; - boolean isTemplatePrivate = false; - DataCenterVO dataCenterVOMock = null; - Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); + Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(null); - boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneSet, isTemplatePrivate); + boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, new HashMap<>(), 0); Mockito.verify(loggerMock, Mockito.times(1)).warn("Unable to find zone by id [{}], so skip downloading template to its image store [{}].", zoneId, dataStoreMock); @@ -442,14 +434,12 @@ public void isZoneAndImageStoreAvailableTestZoneIsNullShouldReturnFalse() { public void isZoneAndImageStoreAvailableTestZoneIsDisabledShouldReturnFalse() { DataStore dataStoreMock = Mockito.mock(DataStore.class); Long zoneId = 1L; - Set zoneSet = null; - boolean isTemplatePrivate = false; DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Disabled); - boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneSet, isTemplatePrivate); + boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, new HashMap<>(), 0); Mockito.verify(loggerMock, Mockito.times(1)).info("Zone [{}] is disabled. Skip downloading template to its image store [{}].", dataCenterVOMock, dataStoreMock); Assert.assertFalse(result); @@ -459,15 +449,13 @@ public void isZoneAndImageStoreAvailableTestZoneIsDisabledShouldReturnFalse() { public void isZoneAndImageStoreAvailableTestImageStoreDoesNotHaveEnoughCapacityShouldReturnFalse() { DataStore dataStoreMock = Mockito.mock(DataStore.class); Long zoneId = 1L; - Set zoneSet = null; - boolean isTemplatePrivate = false; DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); Mockito.when(statsCollectorMock.imageStoreHasEnoughCapacity(any(DataStore.class))).thenReturn(false); - boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneSet, isTemplatePrivate); + boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, new HashMap<>(), 0); Mockito.verify(loggerMock, times(1)).info("Image store doesn't have enough capacity. Skip downloading template to this image store [{}].", dataStoreMock); @@ -475,60 +463,72 @@ public void isZoneAndImageStoreAvailableTestImageStoreDoesNotHaveEnoughCapacityS } @Test - public void isZoneAndImageStoreAvailableTestImageStoreHasEnoughCapacityAndZoneSetIsNullShouldReturnTrue() { + public void isZoneAndImageStoreAvailableTestReplicaLimitZeroShouldCopyToAllStores() { DataStore dataStoreMock = Mockito.mock(DataStore.class); Long zoneId = 1L; - Set zoneSet = null; - boolean isTemplatePrivate = false; DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); + Map zoneCopyCount = new HashMap<>(); + zoneCopyCount.put(zoneId, 999); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); Mockito.when(statsCollectorMock.imageStoreHasEnoughCapacity(any(DataStore.class))).thenReturn(true); - boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneSet, isTemplatePrivate); + boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneCopyCount, 0); - Mockito.verify(loggerMock, times(1)).info(String.format("Zone set is null; therefore, the ISO/template should be allocated in every secondary storage " + - "of zone [%s].", dataCenterVOMock)); Assert.assertTrue(result); + Assert.assertEquals(1000, (int) zoneCopyCount.get(zoneId)); } @Test - public void isZoneAndImageStoreAvailableTestTemplateIsPrivateAndItIsAlreadyAllocatedToTheSameZoneShouldReturnFalse() { + public void isZoneAndImageStoreAvailableTestReplicaLimitReachedShouldReturnFalse() { DataStore dataStoreMock = Mockito.mock(DataStore.class); Long zoneId = 1L; - Set zoneSet = Set.of(1L); - boolean isTemplatePrivate = true; DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); + Map zoneCopyCount = new HashMap<>(); + zoneCopyCount.put(zoneId, 1); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); Mockito.when(statsCollectorMock.imageStoreHasEnoughCapacity(any(DataStore.class))).thenReturn(true); - boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneSet, isTemplatePrivate); + boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneCopyCount, 1); - Mockito.verify(loggerMock, times(1)).info(String.format("The template is private and it is already allocated in a secondary storage in zone [%s]; " + - "therefore, image store [%s] will be skipped.", dataCenterVOMock, dataStoreMock)); + Mockito.verify(loggerMock, times(1)).info("Replica limit of {} reached for zone [{}]; skipping image store [{}].", 1, dataCenterVOMock, dataStoreMock); Assert.assertFalse(result); } @Test - public void isZoneAndImageStoreAvailableTestTemplateIsPrivateAndItIsNotAlreadyAllocatedToTheSameZoneShouldReturnTrue() { + public void isZoneAndImageStoreAvailableTestReplicaLimitNotYetReachedShouldReturnTrueAndIncrementCount() { DataStore dataStoreMock = Mockito.mock(DataStore.class); Long zoneId = 1L; - Set zoneSet = new HashSet<>(); - boolean isTemplatePrivate = true; DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); + Map zoneCopyCount = new HashMap<>(); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); Mockito.when(statsCollectorMock.imageStoreHasEnoughCapacity(any(DataStore.class))).thenReturn(true); - boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneSet, isTemplatePrivate); + boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneCopyCount, 2); - Mockito.verify(loggerMock, times(1)).info(String.format("Private template will be allocated in image store [%s] in zone [%s].", - dataStoreMock, dataCenterVOMock)); Assert.assertTrue(result); + Assert.assertEquals(1, (int) zoneCopyCount.get(zoneId)); + } + + @Test + public void isZoneAndImageStoreAvailableTestReplicaLimitOfTwoShouldCopyToExactlyTwoStores() { + Long zoneId = 1L; + DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); + Map zoneCopyCount = new HashMap<>(); + + Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); + Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + Mockito.when(statsCollectorMock.imageStoreHasEnoughCapacity(any(DataStore.class))).thenReturn(true); + + Assert.assertTrue(_adapter.isZoneAndImageStoreAvailable(Mockito.mock(DataStore.class), zoneId, zoneCopyCount, 2)); + Assert.assertTrue(_adapter.isZoneAndImageStoreAvailable(Mockito.mock(DataStore.class), zoneId, zoneCopyCount, 2)); + Assert.assertFalse(_adapter.isZoneAndImageStoreAvailable(Mockito.mock(DataStore.class), zoneId, zoneCopyCount, 2)); + Assert.assertEquals(2, (int) zoneCopyCount.get(zoneId)); } @Test