From 0d2f3d6864a892fea63cd5b374ddab91399677e3 Mon Sep 17 00:00:00 2001 From: Ortiga Date: Tue, 14 Apr 2026 10:28:10 -0300 Subject: [PATCH] dont download templates to read-only storages --- .../storage/image/TemplateServiceImpl.java | 9 ++++++++ .../image/TemplateServiceImplTest.java | 22 +++++++++++++++++++ .../image/BaseImageStoreDriverImpl.java | 12 +++++++--- .../template/HypervisorTemplateAdapter.java | 5 +++++ 4 files changed, 45 insertions(+), 3 deletions(-) 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 5fc9bbac3522..9374082a8644 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 @@ -64,6 +64,7 @@ import org.apache.cloudstack.storage.command.DeleteCommand; import org.apache.cloudstack.storage.datastore.DataObjectManager; import org.apache.cloudstack.storage.datastore.ObjectInDataStoreManager; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; @@ -163,6 +164,8 @@ public class TemplateServiceImpl implements TemplateService { TemplateDataFactory imageFactory; @Inject StorageOrchestrationService storageOrchestrator; + @Inject + ImageStoreDao dataStoreDao; class TemplateOpContext extends AsyncRpcContext { final TemplateObject template; @@ -295,6 +298,12 @@ public void handleSysTemplateDownload(HypervisorType hostHyper, Long dcId) { } protected boolean shouldDownloadTemplateToStore(VMTemplateVO template, DataStore store) { + if (dataStoreDao.findById(store.getId()).isReadonly()) { + logger.debug("Template [{}] will not be download to image store [{}] because this store is marked as read-only.", template.getUniqueName(), + store.getName()); + return false; + } + Long zoneId = store.getScope().getScopeId(); DataStore directedStore = _tmpltMgr.verifyHeuristicRulesForZone(template, zoneId); if (directedStore != null && store.getId() != directedStore.getId()) { 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 e9eac0458697..062f24e9b30b 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 @@ -32,6 +32,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.Scope; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; +import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; import org.apache.cloudstack.storage.image.store.TemplateObject; @@ -104,6 +106,12 @@ public class TemplateServiceImplTest { @Mock DataCenterDao _dcDao; + @Mock + ImageStoreDao imageStore; + + @Mock + ImageStoreVO imageMock; + Map templatesInSourceStore = new HashMap<>(); @Before @@ -119,11 +127,13 @@ public void setUp() { Mockito.doReturn(templateInfoMock).when(templateDataFactoryMock).getTemplate(2L, sourceStoreMock); Mockito.doReturn(3L).when(dataStoreMock).getId(); Mockito.doReturn(zoneScopeMock).when(dataStoreMock).getScope(); + Mockito.when(imageStore.findById(3L)).thenReturn(imageMock); } @Test public void shouldDownloadTemplateToStoreTestSkipsTemplateDirectedToAnotherStorage() { DataStore destinedStore = Mockito.mock(DataStore.class); + Mockito.when(imageMock.isReadonly()).thenReturn(false); Mockito.doReturn(dataStoreMock.getId() + 1L).when(destinedStore).getId(); Mockito.when(templateManagerMock.verifyHeuristicRulesForZone(tmpltMock, zoneScopeMock.getScopeId())).thenReturn(destinedStore); Assert.assertFalse(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); @@ -131,33 +141,45 @@ public void shouldDownloadTemplateToStoreTestSkipsTemplateDirectedToAnotherStora @Test public void shouldDownloadTemplateToStoreTestDownloadsPublicTemplate() { + Mockito.when(imageMock.isReadonly()).thenReturn(false); Mockito.when(tmpltMock.isPublicTemplate()).thenReturn(true); Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); } @Test public void shouldDownloadTemplateToStoreTestDownloadsFeaturedTemplate() { + Mockito.when(imageMock.isReadonly()).thenReturn(false); Mockito.when(tmpltMock.isFeatured()).thenReturn(true); Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); } @Test public void shouldDownloadTemplateToStoreTestDownloadsSystemTemplate() { + Mockito.when(imageMock.isReadonly()).thenReturn(false); Mockito.when(tmpltMock.getTemplateType()).thenReturn(Storage.TemplateType.SYSTEM); Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); } @Test public void shouldDownloadTemplateToStoreTestDownloadsPrivateNoRefTemplate() { + Mockito.when(imageMock.isReadonly()).thenReturn(false); Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); } @Test public void shouldDownloadTemplateToStoreTestSkipsPrivateExistingTemplate() { + Mockito.when(imageMock.isReadonly()).thenReturn(false); Mockito.when(templateDataStoreDao.findByTemplateZone(tmpltMock.getId(), zoneScopeMock.getScopeId(), DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class)); Assert.assertFalse(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); } + @Test + public void shouldDownloadTemplateToStoreTestSkipsWhenStorageIsReadOnly() { + Mockito.when(imageMock.isReadonly()).thenReturn(true); + Assert.assertFalse(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); + + } + @Test public void tryDownloadingTemplateToImageStoreTestDownloadsTemplateWhenUrlIsNotNull() { Mockito.doReturn("url").when(tmpltMock).getUrl(); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java index 26b39e30776f..35dd7f7b5231 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/BaseImageStoreDriverImpl.java @@ -44,6 +44,7 @@ import org.apache.cloudstack.storage.command.CommandResult; import org.apache.cloudstack.storage.command.CopyCommand; import org.apache.cloudstack.storage.command.DeleteCommand; +import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; @@ -126,6 +127,8 @@ public abstract class BaseImageStoreDriverImpl implements ImageStoreDriver { AgentManager agentMgr; @Inject DataStoreManager dataStoreManager; + @Inject + ImageStoreDao dataStoreDao; protected String _proxy = null; @@ -175,10 +178,13 @@ public void createAsync(DataStore dataStore, DataObject data, AsyncCompletionCal AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); caller.setContext(context); if (data.getType() == DataObjectType.TEMPLATE) { - caller.setCallback(caller.getTarget().createTemplateAsyncCallback(null, null)); - if (logger.isDebugEnabled()) { - logger.debug("Downloading template to data store {}", dataStore); + if (dataStoreDao.findById(dataStore.getId()).isReadonly()) { + logger.debug("Template [{}] will not be download to image store [{}] because this store is marked as read-only.", data.getName(), + dataStore.getName()); + return; } + caller.setCallback(caller.getTarget().createTemplateAsyncCallback(null, null)); + logger.debug("Downloading template [{}] to data store [{}].", data.getName(), dataStore.getName()); _downloadMonitor.downloadTemplateToStorage(data, caller); } else if (data.getType() == DataObjectType.VOLUME) { caller.setCallback(caller.getTarget().createVolumeAsyncCallback(null, null)); diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index 632add684d7a..1f3f268cb90a 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -338,6 +338,11 @@ protected boolean isZoneAndImageStoreAvailable(DataStore imageStore, Long zoneId return false; } + if (_imgStoreDao.findById(imageStore.getId()).isReadonly()) { + logger.info("Image store [{}] is marked as read-only. Skip downloading template to this image store.", imageStore); + return false; + } + if (!_statsCollector.imageStoreHasEnoughCapacity(imageStore)) { logger.info("Image store doesn't have enough capacity. Skip downloading template to this image store [{}].", imageStore); return false;