Skip to content

Commit 0f844ec

Browse files
winterhazelLocharla, Sandeep
winterhazel
authored and
Locharla, Sandeep
committed
Consider secondary storage selectors during template synchronization (apache#10956)
* Consider secondary storage selectors during template synchronization * Fix checkstyle * Remove unused import
1 parent a074b3d commit 0f844ec

File tree

5 files changed

+84
-57
lines changed

5 files changed

+84
-57
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ public interface TemplateManager {
153153

154154
TemplateType validateTemplateType(BaseCmd cmd, boolean isAdmin, boolean isCrossZones, Hypervisor.HypervisorType hypervisorType);
155155

156+
DataStore verifyHeuristicRulesForZone(VMTemplateVO template, Long zoneId);
157+
156158
List<DatadiskTO> getTemplateDisksOnImageStore(VirtualMachineTemplate template, DataStoreRole role, String configurationId);
157159

158160
static Boolean getValidateUrlIsResolvableBeforeRegisteringTemplateValue() {

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

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -291,21 +291,41 @@ public void handleSysTemplateDownload(HypervisorType hostHyper, Long dcId) {
291291
}
292292
}
293293

294-
protected boolean isSkipTemplateStoreDownload(VMTemplateVO template, Long zoneId) {
295-
if (template.isPublicTemplate()) {
294+
protected boolean shouldDownloadTemplateToStore(VMTemplateVO template, DataStore store) {
295+
Long zoneId = store.getScope().getScopeId();
296+
DataStore directedStore = _tmpltMgr.verifyHeuristicRulesForZone(template, zoneId);
297+
if (directedStore != null && store.getId() != directedStore.getId()) {
298+
logger.info("Template [{}] will not be download to image store [{}], as a heuristic rule is directing it to another store.",
299+
template.getUniqueName(), store.getName());
296300
return false;
297301
}
302+
303+
if (template.isPublicTemplate()) {
304+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is public.", template.getUniqueName(),
305+
store.getName());
306+
return true;
307+
}
308+
298309
if (template.isFeatured()) {
299-
return false;
310+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is featured.", template.getUniqueName(),
311+
store.getName());
312+
return true;
300313
}
314+
301315
if (TemplateType.SYSTEM.equals(template.getTemplateType())) {
302-
return false;
316+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is a system VM template.",
317+
template.getUniqueName(),store.getName());
318+
return true;
303319
}
320+
304321
if (zoneId != null && _vmTemplateStoreDao.findByTemplateZone(template.getId(), zoneId, DataStoreRole.Image) == null) {
305-
logger.debug("Template {} is not present on any image store for the zone ID: {}, its download cannot be skipped", template, zoneId);
306-
return false;
322+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is not present on any image store of zone [{}].",
323+
template.getUniqueName(), store.getName(), zoneId);
324+
return true;
307325
}
308-
return true;
326+
327+
logger.info("Skipping download of template [{}] to image store [{}].", template.getUniqueName(), store.getName());
328+
return false;
309329
}
310330

