diff --git a/changelogs/fragments/142_virt_define_improvements.yml b/changelogs/fragments/142_virt_define_improvements.yml new file mode 100644 index 0000000..d2269ff --- /dev/null +++ b/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/). diff --git a/plugins/doc_fragments/virt.py b/plugins/doc_fragments/virt.py index a607299..6e4ec35 100644 --- a/plugins/doc_fragments/virt.py +++ b/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 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'] + """ diff --git a/plugins/modules/virt.py b/plugins/modules/virt.py index d1b3596..529e6cd 100644 --- a/plugins/modules/virt.py +++ b/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 @@ -194,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()) @@ -475,10 +478,27 @@ def define(self, xml): self.__get_conn() return self.conn.define_from_xml(xml) -def handle_define(module: AnsibleModule, v: Virt): + +# A dict of interface types (found in their `type` attribute) to the +# corresponding "source" attribute name of their elements +# user networks don't have a 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") @@ -508,36 +528,152 @@ def handle_define(module: AnsibleModule, v: Virt): res = dict() # 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. + # -- 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 , libvirt will generate one for it. + # If an attempt is made to re-define the same xml (with the same and + # no ), libvirt will complain with the following error: # - # 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 '' already exists with # - # In case a domain would be indeed overwritten, we should protect idempotency: + # If a domain with a similiar but different is defined, + # libvirt complains with the same error. However, if a domain is defined + # with the same and 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_xml = v.get_vm(domain_name).XMLDesc( - libvirt.VIR_DOMAIN_XML_INACTIVE - ) + 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_xml = None + 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 = v.define(xml) - if existing_domain_xml: - # if we are here, then libvirt redefined existing domain as the doc promised - new_domain_xml = domain.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) - if existing_domain_xml != new_domain_xml: - res = {'changed': True, 'change_reason': 'config changed'} + 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: - res = {'changed': True, 'created': domain.name()} + # there was no existing XML, so this is a newly created domain + res.update({'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()) + 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 = {'changed': True, 'change_reason': 'autostart'} + res.update({'changed': True, 'change_reason': 'autostart'}) return res + def core(module): state = module.params.get('state', None) @@ -657,6 +793,7 @@ 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']), ), )