From 9349d20dd36c490eeb6e140b9215ca68653a85d1 Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Fri, 30 Apr 2021 01:17:50 -0300 Subject: [PATCH] vmware: Make deploy-as-is optional (#4901) * [Vmware] Make deploy-as-is optional * Do not use deployasis on create volume test * Update api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java Co-authored-by: Abhishek Kumar * Update api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java Co-authored-by: Abhishek Kumar * Review comments * Refactor condition to select suitable template Co-authored-by: Abhishek Kumar --- .../user/template/RegisterTemplateCmd.java | 14 +++++--- .../template/RegisterTemplateCmdTest.java | 33 ++++++++++++++++++- .../cloud/template/TemplateAdapterBase.java | 2 +- .../smoke/test_kubernetes_clusters.py | 4 +-- test/integration/smoke/test_storage_policy.py | 1 + test/integration/smoke/test_templates.py | 1 + test/integration/smoke/test_volumes.py | 3 +- tools/marvin/marvin/lib/base.py | 31 +++++++++-------- tools/marvin/marvin/lib/common.py | 11 ++++--- ui/public/locales/en.json | 2 +- .../views/image/RegisterOrUploadTemplate.vue | 22 +++++++++---- 11 files changed, 87 insertions(+), 37 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java index 74053ee809d..bb7f7a441b6 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmd.java @@ -72,7 +72,7 @@ public class RegisterTemplateCmd extends BaseCmd implements UserCmd { private String format; @Parameter(name = ApiConstants.HYPERVISOR, type = CommandType.STRING, required = true, description = "the target hypervisor for the template") - private String hypervisor; + protected String hypervisor; @Parameter(name = ApiConstants.IS_FEATURED, type = CommandType.BOOLEAN, description = "true if this template is a featured template, false otherwise") private Boolean featured; @@ -162,6 +162,11 @@ public class RegisterTemplateCmd extends BaseCmd implements UserCmd { description = "true if template should bypass Secondary Storage and be downloaded to Primary Storage on deployment") private Boolean directDownload; + @Parameter(name=ApiConstants.DEPLOY_AS_IS, + type = CommandType.BOOLEAN, + description = "(VMware only) true if VM deployments should preserve all the configurations defined for this template", since = "4.15.1") + protected Boolean deployAsIs; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -274,8 +279,9 @@ public class RegisterTemplateCmd extends BaseCmd implements UserCmd { return directDownload == null ? false : directDownload; } - public Boolean isDeployAsIs() { - return hypervisor != null && hypervisor.equalsIgnoreCase(Hypervisor.HypervisorType.VMware.toString()); + public boolean isDeployAsIs() { + return Hypervisor.HypervisorType.VMware.toString().equalsIgnoreCase(hypervisor) && + Boolean.TRUE.equals(deployAsIs); } ///////////////////////////////////////////////////// @@ -341,7 +347,7 @@ public class RegisterTemplateCmd extends BaseCmd implements UserCmd { "Parameter directdownload is only allowed for KVM templates"); } - if (!getHypervisor().equalsIgnoreCase(Hypervisor.HypervisorType.VMware.toString()) && osTypeId == null) { + if (!isDeployAsIs() && osTypeId == null) { throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Please provide a guest OS type"); } } diff --git a/api/src/test/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmdTest.java b/api/src/test/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmdTest.java index f0cf6a91af2..dfd3b6e2e25 100644 --- a/api/src/test/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmdTest.java +++ b/api/src/test/java/org/apache/cloudstack/api/command/user/template/RegisterTemplateCmdTest.java @@ -20,6 +20,7 @@ package org.apache.cloudstack.api.command.user.template; import com.cloud.exception.ResourceAllocationException; +import com.cloud.hypervisor.Hypervisor; import com.cloud.template.TemplateApiService; import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.ServerApiException; @@ -32,7 +33,7 @@ import org.mockito.runners.MockitoJUnitRunner; import java.util.ArrayList; @RunWith(MockitoJUnitRunner.class) -public class RegisterTemplateCmdTest{ +public class RegisterTemplateCmdTest { @InjectMocks private RegisterTemplateCmd registerTemplateCmd; @@ -108,4 +109,34 @@ public class RegisterTemplateCmdTest{ registerTemplateCmd.zoneId = 1L; Assert.assertEquals((Long)1L,registerTemplateCmd.getZoneIds().get(0)); } + + private void testIsDeployAsIsBase(Hypervisor.HypervisorType hypervisorType, Boolean deployAsIsParameter, boolean expectedResult) { + registerTemplateCmd = new RegisterTemplateCmd(); + registerTemplateCmd.hypervisor = hypervisorType.name(); + registerTemplateCmd.deployAsIs = deployAsIsParameter; + boolean isDeployAsIs = registerTemplateCmd.isDeployAsIs(); + Assert.assertEquals(expectedResult, isDeployAsIs); + } + + @Test + public void testIsDeployAsIsVmwareNullAsIs() { + testIsDeployAsIsBase(Hypervisor.HypervisorType.VMware, null, false); + } + + @Test + public void testIsDeployAsIsVmwareNotAsIs() { + testIsDeployAsIsBase(Hypervisor.HypervisorType.VMware, false, false); + } + + @Test + public void testIsDeployAsIsVmwareAsIs() { + testIsDeployAsIsBase(Hypervisor.HypervisorType.VMware, true, true); + } + + @Test + public void testIsDeployAsIsNonVmware() { + testIsDeployAsIsBase(Hypervisor.HypervisorType.KVM, true, false); + testIsDeployAsIsBase(Hypervisor.HypervisorType.XenServer, true, false); + testIsDeployAsIsBase(Hypervisor.HypervisorType.Any, true, false); + } } diff --git a/server/src/main/java/com/cloud/template/TemplateAdapterBase.java b/server/src/main/java/com/cloud/template/TemplateAdapterBase.java index 2c330f62c69..52628ee2e23 100644 --- a/server/src/main/java/com/cloud/template/TemplateAdapterBase.java +++ b/server/src/main/java/com/cloud/template/TemplateAdapterBase.java @@ -291,7 +291,7 @@ public abstract class TemplateAdapterBase extends AdapterBase implements Templat } Map details = cmd.getDetails(); - if (hypervisorType == HypervisorType.VMware) { + if (cmd.isDeployAsIs()) { if (MapUtils.isNotEmpty(details)) { if (details.containsKey(VmDetailConstants.ROOT_DISK_CONTROLLER)) { s_logger.info("Ignoring the rootDiskController detail provided, as we honour what is defined in the template"); diff --git a/test/integration/smoke/test_kubernetes_clusters.py b/test/integration/smoke/test_kubernetes_clusters.py index 5ea5d772de0..55394c6f323 100644 --- a/test/integration/smoke/test_kubernetes_clusters.py +++ b/test/integration/smoke/test_kubernetes_clusters.py @@ -260,8 +260,8 @@ class TestKubernetesCluster(cloudstackTestCase): if validateList(templates)[0] != PASS: details = None - if hypervisor in ["vmware"]: - details = [{"keyboard": "us"}] + if hypervisor in ["vmware"] and "details" in cks_template: + details = cks_template["details"] template = Template.register(cls.apiclient, cks_template, zoneid=cls.zone.id, hypervisor=hypervisor.lower(), randomize_name=False, details=details) template.download(cls.apiclient) return template, False diff --git a/test/integration/smoke/test_storage_policy.py b/test/integration/smoke/test_storage_policy.py index a7a5f8d0cba..ea35b4db69b 100644 --- a/test/integration/smoke/test_storage_policy.py +++ b/test/integration/smoke/test_storage_policy.py @@ -59,6 +59,7 @@ class TestVMWareStoragePolicies(cloudstackTestCase): cls.apiclient, cls.zone.id, cls.hypervisor, + deploy_as_is=cls.hypervisor.lower() == "vmware" ) cls._cleanup.append(cls.network_offering) return diff --git a/test/integration/smoke/test_templates.py b/test/integration/smoke/test_templates.py index 0947b4b62eb..2ea8ed24693 100644 --- a/test/integration/smoke/test_templates.py +++ b/test/integration/smoke/test_templates.py @@ -84,6 +84,7 @@ class TestCreateTemplateWithChecksum(cloudstackTestCase): self.test_template.displaytext = 'test sha-1' self.test_template.url = "http://dl.openvm.eu/cloudstack/macchinina/x86_64/macchinina-vmware.ova" self.test_template.format = "OVA" + self.test_template.ostypeid = self.getOsType("Other Linux (64-bit)") self.md5 = "27f3c56a8c7ec7b2f3ff2199f7078006" self.sha256 = "a7b04c1eb507f3f5de844bda352df1ea5e20335b465409493ca6ae07dfd0a158" diff --git a/test/integration/smoke/test_volumes.py b/test/integration/smoke/test_volumes.py index aa4eaf13fa4..9cb324bc7ba 100644 --- a/test/integration/smoke/test_volumes.py +++ b/test/integration/smoke/test_volumes.py @@ -308,7 +308,8 @@ class TestVolumes(cloudstackTestCase): cls.apiclient, cls.zone.id, cls.services["ostype"], - cls.hypervisor + cls.hypervisor, + deploy_as_is=cls.hypervisor.lower() == "vmware" ) if cls.template == FAILED: assert False, "get_suitable_test_template() failed to return template with description %s" % cls.services["ostype"] diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index 3de64ef318e..efe7331d0dc 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -1403,24 +1403,22 @@ class Template: elif "hypervisor" in services: cmd.hypervisor = services["hypervisor"] - if cmd.hypervisor.lower() not in ["vmware"]: - # Since version 4.15 VMware templates honour the guest OS defined in the template - if "ostypeid" in services: - cmd.ostypeid = services["ostypeid"] - elif "ostype" in services: - # Find OSTypeId from Os type - sub_cmd = listOsTypes.listOsTypesCmd() - sub_cmd.description = services["ostype"] - ostypes = apiclient.listOsTypes(sub_cmd) + if "ostypeid" in services: + cmd.ostypeid = services["ostypeid"] + elif "ostype" in services: + # Find OSTypeId from Os type + sub_cmd = listOsTypes.listOsTypesCmd() + sub_cmd.description = services["ostype"] + ostypes = apiclient.listOsTypes(sub_cmd) - if not isinstance(ostypes, list): - raise Exception( - "Unable to find Ostype id with desc: %s" % - services["ostype"]) - cmd.ostypeid = ostypes[0].id - else: + if not isinstance(ostypes, list): raise Exception( - "Unable to find Ostype is required for registering template") + "Unable to find Ostype id with desc: %s" % + services["ostype"]) + cmd.ostypeid = ostypes[0].id + else: + raise Exception( + "Unable to find Ostype is required for registering template") cmd.url = services["url"] @@ -1438,6 +1436,7 @@ class Template: cmd.isdynamicallyscalable = services["isdynamicallyscalable"] if "isdynamicallyscalable" in services else False cmd.passwordenabled = services[ "passwordenabled"] if "passwordenabled" in services else False + cmd.deployasis = services["deployasis"] if "deployasis" in services else False if account: cmd.account = account diff --git a/tools/marvin/marvin/lib/common.py b/tools/marvin/marvin/lib/common.py index 18a8a0a1a5e..cd896c909eb 100644 --- a/tools/marvin/marvin/lib/common.py +++ b/tools/marvin/marvin/lib/common.py @@ -348,7 +348,7 @@ def get_template( return list_templatesout[0] -def get_test_template(apiclient, zone_id=None, hypervisor=None, test_templates=None): +def get_test_template(apiclient, zone_id=None, hypervisor=None, test_templates=None, deploy_as_is=False): """ @Name : get_test_template @Desc : Retrieves the test template used to running tests. When the template @@ -373,6 +373,8 @@ def get_test_template(apiclient, zone_id=None, hypervisor=None, test_templates=N return FAILED test_template = test_templates[hypervisor] + if deploy_as_is: + test_template['deployasis'] = True cmd = listTemplates.listTemplatesCmd() cmd.name = test_template['name'] @@ -513,7 +515,7 @@ def get_windows_template( return FAILED -def get_suitable_test_template(apiclient, zoneid, ostypeid, hypervisor): +def get_suitable_test_template(apiclient, zoneid, ostypeid, hypervisor, deploy_as_is=False): ''' @Name : get_suitable_test_template @Desc : Retrieves the test template information based upon inputs provided @@ -525,11 +527,12 @@ def get_suitable_test_template(apiclient, zoneid, ostypeid, hypervisor): template Information matching the inputs ''' template = FAILED - if hypervisor.lower() in ["xenserver"]: + if hypervisor.lower() in ["xenserver"] or (hypervisor.lower() in ["vmware"] and deploy_as_is): template = get_test_template( apiclient, zoneid, - hypervisor) + hypervisor, + deploy_as_is=deploy_as_is) if template == FAILED: template = get_template( apiclient, diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index ba0d2b13d22..cbda4d5094f 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -715,7 +715,7 @@ "label.demote.project.owner": "Demote account to Regular role", "label.demote.project.owner.user": "Demote user to Regular role", "label.deny": "Deny", -"label.deployasis":"Deploy As-Is", +"label.deployasis":"Read VM settings from OVA", "label.deploymentplanner": "Deployment planner", "label.description": "Description", "label.destcidr": "Destination CIDR", diff --git a/ui/src/views/image/RegisterOrUploadTemplate.vue b/ui/src/views/image/RegisterOrUploadTemplate.vue index 13001230758..704cda59ef6 100644 --- a/ui/src/views/image/RegisterOrUploadTemplate.vue +++ b/ui/src/views/image/RegisterOrUploadTemplate.vue @@ -208,8 +208,18 @@ :default-checked="xenServerProvider" /> + + + + + - + - + +