311331
@Override
@@ -533,8 +553,7 @@ public void handleTemplateSync(DataStore store) {
533553
// download.
534554
for (VMTemplateVO tmplt : toBeDownloaded) {
535555
// if this is private template, skip sync to a new image store
536-
if (isSkipTemplateStoreDownload(tmplt, zoneId)) {
537-
logger.info("Skip sync downloading private template {} to a new image store", tmplt);
556+
if (!shouldDownloadTemplateToStore(tmplt, store)) {
538557
continue;
539558
}
540559

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

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package org.apache.cloudstack.storage.image;
2020

2121
import com.cloud.storage.template.TemplateProp;
22+
import com.cloud.template.TemplateManager;
2223
import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
2324
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
2425
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -70,6 +71,9 @@ public class TemplateServiceImplTest {
7071
@Mock
7172
TemplateObject templateInfoMock;
7273

74+
@Mock
75+
DataStore dataStoreMock;
76+
7377
@Mock
7478
DataStore sourceStoreMock;
7579

@@ -82,6 +86,9 @@ public class TemplateServiceImplTest {
8286
@Mock
8387
StorageOrchestrationService storageOrchestrator;
8488

89+
@Mock
90+
TemplateManager templateManagerMock;
91+
8592
Map<String, TemplateProp> templatesInSourceStore = new HashMap<>();
8693

8794
@Before
@@ -96,45 +103,45 @@ public void setUp() {
96103
Mockito.doReturn(null).when(templateService).listTemplate(destStoreMock);
97104
Mockito.doReturn("install-path").when(templateInfoMock).getInstallPath();
98105
Mockito.doReturn(templateInfoMock).when(templateDataFactoryMock).getTemplate(2L, sourceStoreMock);
106+
Mockito.doReturn(3L).when(dataStoreMock).getId();
107+
Mockito.doReturn(zoneScopeMock).when(dataStoreMock).getScope();
108+
}
109+
110+
@Test
111+
public void shouldDownloadTemplateToStoreTestSkipsTemplateDirectedToAnotherStorage() {
112+
DataStore destinedStore = Mockito.mock(DataStore.class);
113+
Mockito.doReturn(dataStoreMock.getId() + 1L).when(destinedStore).getId();
114+
Mockito.when(templateManagerMock.verifyHeuristicRulesForZone(tmpltMock, zoneScopeMock.getScopeId())).thenReturn(destinedStore);
115+
Assert.assertFalse(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock));
99116
}
100117

101118
@Test
102-
public void testIsSkipTemplateStoreDownloadPublicTemplate() {
103-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
104-
Mockito.when(templateVO.isPublicTemplate()).thenReturn(true);
105-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, 1L));
119+
public void shouldDownloadTemplateToStoreTestDownloadsPublicTemplate() {
120+
Mockito.when(tmpltMock.isPublicTemplate()).thenReturn(true);
121+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock));
106122
}
107123

108124
@Test
109-
public void testIsSkipTemplateStoreDownloadFeaturedTemplate() {
110-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
111-
Mockito.when(templateVO.isFeatured()).thenReturn(true);
112-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, 1L));
125+
public void shouldDownloadTemplateToStoreTestDownloadsFeaturedTemplate() {
126+
Mockito.when(tmpltMock.isFeatured()).thenReturn(true);
127+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock));
113128
}
114129

115130
@Test
116-
public void testIsSkipTemplateStoreDownloadSystemTemplate() {
117-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
118-
Mockito.when(templateVO.getTemplateType()).thenReturn(Storage.TemplateType.SYSTEM);
119-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, 1L));
131+
public void shouldDownloadTemplateToStoreTestDownloadsSystemTemplate() {
132+
Mockito.when(tmpltMock.getTemplateType()).thenReturn(Storage.TemplateType.SYSTEM);
133+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock));
120134
}
121135

122136
@Test
123-
public void testIsSkipTemplateStoreDownloadPrivateNoRefTemplate() {
124-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
125-
long id = 1L;
126-
Mockito.when(templateVO.getId()).thenReturn(id);
127-
Mockito.when(templateDataStoreDao.findByTemplateZone(id, id, DataStoreRole.Image)).thenReturn(null);
128-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, id));
137+
public void shouldDownloadTemplateToStoreTestDownloadsPrivateNoRefTemplate() {
138+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock));
129139
}
130140

131141
@Test
132-
public void testIsSkipTemplateStoreDownloadPrivateExistingTemplate() {
133-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
134-
long id = 1L;
135-
Mockito.when(templateVO.getId()).thenReturn(id);
136-
Mockito.when(templateDataStoreDao.findByTemplateZone(id, id, DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class));
137-
Assert.assertTrue(templateService.isSkipTemplateStoreDownload(templateVO, id));
142+
public void shouldDownloadTemplateToStoreTestSkipsPrivateExistingTemplate() {
143+
Mockito.when(templateDataStoreDao.findByTemplateZone(tmpltMock.getId(), zoneScopeMock.getScopeId(), DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class));
144+
Assert.assertFalse(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock));
138145
}
139146

140147
@Test

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

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@
4949
import org.apache.cloudstack.framework.events.Event;
5050
import org.apache.cloudstack.framework.events.EventDistributor;
5151
import org.apache.cloudstack.framework.messagebus.MessageBus;
52-
import org.apache.cloudstack.secstorage.heuristics.HeuristicType;
5352
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
5453
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
5554
import org.apache.cloudstack.storage.heuristics.HeuristicRuleHelper;
@@ -339,7 +338,7 @@ public void createTemplateWithinZonesTestZoneIdsNotNullShouldNotCallListAllZones
339338

340339
Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds);
341340
Mockito.doReturn(null).when(_adapter).getImageStoresThrowsExceptionIfNotFound(Mockito.any(Long.class), Mockito.any(TemplateProfile.class));
342-
Mockito.doReturn(null).when(_adapter).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
341+
Mockito.doReturn(null).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
343342
Mockito.doNothing().when(_adapter).standardImageStoreAllocation(Mockito.isNull(), Mockito.any(VMTemplateVO.class));
344343

