diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java index de26a09c6e51..d572330078fd 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java @@ -60,4 +60,6 @@ public interface DataStoreManager { DataStore getImageStoreByUuid(String uuid); Long getStoreZoneId(long storeId, DataStoreRole role); + + boolean isRemovedOrReadonly(DataStore store); } 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..907616362af9 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 imageStoreDao; class TemplateOpContext extends AsyncRpcContext { final TemplateObject template; @@ -295,10 +298,14 @@ public void handleSysTemplateDownload(HypervisorType hostHyper, Long dcId) { } protected boolean shouldDownloadTemplateToStore(VMTemplateVO template, DataStore store) { + if (_storeMgr.isRemovedOrReadonly(store)) { + return false; + } + 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.", + logger.info("Template [{}] will not be downloaded to image store [{}], as a heuristic rule is directing it to another store.", template.getUniqueName(), store.getName()); return false; } 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..95d115473907 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 imageStoreDao; + + @Mock + ImageStoreVO imageStoreMock; + 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(imageStoreDao.findById(3L)).thenReturn(imageStoreMock); } @Test public void shouldDownloadTemplateToStoreTestSkipsTemplateDirectedToAnotherStorage() { DataStore destinedStore = Mockito.mock(DataStore.class); + Mockito.when(imageStoreMock.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(imageStoreMock.isReadonly()).thenReturn(false); Mockito.when(tmpltMock.isPublicTemplate()).thenReturn(true); Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); } @Test public void shouldDownloadTemplateToStoreTestDownloadsFeaturedTemplate() { + Mockito.when(imageStoreMock.isReadonly()).thenReturn(false); Mockito.when(tmpltMock.isFeatured()).thenReturn(true); Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); } @Test public void shouldDownloadTemplateToStoreTestDownloadsSystemTemplate() { + Mockito.when(imageStoreMock.isReadonly()).thenReturn(false); Mockito.when(tmpltMock.getTemplateType()).thenReturn(Storage.TemplateType.SYSTEM); Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); } @Test public void shouldDownloadTemplateToStoreTestDownloadsPrivateNoRefTemplate() { + Mockito.when(imageStoreMock.isReadonly()).thenReturn(false); Assert.assertTrue(templateService.shouldDownloadTemplateToStore(tmpltMock, dataStoreMock)); } @Test public void shouldDownloadTemplateToStoreTestSkipsPrivateExistingTemplate() { + Mockito.when(imageStoreMock.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(imageStoreMock.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/datastore/DataStoreManagerImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java index 757623e3d044..53adcedcfc19 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java @@ -26,7 +26,11 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope; 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.object.datastore.ObjectStoreProviderManager; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.springframework.stereotype.Component; import org.apache.cloudstack.storage.image.datastore.ImageStoreProviderManager; @@ -37,12 +41,16 @@ @Component public class DataStoreManagerImpl implements DataStoreManager { + protected Logger logger = LogManager.getLogger(DataStoreManagerImpl.class); + @Inject PrimaryDataStoreProviderManager primaryStoreMgr; @Inject ImageStoreProviderManager imageDataStoreMgr; @Inject ObjectStoreProviderManager objectStoreProviderMgr; + @Inject + ImageStoreDao imageStoreDao; @Override public DataStore getDataStore(long storeId, DataStoreRole role) { @@ -199,4 +207,18 @@ public Long getStoreZoneId(long storeId, DataStoreRole role) { } catch (CloudRuntimeException ignored) {} return null; } + + @Override + public boolean isRemovedOrReadonly(DataStore store) { + ImageStoreVO storeVO = imageStoreDao.findById(store.getId()); + if (storeVO == null) { + logger.debug("Could not find image store with id [{}], skipping it.", store.getId()); + return true; + } + if (storeVO.isReadonly()) { + logger.debug("Image store [{}] is read-only, skipping it.", storeVO); + return true; + } + return false; + } } 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..9a65865e3805 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 imageStoreDao; 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 (dataStoreManager.isRemovedOrReadonly(dataStore)) { + DownloadAnswer ans = new DownloadAnswer("Data store is removed or in read-only mode", VMTemplateStorageResourceAssoc.Status.UNKNOWN); + caller.complete(ans); + 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; diff --git a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java index e2a97be469ff..e2f660892db9 100644 --- a/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java +++ b/server/src/test/java/com/cloud/template/HypervisorTemplateAdapterTest.java @@ -49,6 +49,8 @@ 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.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.heuristics.HeuristicRuleHelper; @@ -137,6 +139,9 @@ public class HypervisorTemplateAdapterTest { @Mock StatsCollector statsCollectorMock; + @Mock + ImageStoreDao _imgStoreDao; + @Mock Logger loggerMock; @@ -458,11 +463,13 @@ public void isZoneAndImageStoreAvailableTestZoneIsDisabledShouldReturnFalse() { @Test public void isZoneAndImageStoreAvailableTestImageStoreDoesNotHaveEnoughCapacityShouldReturnFalse() { DataStore dataStoreMock = Mockito.mock(DataStore.class); + ImageStoreVO ImageStoreVOMock = Mockito.mock(ImageStoreVO.class); Long zoneId = 1L; Set zoneSet = null; boolean isTemplatePrivate = false; DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); + Mockito.when(_imgStoreDao.findById(Mockito.anyLong())).thenReturn(ImageStoreVOMock); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); Mockito.when(statsCollectorMock.imageStoreHasEnoughCapacity(any(DataStore.class))).thenReturn(false); @@ -477,11 +484,13 @@ public void isZoneAndImageStoreAvailableTestImageStoreDoesNotHaveEnoughCapacityS @Test public void isZoneAndImageStoreAvailableTestImageStoreHasEnoughCapacityAndZoneSetIsNullShouldReturnTrue() { DataStore dataStoreMock = Mockito.mock(DataStore.class); + ImageStoreVO ImageStoreVOMock = Mockito.mock(ImageStoreVO.class); Long zoneId = 1L; Set zoneSet = null; boolean isTemplatePrivate = false; DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); + Mockito.when(_imgStoreDao.findById(Mockito.anyLong())).thenReturn(ImageStoreVOMock); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); Mockito.when(statsCollectorMock.imageStoreHasEnoughCapacity(any(DataStore.class))).thenReturn(true); @@ -496,11 +505,13 @@ public void isZoneAndImageStoreAvailableTestImageStoreHasEnoughCapacityAndZoneSe @Test public void isZoneAndImageStoreAvailableTestTemplateIsPrivateAndItIsAlreadyAllocatedToTheSameZoneShouldReturnFalse() { DataStore dataStoreMock = Mockito.mock(DataStore.class); + ImageStoreVO ImageStoreVOMock = Mockito.mock(ImageStoreVO.class); Long zoneId = 1L; Set zoneSet = Set.of(1L); boolean isTemplatePrivate = true; DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); + Mockito.when(_imgStoreDao.findById(Mockito.anyLong())).thenReturn(ImageStoreVOMock); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); Mockito.when(statsCollectorMock.imageStoreHasEnoughCapacity(any(DataStore.class))).thenReturn(true); @@ -515,11 +526,13 @@ public void isZoneAndImageStoreAvailableTestTemplateIsPrivateAndItIsAlreadyAlloc @Test public void isZoneAndImageStoreAvailableTestTemplateIsPrivateAndItIsNotAlreadyAllocatedToTheSameZoneShouldReturnTrue() { DataStore dataStoreMock = Mockito.mock(DataStore.class); + ImageStoreVO imageStoreVoMock = Mockito.mock(ImageStoreVO.class); Long zoneId = 1L; Set zoneSet = new HashSet<>(); boolean isTemplatePrivate = true; DataCenterVO dataCenterVOMock = Mockito.mock(DataCenterVO.class); + Mockito.when(_imgStoreDao.findById(Mockito.anyLong())).thenReturn(imageStoreVoMock); Mockito.when(_dcDao.findById(Mockito.anyLong())).thenReturn(dataCenterVOMock); Mockito.when(dataCenterVOMock.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); Mockito.when(statsCollectorMock.imageStoreHasEnoughCapacity(any(DataStore.class))).thenReturn(true);