Skip to content

Commit 15c1be4

Browse files
winterhazelwinterhazel
committed
Consider secondary storage selectors during template synchronization
1 parent 823080c commit 15c1be4

File tree

7 files changed

+111
-69
lines changed

7 files changed

+111
-69
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
@@ -143,6 +143,8 @@ public interface TemplateManager {
143143

144144
TemplateType validateTemplateType(BaseCmd cmd, boolean isAdmin, boolean isCrossZones);
145145

146+
DataStore verifyHeuristicRulesForZone(VMTemplateVO template, Long zoneId);
147+
146148
List<DatadiskTO> getTemplateDisksOnImageStore(VirtualMachineTemplate template, DataStoreRole role, String configurationId);
147149

148150
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
@@ -288,21 +288,41 @@ public void handleSysTemplateDownload(HypervisorType hostHyper, Long dcId) {
288288
}
289289
}
290290

291-
protected boolean isSkipTemplateStoreDownload(VMTemplateVO template, Long zoneId) {
292-
if (template.isPublicTemplate()) {
291+
protected boolean shouldDownloadTemplateToStore(VMTemplateVO template, DataStore store) {
292+
Long zoneId = store.getScope().getScopeId();
293+
DataStore directedStore = _tmpltMgr.verifyHeuristicRulesForZone(template, zoneId);
294+
if (directedStore != null && store.getId() != directedStore.getId()) {
295+
logger.info("Template [{}] will not be download to image store [{}], as a heuristic rule is directing it to another store.",
296+
template.getUniqueName(), store.getName());
293297
return false;
294298
}
299+
300+
if (template.isPublicTemplate()) {
301+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is public.", template.getUniqueName(),
302+
store.getName());
303+
return true;
304+
}
305+
295306
if (template.isFeatured()) {
296-
return false;
307+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is featured.", template.getUniqueName(),
308+
store.getName());
309+
return true;
297310
}
311+
298312
if (TemplateType.SYSTEM.equals(template.getTemplateType())) {
299-
return false;
313+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is a system VM template.",
314+
template.getUniqueName(),store.getName());
315+
return true;
300316
}
317+
301318
if (zoneId != null && _vmTemplateStoreDao.findByTemplateZone(template.getId(), zoneId, DataStoreRole.Image) == null) {
302-
logger.debug("Template {} is not present on any image store for the zone ID: {}, its download cannot be skipped", template, zoneId);
303-
return false;
319+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is not present on any image store of zone [{}].",
320+
template.getUniqueName(), store.getName(), zoneId);
321+
return true;
304322
}
305-
return true;
323+
324+
logger.info("Skipping download of template [{}] to image store [{}].", template.getUniqueName(), store.getName());
325+
return false;
306326
}
307327

308328
@Override
@@ -534,8 +554,7 @@ public void handleTemplateSync(DataStore store) {
534554
continue;
535555
}
536556
// if this is private template, skip sync to a new image store
537-
if (isSkipTemplateStoreDownload(tmplt, zoneId)) {
538-
logger.info("Skip sync downloading private template {} to a new image store", tmplt);
557+
if (!shouldDownloadTemplateToStore(tmplt, store)) {
539558
continue;
540559
}
541560

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

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,13 @@
1818
*/
1919
package org.apache.cloudstack.storage.image;
2020

21+
import com.cloud.template.TemplateManager;
22+
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
23+
import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
2124
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
2225
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
2326
import org.junit.Assert;
27+
import org.junit.Before;
2428
import org.junit.Test;
2529
import org.junit.runner.RunWith;
2630
import org.mockito.InjectMocks;
@@ -43,42 +47,59 @@ public class TemplateServiceImplTest {
4347
@Mock
4448
TemplateDataStoreDao templateDataStoreDao;
4549

50+
@Mock
51+
DataStore dataStoreMock;
52+
53+
@Mock
54+
TemplateManager templateManagerMock;
55+
56+
@Mock
57+
VMTemplateVO templateVoMock;
58+
59+
@Mock
60+
ZoneScope zoneScopeMock;
61+
62+
@Before
63+
public void setUp() {
64+
Mockito.doReturn(3L).when(dataStoreMock).getId();
65+
Mockito.doReturn(4L).when(templateVoMock).getId();
66+
Mockito.doReturn(zoneScopeMock).when(dataStoreMock).getScope();
67+
}
68+
69+
@Test
70+
public void shouldDownloadTemplateToStoreTestSkipsTemplateDirectedToAnotherStorage() {
71+
DataStore destinedStore = Mockito.mock(DataStore.class);
72+
Mockito.doReturn(dataStoreMock.getId() + 1L).when(destinedStore).getId();
73+
Mockito.when(templateManagerMock.verifyHeuristicRulesForZone(templateVoMock, zoneScopeMock.getScopeId())).thenReturn(destinedStore);
74+
Assert.assertFalse(templateService.shouldDownloadTemplateToStore(templateVoMock, dataStoreMock));
75+
}
76+
4677
@Test
47-
public void testIsSkipTemplateStoreDownloadPublicTemplate() {
48-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
49-
Mockito.when(templateVO.isPublicTemplate()).thenReturn(true);
50-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, 1L));
78+
public void shouldDownloadTemplateToStoreTestDownloadsPublicTemplate() {
79+
Mockito.when(templateVoMock.isPublicTemplate()).thenReturn(true);
80+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(templateVoMock, dataStoreMock));
5181
}
5282

