Skip to content

Commit

Permalink
* Distinguish between "standard" and "nonstandard" namespaces (such a…
Browse files Browse the repository at this point in the history
…s vmw:).

* * When adding XML elements with specified ordering, only complain about
    unexpected elements in "standard" namespaces, avoiding noise about elements
    like vmw:CoresPerSocket
* OVF now caches self.platform to avoid repeated checks
* Cleaner handling of attributes and custom elements in OVFItem (fixes #8)
* All properties in OVFItem are now correctly converted to strings
* * This fixes some cases where we were not sharing a profile when it made
    sense to do so, so we generate slightly more optimal XML.
  • Loading branch information
glennmatthews committed Sep 19, 2014
1 parent 97004bb commit 1b31b30
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 45 deletions.
72 changes: 47 additions & 25 deletions COT/ovf.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ def __init__(self, input_file, working_dir, output_file):
for (prefix, URI) in self.NSM.items():
self.register_namespace(prefix, URI)

# Register additional non-standard namespaces we're aware of:
self.register_namespace('vmw', "http://www.vmware.com/schema/ovf")

# Go ahead and set pointers to some of the most useful XML sections
self.envelope = self.root
self.references = self.find_child(self.envelope, self.REFERENCES,
Expand Down Expand Up @@ -219,7 +222,7 @@ def __init__(self, input_file, working_dir, output_file):
except OVFHardwareDataError as e:
raise VMInitError(1, "OVF descriptor is invalid: {0}".format(e))

self.platform = self.get_platform()
self.get_platform()

# Check any file references in the ovf descriptor
self.validate_file_references()
Expand Down Expand Up @@ -623,6 +626,9 @@ def get_default_profile_name(self):

def get_platform(self):
"""Identifies the platform type from the OVF descriptor"""
if self.platform is not None:
return self.platform

platform = None
product_class = None
class_to_platform_map = {
Expand All @@ -633,6 +639,7 @@ def get_platform(self):
'ios-xrv.rp' : Platform.IOSXRvRP,
'ios-xrv.lc' : Platform.IOSXRvLC,
}

if self.product_section is not None:
product_class = self.product_section.get(self.PRODUCT_CLASS)
if product_class is not None:
Expand All @@ -650,7 +657,8 @@ def get_platform(self):
platform = Platform.GenericPlatform
logger.info("OVF product class {0} --> platform {1}"
.format(product_class, platform.__name__))
return platform
self.platform = platform
return self.platform


def validate_file_references(self):
Expand Down Expand Up @@ -1727,17 +1735,20 @@ class OVFNameHelper(object):

def __init__(self, version):
self.ovf_version = version
# We need to tell the XML ElementTree about the various namespaces
# that are involved in an OVF descriptor
self.platform = None
# For the standard namespace URIs in an OVF descriptor, let's define
# shorthand identifiers to be used when writing back out to XML:
cim_uri = "http://schemas.dmtf.org/wbem/wscim/1"
self.NSM = {
'xsi': "http://www.w3.org/2001/XMLSchema-instance",
'cim': cim_uri + "/common",
'rasd': cim_uri + "/cim-schema/2/CIM_ResourceAllocationSettingData",
'vssd': cim_uri + "/cim-schema/2/CIM_VirtualSystemSettingData",
# Namespace for VMWare-specific extensions
'vmw': "http://www.vmware.com/schema/ovf"
}
# Non-standard namespaces (such as VMWare's
# 'http://www.vmware.com/schema/ovf') should not be added to the NSM
# dictionary, but may be registered manually by calling
# self.register_namespace() as needed - see self.write() for examples.

if self.ovf_version < 1.0:
self.NSM['ovf'] = "http://www.vmware.com/schema/ovf/1/envelope"
Expand Down Expand Up @@ -2126,8 +2137,8 @@ def find_all_items(self, resource_type=None, properties=None, profile=None):
break
if valid:
filtered_items.append(ovfitem)
logger.info("Found {0} {1} Items"
.format(len(filtered_items), resource_type))
logger.debug("Found {0} {1} Items"
.format(len(filtered_items), resource_type))
return filtered_items


Expand Down Expand Up @@ -2314,6 +2325,10 @@ class OVFItem:
each of which is a dict of sets of profiles (indexed by element value)
"""

# Magic strings
ATTRIB_KEY_SUFFIX=" {Item attribute}"
ELEMENT_KEY_SUFFIX=" {custom element}"

def __init__(self, ovf, item=None):
"""Create a new OVFItem with contents based on the given Item element
"""
Expand Down Expand Up @@ -2356,7 +2371,7 @@ def add_item(self, item):
for (attrib, value) in item.attrib.items():
if attrib == self.ITEM_CONFIG:
continue
attrib_string = "_attrib_" + attrib
attrib_string = attrib + self.ATTRIB_KEY_SUFFIX
self.set_property(attrib_string, value, profiles, overwrite=False)

# Store any child elements of the Item.
Expand All @@ -2370,8 +2385,9 @@ def add_item(self, item):
# vmw:Config elements, each distinguished by its vmw:key attrib.
# Rather than try to guess how these items do or do not match,
# we simply store the whole item
self.set_property("_element_"+ET.tostring(child).decode(),
ET.tostring(child),
self.set_property((ET.tostring(child).decode().strip() +
self.ELEMENT_KEY_SUFFIX),
ET.tostring(child).decode(),
profiles, overwrite=False)
continue
# Store the value of this element:
Expand All @@ -2396,6 +2412,9 @@ def set_property(self, key, value, profiles=None, overwrite=True):
set for the provided profile will raise a OVFItemDataError.
(Otherwise, by default, the existing data will be overwritten).
"""
# Just to be safe...
value = str(value)

if not profiles:
value_dict = self.dict.get(key, {})
if not value_dict:
Expand All @@ -2407,8 +2426,6 @@ def set_property(self, key, value, profiles=None, overwrite=True):
# or ResourceSubType, replace that reference with a placeholder
# that we can regenerate at output time. That way, if the
# VirtualQuantity or ResourceSubType changes, these can change too.
logger.debug("Setting {0} to {1} under profiles {2}"
.format(key, value, profiles))
if key == self.ELEMENT_NAME or key == self.ITEM_DESCRIPTION:
vq_val = self.get_value(self.VIRTUAL_QUANTITY, profiles)
if vq_val is not None:
Expand All @@ -2421,6 +2438,8 @@ def set_property(self, key, value, profiles=None, overwrite=True):
en_val = self.get_value(self.ELEMENT_NAME, profiles)
if en_val is not None:
value = re.sub(en_val, "_EN_", value)
logger.debug("Setting {0} to {1} under profiles {2}"
.format(key, value, profiles))
if not key in self.dict:
if value == '':
pass
Expand Down Expand Up @@ -2683,35 +2702,38 @@ def generate_items(self):
val = default_val
# Regenerate text that depends on the VirtualQuantity
# or ResourceSubType strings:
if rst_val is not None:
val = re.sub("_RST_", str(rst_val), str(val))
if vq_val is not None:
val = re.sub("_VQ_", str(vq_val), str(val))
if en_val is not None:
val = re.sub("_EN_", str(en_val), str(val))
if key == self.ELEMENT_NAME or key == self.ITEM_DESCRIPTION:
if rst_val is not None:
val = re.sub("_RST_", str(rst_val), str(val))
if vq_val is not None:
val = re.sub("_VQ_", str(vq_val), str(val))
if key == self.ITEM_DESCRIPTION:
if en_val is not None:
val = re.sub("_EN_", str(en_val), str(val))

# Is this an attribute, a child, or a custom element?
attrib_match = re.match("_attrib_(.*)", key)
attrib_match = re.match("(.*)" + self.ATTRIB_KEY_SUFFIX, key)
if attrib_match:
attrib_string = attrib_match.group(1)
child_attrib = re.match("(.*)_attrib_(.*)", key)
custom_elem = re.match("_element_(.*)", key)
custom_elem = re.match("(.*)" + self.ELEMENT_KEY_SUFFIX, key)
if attrib_match:
item.set(attrib_string, val)
elif child_attrib:
child = XML.set_or_make_child(item,
child_attrib.group(1),
None,
ordering=self.ITEM_CHILDREN)
ordering=self.ITEM_CHILDREN,
known_namespaces=self.NSM)
child.set(child_attrib.group(2), val)
elif custom_elem:
# Recreate the element in question and append it
elem = ET.fromstring(custom_elem.group(1))
item.append(elem)
item.append(ET.fromstring(val))
else:
# Children of Item must be in sorted order
XML.set_or_make_child(item, key, val,
ordering=self.ITEM_CHILDREN)
ordering=self.ITEM_CHILDREN,
known_namespaces=self.NSM)
logger.debug("Item is:\n{0}".format(ET.tostring(item)))
item_list.append(item)

