Code fixes

Author: harikrishna-patnala

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java

@@ -325,7 +325,7 @@ public Future<TemplateApiResult> orchestrateTemplateCopyToImageStore(TemplateInf
     public Future<TemplateApiResult> orchestrateTemplateCopyAcrossZones(TemplateInfo templateInfo, DataStore sourceStore, DataStore destStore) {
         Long dstZoneId = destStore.getScope().getScopeId();
         DataCenterVO dstZone = dcDao.findById(dstZoneId);
-        Long userId = CallContext.current().getCallingUserId();
+        long userId = CallContext.current().getCallingUserId();
         return submit(dstZoneId, new CrossZoneCopyTemplateTask(userId, templateInfo, sourceStore, dstZone));
     }
 

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

@@ -622,7 +622,7 @@ protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore d
             return true;
         }
 
-        logger.debug("Template [{}] not found in any image store of zone [{}]. Checking other zones",
+        logger.debug("Template [{}] not found in any image store of zone [{}]. Checking other zones.",
                 tmplt.getUniqueName(), destZoneId);
 
         return searchAndCopyAcrossZones(tmplt, destStore, destZoneId);
@@ -650,6 +650,13 @@ private boolean searchAndCopyAcrossZones(VMTemplateVO tmplt, DataStore destStore
                 continue;
             }
 
+            TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore);
+            if (sourceTmpl.getInstallPath() == null) {
+                logger.warn("Cannot copy template [{}] from image store [{}]; install path is null.",
+                        tmplt.getUniqueName(), sourceStore.getName());
+                continue;
+            }
+
             logger.info("Template [{}] found in zone [{}]. Initiating cross-zone copy to zone [{}].",
                     tmplt.getUniqueName(), otherZoneId, destZoneId);
 
@@ -701,7 +708,7 @@ private boolean copyTemplateAcrossZones(DataStore sourceStore,
         DataCenterVO dstZone = _dcDao.findById(dstZoneId);
 
         if (dstZone == null) {
-            logger.warn("Destination zone [{}] not found for template [{}]",
+            logger.warn("Destination zone [{}] not found for template [{}].",
                     dstZoneId, tmplt.getUniqueName());
             return false;
         }
@@ -711,7 +718,7 @@ private boolean copyTemplateAcrossZones(DataStore sourceStore,
             storageOrchestrator.orchestrateTemplateCopyAcrossZones(sourceTmpl, sourceStore, destStore);
             return true;
         } catch (Exception e) {
-            logger.error("Failed to copy template [{}] from zone [{}] to zone [{}]",
+            logger.error("Failed to copy template [{}] from zone [{}] to zone [{}].",
                     tmplt.getUniqueName(),
                     sourceStore.getScope().getScopeId(),
                     dstZoneId,

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

@@ -201,10 +201,11 @@ public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAno
     }
 
     @Test
-    public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() throws StorageUnavailableException, ResourceAllocationException {
+    public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherZone() {
         Scope scopeMock = Mockito.mock(Scope.class);
         Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
         Mockito.doReturn(1L).when(scopeMock).getScopeId();
+        Mockito.doReturn(100L).when(tmpltMock).getId();
         Mockito.doReturn(List.of(sourceStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
         Mockito.doReturn(null).when(templateService).listTemplate(sourceStoreMock);
         Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
@@ -216,6 +217,10 @@ public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAno
         templatesInOtherZone.put(tmpltMock.getUniqueName(), tmpltPropMock);
         Mockito.doReturn(templatesInOtherZone).when(templateService).listTemplate(otherZoneStoreMock);
 
+        TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class);
+        Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock);
+        Mockito.doReturn("/mnt/secondary/template.qcow2").when(sourceTmplMock).getInstallPath();
+
         DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
         Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);
 
@@ -229,6 +234,7 @@ public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenDestinationZoneIsM
         Scope scopeMock = Mockito.mock(Scope.class);
         Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
         Mockito.doReturn(1L).when(scopeMock).getScopeId();
+        Mockito.doReturn(100L).when(tmpltMock).getId();
         Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
         Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
 
@@ -238,6 +244,10 @@ public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenDestinationZoneIsM
         Map<String, TemplateProp> templates = new HashMap<>();
         templates.put(tmpltMock.getUniqueName(), tmpltPropMock);
         Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock);
+
+        TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class);
+        Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock);
+        Mockito.doReturn("/mnt/secondary/template.qcow2").when(sourceTmplMock).getInstallPath();
         Mockito.doReturn(null).when(_dcDao).findById(1L);
 
         boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock);
@@ -250,23 +260,20 @@ public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenCrossZoneCopyTaskIs
         Scope scopeMock = Mockito.mock(Scope.class);
         Mockito.doReturn(scopeMock).when(destStoreMock).getScope();
         Mockito.doReturn(1L).when(scopeMock).getScopeId();
-
+        Mockito.doReturn(100L).when(tmpltMock).getId();
         Mockito.doReturn(List.of(1L, 2L)).when(_dcDao).listAllIds();
-
-        Mockito.doReturn(List.of())
-                .when(dataStoreManagerMock)
-                .getImageStoresByZoneIds(1L);
+        Mockito.doReturn(List.of()).when(dataStoreManagerMock).getImageStoresByZoneIds(1L);
 
         DataStore otherZoneStoreMock = Mockito.mock(DataStore.class);
-        Mockito.doReturn(List.of(otherZoneStoreMock))
-                .when(dataStoreManagerMock)
-                .getImageStoresByZoneIds(2L);
+        Mockito.doReturn(List.of(otherZoneStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(2L);
 
         Map<String, TemplateProp> templates = new HashMap<>();
         templates.put(tmpltMock.getUniqueName(), tmpltPropMock);
-        Mockito.doReturn(templates)
-                .when(templateService)
-                .listTemplate(otherZoneStoreMock);
+        Mockito.doReturn(templates).when(templateService).listTemplate(otherZoneStoreMock);
+
+        TemplateObject sourceTmplMock = Mockito.mock(TemplateObject.class);
+        Mockito.doReturn(sourceTmplMock).when(templateDataFactoryMock).getTemplate(100L, otherZoneStoreMock);
+        Mockito.doReturn("/mnt/secondary/template.qcow2").when(sourceTmplMock).getInstallPath();
 
         DataCenterVO dstZoneMock = Mockito.mock(DataCenterVO.class);
         Mockito.doReturn(dstZoneMock).when(_dcDao).findById(1L);