Skip to content

Commit 0458c53

Browse files
winterhazelwinterhazel
authored
Consider secondary storage selectors during template synchronization (#10956)
* Consider secondary storage selectors during template synchronization * Fix checkstyle * Remove unused import
1 parent fce69fb commit 0458c53

File tree

7 files changed

+97
-70
lines changed

7 files changed

+97
-70
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
@@ -152,6 +152,8 @@ public interface TemplateManager {
152152

153153
TemplateType validateTemplateType(BaseCmd cmd, boolean isAdmin, boolean isCrossZones);
154154

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

157159
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
@@ -290,21 +290,41 @@ public void handleSysTemplateDownload(HypervisorType hostHyper, Long dcId) {
290290
}
291291
}
292292

293-
protected boolean isSkipTemplateStoreDownload(VMTemplateVO template, Long zoneId) {
294-
if (template.isPublicTemplate()) {
293+
protected boolean shouldDownloadTemplateToStore(VMTemplateVO template, DataStore store) {
294+
Long zoneId = store.getScope().getScopeId();
295+
DataStore directedStore = _tmpltMgr.verifyHeuristicRulesForZone(template, zoneId);
296+
if (directedStore != null && store.getId() != directedStore.getId()) {
297+
logger.info("Template [{}] will not be download to image store [{}], as a heuristic rule is directing it to another store.",
298+
template.getUniqueName(), store.getName());
295299
return false;
296300
}
301+
302+
if (template.isPublicTemplate()) {
303+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is public.", template.getUniqueName(),
304+
store.getName());
305+
return true;
306+
}
307+
297308
if (template.isFeatured()) {
298-
return false;
309+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is featured.", template.getUniqueName(),
310+
store.getName());
311+
return true;
299312
}
313+
300314
if (TemplateType.SYSTEM.equals(template.getTemplateType())) {
301-
return false;
315+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is a system VM template.",
316+
template.getUniqueName(),store.getName());
317+
return true;
302318
}
319+
303320
if (zoneId != null && _vmTemplateStoreDao.findByTemplateZone(template.getId(), zoneId, DataStoreRole.Image) == null) {
304-
logger.debug("Template {} is not present on any image store for the zone ID: {}, its download cannot be skipped", template, zoneId);
305-
return false;
321+
logger.debug("Download of template [{}] to image store [{}] cannot be skipped, as it is not present on any image store of zone [{}].",
322+
template.getUniqueName(), store.getName(), zoneId);
323+
return true;
306324
}
307-
return true;
325+
326+
logger.info("Skipping download of template [{}] to image store [{}].", template.getUniqueName(), store.getName());
327+
return false;
308328
}
309329

310330
@Override
@@ -531,8 +551,7 @@ public void handleTemplateSync(DataStore store) {
531551
// download.
532552
for (VMTemplateVO tmplt : toBeDownloaded) {
533553
// if this is private template, skip sync to a new image store
534-
if (isSkipTemplateStoreDownload(tmplt, zoneId)) {
535-
logger.info("Skip sync downloading private template {} to a new image store", tmplt);
554+
if (!shouldDownloadTemplateToStore(tmplt, store)) {
536555
continue;
537556
}
538557

‎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/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
@@ -2356,6 +2356,17 @@ public TemplateType validateTemplateType(BaseCmd cmd, boolean isAdmin, boolean i
23562356
return templateType;
23572357
}
23582358

2359+
@Override
2360+
public DataStore verifyHeuristicRulesForZone(VMTemplateVO template, Long zoneId) {
2361+
HeuristicType heuristicType;
2362+
if (ImageFormat.ISO.equals(template.getFormat())) {
2363+
heuristicType = HeuristicType.ISO;
2364+
} else {
2365+
heuristicType = HeuristicType.TEMPLATE;
2366+
}
2367+
return heuristicRuleHelper.getImageStoreIfThereIsHeuristicRule(zoneId, heuristicType, template);
2368+
}
2369+
23592370
void validateDetails(VMTemplateVO template, Map<String, String> details) {
23602371
if (MapUtils.isEmpty(details)) {
23612372
return;

‎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)