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

engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java

@@ -67,6 +67,12 @@ public TemplateInfo getTemplate() {
 
     void handleTemplateSync(DataStore store);
 
+    void enforceSecStorageCopyLimit(long templateId, long zoneId);
+
+    boolean canCopyTemplateToImageStore(long templateId, long zoneId);
+
+    void replicateTemplateUpToCap(long templateId, long zoneId);
+
     void downloadBootstrapSysTemplate(DataStore store);
 
     void addSystemVMTemplatesToSecondary(DataStore store);

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 = "secstorage.public.template.copy.max";
+    static final String PrivateTemplateSecStorageCopyCK = "secstorage.private.template.copy.max";
 
     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);
@@ -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);
+
     static final String VMWARE_TOOLS_ISO = "vmware-tools.iso";
     static final String XS_TOOLS_ISO = "xs-tools.iso";
 
@@ -138,6 +152,12 @@ public interface TemplateManager {
 
     List<DataStore> getImageStoreByTemplate(long templateId, Long zoneId);
 
+    /**
+     * Max number of secondary storage copies for the template in this zone; {@code 0} means no limit.
+     * SYSTEM/ROUTING/BUILTIN templates are always exempt (returns {@code 0}).
+     */
+    int getSecStorageCopyLimit(VMTemplateVO template, long zoneId);
+
     TemplateInfo prepareIso(long isoId, long dcId, Long hostId, Long poolId);
 
 

engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java

@@ -295,6 +295,161 @@ public void handleSysTemplateDownload(HypervisorType hostHyper, Long dcId) {
         }
     }
 
+    private int countActiveSecStorageCopies(long templateId, long zoneId) {
+        List<DataStore> stores = _storeMgr.getImageStoresByScope(new ZoneScope(zoneId));
+        if (stores == null || stores.isEmpty()) {
+            return 0;
+        }
+        int count = 0;
+        for (DataStore ds : stores) {
+            List<TemplateDataStoreVO> rows = _vmTemplateStoreDao.listByTemplateStore(templateId, ds.getId());
+            if (rows == null) {
+                continue;
+            }
+            for (TemplateDataStoreVO row : rows) {
+                State st = row.getState();
+                Status ds_state = row.getDownloadState();
+                if (st != State.Failed && st != State.Destroyed
+                        && ds_state != Status.ABANDONED && ds_state != Status.DOWNLOAD_ERROR) {
+                    count++;
+                    break;
+                }
+            }
+        }
+        return count;
+    }
+
+    /**
+     * Central gate for the secondary storage copy limit (secstorage.public/private.template.copy.max).
+     * Every template-landing path (periodic sync, cross-zone copy, register, upload) should consult this
+     * single method before placing another copy of a template on a secondary store in a zone, so the limit
+     * is enforced consistently instead of being re-implemented per call site.
+     *
+     * SYSTEM/ROUTING/BUILTIN templates and a limit of 0 mean "unlimited" (return true). The per-template,
+     * per-zone {@link GlobalLock} serializes concurrent placement decisions so racing SSVM syncs / copies
+     * cannot collectively exceed the limit.
+     */
+    @Override
+    public boolean canCopyTemplateToImageStore(long templateId, long zoneId) {
+        VMTemplateVO template = _templateDao.findById(templateId);
+        if (template == null) {
+            return false;
+        }
+        int copyLimit = _tmpltMgr.getSecStorageCopyLimit(template, zoneId);
+        if (copyLimit <= 0) {
+            logger.debug("Template [{}] has no secondary storage copy limit in zone [{}] (limit={}); copy allowed.",
+                    template.getUniqueName(), zoneId, copyLimit);
+            return true;
+        }
+        int count = countActiveSecStorageCopies(templateId, zoneId);
+        logger.debug("Template [{}] secstorage copy check in zone [{}]: count={}, limit={}",
+                template.getUniqueName(), zoneId, count, copyLimit);
+        return count < copyLimit;
+    }
+
+    private boolean hasReachedSecStorageCopyLimit(VMTemplateVO template, long zoneId) {
+        return !canCopyTemplateToImageStore(template.getId(), zoneId);
+    }
+
+    @Override
+    public void replicateTemplateUpToCap(long templateId, long zoneId) {
+        VMTemplateVO template = _templateDao.findById(templateId);
+        if (template == null) {
+            return;
+        }
+        List<DataStore> stores = _storeMgr.getImageStoresByScope(new ZoneScope(zoneId));
+        if (stores == null || stores.isEmpty()) {
+            return;
+        }
+        for (DataStore store : stores) {
+            if (hasActiveTemplateCopyOnStore(templateId, store.getId())) {
+                continue;
+            }
+            if (!canCopyTemplateToImageStore(templateId, zoneId)) {
+                break;
+            }
+            try {
+                storageOrchestrator.orchestrateTemplateCopyFromSecondaryStores(templateId, store);
+            } catch (Exception e) {
+                logger.warn("Failed to proactively replicate template [{}] to image store [{}] in zone [{}]: {}",
+                        template.getUniqueName(), store.getName(), zoneId, e.getMessage());
+            }
+        }
+    }
+
+    private boolean hasActiveTemplateCopyOnStore(long templateId, long storeId) {
+        List<TemplateDataStoreVO> rows = _vmTemplateStoreDao.listByTemplateStore(templateId, storeId);
+        if (rows == null) {
+            return false;
+        }
+        for (TemplateDataStoreVO row : rows) {
+            State st = row.getState();
+            Status ds = row.getDownloadState();
+            if (st != State.Failed && st != State.Destroyed
+                    && ds != Status.ABANDONED && ds != Status.DOWNLOAD_ERROR) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    @Override
+    public void enforceSecStorageCopyLimit(long templateId, long zoneId) {
+        VMTemplateVO template = _templateDao.findById(templateId);
+        if (template == null) {
+            return;
+        }
+        int copyLimit = _tmpltMgr.getSecStorageCopyLimit(template, zoneId);
+        if (copyLimit <= 0) {
+            return;
+        }
+        if (_tmpltMgr.verifyHeuristicRulesForZone(template, zoneId) != null) {
+            return;
+        }
+        GlobalLock lock = GlobalLock.getInternLock("template.copy.limit." + templateId + "." + zoneId);
+        try {
+            if (!lock.lock(30)) {
+                logger.warn("Could not acquire lock to enforce secondary storage copy limit for template [{}] in zone [{}].",
+                        template.getUniqueName(), zoneId);
+                return;
+            }
+            List<DataStore> stores = _storeMgr.getImageStoresByScope(new ZoneScope(zoneId));
+            if (stores == null) {
+                return;
+            }
+            List<TemplateDataStoreVO> removable = new ArrayList<>();
+            for (DataStore ds : stores) {
+                TemplateDataStoreVO ref = _vmTemplateStoreDao.findByStoreTemplate(ds.getId(), templateId);
+                if (ref != null
+                        && ref.getState() == State.Ready
+                        && ref.getDownloadState() == Status.DOWNLOADED
+                        && (ref.getRefCnt() == null || ref.getRefCnt() == 0)) {
+                    removable.add(ref);
+                }
+            }
+            int excess = removable.size() - copyLimit;
+            if (excess <= 0) {
+                return;
+            }
+            logger.info("Template [{}] has [{}] removable secondary storage copies in zone [{}], limit is [{}]; removing [{}] excess copies.",
+                    template.getUniqueName(), removable.size(), zoneId, copyLimit, excess);
+            for (int i = 0; i < excess; i++) {
+                DataStore ds = _storeMgr.getDataStore(removable.get(i).getDataStoreId(), DataStoreRole.Image);
+                try {
+                    deleteTemplateAsync(_templateFactory.getTemplate(templateId, ds));
+                    logger.info("Removed excess copy of template [{}] from image store [{}] to honor the secondary storage copy limit.",
+                            template.getUniqueName(), ds.getName());
+                } catch (Exception e) {
+                    logger.warn("Failed to remove excess copy of template [{}] from image store [{}]: {}",
+                            template.getUniqueName(), ds, e.getMessage());
+                }
+            }
+        } finally {
+            lock.unlock();
+            lock.releaseRef();
+        }
+    }
+
     protected boolean shouldDownloadTemplateToStore(VMTemplateVO template, DataStore store) {
         Long zoneId = store.getScope().getScopeId();
         DataStore directedStore = _tmpltMgr.verifyHeuristicRulesForZone(template, zoneId);
@@ -304,6 +459,12 @@ protected boolean shouldDownloadTemplateToStore(VMTemplateVO template, DataStore
             return false;
         }
 
+        if (zoneId != null && hasReachedSecStorageCopyLimit(template, zoneId)) {
+            logger.info("Skipping sync of template [{}] to image store [{}]: zone [{}] has reached the configured copy limit.",
+                    template.getUniqueName(), store.getName(), zoneId);
+            return false;
+        }
+
         if (template.isPublicTemplate()) {
             logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is public.", template.getUniqueName(),
                     store.getName());
@@ -328,8 +489,9 @@ protected boolean shouldDownloadTemplateToStore(VMTemplateVO template, DataStore
             return true;
         }
 
-        logger.info("Skipping download of template [{}] to image store [{}].", template.getUniqueName(), store.getName());
-        return false;
+        logger.debug("Copying template [{}] to image store [{}] to reach the configured secondary storage copy limit in zone [{}].",
+                template.getUniqueName(), store.getName(), zoneId);
+        return true;
     }
 
     @Override
@@ -531,10 +693,13 @@ public void handleTemplateSync(DataStore store) {
                                         && tmpltStore.getState() == State.Ready
                                         && tmpltStore.getInstallPath() == null) {
                                     logger.info("Keep fake entry in template store table for migration of previous NFS to object store");
-                                } else {
+                                } else if (tmpltStore.getDownloadState() == VMTemplateStorageResourceAssoc.Status.DOWNLOADED
+                                        || tmpltStore.getState() == State.Ready) {
                                     logger.info("Removing leftover template {} entry from template store table", tmplt);
-                                    // remove those leftover entries
                                     _vmTemplateStoreDao.remove(tmpltStore.getId());
+                                } else {
+                                    logger.debug("Template {} entry on store {} is in pre-download state ({}/{}); not treating as leftover.",
+                                            tmplt, store, tmpltStore.getState(), tmpltStore.getDownloadState());
                                 }
                             }
                         }
