You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by nv...@apache.org on 2021/09/16 19:17:14 UTC
[cloudstack] branch main updated: [Vmware] Fix for ovf templates
with prefix (#5448)
This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 3ca3843 [Vmware] Fix for ovf templates with prefix (#5448)
3ca3843 is described below
commit 3ca3843b025475d197c736cea16deb7d7d8a0a95
Author: Nicolas Vazquez <ni...@gmail.com>
AuthorDate: Thu Sep 16 16:16:41 2021 -0300
[Vmware] Fix for ovf templates with prefix (#5448)
* [Vmware] Fix for ovf templates with prefix
* Support multiple hardware versions
---
.../com/cloud/agent/api/storage/OVFHelper.java | 48 ++++++++++++++-------
.../com/cloud/storage/template/OVAProcessor.java | 36 +---------------
.../image/deployasis/DeployAsIsHelperImpl.java | 31 +++++++++++++-
.../image/deployasis/DeployAsIsHelperImplTest.java | 50 ++++++++++++++++++++++
4 files changed, 112 insertions(+), 53 deletions(-)
diff --git a/api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java b/api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java
index 8d4f780..35835a5 100644
--- a/api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java
+++ b/api/src/main/java/com/cloud/agent/api/storage/OVFHelper.java
@@ -116,6 +116,20 @@ public class OVFHelper {
}
/**
+ * Check if there are elements matching the tag name, otherwise check prepending the prefix
+ */
+ public NodeList getElementsByTagNameAndPrefix(Document doc, String name, String prefix) {
+ if (doc == null) {
+ return null;
+ }
+ NodeList elementsByTagName = doc.getElementsByTagName(name);
+ if (elementsByTagName.getLength() > 0) {
+ return elementsByTagName;
+ }
+ return doc.getElementsByTagName(String.format("%s:%s", prefix, name));
+ }
+
+ /**
* Check if the attribute is present on the element, otherwise check preprending ':'
*/
private String getNodeAttribute(Element element, String prefix, String attr) {
@@ -179,7 +193,7 @@ public class OVFHelper {
return props;
}
int propertyIndex = 0;
- NodeList productSections = doc.getElementsByTagName("ProductSection");
+ NodeList productSections = getElementsByTagNameAndPrefix(doc, "ProductSection", "ovf");
if (productSections != null) {
String lastCategoryFound = null;
for (int i = 0; i < productSections.getLength(); i++) {
@@ -193,10 +207,12 @@ public class OVFHelper {
if (child == null) {
continue;
}
- if (child.getNodeName().equalsIgnoreCase("Category")) {
+ if (child.getNodeName().equalsIgnoreCase("Category") ||
+ child.getNodeName().equalsIgnoreCase("ovf:Category")) {
lastCategoryFound = child.getTextContent();
s_logger.info("Category found " + lastCategoryFound);
- } else if (child.getNodeName().equalsIgnoreCase("Property")) {
+ } else if (child.getNodeName().equalsIgnoreCase("Property") ||
+ child.getNodeName().equalsIgnoreCase("ovf:Property")) {
OVFPropertyTO prop = createOVFPropertyFromNode(child, propertyIndex, lastCategoryFound);
if (prop != null && prop.isUserConfigurable()) {
props.add(prop);
@@ -398,7 +414,7 @@ public class OVFHelper {
}
protected List<OVFFile> extractFilesFromOvfDocumentTree(File ovfFile, Document doc) {
- NodeList files = doc.getElementsByTagName("File");
+ NodeList files = getElementsByTagNameAndPrefix(doc, "File", "ovf");
ArrayList<OVFFile> vf = new ArrayList<>();
boolean toggle = true;
for (int j = 0; j < files.getLength(); j++) {
@@ -514,9 +530,9 @@ public class OVFHelper {
public void rewriteOVFFileForSingleDisk(final String origOvfFilePath, final String newOvfFilePath, final String diskName) {
final Document doc = getDocumentFromFile(origOvfFilePath);
- NodeList disks = doc.getElementsByTagName("Disk");
- NodeList files = doc.getElementsByTagName("File");
- NodeList items = doc.getElementsByTagName("Item");
+ NodeList disks = getElementsByTagNameAndPrefix(doc, "Disk", "ovf");
+ NodeList files = getElementsByTagNameAndPrefix(doc, "File", "ovf");
+ NodeList items = getElementsByTagNameAndPrefix(doc, "Item", "ovf");
String keepfile = null;
List<Element> toremove = new ArrayList<>();
for (int j = 0; j < files.getLength(); j++) {
@@ -677,7 +693,7 @@ public class OVFHelper {
private void checkForOnlyOneSystemNode(Document doc) throws InternalErrorException {
// get hardware VirtualSystem, for now we support only one of those
- NodeList systemElements = doc.getElementsByTagName("VirtualSystem");
+ NodeList systemElements = getElementsByTagNameAndPrefix(doc, "VirtualSystem", "ovf");
if (systemElements.getLength() != 1) {
String msg = "found " + systemElements.getLength() + " system definitions in OVA, can only handle exactly one.";
s_logger.warn(msg);
@@ -686,7 +702,7 @@ public class OVFHelper {
}
private Map<String, OVFNetworkTO> getNetworksFromDocumentTree(Document doc) {
- NodeList networkElements = doc.getElementsByTagName("Network");
+ NodeList networkElements = getElementsByTagNameAndPrefix(doc,"Network", "ovf");
Map<String, OVFNetworkTO> nets = new HashMap<>();
for (int i = 0; i < networkElements.getLength(); i++) {
@@ -746,7 +762,7 @@ public class OVFHelper {
private String getMinimumHardwareVersionFromDocumentTree(Document doc) {
String version = null;
if (doc != null) {
- NodeList systemNodeList = doc.getElementsByTagName("System");
+ NodeList systemNodeList = getElementsByTagNameAndPrefix(doc, "System", "ovf");
if (systemNodeList.getLength() != 0) {
Node systemItem = systemNodeList.item(0);
String hardwareVersions = getChildNodeValue(systemItem, "VirtualSystemType");
@@ -766,7 +782,7 @@ public class OVFHelper {
if (doc == null) {
return options;
}
- NodeList deploymentOptionSection = doc.getElementsByTagName("DeploymentOptionSection");
+ NodeList deploymentOptionSection = getElementsByTagNameAndPrefix(doc,"DeploymentOptionSection", "ovf");
if (deploymentOptionSection.getLength() == 0) {
return options;
}
@@ -775,7 +791,7 @@ public class OVFHelper {
int index = 0;
for (int i = 0; i < childNodes.getLength(); i++) {
Node node = childNodes.item(i);
- if (node != null && node.getNodeName().equals("Configuration")) {
+ if (node != null && (node.getNodeName().equals("Configuration") || node.getNodeName().equals("ovf:Configuration"))) {
Element configuration = (Element) node;
String configurationId = getNodeAttribute(configuration,"ovf","id");
String description = getChildNodeValue(configuration, "Description");
@@ -793,7 +809,7 @@ public class OVFHelper {
if (doc == null) {
return items;
}
- NodeList hardwareSection = doc.getElementsByTagName("VirtualHardwareSection");
+ NodeList hardwareSection = getElementsByTagNameAndPrefix(doc, "VirtualHardwareSection","ovf");
if (hardwareSection.getLength() == 0) {
return items;
}
@@ -801,7 +817,7 @@ public class OVFHelper {
NodeList childNodes = hardwareSectionNode.getChildNodes();
for (int i = 0; i < childNodes.getLength(); i++) {
Node node = childNodes.item(i);
- if (node != null && node.getNodeName().equals("Item")) {
+ if (node != null && (node.getNodeName().equals("Item") || node.getNodeName().equals("ovf:Item"))) {
Element configuration = (Element) node;
String configurationIds = getNodeAttribute(configuration, "ovf", "configuration");
String allocationUnits = getChildNodeValue(configuration, "AllocationUnits");
@@ -869,7 +885,7 @@ public class OVFHelper {
if (doc == null) {
return eulas;
}
- NodeList eulaSections = doc.getElementsByTagName("EulaSection");
+ NodeList eulaSections = getElementsByTagNameAndPrefix(doc, "EulaSection", "ovf");
int eulaIndex = 0;
if (eulaSections.getLength() > 0) {
for (int index = 0; index < eulaSections.getLength(); index++) {
@@ -905,7 +921,7 @@ public class OVFHelper {
if (doc == null) {
return null;
}
- NodeList guesOsList = doc.getElementsByTagName("OperatingSystemSection");
+ NodeList guesOsList = getElementsByTagNameAndPrefix(doc, "OperatingSystemSection", "ovf");
if (guesOsList.getLength() == 0) {
return null;
}
diff --git a/core/src/main/java/com/cloud/storage/template/OVAProcessor.java b/core/src/main/java/com/cloud/storage/template/OVAProcessor.java
index 8315ff4..b5d78ac 100644
--- a/core/src/main/java/com/cloud/storage/template/OVAProcessor.java
+++ b/core/src/main/java/com/cloud/storage/template/OVAProcessor.java
@@ -243,7 +243,7 @@ public class OVAProcessor extends AdapterBase implements Processor {
try {
Document ovfDoc = null;
ovfDoc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new File(ovfFileName));
- NodeList diskElements = ovfDoc.getElementsByTagName("Disk");
+ NodeList diskElements = new OVFHelper().getElementsByTagNameAndPrefix(ovfDoc, "Disk", "ovf");
for (int i = 0; i < diskElements.getLength(); i++) {
Element disk = (Element)diskElements.item(i);
long diskSize = Long.parseLong(disk.getAttribute("ovf:capacity"));
@@ -259,40 +259,6 @@ public class OVAProcessor extends AdapterBase implements Processor {
}
}
- public Pair<Long, Long> getDiskDetails(String ovfFilePath, String diskName) throws InternalErrorException {
- long virtualSize = 0;
- long fileSize = 0;
- String fileId = null;
- try {
- Document ovfDoc = null;
- ovfDoc = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(new File(ovfFilePath));
- NodeList disks = ovfDoc.getElementsByTagName("Disk");
- NodeList files = ovfDoc.getElementsByTagName("File");
- for (int j = 0; j < files.getLength(); j++) {
- Element file = (Element)files.item(j);
- if (file.getAttribute("ovf:href").equals(diskName)) {
- fileSize = Long.parseLong(file.getAttribute("ovf:size"));
- fileId = file.getAttribute("ovf:id");
- break;
- }
- }
- for (int i = 0; i < disks.getLength(); i++) {
- Element disk = (Element)disks.item(i);
- if (disk.getAttribute("ovf:fileRef").equals(fileId)) {
- virtualSize = Long.parseLong(disk.getAttribute("ovf:capacity"));
- String allocationUnits = disk.getAttribute("ovf:capacityAllocationUnits");
- virtualSize = OVFHelper.getDiskVirtualSize(virtualSize, allocationUnits, ovfFilePath);
- break;
- }
- }
- return new Pair<Long, Long>(virtualSize, fileSize);
- } catch (InternalErrorException | IOException | NumberFormatException | ParserConfigurationException | SAXException e) {
- String msg = "getDiskDetails: Unable to parse OVF XML document " + ovfFilePath + " to get the virtual disk " + diskName + " size due to " + e;
- LOGGER.error(msg);
- throw new InternalErrorException(msg);
- }
- }
-
private String getOVFFilePath(String srcOVAFileName) {
File file = new File(srcOVAFileName);
assert (_storage != null);
diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImpl.java
index 6d05af3..9d26da8 100644
--- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImpl.java
+++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImpl.java
@@ -256,8 +256,7 @@ public class DeployAsIsHelperImpl implements DeployAsIsHelper {
/**
* Minimum VMware hosts supported version is 6.0
*/
- protected String getMinimumSupportedHypervisorVersionForHardwareVersion(String hardwareVersion) {
- // From https://kb.vmware.com/s/article/1003746 and https://kb.vmware.com/s/article/2007240
+ protected String mapHardwareVersionToHypervisorVersion(String hardwareVersion) {
String hypervisorVersion = "default";
if (StringUtils.isBlank(hardwareVersion)) {
return hypervisorVersion;
@@ -278,6 +277,34 @@ public class DeployAsIsHelperImpl implements DeployAsIsHelper {
return hypervisorVersion;
}
+ /**
+ * Retrieve the minimal hypervisor version for a single or multiple hardware version(s) in the parameter
+ */
+ protected String getMinimumSupportedHypervisorVersionForHardwareVersion(String hardwareVersion) {
+ // From https://kb.vmware.com/s/article/1003746 and https://kb.vmware.com/s/article/2007240
+ String hypervisorVersion = "default";
+ if (StringUtils.isBlank(hardwareVersion)) {
+ return hypervisorVersion;
+ }
+ if (hardwareVersion.contains(" ")) {
+ String[] versions = hardwareVersion.split(" ");
+ String minVersion = null;
+ for (String version : versions) {
+ String hvVersion = mapHardwareVersionToHypervisorVersion(version);
+ if (minVersion == null) {
+ minVersion = hvVersion;
+ } else if (hvVersion.equalsIgnoreCase("default")) {
+ return minVersion;
+ } else if (hvVersion.compareTo(minVersion) < 0) {
+ minVersion = hvVersion;
+ }
+ }
+ return minVersion;
+ } else {
+ return mapHardwareVersionToHypervisorVersion(hardwareVersion);
+ }
+ }
+
@Override
public Map<String, String> getVirtualMachineDeployAsIsProperties(VirtualMachineProfile vm) {
Map<String, String> map = new HashMap<>();
diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImplTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImplTest.java
new file mode 100644
index 0000000..dd10eda
--- /dev/null
+++ b/engine/storage/src/test/java/org/apache/cloudstack/storage/image/deployasis/DeployAsIsHelperImplTest.java
@@ -0,0 +1,50 @@
+/*
+ * 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.storage.image.deployasis;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class DeployAsIsHelperImplTest {
+
+ private DeployAsIsHelperImpl deployAsIsHelper = new DeployAsIsHelperImpl();
+
+ private static final String singleHardwareVersionDefinition = "vmx-13";
+ private static final String multipleHardwareVersionDefinition = "vmx-10 vmx-11 vmx-13";
+ private static final String multipleHardwareVersionDefinitionUnordered = "vmx-13 vmx-8 vmx-10";
+
+ @Test
+ public void testGetMinimumSupportedHypervisorVersionForHardwareVersion() {
+ String vmwareVersion = deployAsIsHelper.getMinimumSupportedHypervisorVersionForHardwareVersion(singleHardwareVersionDefinition);
+ Assert.assertEquals("6.5", vmwareVersion);
+ }
+
+ @Test
+ public void testGetMinimumSupportedHypervisorVersionForMultipleHardwareVersion() {
+ String vmwareVersion = deployAsIsHelper.getMinimumSupportedHypervisorVersionForHardwareVersion(multipleHardwareVersionDefinition);
+ Assert.assertEquals("6.0", vmwareVersion);
+ }
+
+ @Test
+ public void testGetMinimumSupportedHypervisorVersionForMultipleUnorderedHardwareVersion() {
+ String vmwareVersion = deployAsIsHelper.getMinimumSupportedHypervisorVersionForHardwareVersion(multipleHardwareVersionDefinitionUnordered);
+ Assert.assertEquals("6.0", vmwareVersion);
+ }
+}