From 70fcf755f7acaed8e0193c998b6361eb0e3121bb Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Wed, 8 Jan 2020 22:23:45 +0530 Subject: [PATCH] Allow additional configuration metadata to VMs (#3510) * Suqash commits to a single commit and rebase against master Update marvin tests to use white list * * Fix marvin test failure * Add new marvin negative tests cases * Remove hard-coded hypervisor types in marvin tests * Fix build error after rebase and add hugepagesless * Fix readability of python code * Fix failing test * Adding cleanup of vms for negative tests * Bug fixes - change config checks properly and block extraconfig in details * Trim to compare the keys * CR comments * Don't skip extraconfig without exception Co-authored-by: Boris Stoyanov - a.k.a Bobby --- .../api/command/user/vm/DeployVMCmd.java | 5 +- .../api/command/user/vm/UpdateVMCmd.java | 2 +- .../cloud/vm/VirtualMachineManagerImpl.java | 22 +- .../resource/LibvirtComputingResource.java | 6 +- .../resource/CitrixResourceBase.java | 12 +- .../xenserver/ExtraConfigurationUtility.java | 180 ++++++ .../java/com/cloud/vm/UserVmManagerImpl.java | 247 +++++++- .../com/cloud/vm/UserVmManagerImplTest.java | 19 +- .../smoke/test_deploy_vm_extra_config_data.py | 542 ++++++++++++++++++ 9 files changed, 993 insertions(+), 42 deletions(-) create mode 100644 plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java create mode 100644 test/integration/smoke/test_deploy_vm_extra_config_data.py diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java index 71269df7d7a..c161897032e 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java @@ -26,8 +26,6 @@ import java.util.Map; import javax.annotation.Nonnull; -import org.apache.log4j.Logger; - import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.affinity.AffinityGroupResponse; import org.apache.cloudstack.api.ACL; @@ -52,6 +50,7 @@ import org.apache.cloudstack.api.response.UserVmResponse; import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.cloudstack.context.CallContext; import org.apache.commons.collections.MapUtils; +import org.apache.log4j.Logger; import com.cloud.agent.api.LogLevel; import com.cloud.event.EventTypes; @@ -190,7 +189,7 @@ public class DeployVMCmd extends BaseAsyncCreateCustomIdCmd implements SecurityG @Parameter(name = ApiConstants.DISPLAY_VM, type = CommandType.BOOLEAN, since = "4.2", description = "an optional field, whether to the display the vm to the end user or not.", authorized = {RoleType.Admin}) private Boolean displayVm; - @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, since = "4.3", description = "used to specify the custom parameters.") + @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, since = "4.3", description = "used to specify the custom parameters. 'extraconfig' is not allowed to be passed in details") private Map details; @Parameter(name = ApiConstants.DEPLOYMENT_PLANNER, type = CommandType.STRING, description = "Deployment planner to use for vm allocation. Available to ROOT admin only", since = "4.4", authorized = { RoleType.Admin }) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java index 2fdfd758047..3afee8ac803 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java @@ -98,7 +98,7 @@ public class UpdateVMCmd extends BaseCustomIdCmd implements SecurityGroupAction, @Parameter(name = ApiConstants.INSTANCE_NAME, type = CommandType.STRING, description = "instance name of the user vm", since = "4.4", authorized = {RoleType.Admin}) private String instanceName; - @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "Details in key/value pairs.") + @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "Details in key/value pairs. 'extraconfig' is not allowed to be passed in details.") protected Map details; @ACL diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 712b534e505..b54d548156e 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -39,11 +39,8 @@ import java.util.concurrent.TimeUnit; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.agent.api.PrepareForMigrationAnswer; -import com.cloud.agent.api.to.DpdkTO; -import com.cloud.offering.NetworkOffering; -import com.cloud.offerings.dao.NetworkOfferingDetailsDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; +import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.command.admin.vm.MigrateVMCmd; import org.apache.cloudstack.api.command.admin.volume.MigrateVolumeCmdByAdmin; import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd; @@ -97,6 +94,7 @@ import com.cloud.agent.api.ModifyTargetsCommand; import com.cloud.agent.api.PingRoutingCommand; import com.cloud.agent.api.PlugNicAnswer; import com.cloud.agent.api.PlugNicCommand; +import com.cloud.agent.api.PrepareForMigrationAnswer; import com.cloud.agent.api.PrepareForMigrationCommand; import com.cloud.agent.api.RebootAnswer; import com.cloud.agent.api.RebootCommand; @@ -116,6 +114,7 @@ import com.cloud.agent.api.UnPlugNicCommand; import com.cloud.agent.api.UnregisterVMCommand; import com.cloud.agent.api.routing.NetworkElementCommand; import com.cloud.agent.api.to.DiskTO; +import com.cloud.agent.api.to.DpdkTO; import com.cloud.agent.api.to.GPUDeviceTO; import com.cloud.agent.api.to.NicTO; import com.cloud.agent.api.to.VirtualMachineTO; @@ -166,7 +165,9 @@ import com.cloud.network.dao.NetworkVO; import com.cloud.network.router.VirtualRouter; import com.cloud.offering.DiskOffering; import com.cloud.offering.DiskOfferingInfo; +import com.cloud.offering.NetworkOffering; import com.cloud.offering.ServiceOffering; +import com.cloud.offerings.dao.NetworkOfferingDetailsDao; import com.cloud.org.Cluster; import com.cloud.resource.ResourceManager; import com.cloud.resource.ResourceState; @@ -1123,6 +1124,9 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac vmGuru.finalizeDeployment(cmds, vmProfile, dest, ctx); + // Get VM extraConfig from DB and set to VM TO + addExtraConfig(vmTO); + work = _workDao.findById(work.getId()); if (work == null || work.getStep() != Step.Prepare) { throw new ConcurrentOperationException("Work steps have been changed: " + work); @@ -1287,6 +1291,16 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } + // Add extra config data to the vmTO as a Map + private void addExtraConfig(VirtualMachineTO vmTO) { + Map details = vmTO.getDetails(); + for (String key : details.keySet()) { + if (key.startsWith(ApiConstants.EXTRA_CONFIG)) { + vmTO.addExtraConfig(key, details.get(key)); + } + } + } + // for managed storage on KVM, need to make sure the path field of the volume in question is populated with the IQN private void handlePath(final DiskTO[] disks, final HypervisorType hypervisorType) { if (hypervisorType != HypervisorType.KVM) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 8c0239afcc3..8ee318fa8af 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -2236,7 +2236,11 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv vm.addComp(devices); - addExtraConfigComponent(extraConfig, vm); + // Add extra configuration to User VM Domain XML before starting + if (vmTO.getType().equals(VirtualMachine.Type.User) && MapUtils.isNotEmpty(extraConfig)) { + s_logger.info("Appending extra configuration data to guest VM domain XML"); + addExtraConfigComponent(extraConfig, vm); + } return vm; } diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index 79a9fb22972..4117892b59c 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -49,6 +49,7 @@ import javax.naming.ConfigurationException; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.apache.cloudstack.hypervisor.xenserver.ExtraConfigurationUtility; import org.apache.cloudstack.storage.to.TemplateObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.commons.collections.CollectionUtils; @@ -1404,7 +1405,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } } try { - finalizeVmMetaData(vm, conn, vmSpec); + finalizeVmMetaData(vm, vmr, conn, vmSpec); } catch (final Exception e) { throw new CloudRuntimeException("Unable to finalize VM MetaData: " + vmSpec); } @@ -1859,7 +1860,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } } - protected void finalizeVmMetaData(final VM vm, final Connection conn, final VirtualMachineTO vmSpec) throws Exception { + protected void finalizeVmMetaData(final VM vm, final VM.Record vmr, final Connection conn, final VirtualMachineTO vmSpec) throws Exception { final Map details = vmSpec.getDetails(); if (details != null) { @@ -1890,6 +1891,13 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } } } + + // Add configuration settings VM record for User VM instances before creating VM + Map extraConfig = vmSpec.getExtraConfig(); + if (vmSpec.getType().equals(VirtualMachine.Type.User) && MapUtils.isNotEmpty(extraConfig)) { + s_logger.info("Appending user extra configuration settings to VM"); + ExtraConfigurationUtility.setExtraConfigurationToVm(conn,vmr, vm, extraConfig); + } } /** diff --git a/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java b/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java new file mode 100644 index 00000000000..b58c5f88a9e --- /dev/null +++ b/plugins/hypervisors/xenserver/src/main/java/org/apache/cloudstack/hypervisor/xenserver/ExtraConfigurationUtility.java @@ -0,0 +1,180 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.hypervisor.xenserver; + +import java.util.HashMap; +import java.util.Map; + +import org.apache.log4j.Logger; +import org.apache.xmlrpc.XmlRpcException; + +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.exception.CloudRuntimeException; +import com.xensource.xenapi.Connection; +import com.xensource.xenapi.Types; +import com.xensource.xenapi.VM; + +public class ExtraConfigurationUtility { + private static final Logger LOG = Logger.getLogger(ExtraConfigurationUtility.class); + + public static void setExtraConfigurationToVm(Connection conn, VM.Record vmr, VM vm, Map extraConfig) { + Map recordMap = vmr.toMap(); + for (String key : extraConfig.keySet()) { + String cfg = extraConfig.get(key); + Map configParams = prepareKeyValuePair(cfg); + + // paramKey is either param or param:key for map parameters + String paramKey = configParams.keySet().toString().replaceAll("[\\[\\]]", ""); + String paramValue = configParams.get(paramKey); + + //Map params + if (paramKey.contains(":")) { + applyConfigWithNestedKeyValue(conn, vm, recordMap, paramKey, paramValue); + } else { + applyConfigWithKeyValue(conn, vm, recordMap, paramKey, paramValue); + } + } + } + + private static boolean isValidOperation(Map recordMap, String actualParam) { + return recordMap.containsKey(actualParam); + } + + /** + * Nested keys contain ":" between the paramKey and need to split into operation param and key + * */ + private static void applyConfigWithNestedKeyValue(Connection conn, VM vm, Map recordMap, String paramKey, String paramValue) { + int i = paramKey.indexOf(":"); + String actualParam = paramKey.substring(0, i); + String keyName = paramKey.substring(i + 1); + + if (!isValidOperation(recordMap, actualParam)) { + LOG.error("Unsupported extra configuration has been passed " + actualParam); + throw new InvalidParameterValueException("Unsupported extra configuration option has been passed: " + actualParam); + } + + try { + switch (actualParam) { + case "VCPUs_params": + vm.addToVCPUsParams(conn, keyName, paramValue); + break; + case "platform": + vm.addToOtherConfig(conn, keyName, paramValue); + break; + case "HVM_boot_params": + vm.addToHVMBootParams(conn, keyName, paramValue); + break; + case "other_config": + vm.addToOtherConfig(conn, keyName, paramValue); + break; + case "xenstore_data": + vm.addToXenstoreData(conn, keyName, paramValue); + break; + default: + String msg = String.format("Passed configuration %s is not supported", paramKey); + LOG.warn(msg); + } + } catch (XmlRpcException | Types.XenAPIException e) { + LOG.error("Exception caught while setting VM configuration. exception: " + e.getMessage()); + throw new CloudRuntimeException("Exception caught while setting VM configuration", e); + } + } + + private static void applyConfigWithKeyValue(Connection conn, VM vm, Map recordMap, String paramKey, String paramValue) { + if (!isValidOperation(recordMap, paramKey)) { + LOG.error("Unsupported extra configuration has been passed: " + paramKey); + throw new InvalidParameterValueException("Unsupported extra configuration parameter key has been passed: " + paramKey); + } + + try { + switch (paramKey) { + case "HVM_boot_policy": + vm.setHVMBootPolicy(conn, paramValue); + break; + case "HVM_shadow_multiplier": + vm.setHVMShadowMultiplier(conn, Double.valueOf(paramValue)); + break; + case "PV_kernel": + vm.setPVKernel(conn, paramValue); + break; + case "PV_ramdisk": + vm.setPVRamdisk(conn, paramValue); + break; + case "PV_args": + vm.setPVArgs(conn, paramValue); + break; + case "PV_legacy_args": + vm.setPVLegacyArgs(conn, paramValue); + break; + case "PV_bootloader": + vm.setPVBootloader(conn, paramValue); + break; + case "PV_bootloader_args": + vm.setPVBootloaderArgs(conn, paramValue); + break; + case "ha_restart_priority": + vm.setHaRestartPriority(conn, paramValue); + break; + case "start_delay": + vm.setStartDelay(conn, Long.valueOf(paramValue)); + break; + case "shutdown_delay": + vm.setShutdownDelay(conn, Long.valueOf(paramValue)); + break; + case "order": + vm.setOrder(conn, Long.valueOf(paramValue)); + break; + case "VCPUs_max": + vm.setVCPUsMax(conn, Long.valueOf(paramValue)); + break; + case "VCPUs_at_startup": + vm.setVCPUsAtStartup(conn, Long.valueOf(paramValue)); + break; + case "is-a-template": + vm.setIsATemplate(conn, Boolean.valueOf(paramValue)); + break; + case "memory_static_max": + vm.setMemoryStaticMax(conn, Long.valueOf(paramValue)); + break; + case "memory_static_min": + vm.setMemoryStaticMin(conn, Long.valueOf(paramValue)); + break; + case "memory_dynamic_max": + vm.setMemoryDynamicMax(conn, Long.valueOf(paramValue)); + break; + case "memory_dynamic_min": + vm.setMemoryDynamicMin(conn, Long.valueOf(paramValue)); + break; + default: + String anotherMessage = String.format("Passed configuration %s is not supported", paramKey); + LOG.error(anotherMessage); + } + } catch (XmlRpcException | Types.XenAPIException e) { + LOG.error("Exception caught while setting VM configuration, exception: " + e.getMessage()); + throw new CloudRuntimeException("Exception caught while setting VM configuration: ", e); + } + } + + private static Map prepareKeyValuePair(String cfg) { + Map configKeyPair = new HashMap<>(); + int indexOfEqualSign = cfg.indexOf("="); + String key = cfg.substring(0, indexOfEqualSign).replace("-", "_"); + String value = cfg.substring(indexOfEqualSign + 1); + configKeyPair.put(key, value); + return configKeyPair; + } +} \ No newline at end of file diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index b05135e9f78..8a84db82bcb 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -16,6 +16,8 @@ // under the License. package com.cloud.vm; +import java.io.IOException; +import java.io.StringReader; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.util.ArrayList; @@ -34,13 +36,17 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; import javax.inject.Inject; import javax.naming.ConfigurationException; +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; -import com.cloud.storage.ScopeType; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.affinity.AffinityGroupService; @@ -99,6 +105,11 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; @@ -234,6 +245,7 @@ import com.cloud.storage.DataStoreRole; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.GuestOSCategoryVO; import com.cloud.storage.GuestOSVO; +import com.cloud.storage.ScopeType; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage; @@ -265,7 +277,6 @@ import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.AccountService; -import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; import com.cloud.user.SSHKeyPair; import com.cloud.user.SSHKeyPairVO; @@ -530,8 +541,18 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir private static final ConfigKey AllowDeployVmIfGivenHostFails = new ConfigKey("Advanced", Boolean.class, "allow.deploy.vm.if.deploy.on.given.host.fails", "false", "allow vm to deploy on different host if vm fails to deploy on the given host ", true); - private static final ConfigKey EnableAdditionalVmConfig = new ConfigKey<>("Advanced", Boolean.class, "enable.additional.vm.configuration", - "false", "allow additional arbitrary configuration to vm", true, ConfigKey.Scope.Account); + private static final ConfigKey EnableAdditionalVmConfig = new ConfigKey<>("Advanced", Boolean.class, + "enable.additional.vm.configuration", "false", "allow additional arbitrary configuration to vm", true, ConfigKey.Scope.Account); + + private static final ConfigKey KvmAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class, + "allow.additional.vm.configuration.list.kvm", "", "Comma separated list of allowed additional configuration options.", true); + + private static final ConfigKey XenServerAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class, + "allow.additional.vm.configuration.list.xenserver", "", "Comma separated list of allowed additional configuration options", true); + + private static final ConfigKey VmwareAdditionalConfigAllowList = new ConfigKey<>("Advanced", String.class, + "allow.additional.vm.configuration.list.vmware", "", "Comma separated list of allowed additional configuration options.", true); + private static final ConfigKey VmDestroyForcestop = new ConfigKey("Advanced", Boolean.class, "vm.destroy.forcestop", "false", "On destroy, force-stop takes this value ", true); @@ -2479,6 +2500,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } else { if (MapUtils.isNotEmpty(details)) { + if (details.containsKey("extraconfig")) { + throw new InvalidParameterValueException("'extraconfig' should not be included in details as key"); + } + if (caller != null && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) { // Ensure blacklisted detail is not passed by non-root-admin user for (final String detailName : details.keySet()) { @@ -2505,9 +2530,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir vmInstance.setDetails(details); _vmDao.saveDetails(vmInstance); } - if (StringUtils.isNotBlank(extraConfig) && EnableAdditionalVmConfig.valueIn(accountId)) { - AccountVO account = _accountDao.findById(accountId); - addExtraConfig(vmInstance, account, extraConfig); + if (StringUtils.isNotBlank(extraConfig)) { + if (EnableAdditionalVmConfig.valueIn(accountId)) { + s_logger.info("Adding extra configuration to user vm: " + vmInstance.getUuid()); + addExtraConfig(vmInstance, extraConfig); + } else { + throw new InvalidParameterValueException("attempted setting extraconfig but enable.additional.vm.configuration is disabled"); + } } } return updateVirtualMachine(id, displayName, group, ha, isDisplayVm, osTypeId, userData, isDynamicallyScalable, @@ -5086,8 +5115,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir Account caller = CallContext.current().getCallingAccount(); Long callerId = caller.getId(); String extraConfig = cmd.getExtraConfig(); - if (StringUtils.isNotBlank(extraConfig) && EnableAdditionalVmConfig.valueIn(callerId) ) { - addExtraConfig(vm, caller, extraConfig); + if (StringUtils.isNotBlank(extraConfig)) { + if (EnableAdditionalVmConfig.valueIn(callerId)) { + s_logger.info("Adding extra configuration to user vm: " + vm.getUuid()); + addExtraConfig(vm, extraConfig); + } else { + throw new InvalidParameterValueException("attempted setting extraconfig but enable.additional.vm.configuration is disabled"); + } } if (cmd.getCopyImageTags()) { @@ -5106,24 +5140,119 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } /** - * Persist extra configurations as details for VMware VMs + * Persist extra configuration data in the user_vm_details table as key/value pair + * @param decodedUrl String consisting of the extra config data to appended onto the vmx file for VMware instances */ protected void persistExtraConfigVmware(String decodedUrl, UserVm vm) { - String[] configDataArr = decodedUrl.split("\\r?\\n"); - for (String config: configDataArr) { - String[] keyValue = config.split("="); - try { - userVmDetailsDao.addDetail(vm.getId(), keyValue[0], keyValue[1], true); - } catch (ArrayIndexOutOfBoundsException e) { - throw new CloudRuntimeException("Issue occurred during parsing of:" + config); + boolean isValidConfig = isValidKeyValuePair(decodedUrl); + if (isValidConfig) { + String[] extraConfigs = decodedUrl.split("\\r?\\n"); + for (String cfg : extraConfigs) { + // Validate cfg against unsupported operations set by admin here + String[] allowedKeyList = VmwareAdditionalConfigAllowList.value().split(","); + boolean validXenOrVmwareConfiguration = isValidXenOrVmwareConfiguration(cfg, allowedKeyList); + String[] paramArray = cfg.split("="); + if (validXenOrVmwareConfiguration && paramArray.length == 2) { + userVmDetailsDao.addDetail(vm.getId(), paramArray[0].trim(), paramArray[1].trim(), true); + } else { + throw new CloudRuntimeException("Extra config " + cfg + " is not on the list of allowed keys for VMware hypervisor hosts."); + } } + } else { + throw new CloudRuntimeException("The passed extra config string " + decodedUrl + "contains an invalid key/value pair pattern"); } } /** - * Persist extra configurations as details for hypervisors except Vmware + * Used to persist extra configuration settings in user_vm_details table for the XenServer hypervisor + * persists config as key/value pair e.g key = extraconfig-1 , value="PV-bootloader=pygrub" and so on to extraconfig-N where + * N denotes the number of extra configuration settings passed by user + * + * @param decodedUrl A string containing extra configuration settings as key/value pairs seprated by newline escape character + * e.x PV-bootloader=pygrub\nPV-args=console\nHV-Boot-policy="" */ - protected void persistExtraConfigNonVmware(String decodedUrl, UserVm vm) { + protected void persistExtraConfigXenServer(String decodedUrl, UserVm vm) { + boolean isValidConfig = isValidKeyValuePair(decodedUrl); + if (isValidConfig) { + String[] extraConfigs = decodedUrl.split("\\r?\\n"); + int i = 1; + String extraConfigKey = ApiConstants.EXTRA_CONFIG + "-"; + for (String cfg : extraConfigs) { + // Validate cfg against unsupported operations set by admin here + String[] allowedKeyList = XenServerAdditionalConfigAllowList.value().split(","); + boolean validXenOrVmwareConfiguration = isValidXenOrVmwareConfiguration(cfg, allowedKeyList); + if (validXenOrVmwareConfiguration) { + userVmDetailsDao.addDetail(vm.getId(), extraConfigKey + String.valueOf(i), cfg, true); + i++; + } else { + throw new CloudRuntimeException("Extra config " + cfg + " is not on the list of allowed keys for XenServer hypervisor hosts."); + } + } + } else { + String msg = String.format("The passed extra config string '%s' contains an invalid key/value pair pattern", decodedUrl); + throw new CloudRuntimeException(msg); + } + } + + /** + * Used to valid extraconfig keylvalue pair for Vmware and XenServer + * Example of tested valid config for VMware as taken from VM instance vmx file + *

+ * nvp.vm-uuid=34b3d5ea-1c25-4bb0-9250-8dc3388bfa9b + * migrate.hostLog=i-2-67-VM-5130f8ab.hlog + * ethernet0.address=02:00:5f:51:00:41 + *

+ *

+ * Examples of tested valid configs for XenServer + *

+ * is-a-template=true\nHVM-boot-policy=\nPV-bootloader=pygrub\nPV-args=hvc0 + *

+ * + * Allow the following character set {', ", -, ., =, a-z, 0-9, empty space, \n} + * + * @param decodedUrl String conprising of extra config key/value pairs for XenServer and Vmware + * @return True if extraconfig is valid key/value pair + */ + protected boolean isValidKeyValuePair(String decodedUrl) { + // Valid pairs should look like "key-1=value1, param:key-2=value2, my.config.v0=False" + Pattern pattern = Pattern.compile("^(?:[\\w-\\s\\.:]*=[\\w-\\s\\.'\":]*(?:\\s+|$))+$"); + Matcher matcher = pattern.matcher(decodedUrl); + return matcher.matches(); + } + + /** + * Validates key/value pair strings passed as extra configuration for XenServer and Vmware + * @param cfg configuration key-value pair + * @param allowedKeyList list of allowed configuration keys for XenServer and VMware + * @return + */ + protected boolean isValidXenOrVmwareConfiguration(String cfg, String[] allowedKeyList) { + // This should be of minimum length 1 + // Value is ignored in case it is empty + String[] cfgKeyValuePair = cfg.split("="); + if (cfgKeyValuePair.length >= 1) { + for (String allowedKey : allowedKeyList) { + if (cfgKeyValuePair[0].equalsIgnoreCase(allowedKey.trim())) { + return true; + } + } + } else { + String msg = String.format("An incorrect configuration %s has been passed", cfg); + throw new CloudRuntimeException(msg); + } + return false; + } + + /** + * Persist extra configuration data on KVM + * persisted in the user_vm_details DB as extraconfig-1, and so on depending on the number of configurations + * For KVM, extra config is passed as XML + * @param decodedUrl string containing xml configuration to be persisted into user_vm_details table + * @param vm + */ + protected void persistExtraConfigKvm(String decodedUrl, UserVm vm) { + // validate config against blacklisted cfg commands + validateKvmExtraConfig(decodedUrl); String[] extraConfigs = decodedUrl.split("\n\n"); for (String cfg : extraConfigs) { int i = 1; @@ -5131,7 +5260,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir String extraConfigKey = ApiConstants.EXTRA_CONFIG; String extraConfigValue; if (cfgParts[0].matches("\\S+:$")) { - extraConfigKey += "-" + cfgParts[0].substring(0,cfgParts[0].length() - 1); + extraConfigKey += "-" + cfgParts[0].substring(0, cfgParts[0].length() - 1); extraConfigValue = cfg.replace(cfgParts[0] + "\n", ""); } else { extraConfigKey += "-" + String.valueOf(i); @@ -5142,16 +5271,71 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } - protected void addExtraConfig(UserVm vm, Account caller, String extraConfig) { - String decodedUrl = decodeExtraConfig(extraConfig); - HypervisorType hypervisorType = vm.getHypervisorType(); - if (hypervisorType == HypervisorType.VMware) { - persistExtraConfigVmware(decodedUrl, vm); - } else { - persistExtraConfigNonVmware(decodedUrl, vm); + /** + * This method is called by the persistExtraConfigKvm + * Validates passed extra configuration data for KVM and validates against blacklist of unwanted commands + * controlled by Root admin + * @param decodedUrl string containing xml configuration to be validated + */ + protected void validateKvmExtraConfig(String decodedUrl) { + String[] allowedConfigOptionList = KvmAdditionalConfigAllowList.value().split(","); + // Skip allowed keys validation validation for DPDK + if (!decodedUrl.contains(":")) { + try { + DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + InputSource src = new InputSource(); + src.setCharacterStream(new StringReader(String.format("\n%s\n", decodedUrl))); + Document doc = builder.parse(src); + doc.getDocumentElement().normalize(); + NodeList nodeList=doc.getElementsByTagName("*"); + for (int i = 1; i < nodeList.getLength(); i++) { // First element is config so skip it + Element element = (Element)nodeList.item(i); + boolean isValidConfig = false; + String currentConfig = element.getNodeName().trim(); + for (String tag : allowedConfigOptionList) { + if (currentConfig.equals(tag.trim())) { + isValidConfig = true; + } + } + if (!isValidConfig) { + throw new CloudRuntimeException(String.format("Extra config %s is not on the list of allowed keys for KVM hypervisor hosts", currentConfig)); + } + } + } catch (ParserConfigurationException | IOException | SAXException e) { + throw new CloudRuntimeException("Failed to parse additional XML configuration: " + e.getMessage()); + } } } + /** + * Adds extra config data to guest VM instances + * @param extraConfig Extra Configuration settings to be added in UserVm instances for KVM, XenServer and VMware + */ + protected void addExtraConfig(UserVm vm, String extraConfig) { + String decodedUrl = decodeExtraConfig(extraConfig); + HypervisorType hypervisorType = vm.getHypervisorType(); + + switch (hypervisorType) { + case XenServer: + persistExtraConfigXenServer(decodedUrl, vm); + break; + case KVM: + persistExtraConfigKvm(decodedUrl, vm); + break; + case VMware: + persistExtraConfigVmware(decodedUrl, vm); + break; + default: + String msg = String.format("This hypervisor %s is not supported for use with this feature", hypervisorType.toString()); + throw new CloudRuntimeException(msg); + } + } + + /** + * Decodes an URL encoded string passed as extra configuration for guest VMs + * @param encodeString URL encoded string + * @return String result of decoded URL + */ protected String decodeExtraConfig(String encodeString) { String decodedUrl; try { @@ -5198,6 +5382,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir maxIops = details.get("maxIopsDo"); verifyMinAndMaxIops(minIops, maxIops); + + if (details.containsKey("extraconfig")) { + throw new InvalidParameterValueException("'extraconfig' should not be included in details as key"); + } } } @@ -6732,8 +6920,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, VmIpFetchThreadPoolMax, - VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties}; + return new ConfigKey[] {EnableDynamicallyScaleVm, AllowUserExpungeRecoverVm, VmIpFetchWaitInterval, VmIpFetchTrialMax, + VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties, + KvmAdditionalConfigAllowList, XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList}; } @Override diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 965377b2c7b..f5bfa2ba950 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -16,6 +16,10 @@ // under the License. package com.cloud.vm; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + import java.util.ArrayList; import java.util.HashMap; @@ -23,7 +27,6 @@ import org.apache.cloudstack.api.BaseCmd.HTTPMethod; import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd; import org.apache.cloudstack.context.CallContext; import org.junit.After; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -291,6 +294,18 @@ public class UserVmManagerImplTest { String returnedMacAddress = userVmManagerImpl.validateOrReplaceMacAddress(macAddress, 1l); Mockito.verify(networkModel, Mockito.times(times)).getNextAvailableMacAddressInNetwork(Mockito.anyLong()); - Assert.assertEquals(expectedMacAddress, returnedMacAddress); + assertEquals(expectedMacAddress, returnedMacAddress); + } + + @Test + public void testValidatekeyValuePair() throws Exception { + assertTrue(userVmManagerImpl.isValidKeyValuePair("is-a-template=true\nHVM-boot-policy=\nPV-bootloader=pygrub\nPV-args=hvc0")); + assertTrue(userVmManagerImpl.isValidKeyValuePair("is-a-template=true HVM-boot-policy= PV-bootloader=pygrub PV-args=hvc0")); + assertTrue(userVmManagerImpl.isValidKeyValuePair("nvp.vm-uuid=34b3d5ea-1c25-4bb0-9250-8dc3388bfa9b")); + assertFalse(userVmManagerImpl.isValidKeyValuePair("key")); + //key-1=value1, param:key-2=value2, my.config.v0=False" + assertTrue(userVmManagerImpl.isValidKeyValuePair("key-1=value1")); + assertTrue(userVmManagerImpl.isValidKeyValuePair("param:key-2=value2")); + assertTrue(userVmManagerImpl.isValidKeyValuePair("my.config.v0=False")); } } diff --git a/test/integration/smoke/test_deploy_vm_extra_config_data.py b/test/integration/smoke/test_deploy_vm_extra_config_data.py new file mode 100644 index 00000000000..63e0040d54f --- /dev/null +++ b/test/integration/smoke/test_deploy_vm_extra_config_data.py @@ -0,0 +1,542 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" BVT tests for Virtual Machine additional configuration +""" +# Import System modules +import urllib +import xml.etree.ElementTree as ET + +from lxml import etree +from marvin.cloudstackAPI import (updateVirtualMachine, + deployVirtualMachine, + destroyVirtualMachine, + stopVirtualMachine, + startVirtualMachine, + updateConfiguration, + listVirtualMachines) +# Import Local Modules +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import (Account, + ServiceOffering, + ) +from marvin.lib.common import (get_domain, + get_zone, + get_template, + list_hosts) +from marvin.lib.utils import * +from nose.plugins.attrib import attr + +class TestAddConfigtoDeployVM(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + testClient = super(TestAddConfigtoDeployVM, cls).getClsTestClient() + cls.apiclient = testClient.getApiClient() + cls.services = testClient.getParsedTestDataConfig() + + # Get Zone, Domain and templates + cls.domain = get_domain(cls.apiclient) + cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests()) + cls.hypervisor = testClient.getHypervisorInfo() + cls.services['mode'] = cls.zone.networktype + cls.hostConfig = cls.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][ + 0].__dict__ + + # Set Zones and disk offerings + cls.services["small"]["zoneid"] = cls.zone.id + + cls.services["iso1"]["zoneid"] = cls.zone.id + + cls.services["virtual_machine"]["zoneid"] = cls.zone.id + + # Create an account, network, and IP addresses + cls.account = Account.create( + cls.apiclient, + cls.services["account"], + domainid=cls.domain.id + ) + cls.service_offering = ServiceOffering.create( + cls.apiclient, + cls.services["service_offerings"]["small"] + ) + + cls.cleanup = [ + cls.account, + cls.service_offering + ] + + @classmethod + def tearDownClass(cls): + try: + cls.apiclient = super(TestAddConfigtoDeployVM, cls).getClsTestClient().getApiClient() + # Clean up, terminate the created templates + cleanup_resources(cls.apiclient, cls.cleanup) + + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.hypervisor = self.testClient.getHypervisorInfo() + self.dbclient = self.testClient.getDbConnection() + + """ + Set EnableAdditionalData to true + """ + updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() + updateConfigurationCmd.name = "enable.additional.vm.configuration" + updateConfigurationCmd.value = "true" + updateConfigurationCmd.scopename = "account" + updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd) + self.debug("updated the parameter %s with value %s" % ( + updateConfigurationResponse.name, updateConfigurationResponse.value)) + + # Ste Global Config value + def add_global_config(self, name, value): + self.apiclient = self.testClient.getApiClient() + self.hypervisor = self.testClient.getHypervisorInfo() + self.dbclient = self.testClient.getDbConnection() + + cmd = updateConfiguration.updateConfigurationCmd() + cmd.name = name + cmd.value = value + return self.apiclient.updateConfiguration(cmd) + + # Compare XML Element objects + def elements_equal(self, e1, e2): + if e1.tag != e2.tag: + return False + if e1.attrib != e2.attrib: + return False + if len(e1) != len(e2): + return False + return all(self.elements_equal(c1, c2) for c1, c2 in zip(e1, e2)) + + def destroy_vm(self, vm_id): + cmd = destroyVirtualMachine.destroyVirtualMachineCmd() + cmd.expunge = True + cmd.id = vm_id + return self.apiclient.destroyVirtualMachine(cmd) + + def deploy_vm(self, hypervisor, extra_config=None): + cmd = deployVirtualMachine.deployVirtualMachineCmd() + if extra_config is not None: + cmd.extraconfig = extra_config + + template = get_template( + self.apiclient, + self.zone.id, + hypervisor=hypervisor + ) + cmd.zoneid = self.zone.id + cmd.templateid = template.id + cmd.serviceofferingid = self.service_offering.id + return self.apiclient.deployVirtualMachine(cmd) + + def list_vm(self): + cmd = listVirtualMachines.listVirtualMachinesCmd() + cmd.hypervisor = self.hypervisor + return self.apiclient.listVirtualMachines(cmd)[0] + + def update_vm(self, id, extra_config): + cmd = updateVirtualMachine.updateVirtualMachineCmd() + cmd.id = id + cmd.extraconfig = extra_config + return self.apiclient.updateVirtualMachine(cmd) + + def stop_vm(self, id): + cmd = stopVirtualMachine.stopVirtualMachineCmd() + cmd.id = id + return self.apiclient.stopVirtualMachine(cmd) + + def start_vm(self, id): + cmd = startVirtualMachine.startVirtualMachineCmd() + cmd.id = id + return self.apiclient.startVirtualMachine(cmd) + + # Parse extraconfig for config with that returned by xe vm-param-get ... + def get_xen_param_values(self, config): + equal_sign_index = config.index("=") + cmd_option = config[:equal_sign_index] + cmd_value = config[equal_sign_index + 1:] + return cmd_option, cmd_value + + # Format vm config such that it equals the one from vmx file + def prepare_vmware_config(self, config): + equal_sign_index = config.index("=") + cmd_option = config[:equal_sign_index] + cmd_value = config[equal_sign_index + 1:] + return cmd_option + ' = ' '"{}"'.format(cmd_value) + + # Get vm uuid from xenserver host + def get_vm_uuid(self, instance_name, ssh_client): + cmd = 'xe vm-list name-label={} params=uuid '.format(instance_name) + result = ssh_client.execute(cmd) + uuid_str = result[0] + i = uuid_str.index(":") + return uuid_str[i + 1:].strip() + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_01_deploy_vm_with_extraconfig_throws_exception_kvm(self): + ''' + Test that extra config is not added when element tag is not added on the allowed list global config on KVM hosts + ''' + + hypervisor = self.hypervisor.lower() + if hypervisor != 'kvm': + raise self.skipTest("Skipping test case for non-kvm hypervisor") + + ''' + The following extraconfig is required for enabling hugepages on kvm + + + + url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' + ''' + extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" + + try: + # Clear KVM allow list to show that code throws exception when command is not included in the list + name = 'allow.additional.vm.configuration.list.kvm' + + self.add_global_config(name, "") + self.assertRaises(Exception, + self.deploy_vm(hypervisor, extraconfig), + "Exception was not thrown, check kvm global configuration") + except Exception as e: + logging.debug(e) + finally: + self.destroy_vm(self.list_vm().id) + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_02_deploy_vm_with_extraconfig_kvm(self): + ''' + Test that extra config is added on KVM hosts + ''' + + hypervisor = self.hypervisor.lower() + if hypervisor != 'kvm': + raise self.skipTest("Skipping test case for non-kvm hypervisor") + + name = 'allow.additional.vm.configuration.list.kvm' + value = 'memoryBacking, hugepages, unsedConfigKey' + + add_config_response = self.add_global_config(name, value) + + if add_config_response.name: + try: + ''' + The following extraconfig is required for enabling hugepages on kvm + + + + url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' + ''' + extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" + + response = self.deploy_vm(hypervisor, extraconfig) + + host_id = response.hostid + host = list_hosts( + self.apiclient, + id=host_id, + hypervisor=hypervisor) + + instance_name = response.instancename + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + virsh_cmd = 'virsh dumpxml %s' % instance_name + xml_res = ssh_client.execute(virsh_cmd) + xml_as_str = ''.join(xml_res) + + extraconfig_decoded_xml = '' + urllib.unquote(extraconfig) + '' + + # Root XML Elements + parser = etree.XMLParser(remove_blank_text=True) + domain_xml_root = ET.fromstring(xml_as_str, parser=parser) + decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser) + for child in decoded_xml_root: + find_element_in_domain_xml = domain_xml_root.find(child.tag) + + # Fail if extra config is not found in domain xml + self.assertNotEquals( + 0, + len(find_element_in_domain_xml), + 'Element tag from extra config not added to VM' + ) + + # Compare found XML node with extra config node + is_a_match = self.elements_equal(child, find_element_in_domain_xml) + self.assertEquals( + True, + is_a_match, + 'The element from tags from extra config do not match with those found in domain xml' + ) + finally: + self.destroy_vm(response.id) + self.add_global_config(name, "") + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_03_update_vm_with_extraconfig_kvm(self): + ''' + Test that extra config is added on KVM hosts + ''' + + hypervisor = self.hypervisor.lower() + if hypervisor != 'kvm': + raise self.skipTest("Skipping test case for non-kvm hypervisor") + + name = 'allow.additional.vm.configuration.list.kvm' + value = 'memoryBacking, hugepages' + + add_config_response = self.add_global_config(name, value) + + if add_config_response.name: + try: + ''' + The following extraconfig is required for enabling hugepages on kvm + + + + url encoded extra_config = '%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E' + ''' + extraconfig = "%3CmemoryBacking%3E%0D%0A++%3Chugepages%2F%3E%0D%0A%3C%2FmemoryBacking%3E" + + response = self.deploy_vm(hypervisor) + vm_id = response.id + + ''' + For updateVirtualMachineCmd, the VM must be stopped and restarted for changes to take effect + ''' + self.stop_vm(vm_id) + self.update_vm(vm_id, extraconfig) + start_resp = self.start_vm(vm_id) + + host_id = start_resp.hostid + host = list_hosts( + self.apiclient, + id=host_id, + hypervisor=hypervisor) + + instance_name = response.instancename + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + virsh_cmd = 'virsh dumpxml %s' % instance_name + xml_res = ssh_client.execute(virsh_cmd) + xml_as_str = ''.join(xml_res) + + extraconfig_decoded_xml = '' + urllib.unquote(extraconfig) + '' + + # Root XML Elements + parser = etree.XMLParser(remove_blank_text=True) + domain_xml_root = ET.fromstring(xml_as_str, parser=parser) + decoded_xml_root = ET.fromstring(extraconfig_decoded_xml, parser=parser) + for child in decoded_xml_root: + find_element_in_domain_xml = domain_xml_root.find(child.tag) + + # Fail if extra config is not found in domain xml + self.assertNotEquals( + 0, + len(find_element_in_domain_xml), + 'Element tag from extra config not added to VM' + ) + + # Compare found XML node with extra config node + is_a_match = self.elements_equal(child, find_element_in_domain_xml) + self.assertEquals( + True, + is_a_match, + 'The element from tags from extra config do not match with those found in domain xml' + ) + finally: + self.destroy_vm(vm_id) + self.add_global_config(name, "") + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_04_deploy_vm_with_extraconfig_throws_exception_vmware(self): + ''' + Test that extra config is not added when configuration key is not added on the allowed list global config for VMWARE hosts + ''' + + hypervisor = self.hypervisor.lower() + if hypervisor != 'vmware': + raise self.skipTest("Skipping test case for non-vmware hypervisor") + + ''' + The following extra configuration is used to set Hyper-V instance to run on ESXi host + hypervisor.cpuid.v0 = FALSE + ''' + extraconfig = 'hypervisor.cpuid.v0%3DFALSE' + + try: + # Clear VMWARE allow list to show that code throws exception when command is not included in the list + name = 'allow.additional.vm.configuration.list.vmware' + + self.add_global_config(name, "") + self.assertRaises(Exception, + self.deploy_vm(hypervisor, extraconfig), + "Exception was not thrown, check VMWARE global configuration") + except Exception as e: + logging.debug(e) + finally: + self.destroy_vm(self.list_vm().id) + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_05_deploy_vm_with_extraconfig_vmware(self): + ''' + Test that extra config is added on VMware hosts + ''' + hypervisor = self.hypervisor.lower() + if hypervisor != 'vmware': + raise self.skipTest("Skipping test case for non-vmware hypervisor") + + name = 'allow.additional.vm.configuration.list.vmware' + value = 'hypervisor.cpuid.v0' + + add_config_response = self.add_global_config(name, value) + + if add_config_response.name: + + ''' + The following extra configuration is used to set Hyper-V instance to run on ESXi host + hypervisor.cpuid.v0 = FALSE + ''' + extraconfig = 'hypervisor.cpuid.v0%3DFALSE' + try: + response = self.deploy_vm(hypervisor, extraconfig) + host_id = response.hostid + host = list_hosts( + self.apiclient, + id=host_id) + + instance_name = response.instancename + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + + extraconfig_decoded = urllib.unquote(extraconfig) + config_arr = extraconfig_decoded.splitlines() + + for config in config_arr: + vmx_config = self.prepare_vmware_config(config) + vmx_file_name = "\"$(esxcli vm process list | grep %s | tail -1 | awk '{print $3}')\"" % instance_name + # parse vm instance vmx file to see if extraconfig has been added + grep_config = "cat %s | grep -w '%s'" % (vmx_file_name, vmx_config) + result = ssh_client.execute(grep_config) + # Match exact configuration from vmx file, return empty result array if configuration is not found + self.assertNotEquals( + 0, + len(result), + 'Extra configuration not found in instance vmx file' + ) + finally: + self.destroy_vm(response.id) + self.add_global_config(name, "") + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_06_deploy_vm_with_extraconfig_throws_exception_xenserver(self): + ''' + Test that extra config is not added when configuration key is not added on the allowed list global config for XenServer hosts + ''' + + hypervisor = self.hypervisor.lower() + if hypervisor != 'xenserver': + raise self.skipTest("Skipping test case for non-xenserver hypervisor") + + ''' + Following commands are used to convert a VM from HVM to PV and set using vm-param-set + HVM-boot-policy= + PV-bootloader=pygrub + PV-args=hvc0 + ''' + + extraconfig = 'HVM-boot-policy%3D%0APV-bootloader%3Dpygrub%0APV-args%3Dhvc0' + + try: + # Clear VMWARE allow list to show that code throws exception when command is not included in the list + name = 'allow.additional.vm.configuration.list.xenserver' + + self.add_global_config(name, "") + self.assertRaises(Exception, + self.deploy_vm(hypervisor, extraconfig), + "Exception was not thrown, check XenServer global configuration") + + except Exception as e: + logging.debug(e) + finally: + self.destroy_vm(self.list_vm().id) + + @attr(tags=["devcloud", "advanced", "advancedns", "smoke", "basic", "sg"], required_hardware="true") + def test_07_deploy_vm_with_extraconfig_xenserver(self): + hypervisor = self.hypervisor.lower() + if hypervisor != 'xenserver': + raise self.skipTest("Skipping test case for non-xenserver hypervisor") + """ + Following commands are used to convert a VM from HVM to PV and set using vm-param-set + HVM-boot-policy= + PV-bootloader=pygrub + PV-args=hvc0 + """ + + name = 'allow.additional.vm.configuration.list.xenserver' + value = 'HVM-boot-policy, PV-bootloader, PV-args' + + add_config_response = self.add_global_config(name, value) + + if add_config_response.name: + extraconfig = 'HVM-boot-policy%3D%0APV-bootloader%3Dpygrub%0APV-args%3Dhvc0' + try: + response = self.deploy_vm(hypervisor, extraconfig) + host_id = response.hostid + host = list_hosts( + self.apiclient, + id=host_id) + + host_ipaddress = host[0].ipaddress + + ssh_client = SshClient(host_ipaddress, port=22, + user=self.hostConfig['username'], + passwd=self.hostConfig['password']) + + extraconfig_decoded = urllib.unquote(extraconfig) + config_arr = extraconfig_decoded.splitlines() + + # Get vm instance uuid + instance_uuid = self.get_vm_uuid(response.instancename, ssh_client) + for config in config_arr: + config_tuple = self.get_xen_param_values(config) + # Log on to XenServer host and check the vm-param-get + vm_config_check = 'xe vm-param-get param-name={} uuid={}'.format(config_tuple[0], instance_uuid) + result = ssh_client.execute(vm_config_check) + param_value = config_tuple[1].strip() + # Check if each configuration command has set the configuration as sent with extraconfig + self.assertEquals( + param_value, + result[0], + 'Extra configuration not found in VM param list' + ) + finally: + self.destroy_vm(response.id) + self.add_global_config(name, "")