Refactor as per pr comments

Author: Damans227

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

@@ -45,8 +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 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);

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

@@ -294,10 +294,8 @@ protected void createTemplateWithinZones(TemplateProfile profile, VMTemplateVO t
                 List<DataStore> imageStores = getImageStoresThrowsExceptionIfNotFound(zoneId, profile);
                 standardImageStoreAllocation(imageStores, template, zoneId);
             } else {
-                int replicaLimit = isPrivateTemplate(template)
-                        ? TemplateManager.PrivateTemplateSecStorageCopy.valueIn(zoneId)
-                        : TemplateManager.PublicTemplateSecStorageCopy.valueIn(zoneId);
-                validateSecondaryStorageAndCreateTemplate(List.of(imageStore), template, new HashMap<>(), replicaLimit);
+                int copyLimit = getSecStorageCopyLimit(template, zoneId);
+                validateSecondaryStorageAndCreateTemplate(List.of(imageStore), template, new HashMap<>(), copyLimit);
             }
         }
     }
@@ -311,18 +309,16 @@ protected List<DataStore> getImageStoresThrowsExceptionIfNotFound(long zoneId, T
     }
 
     protected void standardImageStoreAllocation(List<DataStore> imageStores, VMTemplateVO template, long zoneId) {
-        int replicaLimit = isPrivateTemplate(template)
-                ? TemplateManager.PrivateTemplateSecStorageCopy.valueIn(zoneId)
-                : TemplateManager.PublicTemplateSecStorageCopy.valueIn(zoneId);
+        int copyLimit = getSecStorageCopyLimit(template, zoneId);
         Collections.shuffle(imageStores);
-        validateSecondaryStorageAndCreateTemplate(imageStores, template, new HashMap<>(), replicaLimit);
+        validateSecondaryStorageAndCreateTemplate(imageStores, template, new HashMap<>(), copyLimit);
     }
 
-    protected void validateSecondaryStorageAndCreateTemplate(List<DataStore> imageStores, VMTemplateVO template, Map<Long, Integer> zoneCopyCount, int replicaLimit) {
+    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, zoneCopyCount, replicaLimit)) {
+            if (!isZoneAndImageStoreAvailable(imageStore, zoneId, zoneCopyCount, copyLimit)) {
                 continue;
             }
 

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

@@ -167,7 +167,13 @@ protected DataStore verifyHeuristicRulesForZone(VMTemplateVO template, Long zone
         return heuristicRuleHelper.getImageStoreIfThereIsHeuristicRule(zoneId, heuristicType, template);
     }
 
-    protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId, Map<Long, Integer> zoneCopyCount, int replicaLimit) {
+    protected int getSecStorageCopyLimit(VMTemplateVO template, long zoneId) {
+        return isPrivateTemplate(template)
+                ? TemplateManager.PrivateTemplateSecStorageCopy.valueIn(zoneId)
+                : TemplateManager.PublicTemplateSecStorageCopy.valueIn(zoneId);
+    }
+
+    protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId, Map<Long, Integer> zoneCopyCount, int copyLimit) {
         if (zoneId == null) {
             logger.warn(String.format("Zone ID is null, cannot allocate ISO/template in image store [%s].", imageStore));
             return false;
@@ -190,8 +196,8 @@ protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId
         }
 
         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);
+        if (copyLimit > 0 && currentCount >= copyLimit) {
+            logger.info("Copy limit of {} reached for zone [{}]; skipping image store [{}].", copyLimit, zone, imageStore);
             return false;
         }
 
@@ -204,16 +210,13 @@ protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId
      * {@link TemplateProfile#getZoneIdList()}.
      */
     protected void postUploadAllocation(List<DataStore> imageStores, VMTemplateVO template, List<TemplateOrVolumePostUploadCommand> payloads) {
-        boolean isPrivate = isPrivateTemplate(template);
         Map<Long, Integer> zoneCopyCount = new HashMap<>();
         Collections.shuffle(imageStores);
         for (DataStore imageStore : imageStores) {
             Long zoneId_is = imageStore.getScope().getScopeId();
-            int replicaLimit = zoneId_is == null ? 0 : (isPrivate
-                    ? TemplateManager.PrivateTemplateSecStorageCopy.valueIn(zoneId_is)
-                    : TemplateManager.PublicTemplateSecStorageCopy.valueIn(zoneId_is));
+            int copyLimit = zoneId_is == null ? 0 : getSecStorageCopyLimit(template, zoneId_is);
 
-            if (!isZoneAndImageStoreAvailable(imageStore, zoneId_is, zoneCopyCount, replicaLimit)) {
+            if (!isZoneAndImageStoreAvailable(imageStore, zoneId_is, zoneCopyCount, copyLimit)) {
                 continue;
             }
 

server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java

@@ -494,7 +494,7 @@ public void isZoneAndImageStoreAvailableTestReplicaLimitReachedShouldReturnFalse
 
         boolean result = _adapter.isZoneAndImageStoreAvailable(dataStoreMock, zoneId, zoneCopyCount, 1);
 
-        Mockito.verify(loggerMock, times(1)).info("Replica limit of {} reached for zone [{}]; skipping image store [{}].", 1, dataCenterVOMock, dataStoreMock);
+        Mockito.verify(loggerMock, times(1)).info("Copy limit of {} reached for zone [{}]; skipping image store [{}].", 1, dataCenterVOMock, dataStoreMock);
         Assert.assertFalse(result);
     }