PowerFlex/ScaleIO - Wait after SDC service start/restart/stop, and retry to fetch SDC id/guid (apache#11099)

* [PowerFlex/ScaleIO] Added wait time after SDC service start/restart/stop, and retries to fetch SDC id/guid

* Added agent property 'powerflex.sdc.service.wait' for the time (in secs) to wait after SDC service start/restart/stop

* code improvements

Author: sureshanaparti

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java

@@ -653,24 +653,10 @@ public Ternary<Boolean, Map<String, String>, String> prepareStorageClient(String
             if (!ScaleIOUtil.startSDCService()) {
                 return new Ternary<>(false, null, "Couldn't start SDC service on host");
             }
-        }
-
-        if (MapUtils.isNotEmpty(details) && details.containsKey(ScaleIOGatewayClient.STORAGE_POOL_MDMS)) {
-            // Assuming SDC service is started, add mdms
-            String mdms = details.get(ScaleIOGatewayClient.STORAGE_POOL_MDMS);
-            String[] mdmAddresses = mdms.split(",");
-            if (mdmAddresses.length > 0) {
-                if (ScaleIOUtil.isMdmPresent(mdmAddresses[0])) {
-                    return new Ternary<>(true, getSDCDetails(details), "MDM added, no need to prepare the SDC client");
-                }
-
-                ScaleIOUtil.addMdms(mdmAddresses);
-                if (!ScaleIOUtil.isMdmPresent(mdmAddresses[0])) {
-                    return new Ternary<>(false, null, "Failed to add MDMs");
-                } else {
-                    logger.debug(String.format("MDMs %s added to storage pool %s", mdms, uuid));
-                    applyMdmsChangeWaitTime(details);
-                }
+        } else {
+            logger.debug("SDC service is active on host, re-starting it");
+            if (!ScaleIOUtil.restartSDCService()) {
+                return new Ternary<>(false, null, "Couldn't restart SDC service on host");
             }
         }
 
@@ -679,7 +665,7 @@ public Ternary<Boolean, Map<String, String>, String> prepareStorageClient(String
             return new Ternary<>(false, null, "Couldn't get the SDC details on the host");
         }
 
-        return new Ternary<>(true, sdcDetails, "Prepared client successfully");
+        return new Ternary<>( true, sdcDetails, "Prepared client successfully");
     }
 
     public Pair<Boolean, String> unprepareStorageClient(String uuid, Map<String, String> details) {
@@ -798,12 +784,12 @@ private Map<String, String> getSDCDetails(Map<String, String> details) {
             if (sdcId != null) {
                 sdcDetails.put(ScaleIOGatewayClient.SDC_ID, sdcId);
                 return sdcDetails;
-            }
-
-            String sdcGuId = ScaleIOUtil.getSdcGuid();
-            if (sdcGuId != null) {
-                sdcDetails.put(ScaleIOGatewayClient.SDC_GUID, sdcGuId);
-                return sdcDetails;
+            } else {
+                String sdcGuId = ScaleIOUtil.getSdcGuid();
+                if (sdcGuId != null) {
+                    sdcDetails.put(ScaleIOGatewayClient.SDC_GUID, sdcGuId);
+                    return sdcDetails;
+                }
             }
 
             try {

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptorTest.java

@@ -92,6 +92,34 @@ public void testPrepareStorageClient_SDCServiceNotEnabled() {
         Assert.assertEquals("SDC service not enabled on host", result.third());
     }
 
+    @Test
+    public void testPrepareStorageClient_SDCServiceNotRestarted() {
+        when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl status scini"))).thenReturn(3);
+        when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl is-enabled scini"))).thenReturn(0);
+        when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl is-active scini"))).thenReturn(0);
+        when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl restart scini"))).thenReturn(1);
+
+        Ternary<Boolean, Map<String, String>, String> result = scaleIOStorageAdaptor.prepareStorageClient(Storage.StoragePoolType.PowerFlex, poolUuid, new HashMap<>());
+
+        Assert.assertFalse(result.first());
+        Assert.assertNull(result.second());
+        Assert.assertEquals("Couldn't restart SDC service on host", result.third());
+    }
+
+    @Test
+    public void testPrepareStorageClient_SDCServiceRestarted() {
+        when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl status scini"))).thenReturn(3);
+        when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl is-enabled scini"))).thenReturn(0);
+        when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl is-active scini"))).thenReturn(0);
+        when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl restart scini"))).thenReturn(0);
+
+        Ternary<Boolean, Map<String, String>, String> result = scaleIOStorageAdaptor.prepareStorageClient(Storage.StoragePoolType.PowerFlex, poolUuid, new HashMap<>());
+
+        Assert.assertFalse(result.first());
+        Assert.assertNull(result.second());
+        Assert.assertEquals("Couldn't get the SDC details on the host", result.third());
+    }
+
     @Test
     public void testPrepareStorageClient_SDCServiceNotStarted() {
         when(Script.runSimpleBashScriptForExitValue(Mockito.eq("systemctl status scini"))).thenReturn(3);

plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/manager/ScaleIOSDCManagerImpl.java

@@ -185,7 +185,7 @@ public String prepareSDC(Host host, DataStore dataStore) {
                 }
 
                 int waitTimeInSecs = 15; // Wait for 15 secs (usual tests with SDC service start took 10-15 secs)
-                if (isHostSdcConnected(sdcId, dataStore, waitTimeInSecs)) {
+                if (hostSdcConnected(sdcId, dataStore, waitTimeInSecs)) {
                     return sdcId;
                 }
             }
@@ -249,7 +249,7 @@ private String prepareSDCOnHost(Host host, DataStore dataStore, String systemId,
         }
 
         if (StringUtils.isBlank(sdcId)) {
-            logger.warn("Couldn't retrieve PowerFlex storage SDC details from the host: {}, add MDMs if On-demand connect disabled or try (re)install SDC & restart agent", host);
+            logger.warn("Couldn't retrieve PowerFlex storage SDC details from the host: {}, add MDMs if not or try (re)install SDC & restart agent", host);
             return null;
         }
 

plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/provider/ScaleIOHostListener.java

@@ -136,7 +136,7 @@ private String getSdcIdOfHost(HostVO host, DataStore dataStore) {
         }
 
         if (StringUtils.isBlank(sdcId)) {
-            String msg = String.format("Couldn't retrieve PowerFlex storage SDC details from the host: %s, add MDMs if On-demand connect disabled or try (re)install SDC & restart agent", host);
+            String msg = String.format("Couldn't retrieve PowerFlex storage SDC details from the host: %s, add MDMs if not or try (re)install SDC & restart agent", host);
             logger.warn(msg);
             _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_HOST, host.getDataCenterId(), host.getPodId(), "SDC details not found on host: " + host.getUuid(), msg);
             return null;

plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/util/ScaleIOUtil.java

@@ -513,39 +513,4 @@ private static void waitForSdcServiceActionToComplete() {
         } catch (InterruptedException ignore) {
         }
     }
-
-    /**
-     * Represents {@link ScaleIOUtil#DRV_CFG_FILE} MDM entry (SDC and Installation Ids are skipped).
-     */
-    public static class MdmEntry {
-        private String mdmId;
-        private Collection<String> ips;
-
-        /**
-         * MDM entry constructor.
-         *
-         * @param mdmId MDM Id
-         * @param ips   IP Addresses
-         */
-        public MdmEntry(String mdmId, Collection<String> ips) {
-            this.mdmId = mdmId;
-            this.ips = ips;
-        }
-
-        public String getMdmId() {
-            return mdmId;
-        }
-
-        public void setMdmId(String mdmId) {
-            this.mdmId = mdmId;
-        }
-
-        public Collection<String> getIps() {
-            return ips;
-        }
-
-        public void setIps(Collection<String> ips) {
-            this.ips = ips;
-        }
-    }
 }