Improve UI and add checks for force convert to pool parameter

Author: nvazquez

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

@@ -206,6 +206,10 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
     private static final List<Hypervisor.HypervisorType> importUnmanagedInstancesSupportedHypervisors =
             Arrays.asList(Hypervisor.HypervisorType.VMware, Hypervisor.HypervisorType.KVM);
 
+    private static final List<Storage.StoragePoolType> forceConvertToPoolAllowedTypes =
+            Arrays.asList(Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.Filesystem,
+                    Storage.StoragePoolType.SharedMountPoint);
+
     @Inject
     private AgentManager agentManager;
     @Inject
@@ -1655,6 +1659,8 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
                     "Please set all the information for a vCenter IP/Name, datacenter, username and password");
         }
 
+        checkConversionStoragePool(convertStoragePoolId, forceConvertToPool);
+
         checkExtraParamsAllowed(extraParams);
 
         if (existingVcenterId != null) {
@@ -1752,6 +1758,30 @@ protected UserVm importUnmanagedInstanceFromVmwareToKvm(DataCenter zone, Cluster
         }
     }
 
+    /**
+     * Check whether the conversion storage pool exists and is suitable for the conversion or not.
+     * Secondary storage is only allowed when forceConvertToPool is false.
+     * @param convertStoragePoolId the ID of the storage pool (primary or secondary)
+     * @param forceConvertToPool when true, only primary storage pool must be allowed
+     * @throws CloudRuntimeException in case these requirements are not met
+     */
+    protected void checkConversionStoragePool(Long convertStoragePoolId, boolean forceConvertToPool) {
+        if (forceConvertToPool && convertStoragePoolId == null) {
+            String msg = "The parameter forceconverttopool is set to true, but a primary storage pool has not been provided for conversion";
+            logFailureAndThrowException(msg);
+        }
+        if (convertStoragePoolId != null) {
+            StoragePoolVO selectedStoragePool = primaryDataStoreDao.findById(convertStoragePoolId);
+            if (selectedStoragePool == null) {
+                logFailureAndThrowException(String.format("Cannot find a storage pool with ID %s", convertStoragePoolId));
+            }
+            if (forceConvertToPool && !forceConvertToPoolAllowedTypes.contains(selectedStoragePool.getPoolType())) {
+                logFailureAndThrowException(String.format("The selected storage pool %s does not support direct conversion " +
+                        "as its type %s", selectedStoragePool.getName(), selectedStoragePool.getPoolType().name()));
+            }
+        }
+    }
+
     private void checkNetworkingBeforeConvertingVmwareInstance(DataCenter zone, Account owner, String displayName,
                                                                String hostName, UnmanagedInstanceTO sourceVMwareInstance,
                                                                Map<String, Long> nicNetworkMap,

server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java

@@ -1151,4 +1151,41 @@ public void testCheckUnmanagedDiskLimits() {
             Assert.fail("Exception encountered: " + e.getMessage());
         }
     }