Expand Down
41 changes: 35 additions & 6 deletions COT/tests/ovf.py
Original file line number Diff line number Diff line change
Expand Up @@ -753,17 +753,35 @@ def test_set_cpus(self):
"""Set the number of CPUs"""
for cli_opt in ['-c', '--cpus']:
# Change value under a specific profile:
self.call_edit_hardware([cli_opt, '4', '-p', '2CPU-2GB-1NIC'])
self.call_edit_hardware([cli_opt, '8', '-p', '2CPU-2GB-1NIC'])
self.check_diff(
"""
<rasd:Description>Number of Virtual CPUs</rasd:Description>
- <rasd:ElementName>2 virtual CPU(s)</rasd:ElementName>
+ <rasd:ElementName>4 virtual CPU(s)</rasd:ElementName>
+ <rasd:ElementName>8 virtual CPU(s)</rasd:ElementName>
<rasd:InstanceID>1</rasd:InstanceID>
<rasd:ResourceType>3</rasd:ResourceType>
- <rasd:VirtualQuantity>2</rasd:VirtualQuantity>
+ <rasd:VirtualQuantity>4</rasd:VirtualQuantity>
+ <rasd:VirtualQuantity>8</rasd:VirtualQuantity>
<vmw:CoresPerSocket ovf:required="false">1</vmw:CoresPerSocket>
""")
# Change value under a specific profile to match another profile:
self.call_edit_hardware([cli_opt, '4', '-p', '2CPU-2GB-1NIC'])
self.check_diff(
"""
</ovf:Item>
- <ovf:Item ovf:configuration="2CPU-2GB-1NIC">
- <rasd:AllocationUnits>hertz * 10^6</rasd:AllocationUnits>
- <rasd:Description>Number of Virtual CPUs</rasd:Description>
- <rasd:ElementName>2 virtual CPU(s)</rasd:ElementName>
- <rasd:InstanceID>1</rasd:InstanceID>
- <rasd:ResourceType>3</rasd:ResourceType>
- <rasd:VirtualQuantity>2</rasd:VirtualQuantity>
- <vmw:CoresPerSocket ovf:required="false">1</vmw:CoresPerSocket>
- </ovf:Item>
- <ovf:Item ovf:configuration="4CPU-4GB-3NIC">
+ <ovf:Item ovf:configuration="2CPU-2GB-1NIC 4CPU-4GB-3NIC">
<rasd:AllocationUnits>hertz * 10^6</rasd:AllocationUnits>
""")
# Change value under all profiles.
# This results in merging the separate Items together
Expand Down Expand Up @@ -1401,24 +1419,35 @@ def test_set_ide_subtype(self):
def test_profiles(self):
"""Test configuration profile creation"""

