Fixed the PowerFlex/ScaleIO volume name inconsistency issue in the volume path after migration, due to rename failure

Author: sureshanaparti

plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/client/ScaleIOGatewayClientImpl.java

@@ -376,7 +376,7 @@ public boolean renameVolume(final String volumeId, final String newName) {
             checkResponseOK(response);
             return true;
         } catch (final IOException e) {
-            LOG.error("Failed to rename PowerFlex volume due to:", e);
+            LOG.error("Failed to rename PowerFlex volume due to: ", e);
             checkResponseTimeOut(e);
         } finally {
             if (response != null) {
@@ -786,7 +786,6 @@ public boolean migrateVolume(final String srcVolumeId, final String destPoolId,
         Preconditions.checkArgument(!Strings.isNullOrEmpty(destPoolId), "dest pool id cannot be null");
         Preconditions.checkArgument(timeoutInSecs > 0, "timeout must be greater than 0");
 
-        HttpResponse response = null;
         try {
             Volume volume = getVolume(srcVolumeId);
             if (volume == null || Strings.isNullOrEmpty(volume.getVtreeId())) {
@@ -798,10 +797,21 @@ public boolean migrateVolume(final String srcVolumeId, final String destPoolId,
             LOG.debug("Migrating the volume: " + srcVolumeId + " on the src pool: " + srcPoolId + " to the dest pool: " + destPoolId +
                     " in the same PowerFlex cluster");
 
-            response = post(
-                    "/instances/Volume::" + srcVolumeId + "/action/migrateVTree",
-                    String.format("{\"destSPId\":\"%s\"}", destPoolId));
-            checkResponseOK(response);
+            HttpResponse response = null;
+            try {
+                response = post(
+                        "/instances/Volume::" + srcVolumeId + "/action/migrateVTree",
+                        String.format("{\"destSPId\":\"%s\"}", destPoolId));
+                checkResponseOK(response);
+            } catch (final IOException e) {
+                LOG.error("Unable to migrate PowerFlex volume due to: ", e);
+                checkResponseTimeOut(e);
+                throw e;
+            } finally {
+                if (response != null) {
+                    EntityUtils.consumeQuietly(response.getEntity());
+                }
+            }
 
             LOG.debug("Wait until the migration is complete for the volume: " + srcVolumeId);
             long migrationStartTime = System.currentTimeMillis();
@@ -844,28 +854,27 @@ public boolean migrateVolume(final String srcVolumeId, final String destPoolId,
                     }
                 }
             }
-        } catch (final IOException e) {
-            LOG.error("Failed to migrate PowerFlex volume due to:", e);
-            checkResponseTimeOut(e);
-        } finally {
-            if (response != null) {
-                EntityUtils.consumeQuietly(response.getEntity());
-            }
+        } catch (final Exception e) {
+            LOG.error("Failed to migrate PowerFlex volume due to: " + e.getMessage(), e);
+            throw new CloudRuntimeException("Failed to migrate PowerFlex volume due to: " + e.getMessage());
         }
+
         LOG.debug("Migration failed for the volume: " + srcVolumeId);
         return false;
     }
 
-    private boolean waitForVolumeMigrationToComplete(final String volumeTreeId, int waitTimeInSec) {
+    private boolean waitForVolumeMigrationToComplete(final String volumeTreeId, int waitTimeoutInSecs) {
         LOG.debug("Waiting for the migration to complete for the volume-tree " + volumeTreeId);
         if (Strings.isNullOrEmpty(volumeTreeId)) {
             LOG.warn("Invalid volume-tree id, unable to check the migration status of the volume-tree " + volumeTreeId);
             return false;
         }
 
-        while (waitTimeInSec > 0) {
+        int delayTimeInSecs = 3;
+        while (waitTimeoutInSecs > 0) {
             try {
-                Thread.sleep(1000); // Try every sec and return after migration is complete
+                // Wait and try after few secs (reduce no. of client API calls to check the migration status) and return after migration is complete
+                Thread.sleep(delayTimeInSecs * 1000);
 
                 VTreeMigrationInfo.MigrationStatus migrationStatus = getVolumeTreeMigrationStatus(volumeTreeId);
                 if (migrationStatus != null && migrationStatus == VTreeMigrationInfo.MigrationStatus.NotInMigration) {
@@ -876,7 +885,7 @@ private boolean waitForVolumeMigrationToComplete(final String volumeTreeId, int
                 LOG.warn("Exception while checking for migration status of the volume-tree: " + volumeTreeId + " - " + ex.getLocalizedMessage());
                 // don't do anything
             } finally {
-                waitTimeInSec--;
+                waitTimeoutInSecs = waitTimeoutInSecs - delayTimeInSecs;
             }
         }
 
@@ -925,10 +934,10 @@ private boolean rollbackVolumeMigration(final String srcVolumeId) {
             pauseVolumeMigration(srcVolumeId, true); // Pause forcefully
             // Wait few secs for volume migration to change to Paused state
             boolean paused = false;
-            int retryCount = 5;
+            int retryCount = 3;
             while (retryCount > 0) {
                 try {
-                    Thread.sleep(1000); // Try every sec
+                    Thread.sleep(3000); // Try after few secs
                     migrationStatus = getVolumeTreeMigrationStatus(volume.getVtreeId()); // Get updated migration status
                     if (migrationStatus != null && migrationStatus == VTreeMigrationInfo.MigrationStatus.Paused) {
                         LOG.debug("Migration for the volume: " + srcVolumeId + " paused");

plugins/storage/volume/scaleio/src/main/java/org/apache/cloudstack/storage/datastore/driver/ScaleIOPrimaryDataStoreDriver.java

@@ -677,21 +677,29 @@ private Answer migrateVolume(DataObject srcData, DataObject destData) {
             long destPoolId = destStore.getId();
 
             final ScaleIOGatewayClient client = getScaleIOClient(srcPoolId);
-            final String srcVolumeId = ScaleIOUtil.getVolumePath(((VolumeInfo) srcData).getPath());
+            final String srcVolumePath = ((VolumeInfo) srcData).getPath();
+            final String srcVolumeId = ScaleIOUtil.getVolumePath(srcVolumePath);
             final StoragePoolVO destStoragePool = storagePoolDao.findById(destPoolId);
             final String destStoragePoolId = destStoragePool.getPath();
             int migrationTimeout = StorageManager.KvmStorageOfflineMigrationWait.value();
             boolean migrateStatus = client.migrateVolume(srcVolumeId, destStoragePoolId, migrationTimeout);
             if (migrateStatus) {
                 String newVolumeName = String.format("%s-%s-%s-%s", ScaleIOUtil.VOLUME_PREFIX, destData.getId(),
                         destStoragePool.getUuid().split("-")[0].substring(4), ManagementServerImpl.customCsIdentifier.value());
-                client.renameVolume(srcVolumeId, newVolumeName);
+                boolean renamed = client.renameVolume(srcVolumeId, newVolumeName);
 
                 if (srcData.getId() != destData.getId()) {
                     VolumeVO destVolume = volumeDao.findById(destData.getId());
-                    String newVolumePath = ScaleIOUtil.updatedPathWithVolumeName(srcVolumeId, newVolumeName);
-                    destVolume.set_iScsiName(newVolumePath);
-                    destVolume.setPath(newVolumePath);
+                    // Volume Id in the PowerFlex/ScaleIO pool remains the same after the migration
+                    // Update PowerFlex volume name only after it is renamed, to maintain the consistency
+                    if (renamed) {
+                        String newVolumePath = ScaleIOUtil.updatedPathWithVolumeName(srcVolumeId, newVolumeName);
+                        destVolume.set_iScsiName(newVolumePath);
+                        destVolume.setPath(newVolumePath);
+                    } else {
+                        destVolume.set_iScsiName(srcVolumePath);
+                        destVolume.setPath(srcVolumePath);
+                    }
                     volumeDao.update(destData.getId(), destVolume);
 
                     VolumeVO srcVolume = volumeDao.findById(srcData.getId());
@@ -719,10 +727,14 @@ private Answer migrateVolume(DataObject srcData, DataObject destData) {
                         String snapshotVolumeId = ScaleIOUtil.getVolumePath(snapshotStore.getInstallPath());
                         String newSnapshotName = String.format("%s-%s-%s-%s", ScaleIOUtil.SNAPSHOT_PREFIX, snapshot.getId(),
                                 destStoragePool.getUuid().split("-")[0].substring(4), ManagementServerImpl.customCsIdentifier.value());
-                        client.renameVolume(snapshotVolumeId, newSnapshotName);
+                        renamed = client.renameVolume(snapshotVolumeId, newSnapshotName);
 
                         snapshotStore.setDataStoreId(destPoolId);
-                        snapshotStore.setInstallPath(ScaleIOUtil.updatedPathWithVolumeName(snapshotVolumeId, newSnapshotName));
+                        // Snapshot Id in the PowerFlex/ScaleIO pool remains the same after the migration
+                        // Update PowerFlex snapshot name only after it is renamed, to maintain the consistency
+                        if (renamed) {
+                            snapshotStore.setInstallPath(ScaleIOUtil.updatedPathWithVolumeName(snapshotVolumeId, newSnapshotName));
+                        }
                         snapshotDataStoreDao.update(snapshotStore.getId(), snapshotStore);
                     }
                 }