diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/PreparePrimaryStorageForMaintenanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/PreparePrimaryStorageForMaintenanceCmd.java index 9f0efe7f7a1b..d64b6a89ed4f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/PreparePrimaryStorageForMaintenanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/PreparePrimaryStorageForMaintenanceCmd.java @@ -33,8 +33,9 @@ import com.cloud.storage.StoragePool; import com.cloud.user.Account; -@APICommand(name = "enableStorageMaintenance", description = "Puts storage pool into maintenance state", responseObject = StoragePoolResponse.class, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +@APICommand(name = "enableStorageMaintenance", + description = "Puts storage pool into maintenance state. The storage pool must be Up or Disabled", + responseObject = StoragePoolResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) public class PreparePrimaryStorageForMaintenanceCmd extends BaseAsyncCmd { private static final String s_name = "prepareprimarystorageformaintenanceresponse"; diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreLifeCycle.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreLifeCycle.java index 1acaccf09df4..03da78c24b38 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreLifeCycle.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreLifeCycle.java @@ -26,6 +26,8 @@ public interface PrimaryDataStoreLifeCycle extends DataStoreLifeCycle { String CAPACITY_BYTES = "capacityBytes"; String CAPACITY_IOPS = "capacityIops"; + /** Pool detail holding the pool status to restore when maintenance is cancelled (set only for Disabled pools). */ + String STATUS_BEFORE_MAINTENANCE = "statusBeforeMaintenance"; void updateStoragePool(StoragePool storagePool, Map details); void enableStoragePool(DataStore store); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java index d17dae132a04..aec57e9781ac 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java @@ -40,8 +40,11 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProviderManager; import org.apache.cloudstack.engine.subsystem.api.storage.HostScope; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreParameters; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import com.cloud.agent.api.StoragePoolInfo; @@ -83,6 +86,8 @@ public class PrimaryDataStoreHelper { private AnnotationDao annotationDao; @Inject DataStoreProviderManager dataStoreProviderMgr; + @Inject + private StoragePoolDetailsDao storagePoolDetailsDao; public DataStore createPrimaryDataStore(PrimaryDataStoreParameters params) { if (params == null) { @@ -250,11 +255,24 @@ public boolean maintain(DataStore store) { public boolean cancelMaintain(DataStore store) { StoragePoolVO pool = this.dataStoreDao.findById(store.getId()); - pool.setStatus(StoragePoolStatus.Up); + pool.setStatus(getStatusAfterMaintenance(pool.getId())); dataStoreDao.update(store.getId(), pool); + storagePoolDetailsDao.removeDetail(pool.getId(), PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE); return true; } + /** + * A pool that was Disabled before entering maintenance goes back to Disabled, so that cancelling + * maintenance doesn't re-open it for provisioning; any other pool goes back to Up. + */ + protected StoragePoolStatus getStatusAfterMaintenance(long poolId) { + StoragePoolDetailVO statusBeforeMaintenance = storagePoolDetailsDao.findDetail(poolId, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE); + if (statusBeforeMaintenance != null && StoragePoolStatus.Disabled.name().equals(statusBeforeMaintenance.getValue())) { + return StoragePoolStatus.Disabled; + } + return StoragePoolStatus.Up; + } + public boolean disable(DataStore store) { StoragePoolVO pool = this.dataStoreDao.findById(store.getId()); pool.setStatus(StoragePoolStatus.Disabled); diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelperTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelperTest.java index 3927b43f3939..bb2e06a619be 100644 --- a/engine/storage/src/test/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelperTest.java +++ b/engine/storage/src/test/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelperTest.java @@ -21,7 +21,10 @@ import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.junit.Assert; import org.junit.Test; @@ -39,6 +42,7 @@ import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.utils.Pair; @@ -54,6 +58,9 @@ public class PrimaryDataStoreHelperTest { @Mock private StoragePoolHostDao storagePoolHostDao; + @Mock + private StoragePoolDetailsDao storagePoolDetailsDao; + @Spy @InjectMocks PrimaryDataStoreHelper dataStoreHelper; @@ -111,4 +118,35 @@ public void testSwitchToCluster() { Assert.assertEquals(capacity.getPodId(), POD_ID); Assert.assertEquals(capacity.getClusterId(), CLUSTER_ID); } + + private StoragePoolVO cancelMaintainForPoolWithRecordedStatus(StoragePoolDetailVO statusBeforeMaintenance) { + StoragePoolVO pool = new StoragePoolVO(POOL_ID, null, null, Storage.StoragePoolType.NetworkFilesystem, ZONE_ID, POD_ID, 0L, 0L, null, 0, null); + pool.setStatus(StoragePoolStatus.Maintenance); + Mockito.when(dataStoreDao.findById(POOL_ID)).thenReturn(pool); + Mockito.when(storagePoolDetailsDao.findDetail(POOL_ID, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE)).thenReturn(statusBeforeMaintenance); + DataStore storeMock = Mockito.mock(DataStore.class); + Mockito.when(storeMock.getId()).thenReturn(POOL_ID); + + dataStoreHelper.cancelMaintain(storeMock); + + Mockito.verify(dataStoreDao).update(POOL_ID, pool); + Mockito.verify(storagePoolDetailsDao).removeDetail(POOL_ID, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE); + return pool; + } + + @Test + public void cancelMaintainRestoresDisabledStatus() { + StoragePoolDetailVO detail = new StoragePoolDetailVO(POOL_ID, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE, StoragePoolStatus.Disabled.name(), false); + + StoragePoolVO pool = cancelMaintainForPoolWithRecordedStatus(detail); + + Assert.assertEquals(StoragePoolStatus.Disabled, pool.getStatus()); + } + + @Test + public void cancelMaintainSetsUpWhenNoStatusWasRecorded() { + StoragePoolVO pool = cancelMaintainForPoolWithRecordedStatus(null); + + Assert.assertEquals(StoragePoolStatus.Up, pool.getStatus()); + } } diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 69f04393174c..17964a3be6ee 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2550,7 +2550,7 @@ public PrimaryDataStoreInfo preparePrimaryStorageForMaintenance(Long primaryStor throw new InvalidParameterValueException(msg); } - if (!primaryStorage.getStatus().equals(StoragePoolStatus.Up) && !primaryStorage.getStatus().equals(StoragePoolStatus.ErrorInMaintenance)) { + if (!Arrays.asList(StoragePoolStatus.Up, StoragePoolStatus.Disabled, StoragePoolStatus.ErrorInMaintenance).contains(primaryStorage.getStatus())) { throw new InvalidParameterValueException(String.format("Primary storage %s is not ready for migration, as the status is:%s", primaryStorage, primaryStorage.getStatus().toString())); } @@ -2558,6 +2558,7 @@ public PrimaryDataStoreInfo preparePrimaryStorageForMaintenance(Long primaryStor DataStoreLifeCycle lifeCycle = provider.getDataStoreLifeCycle(); DataStore store = _dataStoreMgr.getDataStore(primaryStorage.getId(), DataStoreRole.Primary); + recordStatusBeforeMaintenance(primaryStorage); if (primaryStorage.getPoolType() == StoragePoolType.DatastoreCluster) { if (primaryStorage.getStatus() == StoragePoolStatus.PrepareForMaintenance) { throw new CloudRuntimeException(String.format("There is already a job running for preparation for maintenance of the storage pool %s", primaryStorage)); @@ -2580,6 +2581,7 @@ private void handlePrepareDatastoreClusterMaintenance(DataStoreLifeCycle lifeCyc @Override public void doInTransactionWithoutResult(TransactionStatus status) { for (StoragePoolVO childDatastore : childDatastores) { + recordStatusBeforeMaintenance(childDatastore); // set the pool state to prepare for maintenance, so that VMs will not migrate to the storagepools in the same cluster childDatastore.setStatus(StoragePoolStatus.PrepareForMaintenance); _storagePoolDao.update(childDatastore.getId(), childDatastore); @@ -2606,6 +2608,19 @@ public void doInTransactionWithoutResult(TransactionStatus status) { } } + /** + * Remembers a Disabled pool's status so that cancelling maintenance puts it back to Disabled instead of Up + * (restored in PrimaryDataStoreHelper#cancelMaintain). A retry from ErrorInMaintenance keeps what the + * first attempt recorded. + */ + protected void recordStatusBeforeMaintenance(StoragePoolVO pool) { + if (pool.getStatus() == StoragePoolStatus.Disabled) { + _storagePoolDetailsDao.addDetail(pool.getId(), PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE, StoragePoolStatus.Disabled.name(), false); + } else if (pool.getStatus() == StoragePoolStatus.Up) { + _storagePoolDetailsDao.removeDetail(pool.getId(), PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE); + } + } + @Override @DB @ActionEvent(eventType = EventTypes.EVENT_MAINTENANCE_CANCEL_PRIMARY_STORAGE, @@ -2622,7 +2637,7 @@ public PrimaryDataStoreInfo cancelPrimaryStorageForMaintenance(CancelPrimaryStor throw new InvalidParameterValueException(msg); } - if (primaryStorage.getStatus().equals(StoragePoolStatus.Up) || primaryStorage.getStatus().equals(StoragePoolStatus.PrepareForMaintenance)) { + if (Arrays.asList(StoragePoolStatus.Up, StoragePoolStatus.Disabled, StoragePoolStatus.PrepareForMaintenance).contains(primaryStorage.getStatus())) { throw new StorageUnavailableException("Primary storage " + primaryStorage + " is not ready to complete migration, as the status is:" + primaryStorage.getStatus().toString(), primaryStorageId); } diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index f46699f88cfb..3c6e5c9e23cd 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -31,6 +31,7 @@ import com.cloud.resource.ResourceManager; import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.command.admin.storage.CancelPrimaryStorageMaintenanceCmd; import org.apache.cloudstack.api.command.admin.storage.ChangeStoragePoolScopeCmd; import org.apache.cloudstack.api.command.admin.storage.ConfigureStorageAccessCmd; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -38,7 +39,9 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProviderManager; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle; import org.apache.cloudstack.framework.config.ConfigDepot; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; @@ -1727,4 +1730,73 @@ public void checkUsagedSpaceReturnsFalseForZeroCapacityPool() { Assert.assertFalse(result); } + + @Test + public void recordStatusBeforeMaintenanceStoresDisabledStatus() { + Mockito.when(storagePoolVOMock.getId()).thenReturn(1L); + Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Disabled); + + storageManagerImpl.recordStatusBeforeMaintenance(storagePoolVOMock); + + Mockito.verify(storagePoolDetailsDao).addDetail(1L, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE, StoragePoolStatus.Disabled.name(), false); + } + + @Test + public void recordStatusBeforeMaintenanceClearsStaleStatusForUpPool() { + Mockito.when(storagePoolVOMock.getId()).thenReturn(1L); + Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Up); + + storageManagerImpl.recordStatusBeforeMaintenance(storagePoolVOMock); + + Mockito.verify(storagePoolDetailsDao).removeDetail(1L, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE); + Mockito.verify(storagePoolDetailsDao, Mockito.never()).addDetail(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean()); + } + + @Test + public void recordStatusBeforeMaintenanceKeepsRecordedStatusOnRetry() { + Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.ErrorInMaintenance); + + storageManagerImpl.recordStatusBeforeMaintenance(storagePoolVOMock); + + Mockito.verifyNoInteractions(storagePoolDetailsDao); + } + + @Test + public void preparePrimaryStorageForMaintenanceAllowsDisabledPool() throws Exception { + DataStoreProvider provider = Mockito.mock(DataStoreProvider.class); + DataStoreLifeCycle lifeCycle = Mockito.mock(DataStoreLifeCycle.class); + PrimaryDataStore store = Mockito.mock(PrimaryDataStore.class); + Mockito.when(storagePoolDao.findById(1L)).thenReturn(storagePoolVOMock); + Mockito.when(storagePoolVOMock.getId()).thenReturn(1L); + Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Disabled); + Mockito.when(storagePoolVOMock.getStorageProviderName()).thenReturn("DefaultPrimary"); + Mockito.when(dataStoreProviderMgr.getDataStoreProvider("DefaultPrimary")).thenReturn(provider); + Mockito.when(provider.getDataStoreLifeCycle()).thenReturn(lifeCycle); + Mockito.when(dataStoreMgr.getDataStore(1L, DataStoreRole.Primary)).thenReturn(store); + + storageManagerImpl.preparePrimaryStorageForMaintenance(1L); + + Mockito.verify(storagePoolDetailsDao).addDetail(1L, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE, StoragePoolStatus.Disabled.name(), false); + Mockito.verify(lifeCycle).maintain(store); + } + + @Test + public void preparePrimaryStorageForMaintenanceRejectsPoolAlreadyInMaintenance() { + Mockito.when(storagePoolDao.findById(1L)).thenReturn(storagePoolVOMock); + Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Maintenance); + + assertThrows(InvalidParameterValueException.class, () -> storageManagerImpl.preparePrimaryStorageForMaintenance(1L)); + Mockito.verifyNoInteractions(storagePoolDetailsDao); + } + + @Test + public void cancelPrimaryStorageForMaintenanceRejectsDisabledPool() { + CancelPrimaryStorageMaintenanceCmd cmd = Mockito.mock(CancelPrimaryStorageMaintenanceCmd.class); + Mockito.when(cmd.getId()).thenReturn(1L); + Mockito.when(storagePoolDao.findById(1L)).thenReturn(storagePoolVOMock); + Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Disabled); + + assertThrows(StorageUnavailableException.class, () -> storageManagerImpl.cancelPrimaryStorageForMaintenance(cmd)); + Mockito.verifyNoInteractions(dataStoreProviderMgr); + } } diff --git a/ui/src/config/section/infra/primaryStorages.js b/ui/src/config/section/infra/primaryStorages.js index f127a0853b9e..465d0197991a 100644 --- a/ui/src/config/section/infra/primaryStorages.js +++ b/ui/src/config/section/infra/primaryStorages.js @@ -125,7 +125,7 @@ export default { label: 'label.action.enable.maintenance.mode', message: 'message.action.primarystorage.enable.maintenance.mode', dataView: true, - show: (record) => { return ['Up', 'Connecting', 'Down', 'ErrorInMaintenance'].includes(record.state) } + show: (record) => { return ['Up', 'Disabled', 'Connecting', 'Down', 'ErrorInMaintenance'].includes(record.state) } }, { api: 'cancelStorageMaintenance',