@@ -556,7 +721,7 @@ public void handleTemplateSync(DataStore store) {
                         availHypers.add(HypervisorType.None); // bug 9809: resume ISO
                         // download.
                         for (VMTemplateVO tmplt : toBeDownloaded) {
-                            // if this is private template, skip sync to a new image store
+                            // skip stores excluded by heuristic rules or already at the configured copy limit
                             if (!shouldDownloadTemplateToStore(tmplt, store)) {
                                 continue;
                             }
@@ -580,6 +745,12 @@ public void handleTemplateSync(DataStore store) {
                         }
                     }
 
+                    if (zoneId != null) {
+                        for (VMTemplateVO tmplt : allTemplates) {
+                            enforceSecStorageCopyLimit(tmplt.getId(), zoneId);
+                        }
+                    }
+
                     for (String uniqueName : templateInfos.keySet()) {
                         TemplateProp tInfo = templateInfos.get(uniqueName);
                         if (_tmpltMgr.templateIsDeleteable(tInfo.getId())) {

engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java

@@ -119,6 +119,7 @@ public void setUp() {
         Mockito.doReturn(templateInfoMock).when(templateDataFactoryMock).getTemplate(2L, sourceStoreMock);
         Mockito.doReturn(3L).when(dataStoreMock).getId();
         Mockito.doReturn(zoneScopeMock).when(dataStoreMock).getScope();
+        Mockito.lenient().doReturn(tmpltMock).when(templateDao).findById(2L);
     }
 
     @Test
@@ -153,11 +154,37 @@ public void shouldDownloadTemplateToStoreTestDownloadsPrivateNoRefTemplate() {
     }
 
     @Test
-    public void shouldDownloadTemplateToStoreTestSkipsPrivateExistingTemplate() {
+    public void shouldDownloadTemplateToStoreTestReplicatesPrivateTemplateUnderCopyLimit() {
+        DataStore storeWithCopy = Mockito.mock(DataStore.class);
+        Mockito.doReturn(10L).when(storeWithCopy).getId();
+        Mockito.when(templateManagerMock.getSecStorageCopyLimit(tmpltMock, zoneScopeMock.getScopeId())).thenReturn(2);
+        Mockito.doReturn(List.of(storeWithCopy)).when(dataStoreManagerMock).getImageStoresByScope(Mockito.any());
+        Mockito.doReturn(List.of(Mockito.mock(TemplateDataStoreVO.class))).when(templateDataStoreDao).listByTemplateStore(2L, 10L);
         Mockito.when(templateDataStoreDao.findByTemplateZone(tmpltMock.getId(), zoneScopeMock.getScopeId(), DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class));
+        Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock));
+    }
+
+    @Test
+    public void shouldDownloadTemplateToStoreTestSkipsPrivateTemplateAtCopyLimit() {
+        DataStore storeWithCopy = Mockito.mock(DataStore.class);
+        Mockito.doReturn(10L).when(storeWithCopy).getId();
+        Mockito.when(templateManagerMock.getSecStorageCopyLimit(tmpltMock, zoneScopeMock.getScopeId())).thenReturn(1);
+        Mockito.doReturn(List.of(storeWithCopy)).when(dataStoreManagerMock).getImageStoresByScope(Mockito.any());
+        Mockito.doReturn(List.of(Mockito.mock(TemplateDataStoreVO.class))).when(templateDataStoreDao).listByTemplateStore(2L, 10L);
         Assert.assertFalse(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock));
     }
 
+    @Test
+    public void canCopyTemplateToImageStoreTestUnlimitedWhenLimitIsZero() {
+        Mockito.when(templateManagerMock.getSecStorageCopyLimit(tmpltMock, 1L)).thenReturn(0);
+        Assert.assertTrue(templateService.canCopyTemplateToImageStore(2L, 1L));
+    }
+
+    // The under-limit / at-limit behavior of canCopyTemplateToImageStore is exercised through
+    // shouldDownloadTemplateToStore above (Replicates*UnderCopyLimit / Skips*AtCopyLimit), which run it via
+    // the real call path. Calling the GlobalLock-wrapped method directly on the Mockito spy is not reliable
+    // in the unit-test JVM, so it is not duplicated here.
+
     @Test
     public void tryDownloadingTemplateToImageStoreTestDownloadsTemplateWhenUrlIsNotNull() {
         Mockito.doReturn("url").when(tmpltMock).getUrl();

server/src/main/java/com/cloud/storage/ImageStoreUploadMonitorImpl.java

@@ -451,6 +451,12 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
                             if (logger.isDebugEnabled()) {
                                 logger.debug("Template {} uploaded successfully", tmpTemplate);
                             }
+                            try {
+                                templateService.replicateTemplateUpToCap(tmpTemplate.getId(), vo.getDataCenterId());
+                            } catch (Exception e) {
+                                logger.warn("Failed to schedule additional copies for uploaded template [{}] in zone [{}]: {}",
+                                        tmpTemplate.getUuid(), vo.getDataCenterId(), e.getMessage());
+                            }
                             break;
                         case IN_PROGRESS:
                             if (tmpTemplate.getState() == VirtualMachineTemplate.State.NotUploaded) {

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;
 
@@ -292,9 +292,10 @@ protected void createTemplateWithinZones(TemplateProfile profile, VMTemplateVO t
 
             if (imageStore == null) {
                 List<DataStore> imageStores = getImageStoresThrowsExceptionIfNotFound(zoneId, profile);
-                standardImageStoreAllocation(imageStores, template);
+                standardImageStoreAllocation(imageStores, template, zoneId);
             } else {
-                validateSecondaryStorageAndCreateTemplate(List.of(imageStore), template, null);
+                int copyLimit = getSecStorageCopyLimit(template, zoneId);
+                validateSecondaryStorageAndCreateTemplate(List.of(imageStore), template, new HashMap<>(), copyLimit);
             }
         }
     }
@@ -307,17 +308,17 @@ protected List<DataStore> getImageStoresThrowsExceptionIfNotFound(long zoneId, T
         return imageStores;
     }
 
-    protected void standardImageStoreAllocation(List<DataStore> imageStores, VMTemplateVO template) {
-        Set<Long> zoneSet = new HashSet<Long>();
+    protected void standardImageStoreAllocation(List<DataStore> imageStores, VMTemplateVO template, long zoneId) {
+        int copyLimit = getSecStorageCopyLimit(template, zoneId);
         Collections.shuffle(imageStores);
-        validateSecondaryStorageAndCreateTemplate(imageStores, template, zoneSet);
+        validateSecondaryStorageAndCreateTemplate(imageStores, template, new HashMap<>(), copyLimit);
     }
 
-    protected void validateSecondaryStorageAndCreateTemplate(List<DataStore> imageStores, VMTemplateVO template, Set<Long> zoneSet) {
+    protected void validateSecondaryStorageAndCreateTemplate(List<DataStore> imageStores, VMTemplateVO template, Map<Long, Integer> zoneCopyCount, int copyLimit) {
         for (DataStore imageStore : imageStores) {
             Long zoneId = imageStore.getScope().getScopeId();
 
-            if (!isZoneAndImageStoreAvailable(imageStore, zoneId, zoneSet, isPrivateTemplate(template))) {
+            if (!isZoneAndImageStoreAvailable(imageStore, zoneId, zoneCopyCount, copyLimit)) {
                 continue;
             }