5383
@Test
54-
public void testIsSkipTemplateStoreDownloadFeaturedTemplate() {
55-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
56-
Mockito.when(templateVO.isFeatured()).thenReturn(true);
57-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, 1L));
84+
public void shouldDownloadTemplateToStoreTestDownloadsFeaturedTemplate() {
85+
Mockito.when(templateVoMock.isFeatured()).thenReturn(true);
86+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(templateVoMock, dataStoreMock));
5887
}
5988

6089
@Test
61-
public void testIsSkipTemplateStoreDownloadSystemTemplate() {
62-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
63-
Mockito.when(templateVO.getTemplateType()).thenReturn(Storage.TemplateType.SYSTEM);
64-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, 1L));
90+
public void shouldDownloadTemplateToStoreTestDownloadsSystemTemplate() {
91+
Mockito.when(templateVoMock.getTemplateType()).thenReturn(Storage.TemplateType.SYSTEM);
92+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(templateVoMock, dataStoreMock));
6593
}
6694

6795
@Test
68-
public void testIsSkipTemplateStoreDownloadPrivateNoRefTemplate() {
69-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
70-
long id = 1L;
71-
Mockito.when(templateVO.getId()).thenReturn(id);
72-
Mockito.when(templateDataStoreDao.findByTemplateZone(id, id, DataStoreRole.Image)).thenReturn(null);
73-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, id));
96+
public void shouldDownloadTemplateToStoreTestDownloadsPrivateNoRefTemplate() {
97+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(templateVoMock, dataStoreMock));
7498
}
7599

76100
@Test
77-
public void testIsSkipTemplateStoreDownloadPrivateExistingTemplate() {
78-
VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
79-
long id = 1L;
80-
Mockito.when(templateVO.getId()).thenReturn(id);
81-
Mockito.when(templateDataStoreDao.findByTemplateZone(id, id, DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class));
82-
Assert.assertTrue(templateService.isSkipTemplateStoreDownload(templateVO, id));
101+
public void shouldDownloadTemplateToStoreTestSkipsPrivateExistingTemplate() {
102+
Mockito.when(templateDataStoreDao.findByTemplateZone(templateVoMock.getId(), zoneScopeMock.getScopeId(), DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class));
103+
Assert.assertFalse(templateService.shouldDownloadTemplateToStore(templateVoMock, dataStoreMock));
83104
}
84105
}

