This is an automated email from the ASF dual-hosted git repository.
bstoyanov pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new 0458c5328bf Consider secondary storage selectors during template
synchronization (#10956)
0458c5328bf is described below
commit 0458c5328bf03c1a2f4d291a2e3895e158e7abd7
Author: Fabricio Duarte <[email protected]>
AuthorDate: Tue Jan 27 06:05:09 2026 -0300
Consider secondary storage selectors during template synchronization
(#10956)
* Consider secondary storage selectors during template synchronization
* Fix checkstyle
* Remove unused import
---
.../java/com/cloud/template/TemplateManager.java | 2 +
.../storage/image/TemplateServiceImpl.java | 37 +++++++++++----
.../storage/image/TemplateServiceImplTest.java | 55 ++++++++++++----------
.../cloud/template/HypervisorTemplateAdapter.java | 15 +-----
.../com/cloud/template/TemplateManagerImpl.java | 11 +++++
.../template/HypervisorTemplateAdapterTest.java | 27 ++---------
.../cloud/template/TemplateManagerImplTest.java | 20 ++++++++
7 files changed, 97 insertions(+), 70 deletions(-)
diff --git
a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
index 7191bd5b1d7..1dde19d7a5d 100644
---
a/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
+++
b/engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
@@ -152,6 +152,8 @@ public interface TemplateManager {
TemplateType validateTemplateType(BaseCmd cmd, boolean isAdmin, boolean
isCrossZones);
+ DataStore verifyHeuristicRulesForZone(VMTemplateVO template, Long zoneId);
+
List<DatadiskTO> getTemplateDisksOnImageStore(VirtualMachineTemplate
template, DataStoreRole role, String configurationId);
static Boolean getValidateUrlIsResolvableBeforeRegisteringTemplateValue() {
diff --git
a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java
b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java
index c18be7c7335..bee62955051 100644
---
a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java
+++
b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java
@@ -290,21 +290,41 @@ public class TemplateServiceImpl implements
TemplateService {
}
}
- protected boolean isSkipTemplateStoreDownload(VMTemplateVO template, Long
zoneId) {
- if (template.isPublicTemplate()) {
+ protected boolean shouldDownloadTemplateToStore(VMTemplateVO template,
DataStore store) {
+ Long zoneId = store.getScope().getScopeId();
+ DataStore directedStore =
_tmpltMgr.verifyHeuristicRulesForZone(template, zoneId);
+ if (directedStore != null && store.getId() != directedStore.getId()) {
+ logger.info("Template [{}] will not be download to image store
[{}], as a heuristic rule is directing it to another store.",
+ template.getUniqueName(), store.getName());
return false;
}
+
+ if (template.isPublicTemplate()) {
+ logger.debug("Download of template [{}] to image store [{}] cannot
be skipped, as it is public.", template.getUniqueName(),
+ store.getName());
+ return true;
+ }
+
if (template.isFeatured()) {
- return false;
+ logger.debug("Download of template [{}] to image store [{}] cannot
be skipped, as it is featured.", template.getUniqueName(),
+ store.getName());
+ return true;
}
+
if (TemplateType.SYSTEM.equals(template.getTemplateType())) {
- return false;
+ logger.debug("Download of template [{}] to image store [{}] cannot
be skipped, as it is a system VM template.",
+ template.getUniqueName(),store.getName());
+ return true;
}
+
if (zoneId != null &&
_vmTemplateStoreDao.findByTemplateZone(template.getId(), zoneId,
DataStoreRole.Image) == null) {
- logger.debug("Template {} is not present on any image store for
the zone ID: {}, its download cannot be skipped", template, zoneId);
- return false;
+ logger.debug("Download of template [{}] to image store [{}] cannot
be skipped, as it is not present on any image store of zone [{}].",
+ template.getUniqueName(), store.getName(), zoneId);
+ return true;
}
- return true;
+
+ logger.info("Skipping download of template [{}] to image store [{}].",
template.getUniqueName(), store.getName());
+ return false;
}
@Override
@@ -531,8 +551,7 @@ public class TemplateServiceImpl implements TemplateService
{
// download.
for (VMTemplateVO tmplt : toBeDownloaded) {
// if this is private template, skip sync to a new
image store
- if (isSkipTemplateStoreDownload(tmplt, zoneId)) {
- logger.info("Skip sync downloading private
template {} to a new image store", tmplt);
+ if (!shouldDownloadTemplateToStore(tmplt, store)) {
continue;
}
diff --git
a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java
b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java
index 276581e2e48..cb7994915b3 100644
---
a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java
+++
b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java
@@ -19,6 +19,7 @@
package org.apache.cloudstack.storage.image;
import com.cloud.storage.template.TemplateProp;
+import com.cloud.template.TemplateManager;
import
org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -70,6 +71,9 @@ public class TemplateServiceImplTest {
@Mock
TemplateObject templateInfoMock;
+ @Mock
+ DataStore dataStoreMock;
+
@Mock
DataStore sourceStoreMock;
@@ -82,6 +86,9 @@ public class TemplateServiceImplTest {
@Mock
StorageOrchestrationService storageOrchestrator;
+ @Mock
+ TemplateManager templateManagerMock;
+
Map<String, TemplateProp> templatesInSourceStore = new HashMap<>();
@Before
@@ -96,45 +103,45 @@ public class TemplateServiceImplTest {
Mockito.doReturn(null).when(templateService).listTemplate(destStoreMock);
Mockito.doReturn("install-path").when(templateInfoMock).getInstallPath();
Mockito.doReturn(templateInfoMock).when(templateDataFactoryMock).getTemplate(2L,
sourceStoreMock);
+ Mockito.doReturn(3L).when(dataStoreMock).getId();
+ Mockito.doReturn(zoneScopeMock).when(dataStoreMock).getScope();
+ }
+
+ @Test
+ public void
shouldDownloadTemplateToStoreTestSkipsTemplateDirectedToAnotherStorage() {
+ DataStore destinedStore = Mockito.mock(DataStore.class);
+ Mockito.doReturn(dataStoreMock.getId() +
1L).when(destinedStore).getId();
+
Mockito.when(templateManagerMock.verifyHeuristicRulesForZone(tmpltMock,
zoneScopeMock.getScopeId())).thenReturn(destinedStore);
+
Assert.assertFalse(templateService.shouldDownloadTemplateToStore(tmpltMock,
dataStoreMock));
}
@Test
- public void testIsSkipTemplateStoreDownloadPublicTemplate() {
- VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
- Mockito.when(templateVO.isPublicTemplate()).thenReturn(true);
-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, 1L));
+ public void shouldDownloadTemplateToStoreTestDownloadsPublicTemplate() {
+ Mockito.when(tmpltMock.isPublicTemplate()).thenReturn(true);
+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock,
dataStoreMock));
}
@Test
- public void testIsSkipTemplateStoreDownloadFeaturedTemplate() {
- VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
- Mockito.when(templateVO.isFeatured()).thenReturn(true);
-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, 1L));
+ public void shouldDownloadTemplateToStoreTestDownloadsFeaturedTemplate() {
+ Mockito.when(tmpltMock.isFeatured()).thenReturn(true);
+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock,
dataStoreMock));
}
@Test
- public void testIsSkipTemplateStoreDownloadSystemTemplate() {
- VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
-
Mockito.when(templateVO.getTemplateType()).thenReturn(Storage.TemplateType.SYSTEM);
-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, 1L));
+ public void shouldDownloadTemplateToStoreTestDownloadsSystemTemplate() {
+
Mockito.when(tmpltMock.getTemplateType()).thenReturn(Storage.TemplateType.SYSTEM);
+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock,
dataStoreMock));
}
@Test
- public void testIsSkipTemplateStoreDownloadPrivateNoRefTemplate() {
- VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
- long id = 1L;
- Mockito.when(templateVO.getId()).thenReturn(id);
- Mockito.when(templateDataStoreDao.findByTemplateZone(id, id,
DataStoreRole.Image)).thenReturn(null);
-
Assert.assertFalse(templateService.isSkipTemplateStoreDownload(templateVO, id));
+ public void
shouldDownloadTemplateToStoreTestDownloadsPrivateNoRefTemplate() {
+
Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock,
dataStoreMock));
}
@Test
- public void testIsSkipTemplateStoreDownloadPrivateExistingTemplate() {
- VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class);
- long id = 1L;
- Mockito.when(templateVO.getId()).thenReturn(id);
- Mockito.when(templateDataStoreDao.findByTemplateZone(id, id,
DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class));
-
Assert.assertTrue(templateService.isSkipTemplateStoreDownload(templateVO, id));
+ public void
shouldDownloadTemplateToStoreTestSkipsPrivateExistingTemplate() {
+
Mockito.when(templateDataStoreDao.findByTemplateZone(tmpltMock.getId(),
zoneScopeMock.getScopeId(),
DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class));
+
Assert.assertFalse(templateService.shouldDownloadTemplateToStore(tmpltMock,
dataStoreMock));
}
@Test
diff --git
a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java
b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java
index fb4ea94ae3d..1422e788e24 100644
--- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java
+++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java
@@ -61,7 +61,6 @@ import
org.apache.cloudstack.framework.async.AsyncCompletionCallback;
import org.apache.cloudstack.framework.async.AsyncRpcContext;
import org.apache.cloudstack.framework.messagebus.MessageBus;
import org.apache.cloudstack.framework.messagebus.PublishScope;
-import org.apache.cloudstack.secstorage.heuristics.HeuristicType;
import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
@@ -308,7 +307,7 @@ public class HypervisorTemplateAdapter extends
TemplateAdapterBase {
for (long zoneId : zonesIds) {
- DataStore imageStore = verifyHeuristicRulesForZone(template,
zoneId);
+ DataStore imageStore =
templateMgr.verifyHeuristicRulesForZone(template, zoneId);
if (imageStore == null) {
List<DataStore> imageStores =
getImageStoresThrowsExceptionIfNotFound(zoneId, profile);
@@ -327,16 +326,6 @@ public class HypervisorTemplateAdapter extends
TemplateAdapterBase {
return imageStores;
}
- protected DataStore verifyHeuristicRulesForZone(VMTemplateVO template,
Long zoneId) {
- HeuristicType heuristicType;
- if (ImageFormat.ISO.equals(template.getFormat())) {
- heuristicType = HeuristicType.ISO;
- } else {
- heuristicType = HeuristicType.TEMPLATE;
- }
- return heuristicRuleHelper.getImageStoreIfThereIsHeuristicRule(zoneId,
heuristicType, template);
- }
-
protected void standardImageStoreAllocation(List<DataStore> imageStores,
VMTemplateVO template) {
Set<Long> zoneSet = new HashSet<Long>();
Collections.shuffle(imageStores);
@@ -432,7 +421,7 @@ public class HypervisorTemplateAdapter extends
TemplateAdapterBase {
}
Long zoneId = zoneIdList.get(0);
- DataStore imageStore = verifyHeuristicRulesForZone(template,
zoneId);
+ DataStore imageStore =
templateMgr.verifyHeuristicRulesForZone(template, zoneId);
List<TemplateOrVolumePostUploadCommand> payloads = new
LinkedList<>();
if (imageStore == null) {
diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
index 2c7d2d593e3..5773410c35a 100755
--- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
+++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java
@@ -2356,6 +2356,17 @@ public class TemplateManagerImpl extends ManagerBase
implements TemplateManager,
return templateType;
}
+ @Override
+ public DataStore verifyHeuristicRulesForZone(VMTemplateVO template, Long
zoneId) {
+ HeuristicType heuristicType;
+ if (ImageFormat.ISO.equals(template.getFormat())) {
+ heuristicType = HeuristicType.ISO;
+ } else {
+ heuristicType = HeuristicType.TEMPLATE;
+ }
+ return heuristicRuleHelper.getImageStoreIfThereIsHeuristicRule(zoneId,
heuristicType, template);
+ }
+
void validateDetails(VMTemplateVO template, Map<String, String> details) {
if (MapUtils.isEmpty(details)) {
return;
diff --git
a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java
b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java
index 2a6d7af434a..e2a97be469f 100644
--- a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java
+++ b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java
@@ -49,7 +49,6 @@ import
org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.events.Event;
import org.apache.cloudstack.framework.events.EventDistributor;
import org.apache.cloudstack.framework.messagebus.MessageBus;
-import org.apache.cloudstack.secstorage.heuristics.HeuristicType;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
import org.apache.cloudstack.storage.heuristics.HeuristicRuleHelper;
@@ -339,7 +338,7 @@ public class HypervisorTemplateAdapterTest {
Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds);
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.doReturn(null).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class),
Mockito.anyLong());
Mockito.doNothing().when(_adapter).standardImageStoreAllocation(Mockito.isNull(),
Mockito.any(VMTemplateVO.class));
_adapter.createTemplateWithinZones(templateProfileMock,
vmTemplateVOMock);
@@ -355,7 +354,7 @@ public class HypervisorTemplateAdapterTest {
Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds);
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.doReturn(null).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class),
Mockito.anyLong());
Mockito.doNothing().when(_adapter).standardImageStoreAllocation(Mockito.isNull(),
Mockito.any(VMTemplateVO.class));
_adapter.createTemplateWithinZones(templateProfileMock,
vmTemplateVOMock);
@@ -371,7 +370,7 @@ public class HypervisorTemplateAdapterTest {
List<Long> zoneIds = List.of(1L);
Mockito.when(templateProfileMock.getZoneIdList()).thenReturn(zoneIds);
-
Mockito.doReturn(dataStoreMock).when(_adapter).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class),
Mockito.anyLong());
+
Mockito.doReturn(dataStoreMock).when(_templateMgr).verifyHeuristicRulesForZone(Mockito.any(VMTemplateVO.class),
Mockito.anyLong());
Mockito.doNothing().when(_adapter).validateSecondaryStorageAndCreateTemplate(Mockito.any(List.class),
Mockito.any(VMTemplateVO.class), Mockito.isNull());
_adapter.createTemplateWithinZones(templateProfileMock,
vmTemplateVOMock);
@@ -409,26 +408,6 @@ public class HypervisorTemplateAdapterTest {
_adapter.getImageStoresThrowsExceptionIfNotFound(zoneId,
templateProfileMock);
}
- @Test
- public void
verifyHeuristicRulesForZoneTestTemplateIsISOFormatShouldCheckForISOHeuristicType()
{
- VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class);
-
- Mockito.when(vmTemplateVOMock.getFormat()).thenReturn(ImageFormat.ISO);
- _adapter.verifyHeuristicRulesForZone(vmTemplateVOMock, 1L);
-
- Mockito.verify(heuristicRuleHelperMock,
Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L, HeuristicType.ISO,
vmTemplateVOMock);
- }
-
- @Test
- public void
verifyHeuristicRulesForZoneTestTemplateNotISOFormatShouldCheckForTemplateHeuristicType()
{
- VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class);
-
-
Mockito.when(vmTemplateVOMock.getFormat()).thenReturn(ImageFormat.QCOW2);
- _adapter.verifyHeuristicRulesForZone(vmTemplateVOMock, 1L);
-
- Mockito.verify(heuristicRuleHelperMock,
Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L,
HeuristicType.TEMPLATE, vmTemplateVOMock);
- }
-
@Test
public void
isZoneAndImageStoreAvailableTestZoneIdIsNullShouldReturnFalse() {
DataStore dataStoreMock = Mockito.mock(DataStore.class);
diff --git
a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java
b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java
index 9680fe5e1fd..576930e46f4 100755
--- a/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java
+++ b/server/src/test/java/com/cloud/template/TemplateManagerImplTest.java
@@ -753,6 +753,26 @@ public class TemplateManagerImplTest {
Assert.assertNull(type);
}
+ @Test
+ public void
verifyHeuristicRulesForZoneTestTemplateIsISOFormatShouldCheckForISOHeuristicType()
{
+ VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class);
+
+
Mockito.when(vmTemplateVOMock.getFormat()).thenReturn(Storage.ImageFormat.ISO);
+ templateManager.verifyHeuristicRulesForZone(vmTemplateVOMock, 1L);
+
+ Mockito.verify(heuristicRuleHelperMock,
Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L, HeuristicType.ISO,
vmTemplateVOMock);
+ }
+
+ @Test
+ public void
verifyHeuristicRulesForZoneTestTemplateNotISOFormatShouldCheckForTemplateHeuristicType()
{
+ VMTemplateVO vmTemplateVOMock = Mockito.mock(VMTemplateVO.class);
+
+
Mockito.when(vmTemplateVOMock.getFormat()).thenReturn(Storage.ImageFormat.QCOW2);
+ templateManager.verifyHeuristicRulesForZone(vmTemplateVOMock, 1L);
+
+ Mockito.verify(heuristicRuleHelperMock,
Mockito.times(1)).getImageStoreIfThereIsHeuristicRule(1L,
HeuristicType.TEMPLATE, vmTemplateVOMock);
+ }
+
@Configuration
@ComponentScan(basePackageClasses = {TemplateManagerImpl.class},
includeFilters = {@ComponentScan.Filter(value =
TestConfiguration.Library.class, type = FilterType.CUSTOM)},