+
+    @Test
+    public void testCheckConversionStoragePoolSecondaryStorageStaging() {
+        unmanagedVMsManager.checkConversionStoragePool(null, false);
+        Mockito.verifyNoInteractions(primaryDataStoreDao);
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void testCheckConversionStoragePoolTemporarySecondaryStorageForceConvertToPool() {
+        unmanagedVMsManager.checkConversionStoragePool(null, true);
+    }
+
+    @Test
+    public void testCheckConversionStoragePoolPrimaryStagingPool() {
+        StoragePoolVO destPool = mock(StoragePoolVO.class);
+        long destPoolId = 1L;
+        Mockito.when(primaryDataStoreDao.findById(destPoolId)).thenReturn(destPool);
+        unmanagedVMsManager.checkConversionStoragePool(destPoolId, false);
+    }
+
+    @Test
+    public void testCheckConversionStoragePoolPrimaryStagingPoolTypeAllowedForce() {
+        StoragePoolVO destPool = mock(StoragePoolVO.class);
+        Mockito.when(destPool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem);
+        long destPoolId = 1L;
+        Mockito.when(primaryDataStoreDao.findById(destPoolId)).thenReturn(destPool);
+        unmanagedVMsManager.checkConversionStoragePool(destPoolId, true);
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void testCheckConversionStoragePoolPrimaryStagingPoolTypeNotAllowedForce() {
+        StoragePoolVO destPool = mock(StoragePoolVO.class);
+        Mockito.when(destPool.getPoolType()).thenReturn(Storage.StoragePoolType.RBD);
+        long destPoolId = 1L;
+        Mockito.when(primaryDataStoreDao.findById(destPoolId)).thenReturn(destPool);
+        unmanagedVMsManager.checkConversionStoragePool(destPoolId, true);
+    }
 }

ui/public/locales/en.json

@@ -1083,6 +1083,7 @@
 "label.forcks": "For CKS",
 "label.forbidden": "Forbidden",
 "label.forced": "Force",
+"label.force.convert.to.pool": "Force converting to storage pool directly (not using temporary storage for conversion)",
 "label.force.ms.to.import.vm.files": "Enable to force OVF Download via Management Server. Disable to use KVM Host ovftool (if installed)",
 "label.force.update.os.type": "Force update OS type",
 "label.force.stop": "Force stop",
@@ -3718,6 +3719,7 @@
 "message.select.deselect.desired.options": "Please select / deselect the desired options",
 "message.select.deselect.to.sort": "Please select / deselect to sort the values",
 "message.select.destination.image.stores": "Please select Image Store(s) to which data is to be migrated to",
+"message.select.destination.storage.instance.conversion": "(Optional) Select a Primary Storage destination for the converted disks",
 "message.select.disk.offering": "Please select a disk offering for disk",
 "message.select.end.date.and.time": "Select an end date & time.",
 "message.select.extra.parameters.for.instance.conversion": "(Optional) Pass extra parameters to the virt-v2v command on the conversion host",
@@ -3728,7 +3730,7 @@
 "message.select.nic.network": "Please select a Network for NIC",
 "message.select.security.groups": "Please select security group(s) for your new Instance.",
 "message.select.start.date.and.time": "Select a start date & time.",
-"message.select.temporary.storage.instance.conversion": "(Optional) Select a Storage temporary destination for the converted disks through virt-v2v",
+"message.select.temporary.storage.instance.conversion": "(Optional) Select a staging storage for the converted disks",
 "message.select.zone.description": "Select type of Zone basic/advanced.",
 "message.select.zone.hint": "This is the type of Zone deployment that you want to use. Basic zone: provides a single Network where each Instance is assigned an IP directly from the Network. Guest isolation can be provided through layer-3 means such as security groups (IP address source filtering). Advanced zone: For more sophisticated Network topologies. This Network model provides the most flexibility in defining guest Networks and providing custom Network offerings such as firewall, VPN, or load balancer support.",
 "message.server": "Server : ",

ui/src/views/tools/ImportUnmanagedInstance.vue

@@ -154,9 +154,9 @@
               </a-form-item>
               <a-form-item name="forceconverttopool" ref="forceconverttopool" v-if="selectedVmwareVcenter">
                 <template #label>
-                  <tooltip-label :title="'Force converting to storage pool directly (not using temporary storage for conversion)'" :tooltip="apiParams.forcemstoimportvmfiles.description"/>
+                  <tooltip-label :title="$t('label.force.convert.to.pool')" :tooltip="apiParams.forceconverttopool.description"/>
                 </template>
-                <a-switch v-model:checked="form.forceconverttopool" @change="val => { switches.forceConvertToPool = val }" />
+                <a-switch v-model:checked="form.forceconverttopool" @change="onForceConvertToPoolChange" />
               </a-form-item>
               <a-form-item name="converthostid" ref="converthostid">
                 <check-box-select-pair
@@ -188,13 +188,18 @@
                   v-if="cluster.hypervisortype === 'KVM' && selectedVmwareVcenter"
                   :resourceKey="cluster.id"
                   :selectOptions="storageOptionsForConversion"
-                  :checkBoxLabel="$t('message.select.temporary.storage.instance.conversion')"
+                  :checkBoxLabel="switches.forceConvertToPool ? $t('message.select.destination.storage.instance.conversion') : $t('message.select.temporary.storage.instance.conversion')"
                   :defaultCheckBoxValue="false"
                   :reversed="false"
                   @handle-checkselectpair-change="updateSelectedStorageOptionForConversion"
                 />
               </a-form-item>
-              <a-form-item v-if="showStoragePoolsForConversion" name="convertstoragepool" ref="convertstoragepool" :label="$t('label.storagepool')">
+              <a-form-item
+                v-if="showStoragePoolsForConversion"
+                name="convertstoragepool"
+                ref="convertstoragepool"
+                :label="$t('label.storagepool')"
+              >
                 <a-select
                   v-model:value="form.convertstoragepoolid"
                   defaultActiveFirstOption
@@ -1062,20 +1067,23 @@ export default {
       }
     },
     resetStorageOptionsForConversion () {
-      this.storageOptionsForConversion = [
-        {
-          id: 'secondary',
-          name: 'Secondary Storage'
-        }, {
-          id: 'primary',
-          name: 'Primary Storage'
-        }
-      ]
+      this.storageOptionsForConversion = this.switches.forceConvertToPool ? [] : [{
+        id: 'secondary',
+        name: 'Secondary Storage'
+      }]
+      this.storageOptionsForConversion.push({
+        id: 'primary',
+        name: 'Primary Storage'
+      })
     },
     onSelectRootDisk (val) {
       this.selectedRootDiskIndex = val
       this.updateSelectedRootDisk()
     },
+    onForceConvertToPoolChange (val) {
+      this.switches.forceConvertToPool = val
+      this.resetStorageOptionsForConversion()
+    },
     updateSelectedRootDisk () {
       var rootDisk = this.resource.disk[this.selectedRootDiskIndex]
       rootDisk.size = rootDisk.capacity / (1024 * 1024 * 1024)