Refactor template storage handling to improve secondary storage copy limit logic and update related tests

Author: Damans227

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

@@ -421,8 +421,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
@@ -652,7 +653,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;
                             }

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

@@ -153,8 +153,23 @@ 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));
     }
 

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

@@ -204,8 +204,10 @@ protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId
     }
 
     /**
-     * If the template/ISO is marked as private, then it is allocated to a random secondary storage; otherwise, allocates to every storage pool in every zone given by the
-     * {@link TemplateProfile#getZoneIdList()}.
+     * Allocates the template/ISO to a single image store - the one the file will be uploaded to. The upload can only
+     * target one secondary store, so additional copies (up to the configured secstorage.public/private.template.copy.max)
+     * are propagated later by template sync instead of being pre-allocated here as empty placeholder entries that never
+     * receive the data.
      */
     protected void postUploadAllocation(List<DataStore> imageStores, VMTemplateVO template, List<TemplateOrVolumePostUploadCommand> payloads) {
         Map<Long, Integer> zoneCopyCount = new HashMap<>();
@@ -248,6 +250,11 @@ protected void postUploadAllocation(List<DataStore> imageStores, VMTemplateVO te
             payload.setRequiresHvm(template.requiresHvm());
             payload.setDescription(template.getDisplayText());
             payloads.add(payload);
+
+            // The file can only be uploaded to a single secondary store. Allocate just this one; additional copies
+            // up to the configured secondary storage copy limit are propagated afterwards by template sync, so we do
+            // not create empty placeholder template_store_ref rows on the other stores.
+            break;
         }
     }