From f6802e91983fa93140f58824f0d5f5e704ca20a7 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Tue, 23 Apr 2024 12:06:45 +0200 Subject: [PATCH] server: fix duplicated records for templates if there are multiple zones (#8933) --- .../template/HypervisorTemplateAdapter.java | 8 ++++---- .../HypervisorTemplateAdapterTest.java | 19 ++++++++----------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index d8132df6d28..da47fe66311 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -306,12 +306,12 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { zonesIds = _dcDao.listAllZones().stream().map(DataCenterVO::getId).collect(Collectors.toList()); } - List imageStores = getImageStoresThrowsExceptionIfNotFound(zonesIds, profile); for (long zoneId : zonesIds) { DataStore imageStore = verifyHeuristicRulesForZone(template, zoneId); if (imageStore == null) { + List imageStores = getImageStoresThrowsExceptionIfNotFound(zoneId, profile); standardImageStoreAllocation(imageStores, template); } else { validateSecondaryStorageAndCreateTemplate(List.of(imageStore), template, null); @@ -319,8 +319,8 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { } } - protected List getImageStoresThrowsExceptionIfNotFound(List zonesIds, TemplateProfile profile) { - List imageStores = storeMgr.getImageStoresByZoneIds(zonesIds.toArray(new Long[0])); + protected List getImageStoresThrowsExceptionIfNotFound(long zoneId, TemplateProfile profile) { + List imageStores = storeMgr.getImageStoresByZoneIds(zoneId); if (imageStores == null || imageStores.size() == 0) { throw new CloudRuntimeException(String.format("Unable to find image store to download the template [%s].", profile.getTemplate())); } @@ -426,7 +426,7 @@ public class HypervisorTemplateAdapter extends TemplateAdapterBase { List payloads = new LinkedList<>(); if (imageStore == null) { - List imageStores = getImageStoresThrowsExceptionIfNotFound(List.of(zoneId), profile); + List imageStores = getImageStoresThrowsExceptionIfNotFound(zoneId, profile); postUploadAllocation(imageStores, template, payloads); } else { postUploadAllocation(List.of(imageStore), template, payloads); diff --git a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java index 8657c07b5ef..0dc5e885c40 100644 --- a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java +++ b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java @@ -151,6 +151,8 @@ public class HypervisorTemplateAdapterTest { private AutoCloseable closeable; + private static final long zoneId = 1L; + @Before public void before() { closeable = MockitoAnnotations.openMocks(this); @@ -323,7 +325,6 @@ public class HypervisorTemplateAdapterTest { VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class); Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(null); - Mockito.doReturn(null).when(_adapter).getImageStoresThrowsExceptionIfNotFound(Mockito.any(List.class), Mockito.any(TemplateProfile.class)); _adapter.createTemplateWithinZones(templateProfileMock, vmTemplateVOMock); @@ -337,7 +338,7 @@ public class HypervisorTemplateAdapterTest { List zoneIds = List.of(1L); Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds); - Mockito.doReturn(null).when(_adapter).getImageStoresThrowsExceptionIfNotFound(Mockito.any(List.class), Mockito.any(TemplateProfile.class)); + Mockito.doReturn(null).when(_adapter).getImageStoresThrowsExceptionIfNotFound(Mockito.any(Long.class), Mockito.any(TemplateProfile.class)); Mockito.doReturn(null).when(_adapter).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong()); Mockito.doNothing().when(_adapter).standardImageStoreAllocation(Mockito.isNull(), Mockito.any(VMTemplateVO.class)); @@ -350,10 +351,10 @@ public class HypervisorTemplateAdapterTest { public void createTemplateWithinZonesTestZoneDoesNotHaveActiveHeuristicRulesShouldCallStandardImageStoreAllocation() { TemplateProfile templateProfileMock = Mockito.mock(TemplateProfile.class); VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class); - List zoneIds = List.of(1L); + List zoneIds = List.of(zoneId); Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds); - Mockito.doReturn(null).when(_adapter).getImageStoresThrowsExceptionIfNotFound(Mockito.any(List.class), Mockito.any(TemplateProfile.class)); + Mockito.doReturn(null).when(_adapter).getImageStoresThrowsExceptionIfNotFound(Mockito.any(Long.class), Mockito.any(TemplateProfile.class)); Mockito.doReturn(null).when(_adapter).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong()); Mockito.doNothing().when(_adapter).standardImageStoreAllocation(Mockito.isNull(), Mockito.any(VMTemplateVO.class)); @@ -370,7 +371,6 @@ public class HypervisorTemplateAdapterTest { List zoneIds = List.of(1L); Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds); - Mockito.doReturn(null).when(_adapter).getImageStoresThrowsExceptionIfNotFound(Mockito.any(List.class), Mockito.any(TemplateProfile.class)); Mockito.doReturn(dataStoreMock).when(_adapter).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong()); Mockito.doNothing().when(_adapter).validateSecondaryStorageAndCreateTemplate(Mockito.any(List.class), Mockito.any(VMTemplateVO.class), Mockito.isNull()); @@ -382,34 +382,31 @@ public class HypervisorTemplateAdapterTest { @Test(expected = CloudRuntimeException.class) public void getImageStoresThrowsExceptionIfNotFoundTestNullImageStoreShouldThrowCloudRuntimeException() { TemplateProfile templateProfileMock = Mockito.mock(TemplateProfile.class); - List zoneIds = List.of(1L); Mockito.when(dataStoreManagerMock.getImageStoresByZoneIds(Mockito.anyLong())).thenReturn(null); - _adapter.getImageStoresThrowsExceptionIfNotFound(zoneIds, templateProfileMock); + _adapter.getImageStoresThrowsExceptionIfNotFound(zoneId, templateProfileMock); } @Test(expected = CloudRuntimeException.class) public void getImageStoresThrowsExceptionIfNotFoundTestEmptyImageStoreShouldThrowCloudRuntimeException() { TemplateProfile templateProfileMock = Mockito.mock(TemplateProfile.class); - List zoneIds = List.of(1L); List imageStoresList = new ArrayList<>(); Mockito.when(dataStoreManagerMock.getImageStoresByZoneIds(Mockito.anyLong())).thenReturn(imageStoresList); - _adapter.getImageStoresThrowsExceptionIfNotFound(zoneIds, templateProfileMock); + _adapter.getImageStoresThrowsExceptionIfNotFound(zoneId, templateProfileMock); } @Test public void getImageStoresThrowsExceptionIfNotFoundTestNonEmptyImageStoreShouldNotThrowCloudRuntimeException() { TemplateProfile templateProfileMock = Mockito.mock(TemplateProfile.class); - List zoneIds = List.of(1L); DataStore dataStoreMock = Mockito.mock(DataStore.class); List imageStoresList = List.of(dataStoreMock); Mockito.when(dataStoreManagerMock.getImageStoresByZoneIds(Mockito.anyLong())).thenReturn(imageStoresList); - _adapter.getImageStoresThrowsExceptionIfNotFound(zoneIds, templateProfileMock); + _adapter.getImageStoresThrowsExceptionIfNotFound(zoneId, templateProfileMock); } @Test