345344
_adapter.createTemplateWithinZones(templateProfileMock, vmTemplateVOMock);
@@ -355,7 +354,7 @@ public void createTemplateWithinZonesTestZoneDoesNotHaveActiveHeuristicRulesShou
355354

356355
Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds);
357356
Mockito.doReturn(null).when(_adapter).getImageStoresThrowsExceptionIfNotFound(Mockito.any(Long.class), Mockito.any(TemplateProfile.class));
358-
Mockito.doReturn(null).when(_adapter).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
357+
Mockito.doReturn(null).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
359358
Mockito.doNothing().when(_adapter).standardImageStoreAllocation(Mockito.isNull(), Mockito.any(VMTemplateVO.class));
360359

361360
_adapter.createTemplateWithinZones(templateProfileMock, vmTemplateVOMock);
@@ -371,7 +370,7 @@ public void createTemplateWithinZonesTestZoneWithHeuristicRuleShouldCallValidate
371370
List<Long> zoneIds = List.of(1L);
372371

373372
Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds);
374-
Mockito.doReturn(dataStoreMock).when(_adapter).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
373+
Mockito.doReturn(dataStoreMock).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
375374
Mockito.doNothing().when(_adapter).validateSecondaryStorageAndCreateTemplate(Mockito.any(List.class), Mockito.any(VMTemplateVO.class), Mockito.isNull());
376375

377376
_adapter.createTemplateWithinZones(templateProfileMock, vmTemplateVOMock);
@@ -409,26 +408,6 @@ public void getImageStoresThrowsExceptionIfNotFoundTestNonEmptyImageStoreShouldN
409408
_adapter.getImageStoresThrowsExceptionIfNotFound(zoneId, templateProfileMock);
410409
}
411410

412-
@Test
413-
public void verifyHeuristicRulesForZoneTestTemplateIsISOFormatShouldCheckForISOHeuristicType() {
414-
VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class);
415-
416-
Mockito.when(vmTemplateVOMock.getFormat()).thenReturn(ImageFormat.ISO);
417-
_adapter.verifyHeuristicRulesForZone(vmTemplateVOMock, 1L);
418-
419-
Mockito.verify(heuristicRuleHelperMock, Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L, HeuristicType.ISO, vmTemplateVOMock);
420-
}
421-
422-
@Test
423-
public void verifyHeuristicRulesForZoneTestTemplateNotISOFormatShouldCheckForTemplateHeuristicType() {
424-
VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class);
425-
426-
Mockito.when(vmTemplateVOMock.getFormat()).thenReturn(ImageFormat.QCOW2);
427-
_adapter.verifyHeuristicRulesForZone(vmTemplateVOMock, 1L);
428-
429-
Mockito.verify(heuristicRuleHelperMock, Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L, HeuristicType.TEMPLATE, vmTemplateVOMock);
430-
}
431-
432411
@Test
433412
public void isZoneAndImageStoreAvailableTestZoneIdIsNullShouldReturnFalse() {
434413
DataStore dataStoreMock = Mockito.mock(DataStore.class);

‎server/src/test/java/com/cloud/template/TemplateManagerImplTest.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,26 @@ public void testDeleteTemplateWithTemplateType() {
753753
Assert.assertNull(type);
754754
}
755755

756+
@Test
757+
public void verifyHeuristicRulesForZoneTestTemplateIsISOFormatShouldCheckForISOHeuristicType() {
758+
VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class);
759+
760+
Mockito.when(vmTemplateVOMock.getFormat()).thenReturn(Storage.ImageFormat.ISO);
761+
templateManager.verifyHeuristicRulesForZone(vmTemplateVOMock, 1L);
762+
763+
Mockito.verify(heuristicRuleHelperMock, Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L, HeuristicType.ISO, vmTemplateVOMock);
764+
}
765+
766+
@Test
767+
public void verifyHeuristicRulesForZoneTestTemplateNotISOFormatShouldCheckForTemplateHeuristicType() {
768+
VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class);
769+
770+
Mockito.when(vmTemplateVOMock.getFormat()).thenReturn(Storage.ImageFormat.QCOW2);
771+
templateManager.verifyHeuristicRulesForZone(vmTemplateVOMock, 1L);
772+
773+
Mockito.verify(heuristicRuleHelperMock, Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L, HeuristicType.TEMPLATE, vmTemplateVOMock);
774+
}
775+
756776
@Configuration
757777
@ComponentScan(basePackageClasses = {TemplateManagerImpl.class},
758778
includeFilters = {@ComponentScan.Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)},

0 commit comments

Comments
 (0)