Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
systemsmanagement:Uyuni:Snapshots:2021.08
salt
backport-virt-patches-from-3001-256.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File backport-virt-patches-from-3001-256.patch of Package salt
From 32559016ba2bd306a3a027a2191857f24258fc46 Mon Sep 17 00:00:00 2001 From: Cedric Bosdonnat <cbosdonnat@suse.com> Date: Mon, 7 Sep 2020 15:00:40 +0200 Subject: [PATCH] Backport virt patches from 3001+ (#256) * Fix various spelling mistakes in master branch (#55954) * Fix typo of additional Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of against Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of amount Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of argument Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of attempt Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of bandwidth Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of caught Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of compatibility Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of consistency Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of conversions Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of corresponding Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of dependent Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of dictionary Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of disabled Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of adapters Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of disassociates Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of changes Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of command Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of communicate Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of community Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of configuration Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of default Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of absence Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of attribute Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of container Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of described Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of existence Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of explicit Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of formatted Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of guarantees Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of hexadecimal Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of hierarchy Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of initialize Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of label Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of management Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of mismatch Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of don't Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of manually Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of getting Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of information Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of meant Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of nonexistent Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of occur Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of omitted Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of normally Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of overridden Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of repository Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of separate Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of separator Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of specific Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of successful Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of succeeded Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of support Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of version Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of that's Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of "will be removed" Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of release Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of synchronize Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of python Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of usually Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of override Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of running Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of whether Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of package Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of persist Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of preferred Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of present Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix typo of run Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix spelling mistake of "allows someone to..." "Allows to" is not correct English. It must either be "allows someone to" or "allows doing". Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix spelling mistake of "number of times" Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix spelling mistake of msgpack Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix spelling mistake of daemonized Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix spelling mistake of daemons Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix spelling mistake of extemporaneous Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix spelling mistake of instead Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix spelling mistake of returning Signed-off-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> * Fix literal comparissons * virt: Convert cpu_baseline ElementTree to string In commit 0f5184c (Remove minidom use in virt module) the value of `cpu` become `xml.etree.ElementTree.Element` and no longer has a method `toxml()`. This results in the following error: $ salt '*' virt.cpu_baseline host2: The minion function caused an exception: Traceback (most recent call last): File "/usr/lib/python3.7/site-packages/salt/minion.py", line 1675, in _thread_return return_data = minion_instance.executors[fname](opts, data, func, args, kwargs) File "/usr/lib/python3.7/site-packages/salt/executors/direct_call.py", line 12, in execute return func(*args, **kwargs) File "/usr/lib/python3.7/site-packages/salt/modules/virt.py", line 4410, in cpu_baseline return cpu.toxml() AttributeError: 'xml.etree.ElementTree.Element' object has no attribute 'toxml' Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org> * PR#57374 backport virt: pool secret should be undefined in pool_undefine not pool_delete virt: handle build differently depending on the pool type virt: don't fail if the pool secret has been removed * PR #57396 backport add firmware auto select feature * virt: Update dependencies Closes: #57641 Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org> * use null in sls file to map None object add sls file example reword doc * Update virt module and states and their tests to python3 * PR #57545 backport Move virt.init boot_dev parameter away from the kwargs virt: handle boot device in virt.update() virt: add boot_dev parameter to virt.running state * PR #57431 backport virt: Handle no available hypervisors virt: Remove unused imports * Blacken salt * Add method to remove circular references in data objects and add test (#54930) * Add method to remove circular references in data objects and add test * remove trailing whitespace * Blacken changed files Co-authored-by: xeacott <jeacott@saltstack.com> Co-authored-by: Frode Gundersen <fgundersen@saltstack.com> Co-authored-by: Daniel A. Wozniak <dwozniak@saltstack.com> * PR #58332 backport virt: add debug log with VM XML definition Add xmlutil.get_xml_node() helper function Add salt.utils.data.get_value function Add change_xml() function to xmlutil virt.update: refactor the XML diffing code virt.test_update: move some code to make test more readable Co-authored-by: Benjamin Drung <benjamin.drung@cloud.ionos.com> Co-authored-by: Pedro Algarvio <pedro@algarvio.me> Co-authored-by: Radostin Stoyanov <rstoyanov@fedoraproject.org> Co-authored-by: Firefly <leevn2011@hotmail.com> Co-authored-by: Blacken Salt <jenkins@saltstack.com> Co-authored-by: Joe Eacott <31625359+xeacott@users.noreply.github.com> Co-authored-by: xeacott <jeacott@saltstack.com> Co-authored-by: Frode Gundersen <fgundersen@saltstack.com> Co-authored-by: Daniel A. Wozniak <dwozniak@saltstack.com> --- changelog/56454.fixed | 1 + changelog/57544.added | 1 + changelog/58331.fixed | 1 + salt/modules/virt.py | 270 +++++++++++++---------- salt/states/virt.py | 88 ++++++-- salt/templates/virt/libvirt_domain.jinja | 29 +-- salt/utils/xmlutil.py | 4 +- tests/unit/modules/test_virt.py | 159 +++++++++---- tests/unit/states/test_virt.py | 93 +++++++- tests/unit/utils/test_data.py | 32 --- 10 files changed, 441 insertions(+), 237 deletions(-) create mode 100644 changelog/56454.fixed create mode 100644 changelog/57544.added create mode 100644 changelog/58331.fixed diff --git a/changelog/56454.fixed b/changelog/56454.fixed new file mode 100644 index 0000000000..978b4b6e03 --- /dev/null +++ b/changelog/56454.fixed @@ -0,0 +1 @@ +Better handle virt.pool_rebuild in virt.pool_running and virt.pool_defined states diff --git a/changelog/57544.added b/changelog/57544.added new file mode 100644 index 0000000000..52071cf2c7 --- /dev/null +++ b/changelog/57544.added @@ -0,0 +1 @@ +Allow setting VM boot devices order in virt.running and virt.defined states diff --git a/changelog/58331.fixed b/changelog/58331.fixed new file mode 100644 index 0000000000..4b8f78dd53 --- /dev/null +++ b/changelog/58331.fixed @@ -0,0 +1 @@ +Leave boot parameters untouched if boot parameter is set to None in virt.update diff --git a/salt/modules/virt.py b/salt/modules/virt.py index fb27397baa..ec40f08359 100644 --- a/salt/modules/virt.py +++ b/salt/modules/virt.py @@ -94,17 +94,13 @@ from xml.sax import saxutils import jinja2.exceptions import salt.utils.files import salt.utils.json -import salt.utils.network import salt.utils.path import salt.utils.stringutils import salt.utils.templates -import salt.utils.validate.net -import salt.utils.versions import salt.utils.xmlutil as xmlutil import salt.utils.yaml from salt._compat import ipaddress from salt.exceptions import CommandExecutionError, SaltInvocationError -from salt.ext import six from salt.ext.six.moves import range # pylint: disable=import-error,redefined-builtin from salt.ext.six.moves.urllib.parse import urlparse, urlunparse from salt.utils.virt import check_remote, download_remote @@ -647,6 +643,7 @@ def _gen_xml( arch, graphics=None, boot=None, + boot_dev=None, **kwargs ): """ @@ -680,15 +677,17 @@ def _gen_xml( graphics = None context["graphics"] = graphics - if "boot_dev" in kwargs: - context["boot_dev"] = [] - for dev in kwargs["boot_dev"].split(): - context["boot_dev"].append(dev) - else: - context["boot_dev"] = ["hd"] + context["boot_dev"] = boot_dev.split() if boot_dev is not None else ["hd"] context["boot"] = boot if boot else {} + # if efi parameter is specified, prepare os_attrib + efi_value = context["boot"].get("efi", None) if boot else None + if efi_value is True: + context["boot"]["os_attrib"] = "firmware='efi'" + elif efi_value is not None and type(efi_value) != bool: + raise SaltInvocationError("Invalid efi value") + if os_type == "xen": # Compute the Xen PV boot method if __grains__["os_family"] == "Suse": @@ -1519,17 +1518,24 @@ def _handle_remote_boot_params(orig_boot): new_boot = orig_boot.copy() keys = orig_boot.keys() cases = [ + {"efi"}, + {"kernel", "initrd", "efi"}, + {"kernel", "initrd", "cmdline", "efi"}, {"loader", "nvram"}, {"kernel", "initrd"}, {"kernel", "initrd", "cmdline"}, - {"loader", "nvram", "kernel", "initrd"}, - {"loader", "nvram", "kernel", "initrd", "cmdline"}, + {"kernel", "initrd", "loader", "nvram"}, + {"kernel", "initrd", "cmdline", "loader", "nvram"}, ] try: if keys in cases: for key in keys: - if orig_boot.get(key) is not None and check_remote(orig_boot.get(key)): + if key == "efi" and type(orig_boot.get(key)) == bool: + new_boot[key] = orig_boot.get(key) + elif orig_boot.get(key) is not None and check_remote( + orig_boot.get(key) + ): if saltinst_dir is None: os.makedirs(CACHE_DIR) saltinst_dir = CACHE_DIR @@ -1537,12 +1543,41 @@ def _handle_remote_boot_params(orig_boot): return new_boot else: raise SaltInvocationError( - "Invalid boot parameters, (kernel, initrd) or/and (loader, nvram) must be both present" + "Invalid boot parameters,It has to follow this combination: [(kernel, initrd) or/and cmdline] or/and [(loader, nvram) or efi]" ) except Exception as err: # pylint: disable=broad-except raise err +def _handle_efi_param(boot, desc): + """ + Checks if boot parameter contains efi boolean value, if so, handles the firmware attribute. + :param boot: The boot parameters passed to the init or update functions. + :param desc: The XML description of that domain. + :return: A boolean value. + """ + efi_value = boot.get("efi", None) if boot else None + parent_tag = desc.find("os") + os_attrib = parent_tag.attrib + + # newly defined vm without running, loader tag might not be filled yet + if efi_value is False and os_attrib != {}: + parent_tag.attrib.pop("firmware", None) + return True + + # check the case that loader tag might be present. This happens after the vm ran + elif type(efi_value) == bool and os_attrib == {}: + if efi_value is True and parent_tag.find("loader") is None: + parent_tag.set("firmware", "efi") + if efi_value is False and parent_tag.find("loader") is not None: + parent_tag.remove(parent_tag.find("loader")) + parent_tag.remove(parent_tag.find("nvram")) + return True + elif type(efi_value) != bool: + raise SaltInvocationError("Invalid efi value") + return False + + def init( name, cpu, @@ -1563,6 +1598,7 @@ def init( os_type=None, arch=None, boot=None, + boot_dev=None, **kwargs ): """ @@ -1632,7 +1668,8 @@ def init( This is an optional parameter, all of the keys are optional within the dictionary. The structure of the dictionary is documented in :ref:`init-boot-def`. If a remote path is provided to kernel or initrd, salt will handle the downloading of the specified remote file and modify the XML accordingly. - To boot VM with UEFI, specify loader and nvram path. + To boot VM with UEFI, specify loader and nvram path or specify 'efi': ``True`` if your libvirtd version + is >= 5.2.0 and QEMU >= 3.0.0. .. versionadded:: 3000 @@ -1646,6 +1683,12 @@ def init( 'nvram': '/usr/share/OVMF/OVMF_VARS.ms.fd' } + :param boot_dev: + Space separated list of devices to boot from sorted by decreasing priority. + Values can be ``hd``, ``fd``, ``cdrom`` or ``network``. + + By default, the value will ``"hd"``. + .. _init-boot-def: .. rubric:: Boot parameters definition @@ -1671,6 +1714,11 @@ def init( .. versionadded:: sodium + efi + A boolean value. + + .. versionadded:: sodium + .. _init-nic-def: .. rubric:: Network Interfaces Definitions @@ -1855,6 +1903,8 @@ def init( for x in y } ) + if len(hypervisors) == 0: + raise SaltInvocationError("No supported hypervisors were found") virt_hypervisor = "kvm" if "kvm" in hypervisors else hypervisors[0] # esxi used to be a possible value for the hypervisor: map it to vmware since it's the same @@ -1962,8 +2012,10 @@ def init( arch, graphics, boot, + boot_dev, **kwargs ) + log.debug("New virtual machine definition: %s", vm_xml) conn.defineXML(vm_xml) except libvirt.libvirtError as err: conn.close() @@ -2189,6 +2241,7 @@ def update( live=True, boot=None, test=False, + boot_dev=None, **kwargs ): """ @@ -2248,11 +2301,28 @@ def update( Refer to :ref:`init-boot-def` for the complete boot parameter description. - To update any boot parameters, specify the new path for each. To remove any boot parameters, - pass a None object, for instance: 'kernel': ``None``. + To update any boot parameters, specify the new path for each. To remove any boot parameters, pass ``None`` object, + for instance: 'kernel': ``None``. To switch back to BIOS boot, specify ('loader': ``None`` and 'nvram': ``None``) + or 'efi': ``False``. Please note that ``None`` is mapped to ``null`` in sls file, pass ``null`` in sls file instead. + + SLS file Example: + + .. code-block:: yaml + + - boot: + loader: null + nvram: null .. versionadded:: 3000 + :param boot_dev: + Space separated list of devices to boot from sorted by decreasing priority. + Values can be ``hd``, ``fd``, ``cdrom`` or ``network``. + + By default, the value will ``"hd"``. + + .. versionadded:: Magnesium + :param test: run in dry-run mode if set to True .. versionadded:: sodium @@ -2327,67 +2397,54 @@ def update( cpu_node.set("current", str(cpu)) need_update = True - # Update the kernel boot parameters - boot_tags = ["kernel", "initrd", "cmdline", "loader", "nvram"] - parent_tag = desc.find("os") - - # We need to search for each possible subelement, and update it. - for tag in boot_tags: - # The Existing Tag... - found_tag = parent_tag.find(tag) - - # The new value - boot_tag_value = boot.get(tag, None) if boot else None - - # Existing tag is found and values don't match - if found_tag is not None and found_tag.text != boot_tag_value: - - # If the existing tag is found, but the new value is None - # remove it. If the existing tag is found, and the new value - # doesn't match update it. In either case, mark for update. - if boot_tag_value is None and boot is not None and parent_tag is not None: - parent_tag.remove(found_tag) - else: - found_tag.text = boot_tag_value - - # If the existing tag is loader or nvram, we need to update the corresponding attribute - if found_tag.tag == "loader" and boot_tag_value is not None: - found_tag.set("readonly", "yes") - found_tag.set("type", "pflash") - - if found_tag.tag == "nvram" and boot_tag_value is not None: - found_tag.set("template", found_tag.text) - found_tag.text = None + def _set_loader(node, value): + salt.utils.xmlutil.set_node_text(node, value) + if value is not None: + node.set("readonly", "yes") + node.set("type", "pflash") - need_update = True + def _set_nvram(node, value): + node.set("template", value) - # Need to check for parent tag, and add it if it does not exist. - # Add a subelement and set the value to the new value, and then - # mark for update. - if parent_tag is not None: - child_tag = ElementTree.SubElement(parent_tag, tag) - else: - new_parent_tag = ElementTree.Element("os") - child_tag = ElementTree.SubElement(new_parent_tag, tag) - - child_tag.text = boot_tag_value - - # If the newly created tag is loader or nvram, we need to update the corresponding attribute - if child_tag.tag == "loader": - child_tag.set("readonly", "yes") - child_tag.set("type", "pflash") + def _set_with_mib_unit(node, value): + node.text = str(value) + node.set("unit", "MiB") - if child_tag.tag == "nvram": - child_tag.set("template", child_tag.text) - child_tag.text = None + # Update the kernel boot parameters + params_mapping = [ + {"path": "boot:kernel", "xpath": "os/kernel"}, + {"path": "boot:initrd", "xpath": "os/initrd"}, + {"path": "boot:cmdline", "xpath": "os/cmdline"}, + {"path": "boot:loader", "xpath": "os/loader", "set": _set_loader}, + {"path": "boot:nvram", "xpath": "os/nvram", "set": _set_nvram}, + # Update the memory, note that libvirt outputs all memory sizes in KiB + { + "path": "mem", + "xpath": "memory", + "get": lambda n: int(n.text) / 1024, + "set": _set_with_mib_unit, + }, + { + "path": "mem", + "xpath": "currentMemory", + "get": lambda n: int(n.text) / 1024, + "set": _set_with_mib_unit, + }, + { + "path": "boot_dev:{dev}", + "xpath": "os/boot[$dev]", + "get": lambda n: n.get("dev"), + "set": lambda n, v: n.set("dev", v), + "del": salt.utils.xmlutil.del_attribute("dev"), + }, + ] - # Update the memory, note that libvirt outputs all memory sizes in KiB - for mem_node_name in ["memory", "currentMemory"]: - mem_node = desc.find(mem_node_name) - if mem and int(mem_node.text) != mem * 1024: - mem_node.text = str(mem) - mem_node.set("unit", "MiB") - need_update = True + data = {k: v for k, v in locals().items() if bool(v)} + if boot_dev: + data["boot_dev"] = {i + 1: dev for i, dev in enumerate(boot_dev.split())} + need_update = need_update or salt.utils.xmlutil.change_xml( + desc, data, params_mapping + ) # Update the XML definition with the new disks and diff changes devices_node = desc.find("devices") @@ -2434,9 +2491,9 @@ def update( _disk_volume_create(conn, all_disks[idx]) if not test: - conn.defineXML( - salt.utils.stringutils.to_str(ElementTree.tostring(desc)) - ) + xml_desc = ElementTree.tostring(desc) + log.debug("Update virtual machine definition: %s", xml_desc) + conn.defineXML(salt.utils.stringutils.to_str(xml_desc)) status["definition"] = True except libvirt.libvirtError as err: conn.close() @@ -3218,24 +3275,19 @@ def get_profiles(hypervisor=None, **kwargs): for x in y } ) - default_hypervisor = "kvm" if "kvm" in hypervisors else hypervisors[0] + if len(hypervisors) == 0: + raise SaltInvocationError("No supported hypervisors were found") if not hypervisor: - hypervisor = default_hypervisor + hypervisor = "kvm" if "kvm" in hypervisors else hypervisors[0] virtconf = __salt__["config.get"]("virt", {}) for typ in ["disk", "nic"]: _func = getattr(sys.modules[__name__], "_{}_profile".format(typ)) - ret[typ] = { - "default": _func( - "default", hypervisor if hypervisor else default_hypervisor - ) - } + ret[typ] = {"default": _func("default", hypervisor)} if typ in virtconf: ret.setdefault(typ, {}) for prf in virtconf[typ]: - ret[typ][prf] = _func( - prf, hypervisor if hypervisor else default_hypervisor - ) + ret[typ][prf] = _func(prf, hypervisor) return ret @@ -4043,7 +4095,7 @@ def purge(vm_, dirs=False, removables=False, **kwargs): directories.add(os.path.dirname(disks[disk]["file"])) else: # We may have a volume to delete here - matcher = re.match("^(?P<pool>[^/]+)/(?P<volume>.*)$", disks[disk]["file"],) + matcher = re.match("^(?P<pool>[^/]+)/(?P<volume>.*)$", disks[disk]["file"]) if matcher: pool_name = matcher.group("pool") pool = None @@ -5431,7 +5483,7 @@ def list_networks(**kwargs): def network_info(name=None, **kwargs): """ - Return informations on a virtual network provided its name. + Return information on a virtual network provided its name. :param name: virtual network name :param connection: libvirt connection URI, overriding defaults @@ -6028,15 +6080,19 @@ def _pool_set_secret( if secret_type: # Get the previously defined secret if any secret = None - if usage: - usage_type = ( - libvirt.VIR_SECRET_USAGE_TYPE_CEPH - if secret_type == "ceph" - else libvirt.VIR_SECRET_USAGE_TYPE_ISCSI - ) - secret = conn.secretLookupByUsage(usage_type, usage) - elif uuid: - secret = conn.secretLookupByUUIDString(uuid) + try: + if usage: + usage_type = ( + libvirt.VIR_SECRET_USAGE_TYPE_CEPH + if secret_type == "ceph" + else libvirt.VIR_SECRET_USAGE_TYPE_ISCSI + ) + secret = conn.secretLookupByUsage(usage_type, usage) + elif uuid: + secret = conn.secretLookupByUUIDString(uuid) + except libvirt.libvirtError as err: + # For some reason the secret has been removed. Don't fail since we'll recreate it + log.info("Secret not found: %s", err.get_error_message()) # Create secret if needed if not secret: @@ -6288,7 +6344,7 @@ def list_pools(**kwargs): def pool_info(name=None, **kwargs): """ - Return informations on a storage pool provided its name. + Return information on a storage pool provided its name. :param name: libvirt storage pool name :param connection: libvirt connection URI, overriding defaults @@ -6505,22 +6561,6 @@ def pool_delete(name, **kwargs): conn = __get_conn(**kwargs) try: pool = conn.storagePoolLookupByName(name) - desc = ElementTree.fromstring(pool.XMLDesc()) - - # Is there a secret that we generated and would need to be removed? - # Don't remove the other secrets - auth_node = desc.find("source/auth") - if auth_node is not None: - auth_types = { - "ceph": libvirt.VIR_SECRET_USAGE_TYPE_CEPH, - "iscsi": libvirt.VIR_SECRET_USAGE_TYPE_ISCSI, - } - secret_type = auth_types[auth_node.get("type")] - secret_usage = auth_node.find("secret").get("usage") - if secret_type and "pool_{}".format(name) == secret_usage: - secret = conn.secretLookupByUsage(secret_type, secret_usage) - secret.undefine() - return not bool(pool.delete(libvirt.VIR_STORAGE_POOL_DELETE_NORMAL)) finally: conn.close() diff --git a/salt/states/virt.py b/salt/states/virt.py index cb15d57d8f..b45cf72ed3 100644 --- a/salt/states/virt.py +++ b/salt/states/virt.py @@ -33,6 +33,8 @@ except ImportError: __virtualname__ = "virt" +log = logging.getLogger(__name__) + def __virtual__(): """ @@ -285,6 +287,7 @@ def defined( arch=None, boot=None, update=True, + boot_dev=None, ): """ Starts an existing guest, or defines and starts a new VM with specified arguments. @@ -345,6 +348,14 @@ def defined( .. deprecated:: sodium + :param boot_dev: + Space separated list of devices to boot from sorted by decreasing priority. + Values can be ``hd``, ``fd``, ``cdrom`` or ``network``. + + By default, the value will ``"hd"``. + + .. versionadded:: Magnesium + .. rubric:: Example States Make sure a virtual machine called ``domain_name`` is defined: @@ -355,6 +366,7 @@ def defined( virt.defined: - cpu: 2 - mem: 2048 + - boot_dev: network hd - disk_profile: prod - disks: - name: system @@ -407,6 +419,7 @@ def defined( password=password, boot=boot, test=__opts__["test"], + boot_dev=boot_dev, ) ret["changes"][name] = status if not status.get("definition"): @@ -441,6 +454,7 @@ def defined( password=password, boot=boot, start=False, + boot_dev=boot_dev, ) ret["changes"][name] = {"definition": True} ret["comment"] = "Domain {} defined".format(name) @@ -473,6 +487,7 @@ def running( os_type=None, arch=None, boot=None, + boot_dev=None, ): """ Starts an existing guest, or defines and starts a new VM with specified arguments. @@ -584,6 +599,14 @@ def running( .. versionadded:: 3000 + :param boot_dev: + Space separated list of devices to boot from sorted by decreasing priority. + Values can be ``hd``, ``fd``, ``cdrom`` or ``network``. + + By default, the value will ``"hd"``. + + .. versionadded:: Magnesium + .. rubric:: Example States Make sure an already-defined virtual machine called ``domain_name`` is running: @@ -651,6 +674,7 @@ def running( arch=arch, boot=boot, update=update, + boot_dev=boot_dev, connection=connection, username=username, password=password, @@ -1218,14 +1242,24 @@ def pool_defined( action = "" if info[name]["state"] != "running": - if not __opts__["test"]: - __salt__["virt.pool_build"]( - name, - connection=connection, - username=username, - password=password, - ) - action = ", built" + if ptype in BUILDABLE_POOL_TYPES: + if not __opts__["test"]: + # Storage pools build like disk or logical will fail if the disk or LV group + # was already existing. Since we can't easily figure that out, just log the + # possible libvirt error. + try: + __salt__["virt.pool_build"]( + name, + connection=connection, + username=username, + password=password, + ) + except libvirt.libvirtError as err: + log.warning( + "Failed to build libvirt storage pool: %s", + err.get_error_message(), + ) + action = ", built" action = ( "{}, autostart flag changed".format(action) @@ -1261,9 +1295,22 @@ def pool_defined( password=password, ) - __salt__["virt.pool_build"]( - name, connection=connection, username=username, password=password - ) + if ptype in BUILDABLE_POOL_TYPES: + # Storage pools build like disk or logical will fail if the disk or LV group + # was already existing. Since we can't easily figure that out, just log the + # possible libvirt error. + try: + __salt__["virt.pool_build"]( + name, + connection=connection, + username=username, + password=password, + ) + except libvirt.libvirtError as err: + log.warning( + "Failed to build libvirt storage pool: %s", + err.get_error_message(), + ) if needs_autostart: ret["changes"][name] = "Pool defined, marked for autostart" ret["comment"] = "Pool {} defined, marked for autostart".format(name) @@ -1370,7 +1417,7 @@ def pool_running( is_running = info.get(name, {}).get("state", "stopped") == "running" if is_running: if updated: - action = "built, restarted" + action = "restarted" if not __opts__["test"]: __salt__["virt.pool_stop"]( name, @@ -1378,13 +1425,16 @@ def pool_running( username=username, password=password, ) - if not __opts__["test"]: - __salt__["virt.pool_build"]( - name, - connection=connection, - username=username, - password=password, - ) + # if the disk or LV group is already existing build will fail (issue #56454) + if ptype in BUILDABLE_POOL_TYPES - {"disk", "logical"}: + if not __opts__["test"]: + __salt__["virt.pool_build"]( + name, + connection=connection, + username=username, + password=password, + ) + action = "built, {}".format(action) else: action = "already running" result = True diff --git a/salt/templates/virt/libvirt_domain.jinja b/salt/templates/virt/libvirt_domain.jinja index 439ed83f7f..2a2f5e4141 100644 --- a/salt/templates/virt/libvirt_domain.jinja +++ b/salt/templates/virt/libvirt_domain.jinja @@ -2,32 +2,9 @@ <domain type='{{ hypervisor }}'> <name>{{ name }}</name> <vcpu>{{ cpu }}</vcpu> - {%- if mem.max %} - <maxMemory {{ mem.slots }} unit='KiB'> {{ mem.max }}</maxMemory> - {%- endif %} - {%- if mem.boot %} - <memory unit='KiB'>{{ mem.boot }}</memory> - {%- endif %} - {%- if mem.current %} - <currentMemory unit='KiB'>{{ mem.current }}</currentMemory> - {%- endif %} - {%- if mem %} - <memtune> - {%- if 'hard_limit' in mem and mem.hard_limit %} - <hard_limit unit="KiB">{{ mem.hard_limit }}</hard_limit> - {%- endif %} - {%- if 'soft_limit' in mem and mem.soft_limit %} - <soft_limit unit="KiB">{{ mem.soft_limit }}</soft_limit> - {%- endif %} - {%- if 'swap_hard_limit' in mem and mem.swap_hard_limit %} - <swap_hard_limit unit="KiB">{{ mem.swap_hard_limit }}</swap_hard_limit> - {%- endif %} - {%- if 'min_guarantee' in mem and mem.min_guarantee %} - <min_guarantee unit="KiB">{{ mem.min_guarantee }}</min_guarantee> - {%- endif %} - </memtune> - {%- endif %} - <os {{ boot.os_attrib }}> + <memory unit='KiB'>{{ mem }}</memory> + <currentMemory unit='KiB'>{{ mem }}</currentMemory> + <os {{boot.os_attrib}}> <type arch='{{ arch }}'>{{ os_type }}</type> {% if boot %} {% if 'kernel' in boot %} diff --git a/salt/utils/xmlutil.py b/salt/utils/xmlutil.py index e5c8ad4eec..b9f047820b 100644 --- a/salt/utils/xmlutil.py +++ b/salt/utils/xmlutil.py @@ -25,7 +25,7 @@ def _to_dict(xmltree): """ Converts an XML ElementTree to a dictionary that only contains items. This is the default behavior in version 2017.7. This will default to prevent - unexpected parsing issues on modules dependent on this. + unexpected parsing issues on modules dependant on this. """ # If this object has no children, the for..loop below will return nothing # for it, so just return a single dict representing it. @@ -298,7 +298,7 @@ def change_xml(doc, data, mapping): if convert_fn: new_value = convert_fn(new_value) - if str(current_value) != str(new_value): + if current_value != new_value: set_fn(node, new_value) need_update = True else: diff --git a/tests/unit/modules/test_virt.py b/tests/unit/modules/test_virt.py index f53b4a85c1..4775fec31f 100644 --- a/tests/unit/modules/test_virt.py +++ b/tests/unit/modules/test_virt.py @@ -1843,17 +1843,36 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): "cmdline": "console=ttyS0 ks=http://example.com/f8-i386/os/", } - boot_uefi = { - "loader": "/usr/share/OVMF/OVMF_CODE.fd", - "nvram": "/usr/share/OVMF/OVMF_VARS.ms.fd", - } + # Update boot devices case + define_mock.reset_mock() + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("my_vm", boot_dev="cdrom network hd"), + ) + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual( + ["cdrom", "network", "hd"], + [node.get("dev") for node in setxml.findall("os/boot")], + ) - invalid_boot = { - "loader": "/usr/share/OVMF/OVMF_CODE.fd", - "initrd": "/root/f8-i386-initrd", - } + # Update unchanged boot devices case + define_mock.reset_mock() + self.assertEqual( + { + "definition": False, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("my_vm", boot_dev="hd"), + ) + define_mock.assert_not_called() # Update with boot parameter case + define_mock.reset_mock() self.assertEqual( { "definition": True, @@ -1877,6 +1896,11 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): "console=ttyS0 ks=http://example.com/f8-i386/os/", ) + boot_uefi = { + "loader": "/usr/share/OVMF/OVMF_CODE.fd", + "nvram": "/usr/share/OVMF/OVMF_VARS.ms.fd", + } + self.assertEqual( { "definition": True, @@ -1896,9 +1920,28 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): "/usr/share/OVMF/OVMF_VARS.ms.fd", ) + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("my_vm", boot={"efi": True}), + ) + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual(setxml.find("os").attrib.get("firmware"), "efi") + + invalid_boot = { + "loader": "/usr/share/OVMF/OVMF_CODE.fd", + "initrd": "/root/f8-i386-initrd", + } + with self.assertRaises(SaltInvocationError): virt.update("my_vm", boot=invalid_boot) + with self.assertRaises(SaltInvocationError): + virt.update("my_vm", boot={"efi": "Not a boolean value"}) + # Update memory case setmem_mock = MagicMock(return_value=0) domain_mock.setMemoryFlags = setmem_mock @@ -2390,6 +2433,43 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): ], ) + def test_update_xen_boot_params(self): + """ + Test virt.update() a Xen definition no boot parameter. + """ + root_dir = os.path.join(salt.syspaths.ROOT_DIR, "srv", "salt-images") + xml_boot = """ + <domain type='xen' id='8'> + <name>vm</name> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='auto'>1</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> + </os> + </domain> + """ + domain_mock_boot = self.set_mock_vm("vm", xml_boot) + domain_mock_boot.OSType = MagicMock(return_value="hvm") + define_mock_boot = MagicMock(return_value=True) + define_mock_boot.setVcpusFlags = MagicMock(return_value=0) + self.mock_conn.defineXML = define_mock_boot + self.assertEqual( + { + "cpu": False, + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("vm", cpu=2), + ) + setxml = ET.fromstring(define_mock_boot.call_args[0][0]) + self.assertEqual(setxml.find("os").find("loader").attrib.get("type"), "rom") + self.assertEqual( + setxml.find("os").find("loader").text, "/usr/lib/xen/boot/hvmloader" + ) + def test_update_existing_boot_params(self): """ Test virt.update() with existing boot parameters. @@ -2530,6 +2610,18 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): self.assertEqual(setxml.find("os").find("initrd"), None) self.assertEqual(setxml.find("os").find("cmdline"), None) + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("vm_with_boot_param", boot={"efi": False}), + ) + setxml = ET.fromstring(define_mock_boot.call_args[0][0]) + self.assertEqual(setxml.find("os").find("nvram"), None) + self.assertEqual(setxml.find("os").find("loader"), None) + self.assertEqual( { "definition": True, @@ -4248,7 +4340,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): """ mock_pool = MagicMock() mock_pool.delete = MagicMock(return_value=0) - mock_pool.XMLDesc.return_value = "<pool type='dir'/>" self.mock_conn.storagePoolLookupByName = MagicMock(return_value=mock_pool) res = virt.pool_delete("test-pool") @@ -4262,12 +4353,12 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): self.mock_libvirt.VIR_STORAGE_POOL_DELETE_NORMAL ) - def test_pool_delete_secret(self): + def test_pool_undefine_secret(self): """ - Test virt.pool_delete function where the pool has a secret + Test virt.pool_undefine function where the pool has a secret """ mock_pool = MagicMock() - mock_pool.delete = MagicMock(return_value=0) + mock_pool.undefine = MagicMock(return_value=0) mock_pool.XMLDesc.return_value = """ <pool type='rbd'> <name>test-ses</name> @@ -4284,16 +4375,11 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): mock_undefine = MagicMock(return_value=0) self.mock_conn.secretLookupByUsage.return_value.undefine = mock_undefine - res = virt.pool_delete("test-ses") + res = virt.pool_undefine("test-ses") self.assertTrue(res) self.mock_conn.storagePoolLookupByName.assert_called_once_with("test-ses") - - # Shouldn't be called with another parameter so far since those are not implemented - # and thus throwing exceptions. - mock_pool.delete.assert_called_once_with( - self.mock_libvirt.VIR_STORAGE_POOL_DELETE_NORMAL - ) + mock_pool.undefine.assert_called_once_with() self.mock_conn.secretLookupByUsage.assert_called_once_with( self.mock_libvirt.VIR_SECRET_USAGE_TYPE_CEPH, "pool_test-ses" @@ -4562,24 +4648,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): </source> </pool>""" - expected_xml = ( - '<pool type="rbd">' - "<name>default</name>" - "<uuid>20fbe05c-ab40-418a-9afa-136d512f0ede</uuid>" - '<capacity unit="bytes">1999421108224</capacity>' - '<allocation unit="bytes">713207042048</allocation>' - '<available unit="bytes">1286214066176</available>' - "<source>" - '<host name="ses4.tf.local" />' - '<host name="ses5.tf.local" />' - '<auth type="ceph" username="libvirt">' - '<secret uuid="14e9a0f1-8fbf-4097-b816-5b094c182212" />' - "</auth>" - "<name>iscsi-images</name>" - "</source>" - "</pool>" - ) - mock_secret = MagicMock() self.mock_conn.secretLookupByUUIDString = MagicMock(return_value=mock_secret) @@ -4600,6 +4668,23 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): self.mock_conn.storagePoolDefineXML.assert_not_called() mock_secret.setValue.assert_called_once_with(b"secret") + # Case where the secret can't be found + self.mock_conn.secretLookupByUUIDString = MagicMock( + side_effect=self.mock_libvirt.libvirtError("secret not found") + ) + self.assertFalse( + virt.pool_update( + "default", + "rbd", + source_name="iscsi-images", + source_hosts=["ses4.tf.local", "ses5.tf.local"], + source_auth={"username": "libvirt", "password": "c2VjcmV0"}, + ) + ) + self.mock_conn.storagePoolDefineXML.assert_not_called() + self.mock_conn.secretDefineXML.assert_called_once() + mock_secret.setValue.assert_called_once_with(b"secret") + def test_pool_update_password_create(self): """ Test the pool_update function, where the password only is changed diff --git a/tests/unit/states/test_virt.py b/tests/unit/states/test_virt.py index 6d38829870..8fe892f607 100644 --- a/tests/unit/states/test_virt.py +++ b/tests/unit/states/test_virt.py @@ -333,6 +333,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): "myvm", cpu=2, mem=2048, + boot_dev="cdrom hd", os_type="linux", arch="i686", vm_type="qemu", @@ -355,6 +356,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): "myvm", cpu=2, mem=2048, + boot_dev="cdrom hd", os_type="linux", arch="i686", disk="prod", @@ -463,10 +465,13 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): "comment": "Domain myvm updated with live update(s) failures", } ) - self.assertDictEqual(virt.defined("myvm", cpu=2), ret) + self.assertDictEqual( + virt.defined("myvm", cpu=2, boot_dev="cdrom hd"), ret + ) update_mock.assert_called_with( "myvm", cpu=2, + boot_dev="cdrom hd", mem=None, disk_profile=None, disks=None, @@ -590,6 +595,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): password=None, boot=None, test=True, + boot_dev=None, ) # No changes case @@ -624,6 +630,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): password=None, boot=None, test=True, + boot_dev=None, ) def test_running(self): @@ -700,6 +707,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): install=True, pub_key=None, priv_key=None, + boot_dev=None, connection=None, username=None, password=None, @@ -761,6 +769,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): install=False, pub_key="/path/to/key.pub", priv_key="/path/to/key", + boot_dev="network hd", connection="someconnection", username="libvirtuser", password="supersecret", @@ -785,6 +794,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): start=False, pub_key="/path/to/key.pub", priv_key="/path/to/key", + boot_dev="network hd", connection="someconnection", username="libvirtuser", password="supersecret", @@ -929,6 +939,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): password=None, boot=None, test=False, + boot_dev=None, ) # Failed definition update case @@ -1047,6 +1058,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): password=None, boot=None, test=True, + boot_dev=None, ) start_mock.assert_not_called() @@ -1083,6 +1095,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): password=None, boot=None, test=True, + boot_dev=None, ) def test_stopped(self): @@ -1970,6 +1983,72 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): password="secret", ) + # Define a pool that doesn't handle build + for mock in mocks: + mocks[mock].reset_mock() + with patch.dict( + virt.__salt__, + { # pylint: disable=no-member + "virt.pool_info": MagicMock( + side_effect=[ + {}, + {"mypool": {"state": "stopped", "autostart": True}}, + ] + ), + "virt.pool_define": mocks["define"], + "virt.pool_build": mocks["build"], + "virt.pool_set_autostart": mocks["autostart"], + }, + ): + ret.update( + { + "changes": {"mypool": "Pool defined, marked for autostart"}, + "comment": "Pool mypool defined, marked for autostart", + } + ) + self.assertDictEqual( + virt.pool_defined( + "mypool", + ptype="rbd", + source={ + "name": "libvirt-pool", + "hosts": ["ses2.tf.local", "ses3.tf.local"], + "auth": { + "username": "libvirt", + "password": "AQAz+PRdtquBBRAASMv7nlMZYfxIyLw3St65Xw==", + }, + }, + autostart=True, + ), + ret, + ) + mocks["define"].assert_called_with( + "mypool", + ptype="rbd", + target=None, + permissions=None, + source_devices=None, + source_dir=None, + source_adapter=None, + source_hosts=["ses2.tf.local", "ses3.tf.local"], + source_auth={ + "username": "libvirt", + "password": "AQAz+PRdtquBBRAASMv7nlMZYfxIyLw3St65Xw==", + }, + source_name="libvirt-pool", + source_format=None, + source_initiator=None, + start=False, + transient=False, + connection=None, + username=None, + password=None, + ) + mocks["autostart"].assert_called_with( + "mypool", state="on", connection=None, username=None, password=None, + ) + mocks["build"].assert_not_called() + mocks["update"] = MagicMock(return_value=False) for mock in mocks: mocks[mock].reset_mock() @@ -2019,6 +2098,9 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): for mock in mocks: mocks[mock].reset_mock() mocks["update"] = MagicMock(return_value=True) + mocks["build"] = MagicMock( + side_effect=self.mock_libvirt.libvirtError("Existing VG") + ) with patch.dict( virt.__salt__, { # pylint: disable=no-member @@ -2122,6 +2204,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): ), ret, ) + mocks["build"].assert_not_called() mocks["update"].assert_called_with( "mypool", ptype="logical", @@ -2469,8 +2552,8 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): ): ret.update( { - "changes": {"mypool": "Pool updated, built, restarted"}, - "comment": "Pool mypool updated, built, restarted", + "changes": {"mypool": "Pool updated, restarted"}, + "comment": "Pool mypool updated, restarted", "result": True, } ) @@ -2496,9 +2579,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): mocks["start"].assert_called_with( "mypool", connection=None, username=None, password=None ) - mocks["build"].assert_called_with( - "mypool", connection=None, username=None, password=None - ) + mocks["build"].assert_not_called() mocks["update"].assert_called_with( "mypool", ptype="logical", diff --git a/tests/unit/utils/test_data.py b/tests/unit/utils/test_data.py index 9206979284..aff7384232 100644 --- a/tests/unit/utils/test_data.py +++ b/tests/unit/utils/test_data.py @@ -220,38 +220,6 @@ class DataTestCase(TestCase): ), ) - # Traverse and match integer key in a nested dict - # https://github.com/saltstack/salt/issues/56444 - self.assertEqual( - "it worked", - salt.utils.data.traverse_dict_and_list( - {"foo": {1234: "it worked"}}, "foo:1234", "it didn't work", - ), - ) - # Make sure that we properly return the default value when the initial - # attempt fails and YAML-loading the target key doesn't change its - # value. - self.assertEqual( - "default", - salt.utils.data.traverse_dict_and_list( - {"foo": {"baz": "didn't work"}}, "foo:bar", "default", - ), - ) - - def test_issue_39709(self): - test_two_level_dict_and_list = { - "foo": ["bar", "baz", {"lorem": {"ipsum": [{"dolor": "sit"}]}}] - } - - self.assertEqual( - "sit", - salt.utils.data.traverse_dict_and_list( - test_two_level_dict_and_list, - ["foo", "lorem", "ipsum", "dolor"], - {"not_found": "not_found"}, - ), - ) - def test_compare_dicts(self): ret = salt.utils.data.compare_dicts(old={"foo": "bar"}, new={"foo": "bar"}) self.assertEqual(ret, {}) -- 2.29.2
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
