Virt define improvements

changelogs/fragments/142_virt_define_improvements.yml

@@ -0,0 +1,3 @@
+minor_changes:
+  - virt - support ``--diff`` for ``define`` command (https://github.com/ansible-collections/community.libvirt/pull/142/).
+  - virt - add `mutate_flags` parameter to enable XML mutation (add UUID, MAC addresses from existing domain) (https://github.com/ansible-collections/community.libvirt/pull/142/).

plugins/doc_fragments/virt.py

@@ -65,3 +65,24 @@ class ModuleDocFragment(object):
       - Must be raw XML content using C(lookup). XML cannot be reference to a file.
     type: str
     """
+
+    OPTIONS_MUTATE_FLAGS = r"""
+options:
+  mutate_flags:
+    description:
+      - For each mutate_flag, we will modify the given XML in some way
+      - ADD_UUID will add an existing domain's UUID to the xml if it's missing
+      - ADD_MAC_ADDRESSES will look up interfaces in the existing domain with a
+        matching alias and copy the MAC address over. The matching interface
+        doesn't need to be of the same type or source network.
+      - ADD_MAC_ADDRESSES_FUZZY will try to match incoming interfaces with
+        interfaces of the existing domain sharing the same type and source
+        network/device. It may not always result in the expected outcome,
+        particularly if a domain has multiple interface attached to the same
+        host device and only some of those devices have <mac>s.
+        Use caution, do some testing for your particular use-case!
+    choices: [ ADD_UUID, ADD_MAC_ADDRESSES, ADD_MAC_ADDRESSES_FUZZY ]
+    type: list
+    elements: str
+    default: ['ADD_UUID']
+    """

plugins/modules/virt.py

@@ -46,6 +46,7 @@
     - community.libvirt.virt.options_autostart
     - community.libvirt.virt.options_state
     - community.libvirt.virt.options_command
+    - community.libvirt.virt.options_mutate_flags
     - community.libvirt.requirements
 author:
     - Ansible Core Team
@@ -155,7 +156,12 @@
 else:
     HAS_VIRT = True
 
-import re
+try:
+    from lxml import etree
+except ImportError:
+    HAS_XML = False
+else:
+    HAS_XML = True
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_native
@@ -189,6 +195,8 @@
     'checkpoints_metadata': 16,
 }
 
+MUTATE_FLAGS = ['ADD_UUID', 'ADD_MAC_ADDRESSES', 'ADD_MAC_ADDRESSES_FUZZY']
+
 ALL_FLAGS = []
 ALL_FLAGS.extend(ENTRY_UNDEFINE_FLAGS_MAP.keys())
 
@@ -471,6 +479,201 @@ def define(self, xml):
         return self.conn.define_from_xml(xml)
 
 
+# A dict of interface types (found in their `type` attribute) to the
+# corresponding "source" attribute name of their  <source> elements
+# user networks don't have a <source> element
+#
+# We do not support fuzzy matching against any interface types
+# not defined here
+INTERFACE_SOURCE_ATTRS = {
+    'network': 'network',
+    'bridge': 'bridge',
+    'direct': 'dev',
+    'user': None,
+}
+
+
+def handle_define(module, v):
+    ''' handle `command: define` '''
+    xml = module.params.get('xml', None)
+    guest = module.params.get('name', None)
+    autostart = module.params.get('autostart', None)
+    mutate_flags = module.params.get('mutate_flags', [])
+
+    if not xml:
+        module.fail_json(msg="define requires 'xml' argument")
+    try:
+        incoming_xml = etree.fromstring(xml)
+    except etree.XMLSyntaxError:
+        # TODO: provide info from parser
+        module.fail_json(msg="given XML is invalid")
+
+    # We'll support supplying the domain's name either from 'name' parameter or xml
+    #
+    # But we will fail if both are defined and not equal.
+    domain_name = incoming_xml.findtext("./name")
+    if domain_name is not None:
+        if guest is not None and domain_name != guest:
+            module.fail_json("given 'name' parameter does not match name in XML")
+    else:
+        if guest is None:
+            module.fail_json("missing 'name' parameter and no name provided in XML")
+        domain_name = guest
+        # since there's no <name> in the xml, we'll add it
+        etree.SubElement(incoming_xml, 'name').text = domain_name
+
+    if domain_name == '':
+        module.fail_json(msg="domain name cannot be an empty string")
+
+    res = dict()
+
+    # From libvirt docs (https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDefineXML):
+    # -- A previous definition for this domain with the same UUID and name would
+    # be overridden if it already exists.
+    #
+    # If a domain is defined without a <uuid>, libvirt will generate one for it.
+    # If an attempt is made to re-define the same xml (with the same <name> and
+    # no <uuid>), libvirt will complain with the following error:
+    #
+    # operation failed: domain '<name>' already exists with <uuid>
+    #
+    # If a domain with a similiar <name> but different <uuid> is defined,
+    # libvirt complains with the same error. However, if a domain is defined
+    # with the same <name> and <uuid> as an existing domain, then libvirt will
+    # update the domain with the new definition (automatically handling
+    # addition/removal of devices. some changes may require a boot).
+    try:
+        existing_domain = v.get_vm(domain_name)
+        existing_xml_raw = existing_domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)
+        existing_xml = etree.fromstring(existing_xml_raw)
+    except VMNotFound:
+        existing_domain = None
+        existing_xml_raw = None
+        existing_xml = None
+
+    if existing_domain is not None:
+        # we are updating a domain's definition
+
+        incoming_uuid = incoming_xml.findtext('./uuid')
+        existing_uuid = existing_domain.UUIDString()
+
+        if incoming_uuid is not None and incoming_uuid != existing_uuid:
+            # A user should not try defining a domain with the same name but
+            # different UUID
+            module.fail_json(msg="attempting to re-define domain %s/%s with a different UUID: %s" % (
+                domain_name, existing_uuid, incoming_uuid
+            ))
+        else:
+            if 'ADD_UUID' in mutate_flags and incoming_uuid is None:
+                # Users will often want to define their domains without an explicit
+                # UUID, instead giving them a unique name - so we support bringing
+                # over the UUID from the existing domain
+                etree.SubElement(incoming_xml, 'uuid').text = existing_uuid
+
+            existing_devices = existing_xml.find('./devices')
+
+            if 'ADD_MAC_ADDRESSES' in mutate_flags:
+                for interface in incoming_xml.xpath('./devices/interface[not(mac) and alias]'):
+                    search_alias = interface.find('alias').get('name')
+                    xpath = "./interface[alias[@name='%s']]" % search_alias
+                    try:
+                        matched_interface = existing_devices.xpath(xpath)[0]
+                        existing_devices.remove(matched_interface)
+                        etree.SubElement(interface, 'mac', {
+                            'address': matched_interface.find('mac').get('address')
+                        })
+                    except IndexError:
+                        module.warn("Could not match interface %i of incoming XML by alias %s." % (
+                            interface.getparent().index(interface) + 1, search_alias
+                        ))
+
+            if 'ADD_MAC_ADDRESSES_FUZZY' in mutate_flags:
+                # the counts of interfaces of a similar type/source
+                # key'd with tuple of (type, source)
+                similar_interface_counts = {}
+
+                def get_interface_count(_type, source=None):
+                    key = (_type, source if _type != "user" else None)
+                    if key not in similar_interface_counts:
+                        similar_interface_counts[key] = 1
+                    else:
+                        similar_interface_counts[key] += 1
+                    return similar_interface_counts[key]
+
+                # iterate user-defined interfaces
+                for interface in incoming_xml.xpath('./devices/interface'):
+                    _type = interface.get('type')
+
+                    if interface.find('mac') is not None and interface.find('alias') is not None:
+                        continue
+
+                    if _type not in INTERFACE_SOURCE_ATTRS:
+                        module.warn("Skipping fuzzy MAC matching for interface %i of incoming XML: unsupported interface type '%s'." % (
+                            interface.getparent().index(interface) + 1, _type
+                        ))
+                        continue
+
+                    source_attr = INTERFACE_SOURCE_ATTRS[_type]
+                    source = interface.find('source').get(source_attr) if source_attr else None
+                    similar_count = get_interface_count(_type, source)
+
+                    if interface.find('mac') is not None:
+                        # we want to count these, but not try to change their MAC address
+                        continue
+
+                    if source:
+                        xpath = "./interface[@type='%s' and source[@%s='%s']]" % (
+                            _type, source_attr, source)
+                    else:
+                        xpath = "./interface[@type = '%s']" % source_attr
+
+                    matching_interfaces = existing_devices.xpath(xpath)
+                    try:
+                        matched_interface = matching_interfaces[similar_count - 1]
+                        etree.SubElement(interface, 'mac', {
+                            'address': matched_interface.find('./mac').get('address'),
+                        })
+                    except IndexError:
+                        module.warn("Could not fuzzy match interface %i of incoming XML." % (
+                            interface.getparent().index(interface) + 1
+                        ))
+
+    try:
+        domain_xml = etree.tostring(incoming_xml).decode()
+
+        # TODO: support check mode
+        domain = v.define(domain_xml)
+
+        if existing_domain is not None:
+            # In this case, we may have updated the definition or it might be the same.
+            # We compare the domain's previous xml with its new state and diff
+            # the changes. This allows users to fix their xml if it results in
+            # non-idempotent behaviour (e.g. libvirt mutates it each time)
+            new_xml = domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)
+            if existing_xml_raw != new_xml:
+                res.update({
+                    'changed': True,
+                    'change_reason': 'domain definition changed',
+                    'diff': {
+                        'before': existing_xml_raw,
+                        'after': new_xml
+                    }
+                })
+        else:
+            # there was no existing XML, so this is a newly created domain
+            res.update({'changed': True, 'created': domain.name()})
+
+    except libvirtError as e:
+        module.fail_json(msg='libvirtError: %s' % e.get_error_message())
+    except Exception as e:
+        module.fail_json(msg='an unknown error occured: %s' % e)
+
+    if autostart is not None and v.autostart(domain_name, autostart):
+        res.update({'changed': True, 'change_reason': 'autostart'})
+
+    return res
+
+
 def core(module):
 
     state = module.params.get('state', None)
@@ -480,7 +683,6 @@ def core(module):
     force = module.params.get('force', None)
     flags = module.params.get('flags', None)
     uri = module.params.get('uri', None)
-    xml = module.params.get('xml', None)
 
     v = Virt(uri, module)
     res = dict()
@@ -533,43 +735,7 @@ def core(module):
     if command:
         if command in VM_COMMANDS:
             if command == 'define':
-                if not xml:
-                    module.fail_json(msg="define requires xml argument")
-                if guest:
-                    # there might be a mismatch between quest 'name' in the module and in the xml
-                    module.warn("'xml' is given - ignoring 'name'")
-                try:
-                    domain_name = re.search('<name>(.*)</name>', xml).groups()[0]
-                except AttributeError:
-                    module.fail_json(msg="Could not find domain 'name' in xml")
-
-                # From libvirt docs (https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDefineXML):
-                # -- A previous definition for this domain would be overridden if it already exists.
-                #
-                # In real world testing with libvirt versions 1.2.17-13, 2.0.0-10 and 3.9.0-14
-                # on qemu and lxc domains results in:
-                # operation failed: domain '<name>' already exists with <uuid>
-                #
-                # In case a domain would be indeed overwritten, we should protect idempotency:
-                try:
-                    existing_domain_xml = v.get_vm(domain_name).XMLDesc(
-                        libvirt.VIR_DOMAIN_XML_INACTIVE
-                    )
-                except VMNotFound:
-                    existing_domain_xml = None
-                try:
-                    domain = v.define(xml)
-                    if existing_domain_xml:
-                        # if we are here, then libvirt redefined existing domain as the doc promised
-                        if existing_domain_xml != domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE):
-                            res = {'changed': True, 'change_reason': 'config changed'}
-                    else:
-                        res = {'changed': True, 'created': domain.name()}
-                except libvirtError as e:
-                    if e.get_error_code() != 9:  # 9 means 'domain already exists' error
-                        module.fail_json(msg='libvirtError: %s' % e.get_error_message())
-                if autostart is not None and v.autostart(domain_name, autostart):
-                    res = {'changed': True, 'change_reason': 'autostart'}
+                res.update(handle_define(module, v))
 
             elif not guest:
                 module.fail_json(msg="%s requires 1 argument: guest" % command)
@@ -627,11 +793,19 @@ def main():
             force=dict(type='bool'),
             uri=dict(type='str', default='qemu:///system'),
             xml=dict(type='str'),
+            mutate_flags=dict(type='list', elements='str', choices=MUTATE_FLAGS, default=['ADD_UUID']),
         ),
     )
 
     if not HAS_VIRT:
-        module.fail_json(msg='The `libvirt` module is not importable. Check the requirements.')
+        module.fail_json(
+            msg='The `libvirt` module is not importable. Check the requirements.'
+        )
+
+    if not HAS_XML:
+        module.fail_json(
+            msg='The `lxml` module is not importable. Check the requirements.'
+        )
 
     rc = VIRT_SUCCESS
     try: