Implement enforcement of secondary storage copy limits for templates based on zone configuration

Author: Damans227

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

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

engine/components-api/src/main/java/com/cloud/template/TemplateManager.java

@@ -153,11 +153,8 @@ public interface TemplateManager {
     List<DataStore> getImageStoreByTemplate(long templateId, Long zoneId);
 
     /**
-     * Returns the maximum number of secondary storage pools a template should be copied to in the given zone,
-     * based on {@link #PublicTemplateSecStorageCopy} / {@link #PrivateTemplateSecStorageCopy}.
-     * A return value of {@code 0} (or less) means "no limit" (copy to all secondary storage pools).
-     * System, routing and builtin templates are always exempt from the limit (returns {@code 0}), as they
-     * must be available on every secondary storage pool.
+     * 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);
 

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

@@ -325,6 +325,63 @@ private boolean hasReachedSecStorageCopyLimit(VMTemplateVO template, long zoneId
         return count >= copyLimit;
     }
 
+    @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);
@@ -619,6 +676,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())) {

server/src/main/java/com/cloud/template/TemplateManagerImpl.java

@@ -1693,6 +1693,12 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
             _launchPermissionDao.removeAllPermissions(id);
             _messageBus.publish(_name, TemplateManager.MESSAGE_RESET_TEMPLATE_PERMISSION_EVENT, PublishScope.LOCAL, template.getId());
         }
+
+        if (isPublic != null || isFeatured != null || "reset".equalsIgnoreCase(operation)) {
+            for (VMTemplateZoneVO templateZone : _tmpltZoneDao.listByTemplateId(template.getId())) {
+                _tmpltSvr.enforceSecStorageCopyLimit(template.getId(), templateZone.getZoneId());
+            }
+        }
         return true;
     }
 
@@ -2183,15 +2189,12 @@ public int getSecStorageCopyLimit(VMTemplateVO template, long zoneId) {
             return 0;
         }
         TemplateType type = template.getTemplateType();
-        // System, routing and builtin templates must be available on every secondary storage pool,
-        // so they are never subject to the configured copy limit.
         if (type == TemplateType.SYSTEM || type == TemplateType.ROUTING || type == TemplateType.BUILTIN) {
             return 0;
         }
-        boolean isPrivate = !template.isPublicTemplate() && !template.isFeatured();
-        return isPrivate
-                ? PrivateTemplateSecStorageCopy.valueIn(zoneId)
-                : PublicTemplateSecStorageCopy.valueIn(zoneId);
+        return template.isPublicTemplate()
+                ? PublicTemplateSecStorageCopy.valueIn(zoneId)
+                : PrivateTemplateSecStorageCopy.valueIn(zoneId);
     }
 
     @Override