# Create a new profile:
# Add a profile sharing attributes with the default profile:
self.call_edit_hardware(['--profile', 'UT', '--cpus', '1'])
self.check_diff("""
</ovf:Configuration>
+ <ovf:Configuration ovf:id="UT">
+ <ovf:Label>UT</ovf:Label>
+ <ovf:Description>UT</ovf:Description>
+ </ovf:Configuration>
</ovf:DeploymentOptionSection>
""")

# Create a new profile:
self.call_edit_hardware(['--profile', 'UT', '--cpus', '8'])
self.check_diff("""
</ovf:Configuration>
+ <ovf:Configuration ovf:id="UT">
+ <ovf:Label>UT</ovf:Label>
+ <ovf:Description>UT</ovf:Description>
+ </ovf:Configuration>
</ovf:DeploymentOptionSection>
...
</ovf:Item>
+ <ovf:Item ovf:configuration="UT">
+ <rasd:AllocationUnits>hertz * 10^6</rasd:AllocationUnits>
+ <rasd:Description>Number of Virtual CPUs</rasd:Description>
+ <rasd:ElementName>1 virtual CPU(s)</rasd:ElementName>
+ <rasd:ElementName>8 virtual CPU(s)</rasd:ElementName>
+ <rasd:InstanceID>1</rasd:InstanceID>
+ <rasd:ResourceType>3</rasd:ResourceType>
+ <rasd:VirtualQuantity>1</rasd:VirtualQuantity>
+ <rasd:VirtualQuantity>8</rasd:VirtualQuantity>
+ <vmw:CoresPerSocket ovf:required="false">1</vmw:CoresPerSocket>
+ </ovf:Item>
<ovf:Item>
Expand Down
32 changes: 18 additions & 14 deletions COT/xml_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,18 +172,20 @@ def find_all_children(cls, parent, tag, children={}, attrib={}):


@classmethod
def add_child(cls, parent, new_child, ordering=None):
def add_child(cls, parent, new_child, ordering=None, known_namespaces=None):
"""Add the given child element under the given parent element.
If ordering is unspecified, the child will be appended after
all existing children; otherwise, the placement of the child
relative to other children will respect this ordering.
"""
if ordering and not (new_child.tag in ordering):
logger.warning("New child '{0}' is not in the list of "
"expected children under '{1}': {2}"
.format(tag,
XML.strip_ns(parent.tag),
ordering))
if (known_namespaces and
(XML.get_ns(new_child.tag) in known_namespaces)):
logger.warning("New child '{0}' is not in the list of "
"expected children under '{1}': {2}"
.format(new_child.tag,
XML.strip_ns(parent.tag),
ordering))
# Assume this is some sort of custom element, which
# implicitly goes at the end of the list.
ordering = None
Expand All @@ -200,12 +202,14 @@ def add_child(cls, parent, new_child, ordering=None):
found_position = True
break
except ValueError as e:
logger.warning("Existing child element '{0}' is not in "
"expected list of children under '{1}': "
"\n{2}"
.format(child.tag,
XML.strip_ns(parent.tag),
ordering))
if (known_namespaces and (XML.get_ns(child.tag) in
known_namespaces)):
logger.warning("Existing child element '{0}' is not in "
"expected list of children under '{1}': "
"\n{2}"
.format(child.tag,
XML.strip_ns(parent.tag),
ordering))
# Assume this is some sort of custom element - all known
# elements should implicitly come before it.
found_position = True
Expand All @@ -219,7 +223,7 @@ def add_child(cls, parent, new_child, ordering=None):

@classmethod
def set_or_make_child(cls, parent, tag, text=None, attrib=None,
ordering=None):
ordering=None, known_namespaces=None):
"""Update or create a child element with the desired text
and/or attributes under the specified parent element.
The optional 'ordering' parameter is used to provide a list of
Expand All @@ -234,7 +238,7 @@ def set_or_make_child(cls, parent, tag, text=None, attrib=None,
logger.debug("Creating new {0} under {1}"
.format(XML.strip_ns(tag), XML.strip_ns(parent.tag)))
element = ET.Element(tag)
XML.add_child(parent, element, ordering)
XML.add_child(parent, element, ordering, known_namespaces)
if text is not None:
element.text = str(text)
for a in attrib:
Expand Down

0 comments on commit 1b31b30

Please sign in to comment.