From 9cf1e0e86977147e8680c745e760cceda4169c11 Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Mon, 12 Apr 2021 09:34:58 -0300 Subject: [PATCH] vmware: Fix VMware OVF properties copy from template (#4738) * Fix VMware OVF properties copy from template * Fix vapp marvin test * Remove unused code * Fix check for deploy as is details * Access class fields --- .../vmware/resource/VmwareResource.java | 35 +++++++++------- test/integration/smoke/test_vm_life_cycle.py | 40 ++----------------- tools/marvin/marvin/lib/common.py | 8 ++-- 3 files changed, 29 insertions(+), 54 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 6dfdeb5dd81..9963b7589c2 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -59,6 +59,7 @@ import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo; import org.apache.cloudstack.vm.UnmanagedInstanceTO; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.math.NumberUtils; @@ -2336,7 +2337,7 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa configBasicExtraOption(extraOptions, vmSpec); if (deployAsIs) { - setDeployAsIsProperties(vmMo, deployAsIsInfo, vmConfigSpec); + setDeployAsIsProperties(vmMo, deployAsIsInfo, vmConfigSpec, hyperHost); } configNvpExtraOption(extraOptions, vmSpec, nicUuidToDvSwitchUuid); @@ -2563,12 +2564,12 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa * Set OVF properties (if available) */ private void setDeployAsIsProperties(VirtualMachineMO vmMo, DeployAsIsInfoTO deployAsIsInfo, - VirtualMachineConfigSpec vmConfigSpec) throws Exception { - if (deployAsIsInfo != null) { + VirtualMachineConfigSpec vmConfigSpec, VmwareHypervisorHost hyperHost) throws Exception { + if (deployAsIsInfo != null && MapUtils.isNotEmpty(deployAsIsInfo.getProperties())) { Map properties = deployAsIsInfo.getProperties(); VmConfigInfo vAppConfig = vmMo.getConfigInfo().getVAppConfig(); s_logger.info("Copying OVF properties to the values the user provided"); - setVAppPropertiesToConfigSpec(vAppConfig, properties, vmConfigSpec); + setVAppPropertiesToConfigSpec(vAppConfig, properties, vmConfigSpec, hyperHost); } } @@ -2660,13 +2661,13 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa /** * Set the ovf section spec from existing vApp configuration */ - protected List copyVAppConfigOvfSectionFromOVF(VmConfigInfo vAppConfig) { + protected List copyVAppConfigOvfSectionFromOVF(VmConfigInfo vAppConfig, boolean useEdit) { List ovfSection = vAppConfig.getOvfSection(); List specs = new ArrayList<>(); for (VAppOvfSectionInfo info : ovfSection) { VAppOvfSectionSpec spec = new VAppOvfSectionSpec(); spec.setInfo(info); - spec.setOperation(ArrayUpdateOperation.ADD); + spec.setOperation(useEdit ? ArrayUpdateOperation.EDIT : ArrayUpdateOperation.ADD); specs.add(spec); } return specs; @@ -2694,7 +2695,8 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa /** * Set the properties section from existing vApp configuration and values set on ovfProperties */ - protected List copyVAppConfigPropertySectionFromOVF(VmConfigInfo vAppConfig, Map ovfProperties) { + protected List copyVAppConfigPropertySectionFromOVF(VmConfigInfo vAppConfig, Map ovfProperties, + boolean useEdit) { List productFromOvf = vAppConfig.getProperty(); List specs = new ArrayList<>(); for (VAppPropertyInfo info : productFromOvf) { @@ -2702,9 +2704,10 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa if (ovfProperties.containsKey(info.getId())) { String value = ovfProperties.get(info.getId()); info.setValue(value); + s_logger.info("Setting OVF property ID = " + info.getId() + " VALUE = " + value); } spec.setInfo(info); - spec.setOperation(ArrayUpdateOperation.ADD); + spec.setOperation(useEdit ? ArrayUpdateOperation.EDIT : ArrayUpdateOperation.ADD); specs.add(spec); } return specs; @@ -2713,13 +2716,14 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa /** * Set the product section spec from existing vApp configuration */ - protected List copyVAppConfigProductSectionFromOVF(VmConfigInfo vAppConfig) { + protected List copyVAppConfigProductSectionFromOVF(VmConfigInfo vAppConfig, boolean useEdit) { List productFromOvf = vAppConfig.getProduct(); List specs = new ArrayList<>(); for (VAppProductInfo info : productFromOvf) { VAppProductSpec spec = new VAppProductSpec(); spec.setInfo(info); - spec.setOperation(ArrayUpdateOperation.ADD); + s_logger.info("Procuct info KEY " + info.getKey()); + spec.setOperation(useEdit ? ArrayUpdateOperation.EDIT : ArrayUpdateOperation.ADD); specs.add(spec); } return specs; @@ -2731,16 +2735,19 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa */ protected void setVAppPropertiesToConfigSpec(VmConfigInfo vAppConfig, Map ovfProperties, - VirtualMachineConfigSpec vmConfig) throws Exception { + VirtualMachineConfigSpec vmConfig, VmwareHypervisorHost hyperHost) throws Exception { VmConfigSpec vmConfigSpec = new VmConfigSpec(); vmConfigSpec.getEula().addAll(vAppConfig.getEula()); vmConfigSpec.setInstallBootStopDelay(vAppConfig.getInstallBootStopDelay()); vmConfigSpec.setInstallBootRequired(vAppConfig.isInstallBootRequired()); vmConfigSpec.setIpAssignment(vAppConfig.getIpAssignment()); vmConfigSpec.getOvfEnvironmentTransport().addAll(vAppConfig.getOvfEnvironmentTransport()); - vmConfigSpec.getProduct().addAll(copyVAppConfigProductSectionFromOVF(vAppConfig)); - vmConfigSpec.getProperty().addAll(copyVAppConfigPropertySectionFromOVF(vAppConfig, ovfProperties)); - vmConfigSpec.getOvfSection().addAll(copyVAppConfigOvfSectionFromOVF(vAppConfig)); + + // For backward compatibility, prior to Vmware 6.5 use EDIT operation instead of ADD + boolean useEditOperation = hyperHost.getContext().getServiceContent().getAbout().getApiVersion().compareTo("6.5") < 1; + vmConfigSpec.getProduct().addAll(copyVAppConfigProductSectionFromOVF(vAppConfig, useEditOperation)); + vmConfigSpec.getProperty().addAll(copyVAppConfigPropertySectionFromOVF(vAppConfig, ovfProperties, useEditOperation)); + vmConfigSpec.getOvfSection().addAll(copyVAppConfigOvfSectionFromOVF(vAppConfig, useEditOperation)); vmConfig.setVAppConfig(vmConfigSpec); } diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py index d1aa4cbad63..6cbbc91e826 100644 --- a/test/integration/smoke/test_vm_life_cycle.py +++ b/test/integration/smoke/test_vm_life_cycle.py @@ -61,7 +61,6 @@ from operator import itemgetter _multiprocess_shared_ = True - class TestDeployVM(cloudstackTestCase): @classmethod @@ -1750,9 +1749,6 @@ class TestVAppsVM(cloudstackTestCase): cls.l2_network_offering ] - # Uncomment when tests are finished, to cleanup the test templates - for template in cls.templates: - cls._cleanup.append(template) @classmethod def tearDownClass(cls): @@ -1774,21 +1770,21 @@ class TestVAppsVM(cloudstackTestCase): def get_ova_parsed_information_from_template(self, template): if not template: return None - details = template.details.__dict__ + details = template.deployasisdetails.__dict__ configurations = [] disks = [] isos = [] networks = [] for propKey in details: - if propKey.startswith('ACS-configuration'): + if propKey.startswith('configuration'): configurations.append(json.loads(details[propKey])) - elif propKey.startswith('ACS-disk'): + elif propKey.startswith('disk'): detail = json.loads(details[propKey]) if detail['isIso'] == True: isos.append(detail) else: disks.append(detail) - elif propKey.startswith('ACS-network'): + elif propKey.startswith('network'): networks.append(json.loads(details[propKey])) return configurations, disks, isos, networks @@ -1818,32 +1814,6 @@ class TestVAppsVM(cloudstackTestCase): msg="VM NIC(InstanceID: {}) network mismatch, expected = {}, result = {}".format(nic_network["nic"], nic_network["network"], nic.networkid) ) - def verify_disks(self, template_disks, vm_id): - cmd = listVolumes.listVolumesCmd() - cmd.virtualmachineid = vm_id - cmd.listall = True - vm_volumes = self.apiclient.listVolumes(cmd) - self.assertEqual( - isinstance(vm_volumes, list), - True, - "Check listVolumes response returns a valid list" - ) - self.assertEqual( - len(template_disks), - len(vm_volumes), - msg="VM volumes count is different, expected = {}, result = {}".format(len(template_disks), len(vm_volumes)) - ) - template_disks.sort(key=itemgetter('diskNumber')) - vm_volumes.sort(key=itemgetter('deviceid')) - for j in range(len(vm_volumes)): - volume = vm_volumes[j] - disk = template_disks[j] - self.assertEqual( - volume.size, - disk["virtualSize"], - msg="VM Volume(diskNumber: {}) network mismatch, expected = {}, result = {}".format(disk["diskNumber"], disk["virtualSize"], volume.size) - ) - @attr(tags=["advanced", "advancedns", "smoke", "sg", "dev"], required_hardware="false") @skipTestIf("hypervisorNotSupported") def test_01_vapps_vm_cycle(self): @@ -1944,8 +1914,6 @@ class TestVAppsVM(cloudstackTestCase): # Verify nics self.verify_nics(nicnetworklist, vm.id) - # Verify disks - self.verify_disks(disks, vm.id) # Verify properties original_properties = vm_service['properties'] vm_properties = get_vm_vapp_configs(self.apiclient, self.config, self.zone, vm.instancename) diff --git a/tools/marvin/marvin/lib/common.py b/tools/marvin/marvin/lib/common.py index fb69f8005d3..18a8a0a1a5e 100644 --- a/tools/marvin/marvin/lib/common.py +++ b/tools/marvin/marvin/lib/common.py @@ -429,21 +429,21 @@ def get_test_ovf_templates(apiclient, zone_id=None, test_ovf_templates=None, hyp template = Template.register(apiclient, test_template, zoneid=zone_id, hypervisor=hypervisor.lower(), randomize_name=False) template.download(apiclient) retries = 3 - while (template.details == None or len(template.details.__dict__) == 0) and retries > 0: + while (not hasattr(template, 'deployasisdetails') or len(template.deployasisdetails.__dict__) == 0) and retries > 0: time.sleep(10) template_list = Template.list(apiclient, id=template.id, zoneid=zone_id, templatefilter='all') if isinstance(template_list, list): template = Template(template_list[0].__dict__) retries = retries - 1 - if template.details == None or len(template.details.__dict__) == 0: + if not hasattr(template, 'deployasisdetails') or len(template.deployasisdetails.__dict__) == 0: template.delete(apiclient) else: result.append(template) if templates: for template in templates: - if template.isready and template.ispublic and template.details != None and len(template.details.__dict__) > 0: - result.append(template.__dict__) + if template.isready and template.ispublic and template.deployasisdetails and len(template.deployasisdetails.__dict__) > 0: + result.append(template) return result