‎server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161
import org.apache.cloudstack.framework.async.AsyncRpcContext;
6262
import org.apache.cloudstack.framework.messagebus.MessageBus;
6363
import org.apache.cloudstack.framework.messagebus.PublishScope;
64-
import org.apache.cloudstack.secstorage.heuristics.HeuristicType;
6564
import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
6665
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
6766
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
@@ -308,7 +307,7 @@ protected void createTemplateWithinZones(TemplateProfile profile, VMTemplateVO t
308307

309308

310309
for (long zoneId : zonesIds) {
311-
DataStore imageStore = verifyHeuristicRulesForZone(template, zoneId);
310+
DataStore imageStore = templateMgr.verifyHeuristicRulesForZone(template, zoneId);
312311

313312
if (imageStore == null) {
314313
List<DataStore> imageStores = getImageStoresThrowsExceptionIfNotFound(zoneId, profile);
@@ -327,16 +326,6 @@ protected List<DataStore> getImageStoresThrowsExceptionIfNotFound(long zoneId, T
327326
return imageStores;
328327
}
329328

330-
protected DataStore verifyHeuristicRulesForZone(VMTemplateVO template, Long zoneId) {
331-
HeuristicType heuristicType;
332-
if (ImageFormat.ISO.equals(template.getFormat())) {
333-
heuristicType = HeuristicType.ISO;
334-
} else {
335-
heuristicType = HeuristicType.TEMPLATE;
336-
}
337-
return heuristicRuleHelper.getImageStoreIfThereIsHeuristicRule(zoneId, heuristicType, template);
338-
}
339-
340329
protected void standardImageStoreAllocation(List<DataStore> imageStores, VMTemplateVO template) {
341330
Set<Long> zoneSet = new HashSet<Long>();
342331
Collections.shuffle(imageStores);
@@ -432,7 +421,7 @@ public List<TemplateOrVolumePostUploadCommand> doInTransaction(TransactionStatus
432421
}
433422

434423
Long zoneId = zoneIdList.get(0);
435-
DataStore imageStore = verifyHeuristicRulesForZone(template, zoneId);
424+
DataStore imageStore = templateMgr.verifyHeuristicRulesForZone(template, zoneId);
436425
List<TemplateOrVolumePostUploadCommand> payloads = new LinkedList<>();
437426

438427
if (imageStore == null) {

‎server/src/main/java/com/cloud/template/TemplateManagerImpl.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2327,6 +2327,17 @@ public TemplateType validateTemplateType(BaseCmd cmd, boolean isAdmin, boolean i
23272327
return templateType;
23282328
}
23292329

2330+
@Override
2331+
public DataStore verifyHeuristicRulesForZone(VMTemplateVO template, Long zoneId) {
2332+
HeuristicType heuristicType;
2333+
if (ImageFormat.ISO.equals(template.getFormat())) {
2334+
heuristicType = HeuristicType.ISO;
2335+
} else {
2336+
heuristicType = HeuristicType.TEMPLATE;
2337+
}
2338+
return heuristicRuleHelper.getImageStoreIfThereIsHeuristicRule(zoneId, heuristicType, template);
2339+
}
2340+
23302341
void validateDetails(VMTemplateVO template, Map<String, String> details) {
23312342
if (MapUtils.isEmpty(details)) {
23322343
return;

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

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ public void createTemplateWithinZonesTestZoneIdsNotNullShouldNotCallListAllZones
339339

340340
Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds);
341341
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());
342+
Mockito.doReturn(null).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
343343
Mockito.doNothing().when(_adapter).standardImageStoreAllocation(Mockito.isNull(), Mockito.any(VMTemplateVO.class));
344344

345345
_adapter.createTemplateWithinZones(templateProfileMock, vmTemplateVOMock);
@@ -355,7 +355,7 @@ public void createTemplateWithinZonesTestZoneDoesNotHaveActiveHeuristicRulesShou
355355

356356
Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds);
357357
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());
358+
Mockito.doReturn(null).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
359359
Mockito.doNothing().when(_adapter).standardImageStoreAllocation(Mockito.isNull(), Mockito.any(VMTemplateVO.class));
360360

361361
_adapter.createTemplateWithinZones(templateProfileMock, vmTemplateVOMock);
@@ -371,7 +371,7 @@ public void createTemplateWithinZonesTestZoneWithHeuristicRuleShouldCallValidate
371371
List<Long> zoneIds = List.of(1L);
372372

373373
Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds);
374-
Mockito.doReturn(dataStoreMock).when(_adapter).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
374+
Mockito.doReturn(dataStoreMock).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class), Mockito.anyLong());
375375
Mockito.doNothing().when(_adapter).validateSecondaryStorageAndCreateTemplate(Mockito.any(List.class), Mockito.any(VMTemplateVO.class), Mockito.isNull());
376376

377377
_adapter.createTemplateWithinZones(templateProfileMock, vmTemplateVOMock);
@@ -409,26 +409,6 @@ public void getImageStoresThrowsExceptionIfNotFoundTestNonEmptyImageStoreShouldN
409409
_adapter.getImageStoresThrowsExceptionIfNotFound(zoneId, templateProfileMock);
410410
}
411411

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-
432412
@Test
433413
public void isZoneAndImageStoreAvailableTestZoneIdIsNullShouldReturnFalse() {
434414
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
@@ -750,6 +750,26 @@ public void testDeleteTemplateWithTemplateType() {
750750
Assert.assertNull(type);
751751
}
752752

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

0 commit comments

Comments
 (0)