Skip to content

Add config keys for controlling public/private template secondary storage replica counts #12877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Damans227 wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Add config keys for controlling public/private template secondary storage replica counts #12877

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
40b9e4d
Add config keys for controlling public/private template secondary sto…
Mar 23, 2026
f997444
Allow configurable secondary storage replica count for public and pri…
Mar 23, 2026
0708236
Add unit tests
Mar 23, 2026
e5bfeab
Refactor secondary storage replica limit handling to use zone-specifi…
Damans227 Apr 15, 2026
6dbeb52
Refactor as per pr comments
Damans227 Apr 15, 2026
bbc0560
Implement secondary storage copy limit check for templates based on z…
Damans227 Apr 29, 2026
3b69432
Enhance secondary storage copy limit check to consider image stores b…
Damans227 Apr 29, 2026
c60d927
Improve logging for template store cleanup by adding debug informatio…
Damans227 Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Comment thread
Damans227 marked this conversation as resolved.
Outdated
static final String PrivateTemplateSecStorageCopyCK = "private.template.secstorage.copy";
Comment thread
Damans227 marked this conversation as resolved.
Outdated

static final ConfigKey<Boolean> AllowPublicUserTemplates = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowPublicUserTemplatesCK, "true",
"If false, users will not be able to create public Templates.", true, ConfigKey.Scope.Account);
Expand All @@ -64,6 +66,18 @@ public interface TemplateManager {
true,
ConfigKey.Scope.Global);

ConfigKey<Integer> PublicTemplateSecStorageCopy = new ConfigKey<Integer>("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<Integer> PrivateTemplateSecStorageCopy = new ConfigKey<Integer>("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);
Comment on lines +69 to +79
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The config key descriptions don’t mention that the limits are evaluated per-zone (and the keys themselves are zone-scoped). Consider clarifying the wording (e.g., “per zone”) so operators understand how replica limits are applied across multiple zones/image stores.

Copilot uses AI. Check for mistakes.

static final String VMWARE_TOOLS_ISO = "vmware-tools.iso";
static final String XS_TOOLS_ISO = "xs-tools.iso";

Expand Down
19 changes: 12 additions & 7 deletions server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -294,7 +294,10 @@ protected void createTemplateWithinZones(TemplateProfile profile, VMTemplateVO t
List<DataStore> imageStores = getImageStoresThrowsExceptionIfNotFound(zoneId, profile);
standardImageStoreAllocation(imageStores, template);
} else {
validateSecondaryStorageAndCreateTemplate(List.of(imageStore), template, null);
int replicaLimit = isPrivateTemplate(template)
? TemplateManager.PrivateTemplateSecStorageCopy.value()
: TemplateManager.PublicTemplateSecStorageCopy.value();
Comment thread
Damans227 marked this conversation as resolved.
Outdated
validateSecondaryStorageAndCreateTemplate(List.of(imageStore), template, new HashMap<>(), replicaLimit);
}
}
}
Expand All @@ -308,16 +311,18 @@ protected List<DataStore> getImageStoresThrowsExceptionIfNotFound(long zoneId, T
}

protected void standardImageStoreAllocation(List<DataStore> imageStores, VMTemplateVO template) {
Set<Long> zoneSet = new HashSet<Long>();
int replicaLimit = isPrivateTemplate(template)
Comment thread
Damans227 marked this conversation as resolved.
Outdated
? TemplateManager.PrivateTemplateSecStorageCopy.value()
: TemplateManager.PublicTemplateSecStorageCopy.value();
Collections.shuffle(imageStores);
validateSecondaryStorageAndCreateTemplate(imageStores, template, zoneSet);
validateSecondaryStorageAndCreateTemplate(imageStores, template, new HashMap<>(), replicaLimit);
Comment thread
DaanHoogland marked this conversation as resolved.
Outdated
}

protected void validateSecondaryStorageAndCreateTemplate(List<DataStore> imageStores, VMTemplateVO template, Set<Long> zoneSet) {
protected void validateSecondaryStorageAndCreateTemplate(List<DataStore> imageStores, VMTemplateVO template, Map<Long, Integer> 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;
}

Expand Down
25 changes: 10 additions & 15 deletions server/src/main/java/com/cloud/template/TemplateAdapterBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<Long> zoneSet, boolean isTemplatePrivate) {
protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId, Map<Long, Integer> 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;
Expand All @@ -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);
Comment thread
DaanHoogland marked this conversation as resolved.
Outdated
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;
}

