Skip to content

Fix VMware OVF properties copy from template #4738

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yadvr merged 5 commits into from
Apr 12, 2021
Merged

Fix VMware OVF properties copy from template #4738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
07b3dd7
Fix VMware OVF properties copy from template
nvazquez Mar 1, 2021
d44880e
Fix vapp marvin test
nvazquez Apr 7, 2021
0a25c52
Remove unused code
nvazquez Apr 7, 2021
090d963
Fix check for deploy as is details
nvazquez Apr 9, 2021
fee1a52
Access class fields
nvazquez Apr 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions ...hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<String, String> 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);
}
}

Expand Down Expand Up @@ -2654,13 +2655,13 @@ private void setBootOptions(VirtualMachineTO vmSpec, String bootMode, VirtualMac
/**
* Set the ovf section spec from existing vApp configuration
*/
protected List<VAppOvfSectionSpec> copyVAppConfigOvfSectionFromOVF(VmConfigInfo vAppConfig) {
protected List<VAppOvfSectionSpec> copyVAppConfigOvfSectionFromOVF(VmConfigInfo vAppConfig, boolean useEdit) {
List<VAppOvfSectionInfo> ovfSection = vAppConfig.getOvfSection();
List<VAppOvfSectionSpec> 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;
Expand Down Expand Up @@ -2688,17 +2689,19 @@ private String getPropertyValue(OVFPropertyTO prop) {
/**
* Set the properties section from existing vApp configuration and values set on ovfProperties
*/
protected List<VAppPropertySpec> copyVAppConfigPropertySectionFromOVF(VmConfigInfo vAppConfig, Map<String, String> ovfProperties) {
protected List<VAppPropertySpec> copyVAppConfigPropertySectionFromOVF(VmConfigInfo vAppConfig, Map<String, String> ovfProperties,
boolean useEdit) {
List<VAppPropertyInfo> productFromOvf = vAppConfig.getProperty();
List<VAppPropertySpec> specs = new ArrayList<>();
for (VAppPropertyInfo info : productFromOvf) {
VAppPropertySpec spec = new VAppPropertySpec();
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;
Expand All @@ -2707,13 +2710,14 @@ protected List<VAppPropertySpec> copyVAppConfigPropertySectionFromOVF(VmConfigIn
/**
* Set the product section spec from existing vApp configuration
*/
protected List<VAppProductSpec> copyVAppConfigProductSectionFromOVF(VmConfigInfo vAppConfig) {
protected List<VAppProductSpec> copyVAppConfigProductSectionFromOVF(VmConfigInfo vAppConfig, boolean useEdit) {
List<VAppProductInfo> productFromOvf = vAppConfig.getProduct();
List<VAppProductSpec> 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());
Copy link
Contributor

@sureshanaparti sureshanaparti Mar 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Product - typo

spec.setOperation(useEdit ? ArrayUpdateOperation.EDIT : ArrayUpdateOperation.ADD);
specs.add(spec);
}
return specs;
Expand All @@ -2725,16 +2729,19 @@ protected List<VAppProductSpec> copyVAppConfigProductSectionFromOVF(VmConfigInfo
*/
protected void setVAppPropertiesToConfigSpec(VmConfigInfo vAppConfig,
Map<String, String> 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;
Copy link
Contributor

@sureshanaparti sureshanaparti Mar 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
boolean useEditOperation = hyperHost.getContext().getServiceContent().getAbout().getApiVersion().compareTo("6.5") < 1;
boolean useEditOperation = ((HostMO) hyperHost).getHostAboutInfo().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);
}

Expand Down
40 changes: 4 additions & 36 deletions test/integration/smoke/test_vm_life_cycle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@

_multiprocess_shared_ = True


class TestDeployVM(cloudstackTestCase):

@classmethod
Expand Down Expand Up @@ -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):
Expand All @@ -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
Expand Down Expand Up @@ -1813,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):
Expand Down Expand Up @@ -1939,8 +1909,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)
Expand Down
8 changes: 4 additions & 4 deletions tools/marvin/marvin/lib/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down