Skip to content

Commit 9cf1e0e

Browse files
authored
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
1 parent a64ad9d commit 9cf1e0e

3 files changed

Lines changed: 29 additions & 54 deletions

File tree

plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
6060
import org.apache.cloudstack.vm.UnmanagedInstanceTO;
6161
import org.apache.commons.collections.CollectionUtils;
62+
import org.apache.commons.collections.MapUtils;
6263
import org.apache.commons.lang.ArrayUtils;
6364
import org.apache.commons.lang.StringUtils;
6465
import org.apache.commons.lang.math.NumberUtils;
@@ -2336,7 +2337,7 @@ protected StartAnswer execute(StartCommand cmd) {
23362337
configBasicExtraOption(extraOptions, vmSpec);
23372338

23382339
if (deployAsIs) {
2339-
setDeployAsIsProperties(vmMo, deployAsIsInfo, vmConfigSpec);
2340+
setDeployAsIsProperties(vmMo, deployAsIsInfo, vmConfigSpec, hyperHost);
23402341
}
23412342

23422343
configNvpExtraOption(extraOptions, vmSpec, nicUuidToDvSwitchUuid);
@@ -2563,12 +2564,12 @@ private String getBootModeFromVmSpec(VirtualMachineTO vmSpec, boolean deployAsIs
25632564
* Set OVF properties (if available)
25642565
*/
25652566
private void setDeployAsIsProperties(VirtualMachineMO vmMo, DeployAsIsInfoTO deployAsIsInfo,
2566-
VirtualMachineConfigSpec vmConfigSpec) throws Exception {
2567-
if (deployAsIsInfo != null) {
2567+
VirtualMachineConfigSpec vmConfigSpec, VmwareHypervisorHost hyperHost) throws Exception {
2568+
if (deployAsIsInfo != null && MapUtils.isNotEmpty(deployAsIsInfo.getProperties())) {
25682569
Map<String, String> properties = deployAsIsInfo.getProperties();
25692570
VmConfigInfo vAppConfig = vmMo.getConfigInfo().getVAppConfig();
25702571
s_logger.info("Copying OVF properties to the values the user provided");
2571-
setVAppPropertiesToConfigSpec(vAppConfig, properties, vmConfigSpec);
2572+
setVAppPropertiesToConfigSpec(vAppConfig, properties, vmConfigSpec, hyperHost);
25722573
}
25732574
}
25742575

@@ -2660,13 +2661,13 @@ private void setBootOptions(VirtualMachineTO vmSpec, String bootMode, VirtualMac
26602661
/**
26612662
* Set the ovf section spec from existing vApp configuration
26622663
*/
2663-
protected List<VAppOvfSectionSpec> copyVAppConfigOvfSectionFromOVF(VmConfigInfo vAppConfig) {
2664+
protected List<VAppOvfSectionSpec> copyVAppConfigOvfSectionFromOVF(VmConfigInfo vAppConfig, boolean useEdit) {
26642665
List<VAppOvfSectionInfo> ovfSection = vAppConfig.getOvfSection();
26652666
List<VAppOvfSectionSpec> specs = new ArrayList<>();
26662667
for (VAppOvfSectionInfo info : ovfSection) {
26672668
VAppOvfSectionSpec spec = new VAppOvfSectionSpec();
26682669
spec.setInfo(info);
2669-
spec.setOperation(ArrayUpdateOperation.ADD);
2670+
spec.setOperation(useEdit ? ArrayUpdateOperation.EDIT : ArrayUpdateOperation.ADD);
26702671
specs.add(spec);
26712672
}
26722673
return specs;
@@ -2694,17 +2695,19 @@ private String getPropertyValue(OVFPropertyTO prop) {
26942695
/**
26952696
* Set the properties section from existing vApp configuration and values set on ovfProperties
26962697
*/
2697-
protected List<VAppPropertySpec> copyVAppConfigPropertySectionFromOVF(VmConfigInfo vAppConfig, Map<String, String> ovfProperties) {
2698+
protected List<VAppPropertySpec> copyVAppConfigPropertySectionFromOVF(VmConfigInfo vAppConfig, Map<String, String> ovfProperties,
2699+
boolean useEdit) {
26982700
List<VAppPropertyInfo> productFromOvf = vAppConfig.getProperty();
26992701
List<VAppPropertySpec> specs = new ArrayList<>();
27002702
for (VAppPropertyInfo info : productFromOvf) {
27012703
VAppPropertySpec spec = new VAppPropertySpec();
27022704
if (ovfProperties.containsKey(info.getId())) {
27032705
String value = ovfProperties.get(info.getId());
27042706
info.setValue(value);
2707+
s_logger.info("Setting OVF property ID = " + info.getId() + " VALUE = " + value);
27052708
}
27062709
spec.setInfo(info);
2707-
spec.setOperation(ArrayUpdateOperation.ADD);
2710+
spec.setOperation(useEdit ? ArrayUpdateOperation.EDIT : ArrayUpdateOperation.ADD);
27082711
specs.add(spec);
27092712
}
27102713
return specs;
@@ -2713,13 +2716,14 @@ protected List<VAppPropertySpec> copyVAppConfigPropertySectionFromOVF(VmConfigIn
27132716
/**
27142717
* Set the product section spec from existing vApp configuration
27152718
*/
2716-
protected List<VAppProductSpec> copyVAppConfigProductSectionFromOVF(VmConfigInfo vAppConfig) {
2719+
protected List<VAppProductSpec> copyVAppConfigProductSectionFromOVF(VmConfigInfo vAppConfig, boolean useEdit) {
27172720
List<VAppProductInfo> productFromOvf = vAppConfig.getProduct();
27182721
List<VAppProductSpec> specs = new ArrayList<>();
27192722
for (VAppProductInfo info : productFromOvf) {
27202723
VAppProductSpec spec = new VAppProductSpec();
27212724
spec.setInfo(info);
2722-
spec.setOperation(ArrayUpdateOperation.ADD);
2725+
s_logger.info("Procuct info KEY " + info.getKey());
2726+
spec.setOperation(useEdit ? ArrayUpdateOperation.EDIT : ArrayUpdateOperation.ADD);
27232727
specs.add(spec);
27242728
}
27252729
return specs;
@@ -2731,16 +2735,19 @@ protected List<VAppProductSpec> copyVAppConfigProductSectionFromOVF(VmConfigInfo
27312735
*/
27322736
protected void setVAppPropertiesToConfigSpec(VmConfigInfo vAppConfig,
27332737
Map<String, String> ovfProperties,
2734-
VirtualMachineConfigSpec vmConfig) throws Exception {
2738+
VirtualMachineConfigSpec vmConfig, VmwareHypervisorHost hyperHost) throws Exception {
27352739
VmConfigSpec vmConfigSpec = new VmConfigSpec();
27362740
vmConfigSpec.getEula().addAll(vAppConfig.getEula());
27372741
vmConfigSpec.setInstallBootStopDelay(vAppConfig.getInstallBootStopDelay());
27382742
vmConfigSpec.setInstallBootRequired(vAppConfig.isInstallBootRequired());
27392743
vmConfigSpec.setIpAssignment(vAppConfig.getIpAssignment());
27402744
vmConfigSpec.getOvfEnvironmentTransport().addAll(vAppConfig.getOvfEnvironmentTransport());
2741-
vmConfigSpec.getProduct().addAll(copyVAppConfigProductSectionFromOVF(vAppConfig));
2742-
vmConfigSpec.getProperty().addAll(copyVAppConfigPropertySectionFromOVF(vAppConfig, ovfProperties));
2743-
vmConfigSpec.getOvfSection().addAll(copyVAppConfigOvfSectionFromOVF(vAppConfig));
2745+
2746+
// For backward compatibility, prior to Vmware 6.5 use EDIT operation instead of ADD
2747+
boolean useEditOperation = hyperHost.getContext().getServiceContent().getAbout().getApiVersion().compareTo("6.5") < 1;
2748+
vmConfigSpec.getProduct().addAll(copyVAppConfigProductSectionFromOVF(vAppConfig, useEditOperation));
2749+
vmConfigSpec.getProperty().addAll(copyVAppConfigPropertySectionFromOVF(vAppConfig, ovfProperties, useEditOperation));
2750+
vmConfigSpec.getOvfSection().addAll(copyVAppConfigOvfSectionFromOVF(vAppConfig, useEditOperation));
27442751
vmConfig.setVAppConfig(vmConfigSpec);
27452752
}
27462753

test/integration/smoke/test_vm_life_cycle.py

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161

6262
_multiprocess_shared_ = True
6363

64-
6564
class TestDeployVM(cloudstackTestCase):
6665

6766
@classmethod
@@ -1750,9 +1749,6 @@ def setUpClass(cls):
17501749
cls.l2_network_offering
17511750
]
17521751

1753-
# Uncomment when tests are finished, to cleanup the test templates
1754-
for template in cls.templates:
1755-
cls._cleanup.append(template)
17561752

17571753
@classmethod
17581754
def tearDownClass(cls):
@@ -1774,21 +1770,21 @@ def tearDown(self):
17741770
def get_ova_parsed_information_from_template(self, template):
17751771
if not template:
17761772
return None
1777-
details = template.details.__dict__
1773+
details = template.deployasisdetails.__dict__
17781774
configurations = []
17791775
disks = []
17801776
isos = []
17811777
networks = []
17821778
for propKey in details:
1783-
if propKey.startswith('ACS-configuration'):
1779+
if propKey.startswith('configuration'):
17841780
configurations.append(json.loads(details[propKey]))
1785-
elif propKey.startswith('ACS-disk'):
1781+
elif propKey.startswith('disk'):
17861782
detail = json.loads(details[propKey])
17871783
if detail['isIso'] == True:
17881784
isos.append(detail)
17891785
else:
17901786
disks.append(detail)
1791-
elif propKey.startswith('ACS-network'):
1787+
elif propKey.startswith('network'):
17921788
networks.append(json.loads(details[propKey]))
17931789

17941790
return configurations, disks, isos, networks
@@ -1818,32 +1814,6 @@ def verify_nics(self, nic_networks, vm_id):
18181814
msg="VM NIC(InstanceID: {}) network mismatch, expected = {}, result = {}".format(nic_network["nic"], nic_network["network"], nic.networkid)
18191815
)
18201816

1821-
def verify_disks(self, template_disks, vm_id):
1822-
cmd = listVolumes.listVolumesCmd()
1823-
cmd.virtualmachineid = vm_id
1824-
cmd.listall = True
1825-
vm_volumes = self.apiclient.listVolumes(cmd)
1826-
self.assertEqual(
1827-
isinstance(vm_volumes, list),
1828-
True,
1829-
"Check listVolumes response returns a valid list"
1830-
)
1831-
self.assertEqual(
1832-
len(template_disks),
1833-
len(vm_volumes),
1834-
msg="VM volumes count is different, expected = {}, result = {}".format(len(template_disks), len(vm_volumes))
1835-
)
1836-
template_disks.sort(key=itemgetter('diskNumber'))
1837-
vm_volumes.sort(key=itemgetter('deviceid'))
1838-
for j in range(len(vm_volumes)):
1839-
volume = vm_volumes[j]
1840-
disk = template_disks[j]
1841-
self.assertEqual(
1842-
volume.size,
1843-
disk["virtualSize"],
1844-
msg="VM Volume(diskNumber: {}) network mismatch, expected = {}, result = {}".format(disk["diskNumber"], disk["virtualSize"], volume.size)
1845-
)
1846-
18471817
@attr(tags=["advanced", "advancedns", "smoke", "sg", "dev"], required_hardware="false")
18481818
@skipTestIf("hypervisorNotSupported")
18491819
def test_01_vapps_vm_cycle(self):
@@ -1944,8 +1914,6 @@ def test_01_vapps_vm_cycle(self):
19441914

19451915
# Verify nics
19461916
self.verify_nics(nicnetworklist, vm.id)
1947-
# Verify disks
1948-
self.verify_disks(disks, vm.id)
19491917
# Verify properties
19501918
original_properties = vm_service['properties']
19511919
vm_properties = get_vm_vapp_configs(self.apiclient, self.config, self.zone, vm.instancename)

tools/marvin/marvin/lib/common.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -429,21 +429,21 @@ def get_test_ovf_templates(apiclient, zone_id=None, test_ovf_templates=None, hyp
429429
template = Template.register(apiclient, test_template, zoneid=zone_id, hypervisor=hypervisor.lower(), randomize_name=False)
430430
template.download(apiclient)
431431
retries = 3
432-
while (template.details == None or len(template.details.__dict__) == 0) and retries > 0:
432+
while (not hasattr(template, 'deployasisdetails') or len(template.deployasisdetails.__dict__) == 0) and retries > 0:
433433
time.sleep(10)
434434
template_list = Template.list(apiclient, id=template.id, zoneid=zone_id, templatefilter='all')
435435
if isinstance(template_list, list):
436436
template = Template(template_list[0].__dict__)
437437
retries = retries - 1
438-
if template.details == None or len(template.details.__dict__) == 0:
438+
if not hasattr(template, 'deployasisdetails') or len(template.deployasisdetails.__dict__) == 0:
439439
template.delete(apiclient)
440440
else:
441441
result.append(template)
442442

443443
if templates:
444444
for template in templates:
445-
if template.isready and template.ispublic and template.details != None and len(template.details.__dict__) > 0:
446-
result.append(template.__dict__)
445+
if template.isready and template.ispublic and template.deployasisdetails and len(template.deployasisdetails.__dict__) > 0:
446+
result.append(template)
447447

448448
return result
449449

0 commit comments

Comments
 (0)