Skip to content

Commit 54c61c9

Browse files
code refactoring
1 parent d277218 commit 54c61c9

File tree

2 files changed

+19
-16
lines changed

2 files changed

+19
-16
lines changed

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

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,11 @@ protected TemplateObject findUsableTemplate(VMTemplateVO tmplt, List<DataStore>
665665
continue;
666666
}
667667

668+
Map<String, TemplateProp> templates = listTemplate(store);
669+
if (templates == null || !templates.containsKey(tmplt.getUniqueName())) {
670+
continue;
671+
}
672+
668673
if (tmpl.getInstallPath() == null) {
669674
logger.debug("Template [{}] found in image store [{}] but install path is null. Skipping.",
670675
tmplt.getUniqueName(), store.getName());
@@ -678,24 +683,14 @@ protected TemplateObject findUsableTemplate(VMTemplateVO tmplt, List<DataStore>
678683
private boolean searchAndCopyWithinZone(VMTemplateVO tmplt, DataStore destStore) {
679684
Long destZoneId = destStore.getScope().getScopeId();
680685
List<DataStore> storesInSameZone = _storeMgr.getImageStoresByZoneIds(destZoneId);
681-
for (DataStore sourceStore : storesInSameZone) {
682-
Map<String, TemplateProp> existingTemplatesInSourceStore = listTemplate(sourceStore);
683-
if (existingTemplatesInSourceStore == null ||
684-
!existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) {
685-
logger.debug("Template [{}] does not exist on image store [{}]; searching another.", tmplt.getUniqueName(), sourceStore.getName());
686-
continue;
687-
}
688686

689-
TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore);
690-
if (sourceTmpl.getInstallPath() == null) {
691-
logger.warn("Cannot copy template [{}] from image store [{}]; install path is null.", tmplt.getUniqueName(), sourceStore.getName());
692-
continue;
693-
}
694-
695-
storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore);
696-
return true;
687+
TemplateObject sourceTmpl = findUsableTemplate(tmplt, storesInSameZone);
688+
if (sourceTmpl == null) {
689+
return false;
697690
}
698-
return false;
691+
692+
storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore);
693+
return true;
699694
}
700695

701696
private boolean copyTemplateAcrossZones(DataStore destStore, TemplateObject sourceTmpl) {

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,8 @@ public void testFindUsableTemplateReturnsTemplateWithNonNullInstallPath() {
302302
Mockito.when(storeWithNullPath.getName()).thenReturn("store-null");
303303

304304
DataStore storeWithValidPath = Mockito.mock(DataStore.class);
305+
Mockito.when(storeWithValidPath.getName()).thenReturn("store-valid");
306+
305307
TemplateObject tmplWithNullPath = Mockito.mock(TemplateObject.class);
306308
Mockito.when(tmplWithNullPath.getInstallPath()).thenReturn(null);
307309

@@ -311,6 +313,12 @@ public void testFindUsableTemplateReturnsTemplateWithNonNullInstallPath() {
311313
Mockito.doReturn(tmplWithNullPath).when(templateDataFactoryMock).getTemplate(10L, storeWithNullPath);
312314
Mockito.doReturn(tmplWithValidPath).when(templateDataFactoryMock).getTemplate(10L, storeWithValidPath);
313315

316+
Map<String, TemplateProp> templates = new HashMap<>();
317+
templates.put("test-template", Mockito.mock(TemplateProp.class));
318+
319+
Mockito.doReturn(templates).when(templateService).listTemplate(storeWithNullPath);
320+
Mockito.doReturn(templates).when(templateService).listTemplate(storeWithValidPath);
321+
314322
List<DataStore> imageStores = List.of(storeWithNullPath, storeWithValidPath);
315323

316324
TemplateObject result = templateService.findUsableTemplate(template, imageStores);

0 commit comments

Comments
 (0)