[CLOUDSTACK-10323] Allow changing disk offering during volume migrati…

…on (#2486)

* [CLOUDSTACK-10323] Allow changing disk offering during volume migration

This is a continuation of work developed on PR #2425 (CLOUDSTACK-10240), which provided root admins an override mechanism to move volumes between storage systems types (local/shared) even when the disk offering would not allow such operation. To complete the work, we will now provide a way for administrators to enter a new disk offering that can reflect the new placement of the volume. We will add an extra parameter to allow the root admin inform a new disk offering for the volume. Therefore, when the volume is being migrated, it will be possible to replace the disk offering to reflect the new placement of the volume.

The API method will have the following parameters:

* storageid (required)
* volumeid (required)
* livemigrate(optional)
* newdiskofferingid (optional) – this is the new parameter

The expected behavior is the following:

* If “newdiskofferingid” is not provided the current behavior is maintained. Override mechanism will also keep working as we have seen so far.
* If the “newdiskofferingid” is provided by the admin, we will execute the following checks
** new disk offering mode (local/shared) must match the target storage mode. If it does not match, an exception will be thrown and the operator will receive a message indicating the problem.
** we will check if the new disk offering tags match the target storage tags. If it does not match, an exception will be thrown and the operator will receive a message indicating the problem.
** check if the target storage has the capacity for the new volume. If it does not have enough space, then an exception is thrown and the operator will receive a message indicating the problem.
** check if the size of the volume is the same as the size of the new disk offering. If it is not the same, we will ALLOW the change of the service offering, and a warning message will be logged.

We execute the change of the Disk offering as soon as the migration of the volume finishes. Therefore, if an error happens during the migration and the volume remains in the original storage system, the disk offering will keep reflecting this situation.

* Code formatting

* Adding a test to cover migration with new disk offering (#4)

* Adding a test to cover migration with new disk offering

* Update test_volumes.py

* Update test_volumes.py

* fix test_11_migrate_volume_and_change_offering

* Fix typo in Java doc

api/src/main/java/org/apache/cloudstack/api/ApiConstants.java

@@ -86,6 +86,7 @@ public class ApiConstants {
     public static final String DEVICE_ID = "deviceid";
     public static final String DIRECT_DOWNLOAD = "directdownload";
     public static final String DISK_OFFERING_ID = "diskofferingid";
+    public static final String NEW_DISK_OFFERING_ID = "newdiskofferingid";
     public static final String DISK_SIZE = "disksize";
     public static final String UTILIZATION = "utilization";
     public static final String DRIVER = "driver";

api/src/main/java/org/apache/cloudstack/api/command/admin/volume/MigrateVolumeCmdByAdmin.java

@@ -25,17 +25,13 @@
 
 import com.cloud.storage.Volume;
 
-
-@APICommand(name = "migrateVolume", description = "Migrate volume", responseObject = VolumeResponse.class, since = "3.0.0", responseView = ResponseView.Full, entityType = {Volume.class},
-        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+@APICommand(name = "migrateVolume", description = "Migrate volume", responseObject = VolumeResponse.class, since = "3.0.0", responseView = ResponseView.Full, entityType = {
+        Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
 public class MigrateVolumeCmdByAdmin extends MigrateVolumeCmd {
 
-
     @Override
-    public void execute(){
-        Volume result;
-
-        result = _volumeService.migrateVolume(this);
+    public void execute() {
+        Volume result = _volumeService.migrateVolume(this);
         if (result != null) {
             VolumeResponse response = _responseGenerator.createVolumeResponse(ResponseView.Full, result);
             response.setResponseName(getCommandName());

api/src/main/java/org/apache/cloudstack/api/command/user/volume/MigrateVolumeCmd.java

@@ -30,31 +30,27 @@
 import com.cloud.storage.Volume;
 import com.cloud.user.Account;
 
-@APICommand(name = "migrateVolume", description = "Migrate volume", responseObject = VolumeResponse.class, since = "3.0.0", responseView = ResponseView.Restricted, entityType = {Volume.class},
-        requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+@APICommand(name = "migrateVolume", description = "Migrate volume", responseObject = VolumeResponse.class, since = "3.0.0", responseView = ResponseView.Restricted, entityType = {
+        Volume.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
 public class MigrateVolumeCmd extends BaseAsyncCmd {
     private static final String s_name = "migratevolumeresponse";
 
-     /////////////////////////////////////////////////////
+    /////////////////////////////////////////////////////
     //////////////// API parameters /////////////////////
     /////////////////////////////////////////////////////
 
     @Parameter(name = ApiConstants.VOLUME_ID, type = CommandType.UUID, entityType = VolumeResponse.class, required = true, description = "the ID of the volume")
     private Long volumeId;
 
-    @Parameter(name = ApiConstants.STORAGE_ID,
-               type = CommandType.UUID,
-               entityType = StoragePoolResponse.class,
-               required = true,
-               description = "destination storage pool ID to migrate the volume to")
+    @Parameter(name = ApiConstants.STORAGE_ID, type = CommandType.UUID, entityType = StoragePoolResponse.class, required = true, description = "destination storage pool ID to migrate the volume to")
     private Long storageId;
 
-    @Parameter(name = ApiConstants.LIVE_MIGRATE,
-               type = CommandType.BOOLEAN,
-               required = false,
-               description = "if the volume should be live migrated when it is attached to a running vm")
+    @Parameter(name = ApiConstants.LIVE_MIGRATE, type = CommandType.BOOLEAN, required = false, description = "if the volume should be live migrated when it is attached to a running vm")
     private Boolean liveMigrate;
 
+    @Parameter(name = ApiConstants.NEW_DISK_OFFERING_ID, type = CommandType.STRING, description = "The new disk offering ID that replaces the current one used by the volume. This new disk offering is used to better reflect the new storage where the volume is going to be migrated to.")
+    private String newDiskOfferingUuid;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -87,12 +83,12 @@ public String getCommandName() {
 
     @Override
     public long getEntityOwnerId() {
-          Volume volume = _entityMgr.findById(Volume.class, getVolumeId());
-          if (volume != null) {
-              return volume.getAccountId();
-          }
+        Volume volume = _entityMgr.findById(Volume.class, getVolumeId());
+        if (volume != null) {
+            return volume.getAccountId();
+        }
 
-          return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked
+        return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked
     }
 
     @Override
@@ -105,6 +101,10 @@ public String getEventDescription() {
         return "Attempting to migrate volume Id: " + getVolumeId() + " to storage pool Id: " + getStoragePoolId();
     }
 
+    public String getNewDiskOfferingUuid() {
+        return newDiskOfferingUuid;
+    }
+
     @Override
     public void execute() {
         Volume result;

engine/components-api/src/main/java/com/cloud/vm/VmWorkMigrateVolume.java

@@ -22,12 +22,14 @@ public class VmWorkMigrateVolume extends VmWork {
     private long volumeId;
     private long destPoolId;
     private boolean liveMigrate;
+    private Long newDiskOfferingId;
 
-    public VmWorkMigrateVolume(long userId, long accountId, long vmId, String handlerName, long volumeId, long destPoolId, boolean liveMigrate) {
+    public VmWorkMigrateVolume(long userId, long accountId, long vmId, String handlerName, long volumeId, long destPoolId, boolean liveMigrate, Long newDiskOfferingId) {
         super(userId, accountId, vmId, handlerName);
         this.volumeId = volumeId;
         this.destPoolId = destPoolId;
         this.liveMigrate = liveMigrate;
+        this.newDiskOfferingId = newDiskOfferingId;
     }
 
     public long getVolumeId() {
@@ -41,4 +43,8 @@ public long getDestPoolId() {
     public boolean isLiveMigrate() {
         return liveMigrate;
     }
+
+    public Long getNewDiskOfferingId() {
+        return newDiskOfferingId;
+    }
 }