Don't remove MDM IPs from SDC when any volumes mapped to SDC

sureshanaparti · sureshanaparti · commit b065389271b3 · 2024-12-27T19:31:40.000+05:30
diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java
@@ -137,6 +137,14 @@ default boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool) {
         return false;
     }
 
+    /**
+     * intended for managed storage
+     * returns true if the host can be disconnected from storage pool
+     */
+    default boolean canDisconnectHostFromStoragePool(Host host, StoragePool pool) {
+        return true;
+    }
+
     /**
      * Used by storage pools which want to keep VMs' information
      * @return true if additional VM info is needed (intended for storage pools).
diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java
@@ -308,6 +308,8 @@ static Boolean getFullCloneConfiguration(Long storeId) {
 
     boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool);
 
+    boolean canDisconnectHostFromStoragePool(Host host, StoragePool pool);
+
     Host getHost(long hostId);
 
     Host updateSecondaryStorage(long secStorageId, String newUrl);
diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java
@@ -1469,6 +1469,31 @@ public boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool) {
         return sdcManager.areSDCConnectionsWithinLimit(pool.getId());
     }
 
+    @Override
+    public boolean canDisconnectHostFromStoragePool(Host host, StoragePool pool) {
+        if (host == null || pool == null) {
+            return false;
+        }
+
+        StoragePoolHostVO poolHostVO = storagePoolHostDao.findByPoolHost(pool.getId(), host.getId());
+        if (poolHostVO == null) {
+            return false;
+        }
+
+        final String sdcId = poolHostVO.getLocalPath();
+        if (StringUtils.isBlank(sdcId)) {
+            return false;
+        }
+
+        try {
+            final ScaleIOGatewayClient client = getScaleIOClient(pool.getId());
+            return client.listVolumesMappedToSdc(sdcId).isEmpty();
+        } catch (Exception e) {
+            logger.warn("Unable to check whether the host: " + host.getId() + " can be disconnected from storage pool: " + pool.getId() + ", due to " + e.getMessage(), e);
+            return false;
+        }
+    }
+
     private void alertHostSdcDisconnection(Host host) {
         if (host == null) {
             return;
diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ScaleIOPrimaryDataStoreLifeCycle.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/ScaleIOPrimaryDataStoreLifeCycle.java
@@ -336,8 +336,8 @@ public boolean maintain(DataStore store) {
             StoragePoolDetailVO mdmsDetail = storagePoolDetailsDao.findDetail(store.getId(), ScaleIOGatewayClient.STORAGE_POOL_MDMS);
             if (mdmsDetail != null) {
                 details.put(ScaleIOGatewayClient.STORAGE_POOL_MDMS, mdmsDetail.getValue());
+                details.put(ScaleIOSDCManager.ConnectOnDemand.key(), "false");
             }
-            details.put(ScaleIOSDCManager.ConnectOnDemand.key(), "false");
         }
 
         storagePoolAutomation.maintain(store, details);
diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManager.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManager.java
@@ -27,8 +27,8 @@ public interface ScaleIOSDCManager {
             Boolean.class,
             "powerflex.connect.on.demand",
             Boolean.FALSE.toString(),
-            "Connect PowerFlex client on Host when first Volume is mapped to SDC and disconnect when last Volume is unmapped from SDC," +
-                    " otherwise no action (that is connection remains in the same state whichever it is, connected or disconnected).",
+            "When true, connects PowerFlex client on Host when first Volume is mapped to SDC & client connections configured 'storage.pool.connected.clients.limit' are within the limit and disconnects when last Volume is unmapped from SDC; " +
+                    "and When false, connects PowerFlex client on Host when host connects to storage pool & client connections configured 'storage.pool.connected.clients.limit' are within the limit and disconnects when host disconnects from storage pool & no volumes mapped to SDC.",
             Boolean.TRUE,
             ConfigKey.Scope.Zone);
 
@@ -63,6 +63,14 @@ public interface ScaleIOSDCManager {
      */
     boolean unprepareSDC(Host host, DataStore dataStore);
 
