Skip to content

Commit f9d607e

Browse files
vishesh92vishesh92
committed
Allow maintenance of a disabled primary storage
1 parent d3f0700 commit f9d607e

7 files changed

Lines changed: 152 additions & 6 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/api/command/admin/storage/PreparePrimaryStorageForMaintenanceCmd.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@
3333
import com.cloud.storage.StoragePool;
3434
import com.cloud.user.Account;
3535

36-
@APICommand(name = "enableStorageMaintenance", description = "Puts storage pool into maintenance state", responseObject = StoragePoolResponse.class,
37-
requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
36+
@APICommand(name = "enableStorageMaintenance",
37+
description = "Puts storage pool into maintenance state. The storage pool must be Up or Disabled",
38+
responseObject = StoragePoolResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
3839
public class PreparePrimaryStorageForMaintenanceCmd extends BaseAsyncCmd {
3940
private static final String s_name = "prepareprimarystorageformaintenanceresponse";
4041

‎engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreLifeCycle.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
public interface PrimaryDataStoreLifeCycle extends DataStoreLifeCycle {
2727
String CAPACITY_BYTES = "capacityBytes";
2828
String CAPACITY_IOPS = "capacityIops";
29+
/** Pool detail holding the pool status to restore when maintenance is cancelled (set only for Disabled pools). */
30+
String STATUS_BEFORE_MAINTENANCE = "statusBeforeMaintenance";
2931

3032
void updateStoragePool(StoragePool storagePool, Map<String, String> details);
3133
void enableStoragePool(DataStore store);

‎engine/storage/src/main/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,11 @@
4040
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
4141
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProviderManager;
4242
import org.apache.cloudstack.engine.subsystem.api.storage.HostScope;
43+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle;
4344
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreParameters;
4445
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
46+
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
47+
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
4548
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
4649

4750
import com.cloud.agent.api.StoragePoolInfo;
@@ -83,6 +86,8 @@ public class PrimaryDataStoreHelper {
8386
private AnnotationDao annotationDao;
8487
@Inject
8588
DataStoreProviderManager dataStoreProviderMgr;
89+
@Inject
90+
private StoragePoolDetailsDao storagePoolDetailsDao;
8691

8792
public DataStore createPrimaryDataStore(PrimaryDataStoreParameters params) {
8893
if (params == null) {
@@ -250,11 +255,24 @@ public boolean maintain(DataStore store) {
250255

251256
public boolean cancelMaintain(DataStore store) {
252257
StoragePoolVO pool = this.dataStoreDao.findById(store.getId());
253-
pool.setStatus(StoragePoolStatus.Up);
258+
pool.setStatus(getStatusAfterMaintenance(pool.getId()));
254259
dataStoreDao.update(store.getId(), pool);
260+
storagePoolDetailsDao.removeDetail(pool.getId(), PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE);
255261
return true;
256262
}
257263

264+
/**
265+
* A pool that was Disabled before entering maintenance goes back to Disabled, so that cancelling
266+
* maintenance doesn't re-open it for provisioning; any other pool goes back to Up.
267+
*/
268+
protected StoragePoolStatus getStatusAfterMaintenance(long poolId) {
269+
StoragePoolDetailVO statusBeforeMaintenance = storagePoolDetailsDao.findDetail(poolId, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE);
270+
if (statusBeforeMaintenance != null && StoragePoolStatus.Disabled.name().equals(statusBeforeMaintenance.getValue())) {
271+
return StoragePoolStatus.Disabled;
272+
}
273+
return StoragePoolStatus.Up;
274+
}
275+
258276
public boolean disable(DataStore store) {
259277
StoragePoolVO pool = this.dataStoreDao.findById(store.getId());
260278
pool.setStatus(StoragePoolStatus.Disabled);

‎engine/storage/src/test/java/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelperTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@
2121

2222
import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope;
2323
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
24+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle;
2425
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
26+
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
27+
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
2528
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
2629
import org.junit.Assert;
2730
import org.junit.Test;
@@ -39,6 +42,7 @@
3942
import com.cloud.storage.ScopeType;
4043
import com.cloud.storage.Storage;
4144
import com.cloud.storage.StoragePoolHostVO;
45+
import com.cloud.storage.StoragePoolStatus;
4246
import com.cloud.storage.dao.StoragePoolHostDao;
4347
import com.cloud.utils.Pair;
4448

@@ -54,6 +58,9 @@ public class PrimaryDataStoreHelperTest {
5458
@Mock
5559
private StoragePoolHostDao storagePoolHostDao;
5660

61+
@Mock
62+
private StoragePoolDetailsDao storagePoolDetailsDao;
63+
5764
@Spy
5865
@InjectMocks
5966
PrimaryDataStoreHelper dataStoreHelper;
@@ -111,4 +118,35 @@ public void testSwitchToCluster() {
111118
Assert.assertEquals(capacity.getPodId(), POD_ID);
112119
Assert.assertEquals(capacity.getClusterId(), CLUSTER_ID);
113120
}
121+
122+
private StoragePoolVO cancelMaintainForPoolWithRecordedStatus(StoragePoolDetailVO statusBeforeMaintenance) {
123+
StoragePoolVO pool = new StoragePoolVO(POOL_ID, null, null, Storage.StoragePoolType.NetworkFilesystem, ZONE_ID, POD_ID, 0L, 0L, null, 0, null);
124+
pool.setStatus(StoragePoolStatus.Maintenance);
125+
Mockito.when(dataStoreDao.findById(POOL_ID)).thenReturn(pool);
126+
Mockito.when(storagePoolDetailsDao.findDetail(POOL_ID, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE)).thenReturn(statusBeforeMaintenance);
127+
DataStore storeMock = Mockito.mock(DataStore.class);
128+
Mockito.when(storeMock.getId()).thenReturn(POOL_ID);
129+
130+
dataStoreHelper.cancelMaintain(storeMock);
131+
132+
Mockito.verify(dataStoreDao).update(POOL_ID, pool);
133+
Mockito.verify(storagePoolDetailsDao).removeDetail(POOL_ID, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE);
134+
return pool;
135+
}
136+
137+
@Test
138+
public void cancelMaintainRestoresDisabledStatus() {
139+
StoragePoolDetailVO detail = new StoragePoolDetailVO(POOL_ID, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE, StoragePoolStatus.Disabled.name(), false);
140+
141+
StoragePoolVO pool = cancelMaintainForPoolWithRecordedStatus(detail);
142+
143+
Assert.assertEquals(StoragePoolStatus.Disabled, pool.getStatus());
144+
}
145+
146+
@Test
147+
public void cancelMaintainSetsUpWhenNoStatusWasRecorded() {
148+
StoragePoolVO pool = cancelMaintainForPoolWithRecordedStatus(null);
149+
150+
Assert.assertEquals(StoragePoolStatus.Up, pool.getStatus());
151+
}
114152
}

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2550,14 +2550,15 @@ public PrimaryDataStoreInfo preparePrimaryStorageForMaintenance(Long primaryStor
25502550
throw new InvalidParameterValueException(msg);
25512551
}
25522552

2553-
if (!primaryStorage.getStatus().equals(StoragePoolStatus.Up) && !primaryStorage.getStatus().equals(StoragePoolStatus.ErrorInMaintenance)) {
2553+
if (!Arrays.asList(StoragePoolStatus.Up, StoragePoolStatus.Disabled, StoragePoolStatus.ErrorInMaintenance).contains(primaryStorage.getStatus())) {
25542554
throw new InvalidParameterValueException(String.format("Primary storage %s is not ready for migration, as the status is:%s", primaryStorage, primaryStorage.getStatus().toString()));
25552555
}
25562556

25572557
DataStoreProvider provider = _dataStoreProviderMgr.getDataStoreProvider(primaryStorage.getStorageProviderName());
25582558
DataStoreLifeCycle lifeCycle = provider.getDataStoreLifeCycle();
25592559
DataStore store = _dataStoreMgr.getDataStore(primaryStorage.getId(), DataStoreRole.Primary);
25602560

2561+
recordStatusBeforeMaintenance(primaryStorage);
25612562
if (primaryStorage.getPoolType() == StoragePoolType.DatastoreCluster) {
25622563
if (primaryStorage.getStatus() == StoragePoolStatus.PrepareForMaintenance) {
25632564
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
25802581
@Override
25812582
public void doInTransactionWithoutResult(TransactionStatus status) {
25822583
for (StoragePoolVO childDatastore : childDatastores) {
2584+
recordStatusBeforeMaintenance(childDatastore);
25832585
// set the pool state to prepare for maintenance, so that VMs will not migrate to the storagepools in the same cluster
25842586
childDatastore.setStatus(StoragePoolStatus.PrepareForMaintenance);
25852587
_storagePoolDao.update(childDatastore.getId(), childDatastore);
@@ -2606,6 +2608,19 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
26062608
}
26072609
}
26082610

2611+
/**
2612+
* Remembers a Disabled pool's status so that cancelling maintenance puts it back to Disabled instead of Up
2613+
* (restored in PrimaryDataStoreHelper#cancelMaintain). A retry from ErrorInMaintenance keeps what the
2614+
* first attempt recorded.
2615+
*/
2616+
protected void recordStatusBeforeMaintenance(StoragePoolVO pool) {
2617+
if (pool.getStatus() == StoragePoolStatus.Disabled) {
2618+
_storagePoolDetailsDao.addDetail(pool.getId(), PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE, StoragePoolStatus.Disabled.name(), false);
2619+
} else if (pool.getStatus() == StoragePoolStatus.Up) {
2620+
_storagePoolDetailsDao.removeDetail(pool.getId(), PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE);
2621+
}
2622+
}
2623+
26092624
@Override
26102625
@DB
26112626
@ActionEvent(eventType = EventTypes.EVENT_MAINTENANCE_CANCEL_PRIMARY_STORAGE,
@@ -2622,7 +2637,7 @@ public PrimaryDataStoreInfo cancelPrimaryStorageForMaintenance(CancelPrimaryStor
26222637
throw new InvalidParameterValueException(msg);
26232638
}
26242639

2625-
if (primaryStorage.getStatus().equals(StoragePoolStatus.Up) || primaryStorage.getStatus().equals(StoragePoolStatus.PrepareForMaintenance)) {
2640+
if (Arrays.asList(StoragePoolStatus.Up, StoragePoolStatus.Disabled, StoragePoolStatus.PrepareForMaintenance).contains(primaryStorage.getStatus())) {
26262641
throw new StorageUnavailableException("Primary storage " + primaryStorage + " is not ready to complete migration, as the status is:" + primaryStorage.getStatus().toString(),
26272642
primaryStorageId);
26282643
}

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,17 @@
3131
import com.cloud.resource.ResourceManager;
3232
import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao;
3333
import org.apache.cloudstack.api.ApiConstants;
34+
import org.apache.cloudstack.api.command.admin.storage.CancelPrimaryStorageMaintenanceCmd;
3435
import org.apache.cloudstack.api.command.admin.storage.ChangeStoragePoolScopeCmd;
3536
import org.apache.cloudstack.api.command.admin.storage.ConfigureStorageAccessCmd;
3637
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
3738
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreLifeCycle;
3839
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
3940
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProvider;
4041
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProviderManager;
42+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
4143
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver;
44+
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreLifeCycle;
4245
import org.apache.cloudstack.framework.config.ConfigDepot;
4346
import org.apache.cloudstack.framework.config.ConfigKey;
4447
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
@@ -1727,4 +1730,73 @@ public void checkUsagedSpaceReturnsFalseForZeroCapacityPool() {
17271730

17281731
Assert.assertFalse(result);
17291732
}
1733+
1734+
@Test
1735+
public void recordStatusBeforeMaintenanceStoresDisabledStatus() {
1736+
Mockito.when(storagePoolVOMock.getId()).thenReturn(1L);
1737+
Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Disabled);
1738+
1739+
storageManagerImpl.recordStatusBeforeMaintenance(storagePoolVOMock);
1740+
1741+
Mockito.verify(storagePoolDetailsDao).addDetail(1L, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE, StoragePoolStatus.Disabled.name(), false);
1742+
}
1743+
1744+
@Test
1745+
public void recordStatusBeforeMaintenanceClearsStaleStatusForUpPool() {
1746+
Mockito.when(storagePoolVOMock.getId()).thenReturn(1L);
1747+
Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Up);
1748+
1749+
storageManagerImpl.recordStatusBeforeMaintenance(storagePoolVOMock);
1750+
1751+
Mockito.verify(storagePoolDetailsDao).removeDetail(1L, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE);
1752+
Mockito.verify(storagePoolDetailsDao, Mockito.never()).addDetail(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean());
1753+
}
1754+
1755+
@Test
1756+
public void recordStatusBeforeMaintenanceKeepsRecordedStatusOnRetry() {
1757+
Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.ErrorInMaintenance);
1758+
1759+
storageManagerImpl.recordStatusBeforeMaintenance(storagePoolVOMock);
1760+
1761+
Mockito.verifyNoInteractions(storagePoolDetailsDao);
1762+
}
1763+
1764+
@Test
1765+
public void preparePrimaryStorageForMaintenanceAllowsDisabledPool() throws Exception {
1766+
DataStoreProvider provider = Mockito.mock(DataStoreProvider.class);
1767+
DataStoreLifeCycle lifeCycle = Mockito.mock(DataStoreLifeCycle.class);
1768+
PrimaryDataStore store = Mockito.mock(PrimaryDataStore.class);
1769+
Mockito.when(storagePoolDao.findById(1L)).thenReturn(storagePoolVOMock);
1770+
Mockito.when(storagePoolVOMock.getId()).thenReturn(1L);
1771+
Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Disabled);
1772+
Mockito.when(storagePoolVOMock.getStorageProviderName()).thenReturn("DefaultPrimary");
1773+
Mockito.when(dataStoreProviderMgr.getDataStoreProvider("DefaultPrimary")).thenReturn(provider);
1774+
Mockito.when(provider.getDataStoreLifeCycle()).thenReturn(lifeCycle);
1775+
Mockito.when(dataStoreMgr.getDataStore(1L, DataStoreRole.Primary)).thenReturn(store);
1776+
1777+
storageManagerImpl.preparePrimaryStorageForMaintenance(1L);
1778+
1779+
Mockito.verify(storagePoolDetailsDao).addDetail(1L, PrimaryDataStoreLifeCycle.STATUS_BEFORE_MAINTENANCE, StoragePoolStatus.Disabled.name(), false);
1780+
Mockito.verify(lifeCycle).maintain(store);
1781+
}
1782+
1783+
@Test
1784+
public void preparePrimaryStorageForMaintenanceRejectsPoolAlreadyInMaintenance() {
1785+
Mockito.when(storagePoolDao.findById(1L)).thenReturn(storagePoolVOMock);
1786+
Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Maintenance);
1787+
1788+
assertThrows(InvalidParameterValueException.class, () -> storageManagerImpl.preparePrimaryStorageForMaintenance(1L));
1789+
Mockito.verifyNoInteractions(storagePoolDetailsDao);
1790+
}
1791+
1792+
@Test
1793+
public void cancelPrimaryStorageForMaintenanceRejectsDisabledPool() {
1794+
CancelPrimaryStorageMaintenanceCmd cmd = Mockito.mock(CancelPrimaryStorageMaintenanceCmd.class);
1795+
Mockito.when(cmd.getId()).thenReturn(1L);
1796+
Mockito.when(storagePoolDao.findById(1L)).thenReturn(storagePoolVOMock);
1797+
Mockito.when(storagePoolVOMock.getStatus()).thenReturn(StoragePoolStatus.Disabled);
1798+
1799+
assertThrows(StorageUnavailableException.class, () -> storageManagerImpl.cancelPrimaryStorageForMaintenance(cmd));
1800+
Mockito.verifyNoInteractions(dataStoreProviderMgr);
1801+
}
17301802
}

‎ui/src/config/section/infra/primaryStorages.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export default {
125125
label: 'label.action.enable.maintenance.mode',
126126
message: 'message.action.primarystorage.enable.maintenance.mode',
127127
dataView: true,
128-
show: (record) => { return ['Up', 'Connecting', 'Down', 'ErrorInMaintenance'].includes(record.state) }
128+
show: (record) => { return ['Up', 'Disabled', 'Connecting', 'Down', 'ErrorInMaintenance'].includes(record.state) }
129129
},
130130
{
131131
api: 'cancelStorageMaintenance',

0 commit comments

Comments
 (0)