Expand All @@ -212,12 +204,15 @@ protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId
* {@link TemplateProfile#getZoneIdList()}.
*/
protected void postUploadAllocation(List<DataStore> imageStores, VMTemplateVO template, List<TemplateOrVolumePostUploadCommand> payloads) {
Comment on lines 208 to 212
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method JavaDoc no longer matches the behavior: allocation is now governed by replica-limit config keys (public/private can both be limited), not strictly by template visibility (private=random vs public=all). Please update the comment to reflect the new replica-limit semantics (and that the limits are zone-scoped).

Copilot uses AI. Check for mistakes.
Set<Long> zoneSet = new HashSet<>();
int replicaLimit = isPrivateTemplate(template)
? TemplateManager.PrivateTemplateSecStorageCopy.value()
: TemplateManager.PublicTemplateSecStorageCopy.value();
Map<Long, Integer> zoneCopyCount = new HashMap<>();
Comment thread
DaanHoogland marked this conversation as resolved.
Outdated
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;
}

Expand Down
4 changes: 3 additions & 1 deletion server/src/main/java/com/cloud/template/TemplateManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2493,7 +2493,9 @@ public ConfigKey<?>[] getConfigKeys() {
return new ConfigKey<?>[] {AllowPublicUserTemplates,
TemplatePreloaderPoolSize,
ValidateUrlIsResolvableBeforeRegisteringTemplate,
TemplateDeleteFromPrimaryStorage};
TemplateDeleteFromPrimaryStorage,
PublicTemplateSecStorageCopy,
PrivateTemplateSecStorageCopy};
}

public List<TemplateAdapter> getTemplateAdapters() {
Expand Down
74 changes: 37 additions & 37 deletions server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -411,11 +409,8 @@ public void getImageStoresThrowsExceptionIfNotFoundTestNonEmptyImageStoreShouldN
@Test
public void isZoneAndImageStoreAvailableTestZoneIdIsNullShouldReturnFalse() {
DataStore dataStoreMock = Mockito.mock(DataStore.class);
Long zoneId = null;
Set<Long> 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);
Expand All @@ -425,13 +420,10 @@ public void isZoneAndImageStoreAvailableTestZoneIdIsNullShouldReturnFalse() {
public void isZoneAndImageStoreAvailableTestZoneIsNullShouldReturnFalse() {
DataStore dataStoreMock = Mockito.mock(DataStore.class);
Long zoneId = 1L;
Set<Long> 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);
Expand All @@ -442,14 +434,12 @@ public void isZoneAndImageStoreAvailableTestZoneIsNullShouldReturnFalse() {
public void isZoneAndImageStoreAvailableTestZoneIsDisabledShouldReturnFalse() {
DataStore dataStoreMock = Mockito.mock(DataStore.class);
Long zoneId = 1L;
Set<Long> 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);
Expand All @@ -459,76 +449,86 @@ public void isZoneAndImageStoreAvailableTestZoneIsDisabledShouldReturnFalse() {
public void isZoneAndImageStoreAvailableTestImageStoreDoesNotHaveEnoughCapacityShouldReturnFalse() {
DataStore dataStoreMock = Mockito.mock(DataStore.class);
Long zoneId = 1L;
Set<Long> 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);
Assert.assertFalse(result);
}

@Test
public void isZoneAndImageStoreAvailableTestImageStoreHasEnoughCapacityAndZoneSetIsNullShouldReturnTrue() {
public void isZoneAndImageStoreAvailableTestReplicaLimitZeroShouldCopyToAllStores() {
DataStore dataStoreMock = Mockito.mock(DataStore.class);
Long zoneId = 1L;
Set<Long> zoneSet = null;
boolean isTemplatePrivate = false;
DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class);
Map<Long, Integer> 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<Long> zoneSet = Set.of(1L);
boolean isTemplatePrivate = true;
DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class);
Map<Long, Integer> 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<Long> zoneSet = new HashSet<>();
boolean isTemplatePrivate = true;
DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class);
Map<Long, Integer> 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<Long, Integer> 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
Expand Down
Loading