Allow maintenance of a disabled primary storage

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";
 

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<String, String> details);
     void enableStoragePool(DataStore store);

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

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());
+    }
 }

server/src/main/java/com/cloud/storage/StorageManagerImpl.java

@@ -2550,14 +2550,15 @@ 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()));
         }
 
         DataStoreProvider provider = _dataStoreProviderMgr.getDataStoreProvider(primaryStorage.getStorageProviderName());
         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);
         }

server/src/test/java/com/cloud/storage/StorageManagerImplTest.java

@@ -31,14 +31,17 @@
 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;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreLifeCycle;
 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);
+    }
 }

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',