From 07b3dd7a9cbdf4d2a7298c2ef37b34b386599b8f Mon Sep 17 00:00:00 2001 From: nvazquez Date: Mon, 1 Mar 2021 09:03:57 -0300 Subject: [PATCH 1/5] Fix VMware OVF properties copy from template --- .../vmware/resource/VmwareResource.java | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 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 56d08a4e088e..eefc9a22d3b6 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 @@ -55,6 +55,7 @@ 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; @@ -2330,7 +2331,7 @@ protected StartAnswer execute(StartCommand cmd) { configBasicExtraOption(extraOptions, vmSpec); if (deployAsIs) { - setDeployAsIsProperties(vmMo, deployAsIsInfo, vmConfigSpec); + setDeployAsIsProperties(vmMo, deployAsIsInfo, vmConfigSpec, hyperHost); } configNvpExtraOption(extraOptions, vmSpec, nicUuidToDvSwitchUuid); @@ -2557,12 +2558,12 @@ private String getBootModeFromVmSpec(VirtualMachineTO vmSpec, boolean deployAsIs * 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); } } @@ -2654,13 +2655,13 @@ private void setBootOptions(VirtualMachineTO vmSpec, String bootMode, VirtualMac /** * 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; @@ -2688,7 +2689,8 @@ private String getPropertyValue(OVFPropertyTO prop) { /** * 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) { @@ -2696,9 +2698,10 @@ protected List copyVAppConfigPropertySectionFromOVF(VmConfigIn 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; @@ -2707,13 +2710,14 @@ protected List copyVAppConfigPropertySectionFromOVF(VmConfigIn /** * 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; @@ -2725,16 +2729,19 @@ protected List copyVAppConfigProductSectionFromOVF(VmConfigInfo */ 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); } From d44880e1d5eec56805536edec106c4f410f48c92 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 7 Apr 2021 16:41:13 -0300 Subject: [PATCH 2/5] Fix vapp marvin test --- test/integration/smoke/test_vm_life_cycle.py | 14 ++++---------- tools/marvin/marvin/lib/common.py | 8 ++++---- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py index 08668e47b727..11611a76705e 100644 --- a/test/integration/smoke/test_vm_life_cycle.py +++ b/test/integration/smoke/test_vm_life_cycle.py @@ -61,7 +61,6 @@ _multiprocess_shared_ = True - class TestDeployVM(cloudstackTestCase): @classmethod @@ -1745,9 +1744,6 @@ def setUpClass(cls): 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): @@ -1769,21 +1765,21 @@ def tearDown(self): 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 @@ -1939,8 +1935,6 @@ def test_01_vapps_vm_cycle(self): # 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 fb69f8005d34..091071723762 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 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 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 != None and len(template.deployasisdetails.__dict__) > 0: + result.append(template) return result From 0a25c525c390d27e8285cfd7c6287dcd9c23c4c1 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 7 Apr 2021 17:15:00 -0300 Subject: [PATCH 3/5] Remove unused code --- test/integration/smoke/test_vm_life_cycle.py | 30 ++------------------ 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py index 11611a76705e..461bbf604a9b 100644 --- a/test/integration/smoke/test_vm_life_cycle.py +++ b/test/integration/smoke/test_vm_life_cycle.py @@ -1809,32 +1809,6 @@ def verify_nics(self, nic_networks, vm_id): 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): @@ -1864,7 +1838,7 @@ def test_01_vapps_vm_cycle(self): nicnetworklist = [] networks = [] - vm_service = self.services["virtual_machine_vapps"][template.name] + vm_service = self.services["virtual_machine_vapps"][template['name']] network_mappings = vm_service["nicnetworklist"] for network_mapping in network_mappings: network_service = self.services["isolated_network"] @@ -1888,7 +1862,7 @@ def test_01_vapps_vm_cycle(self): vm_service, accountid=self.account.name, domainid=self.account.domainid, - templateid=template.id, + templateid=template['id'], serviceofferingid=self.custom_offering.id, zoneid=self.zone.id, customcpunumber=cpu_number, From 090d9637a81be7629ad988a473f5826220ca7e65 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Fri, 9 Apr 2021 11:19:46 -0300 Subject: [PATCH 4/5] Fix check for deploy as is details --- tools/marvin/marvin/lib/common.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/marvin/marvin/lib/common.py b/tools/marvin/marvin/lib/common.py index 091071723762..18a8a0a1a5ed 100644 --- a/tools/marvin/marvin/lib/common.py +++ b/tools/marvin/marvin/lib/common.py @@ -429,20 +429,20 @@ 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 (not template.deployasisdetails or len(template.deployasisdetails.__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 not template.deployasisdetails or len(template.deployasisdetails.__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.deployasisdetails != None and len(template.deployasisdetails.__dict__) > 0: + if template.isready and template.ispublic and template.deployasisdetails and len(template.deployasisdetails.__dict__) > 0: result.append(template) return result From fee1a52bdaaaec564faa84cf8f6567bd2f8362ee Mon Sep 17 00:00:00 2001 From: nvazquez Date: Sat, 10 Apr 2021 01:56:29 -0300 Subject: [PATCH 5/5] Access class fields --- test/integration/smoke/test_vm_life_cycle.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/smoke/test_vm_life_cycle.py b/test/integration/smoke/test_vm_life_cycle.py index 461bbf604a9b..fff2d329bd48 100644 --- a/test/integration/smoke/test_vm_life_cycle.py +++ b/test/integration/smoke/test_vm_life_cycle.py @@ -1765,7 +1765,7 @@ def tearDown(self): def get_ova_parsed_information_from_template(self, template): if not template: return None - details = template['deployasisdetails'].__dict__ + details = template.deployasisdetails.__dict__ configurations = [] disks = [] isos = [] @@ -1838,7 +1838,7 @@ def test_01_vapps_vm_cycle(self): nicnetworklist = [] networks = [] - vm_service = self.services["virtual_machine_vapps"][template['name']] + vm_service = self.services["virtual_machine_vapps"][template.name] network_mappings = vm_service["nicnetworklist"] for network_mapping in network_mappings: network_service = self.services["isolated_network"] @@ -1862,7 +1862,7 @@ def test_01_vapps_vm_cycle(self): vm_service, accountid=self.account.name, domainid=self.account.domainid, - templateid=template['id'], + templateid=template.id, serviceofferingid=self.custom_offering.id, zoneid=self.zone.id, customcpunumber=cpu_number,