+    /**
+     * Checks if the SDC can be unprepared on the host (don't remove MDM IPs from SDC if any volumes mapped to SDC).
+     * @param host the host
+     * @param dataStore the datastore
+     * @return true if SDC can be unprepared on the host
+     */
+    boolean canUnprepareSDC(Host host, DataStore dataStore);
+
     /**
      * Returns the SDC Id of the host for the pool.
      * @param sdcGuid the SDC GUID
diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManagerImpl.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManagerImpl.java
@@ -284,6 +284,11 @@ public boolean unprepareSDC(Host host, DataStore dataStore) {
                 return true;
             }
 
+            if (!canUnprepareSDC(host, dataStore)) {
+                logger.debug("Cannot unprepare SDC, there might be some volumes mapped to the SDC that belongs to the storage pools of PowerFlex storage cluster");
+                return false;
+            }
+
             String mdms = getMdms(dataStore.getId());;
             boolean unprepareSDCStatus = unprepareSDCOnHost(host, dataStore, mdms);
             if (unprepareSDCStatus) {
@@ -327,6 +332,31 @@ private boolean unprepareSDCOnHost(Host host, DataStore dataStore, String mdms)
         return true;
     }
 
+    @Override
+    public boolean canUnprepareSDC(Host host, DataStore dataStore) {
+        if (host == null || dataStore == null) {
+            return false;
+        }
+
+        StoragePoolHostVO poolHostVO = storagePoolHostDao.findByPoolHost(dataStore.getId(), host.getId());
+        if (poolHostVO == null) {
+            return false;
+        }
+
+        final String sdcId = poolHostVO.getLocalPath();
+        if (StringUtils.isBlank(sdcId)) {
+            return false;
+        }
+
+        try {
+            final ScaleIOGatewayClient client = getScaleIOClient(dataStore.getId());
+            return client.listVolumesMappedToSdc(sdcId).isEmpty();
+        } catch (Exception e) {
+            logger.warn("Unable to check whether the SDC of the pool: " + dataStore.getId() + " can be unprepared on the host: " + host.getId() + ", due to " + e.getMessage(), e);
+            return false;
+        }
+    }
+
     @Override
     public String getHostSdcId(String sdcGuid, long poolId) {
         try {
diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java
@@ -202,10 +202,12 @@ public boolean hostDisconnected(long hostId, long poolId) {
         }
         Map<String,String> details = new HashMap<>();
         details.put(ScaleIOGatewayClient.STORAGE_POOL_SYSTEM_ID, systemId);
-        details.put(ScaleIOSDCManager.ConnectOnDemand.key(), String.valueOf(ScaleIOSDCManager.ConnectOnDemand.valueIn(host.getDataCenterId())));
         _sdcManager = ComponentContext.inject(_sdcManager);
-        String mdms = _sdcManager.getMdms(poolId);
-        details.put(ScaleIOGatewayClient.STORAGE_POOL_MDMS, mdms);
+        if (_sdcManager.canUnprepareSDC(host, dataStore)) {
+            details.put(ScaleIOSDCManager.ConnectOnDemand.key(), String.valueOf(ScaleIOSDCManager.ConnectOnDemand.valueIn(host.getDataCenterId())));
+            String mdms = _sdcManager.getMdms(poolId);
+            details.put(ScaleIOGatewayClient.STORAGE_POOL_MDMS, mdms);
+        }
 
         ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(false, storagePool, storagePool.getPath(), details);
         ModifyStoragePoolAnswer answer  = sendModifyStoragePoolCommand(cmd, storagePool, hostId);
diff --git a/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java b/plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java
@@ -19,6 +19,7 @@
 
 import java.util.List;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.logging.log4j.Logger;
 import org.apache.logging.log4j.LogManager;
 
@@ -91,6 +92,9 @@ public class ScaleIOUtil {
     private static final String DRV_CFG_FILE = "/etc/emc/scaleio/drv_cfg.txt";
 
     public static void addMdms(List<String> mdmAddresses) {
+        if (CollectionUtils.isEmpty(mdmAddresses)) {
+            return;
+        }
         // Sample Cmd - /opt/emc/scaleio/sdc/bin/drv_cfg --add_mdm --ip x.x.x.x,x.x.x.x --file /etc/emc/scaleio/drv_cfg.txt
         String addMdmsCmd = ScaleIOUtil.SDC_HOME_PATH + "/bin/" + ScaleIOUtil.ADD_MDMS_CMD;
         addMdmsCmd += " --ip " + String.join(",", mdmAddresses);
@@ -102,14 +106,23 @@ public static void addMdms(List<String> mdmAddresses) {
     }
 
     public static void removeMdms(List<String> mdmAddresses) {
+        if (CollectionUtils.isEmpty(mdmAddresses)) {
+            return;
+        }
         // (i) Remove MDMs from config file (ii) Restart scini
         //  Sample Cmd - sed -i '/x.x.x.x\,/d' /etc/emc/scaleio/drv_cfg.txt
+        boolean restartSDC = false;
         String removeMdmsCmdFormat = "sed -i '/%s\\,/d' %s";
         for (String mdmAddress : mdmAddresses) {
+            if (mdmAdded(mdmAddress)) {
+                restartSDC = true;
+            }
             String removeMdmsCmd = String.format(removeMdmsCmdFormat, mdmAddress, DRV_CFG_FILE);
             Script.runSimpleBashScript(removeMdmsCmd);
         }
-        restartSDCService();
+        if (restartSDC) {
+            restartSDCService();
+        }
     }
 
     public static boolean mdmAdded(String mdmAddress) {
diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java
@@ -2826,6 +2826,17 @@ public boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool) {
         return storeDriver instanceof PrimaryDataStoreDriver && ((PrimaryDataStoreDriver)storeDriver).canHostPrepareStoragePoolAccess(host, pool);
     }
 
+    @Override
+    public boolean canDisconnectHostFromStoragePool(Host host, StoragePool pool) {
+        if (pool == null || !pool.isManaged()) {
+            return true;
+        }
+
+        DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(pool.getStorageProviderName());
+        DataStoreDriver storeDriver = storeProvider.getDataStoreDriver();
+        return storeDriver instanceof PrimaryDataStoreDriver && ((PrimaryDataStoreDriver)storeDriver).canDisconnectHostFromStoragePool(host, pool);
+    }
+
     @Override
     @DB
     public Host getHost(long hostId) {
diff --git a/server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java b/server/src/main/java/com/cloud/storage/StoragePoolAutomationImpl.java
@@ -168,7 +168,7 @@ public boolean maintain(DataStore store, Map<String,String> details) {
             // remove heartbeat
             for (HostVO host : hosts) {
                 ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(false, storagePool);
-                if (MapUtils.isNotEmpty(details)) {
+                if (MapUtils.isNotEmpty(details) && storageManager.canDisconnectHostFromStoragePool(host, storagePool)) {
                     cmd.setDetails(details);
                 }
                 final Answer answer = agentMgr.easySend(host.getId(), cmd);

Original file line number	Diff line number	Diff line change
`@@ -336,8 +336,8 @@ public boolean maintain(DataStore store) {`
`336`	`336`	`StoragePoolDetailVO mdmsDetail = storagePoolDetailsDao.findDetail(store.getId(), ScaleIOGatewayClient.STORAGE_POOL_MDMS);`
`337`	`337`	`if (mdmsDetail != null) {`
`338`	`338`	`details.put(ScaleIOGatewayClient.STORAGE_POOL_MDMS, mdmsDetail.getValue());`
	`339`	`+ details.put(ScaleIOSDCManager.ConnectOnDemand.key(), "false");`
`339`	`340`	`}`
`340`		`- details.put(ScaleIOSDCManager.ConnectOnDemand.key(), "false");`
`341`	`341`	`}`
`342`	`342`
`343`	`343`	`storagePoolAutomation.maintain(store, details);`
Original file line number	Diff line number	Diff line change
`@@ -168,7 +168,7 @@ public boolean maintain(DataStore store, Map<String,String> details) {`
`168`	`168`	`// remove heartbeat`
`169`	`169`	`for (HostVO host : hosts) {`
`170`	`170`	`ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(false, storagePool);`
`171`		`- if (MapUtils.isNotEmpty(details)) {`
	`171`	`+ if (MapUtils.isNotEmpty(details) && storageManager.canDisconnectHostFromStoragePool(host, storagePool)) {`
`172`	`172`	`cmd.setDetails(details);`
`173`	`173`	`}`
`174`	`174`	`final Answer answer = agentMgr.easySend(host.getId(), cmd);`