Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:Newbie08
salt
backport-a-few-virt-prs-272.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File backport-a-few-virt-prs-272.patch of Package salt
From acee2074e9fe4da2731e61a554639e773c04e43a Mon Sep 17 00:00:00 2001 From: Cedric Bosdonnat <cbosdonnat@suse.com> Date: Mon, 5 Oct 2020 16:49:59 +0200 Subject: [PATCH] Backport a few virt PRs (#272) * Fix virt update when cpu and memory are changed If CPU is changed, the memory change would be short circuited. This is a regression introduced by PR #58332 * virt: add VM memory tunning support * avoid comparing string with integer * fix pre-commit failure * Properly fix memory setting regression in virt.update The 'mem' property in the virt.update value should indicate the result of a live memory setting. The value should be an int in KiB. Fixing the code and tests for this. * virt: add stop_on_reboot parameter in guest states and definition It can be needed to force a VM to stop instead of rebooting. A typical example of this is when creating a VM using a install CDROM ISO or when using an autoinstallation profile. Forcing a shutdown allows libvirt to pick up another XML definition for the new start to remove the firstboot-only options. * virt: expose live parameter in virt.defined state Allow updating the definition of a VM without touching the live instance. This can be helpful since live update may change the device names in the guest. Co-authored-by: firefly <guoqing_li@pm.me> Co-authored-by: gqlo <escita@pm.me> --- changelog/57639.added | 1 + changelog/58589.added | 1 + salt/modules/virt.py | 284 ++++++++++++++++++-- salt/states/virt.py | 71 ++++- salt/templates/virt/libvirt_domain.jinja | 30 ++- salt/utils/xmlutil.py | 2 +- tests/unit/modules/test_virt.py | 318 ++++++++++++++++++++++- tests/unit/states/test_virt.py | 14 +- 8 files changed, 687 insertions(+), 34 deletions(-) create mode 100644 changelog/57639.added create mode 100644 changelog/58589.added diff --git a/changelog/57639.added b/changelog/57639.added new file mode 100644 index 0000000000..c0281e9319 --- /dev/null +++ b/changelog/57639.added @@ -0,0 +1 @@ +Memory Tuning Support which allows much greater control of memory allocation diff --git a/changelog/58589.added b/changelog/58589.added new file mode 100644 index 0000000000..5960555ec6 --- /dev/null +++ b/changelog/58589.added @@ -0,0 +1 @@ +Allow handling special first boot definition on virtual machine diff --git a/salt/modules/virt.py b/salt/modules/virt.py index e306bc0679..8e2180608a 100644 --- a/salt/modules/virt.py +++ b/salt/modules/virt.py @@ -71,6 +71,50 @@ The calls not using the libvirt connection setup are: - `libvirt URI format <http://libvirt.org/uri.html#URI_config>`_ - `libvirt authentication configuration <http://libvirt.org/auth.html#Auth_client_config>`_ +Units +========== +.. _virt-units: +.. rubric:: Units specification +.. versionadded:: Magnesium + +The string should contain a number optionally followed +by a unit. The number may have a decimal fraction. If +the unit is not given then MiB are set by default. +Units can optionally be given in IEC style (such as MiB), +although the standard single letter style (such as M) is +more convenient. + +Valid units include: + +========== ===== ========== ========== ====== +Standard IEC Standard IEC + Unit Unit Name Name Factor +========== ===== ========== ========== ====== + B Bytes 1 + K KiB Kilobytes Kibibytes 2**10 + M MiB Megabytes Mebibytes 2**20 + G GiB Gigabytes Gibibytes 2**30 + T TiB Terabytes Tebibytes 2**40 + P PiB Petabytes Pebibytes 2**50 + E EiB Exabytes Exbibytes 2**60 + Z ZiB Zettabytes Zebibytes 2**70 + Y YiB Yottabytes Yobibytes 2**80 +========== ===== ========== ========== ====== + +Additional decimal based units: + +====== ======= +Unit Factor +====== ======= +KB 10**3 +MB 10**6 +GB 10**9 +TB 10**12 +PB 10**15 +EB 10**18 +ZB 10**21 +YB 10**24 +====== ======= """ # Special Thanks to Michael Dehann, many of the concepts, and a few structures # of his in the virt func module have been used @@ -719,6 +763,39 @@ def _disk_from_pool(conn, pool, pool_xml, volume_name): return disk_context +def _handle_unit(s, def_unit="m"): + """ + Handle the unit conversion, return the value in bytes + """ + m = re.match(r"(?P<value>[0-9.]*)\s*(?P<unit>.*)$", str(s).strip()) + value = m.group("value") + # default unit + unit = m.group("unit").lower() or def_unit + try: + value = int(value) + except ValueError: + try: + value = float(value) + except ValueError: + raise SaltInvocationError("invalid number") + # flag for base ten + dec = False + if re.match(r"[kmgtpezy]b$", unit): + dec = True + elif not re.match(r"(b|[kmgtpezy](ib)?)$", unit): + raise SaltInvocationError("invalid units") + p = "bkmgtpezy".index(unit[0]) + value *= 10 ** (p * 3) if dec else 2 ** (p * 10) + return int(value) + + +def nesthash(): + """ + create default dict that allows arbitrary level of nesting + """ + return collections.defaultdict(nesthash) + + def _gen_xml( conn, name, @@ -732,18 +809,32 @@ def _gen_xml( graphics=None, boot=None, boot_dev=None, + stop_on_reboot=False, **kwargs ): """ Generate the XML string to define a libvirt VM """ - mem = int(mem) * 1024 # MB context = { "hypervisor": hypervisor, "name": name, "cpu": str(cpu), - "mem": str(mem), + "on_reboot": "destroy" if stop_on_reboot else "restart", } + + context["mem"] = nesthash() + if isinstance(mem, int): + mem = int(mem) * 1024 # MB + context["mem"]["boot"] = str(mem) + context["mem"]["current"] = str(mem) + elif isinstance(mem, dict): + for tag, val in mem.items(): + if val: + if tag == "slots": + context["mem"]["slots"] = "{}='{}'".format(tag, val) + else: + context["mem"][tag] = str(int(_handle_unit(val) / 1024)) + if hypervisor in ["qemu", "kvm"]: context["controller_model"] = False elif hypervisor == "vmware": @@ -863,7 +954,6 @@ def _gen_xml( except jinja2.exceptions.TemplateNotFound: log.error("Could not load template %s", fn_) return "" - return template.render(**context) @@ -1662,6 +1752,7 @@ def init( arch=None, boot=None, boot_dev=None, + stop_on_reboot=False, **kwargs ): """ @@ -1669,7 +1760,28 @@ def init( :param name: name of the virtual machine to create :param cpu: Number of virtual CPUs to assign to the virtual machine - :param mem: Amount of memory to allocate to the virtual machine in MiB. + :param mem: Amount of memory to allocate to the virtual machine in MiB. Since Magnesium, a dictionary can be used to + contain detailed configuration which support memory allocation or tuning. Supported parameters are ``boot``, + ``current``, ``max``, ``slots``, ``hard_limit``, ``soft_limit``, ``swap_hard_limit`` and ``min_guarantee``. The + structure of the dictionary is documented in :ref:`init-mem-def`. Both decimal and binary base are supported. + Detail unit specification is documented in :ref:`virt-units`. Please note that the value for ``slots`` must be + an integer. + + .. code-block:: python + + { + 'boot': 1g, + 'current': 1g, + 'max': 1g, + 'slots': 10, + 'hard_limit': '1024' + 'soft_limit': '512m' + 'swap_hard_limit': '1g' + 'min_guarantee': '512mib' + } + + .. versionchanged:: Magnesium + :param nic: NIC profile to use (Default: ``'default'``). The profile interfaces can be customized / extended with the interfaces parameter. If set to ``None``, no profile will be used. @@ -1726,6 +1838,15 @@ def init( :param password: password to connect with, overriding defaults .. versionadded:: 2019.2.0 + + :param stop_on_reboot: + If set to ``True`` the guest will stop instead of rebooting. + This is specially useful when creating a virtual machine with an installation cdrom or + an autoinstallation needing a special first boot configuration. + Defaults to ``False`` + + .. versionadded:: Aluminium + :param boot: Specifies kernel, initial ramdisk and kernel command line parameters for the virtual machine. This is an optional parameter, all of the keys are optional within the dictionary. The structure of @@ -1782,6 +1903,36 @@ def init( .. versionadded:: sodium + .. _init-mem-def: + + .. rubric:: Memory parameter definition + + Memory parameter can contain the following properties: + + boot + The maximum allocation of memory for the guest at boot time + + current + The actual allocation of memory for the guest + + max + The run time maximum memory allocation of the guest + + slots + specifies the number of slots available for adding memory to the guest + + hard_limit + the maximum memory the guest can use + + soft_limit + memory limit to enforce during memory contention + + swap_hard_limit + the maximum memory plus swap the guest can use + + min_guarantee + the guaranteed minimum memory allocation for the guest + .. _init-nic-def: .. rubric:: Network Interfaces Definitions @@ -2076,6 +2227,7 @@ def init( graphics, boot, boot_dev, + stop_on_reboot, **kwargs ) log.debug("New virtual machine definition: %s", vm_xml) @@ -2305,6 +2457,7 @@ def update( boot=None, test=False, boot_dev=None, + stop_on_reboot=False, **kwargs ): """ @@ -2312,7 +2465,7 @@ def update( :param name: Name of the domain to update :param cpu: Number of virtual CPUs to assign to the virtual machine - :param mem: Amount of memory to allocate to the virtual machine in MiB. Since 3002, a dictionary can be used to + :param mem: Amount of memory to allocate to the virtual machine in MiB. Since Magnesium, a dictionary can be used to contain detailed configuration which support memory allocation or tuning. Supported parameters are ``boot``, ``current``, ``max``, ``slots``, ``hard_limit``, ``soft_limit``, ``swap_hard_limit`` and ``min_guarantee``. The structure of the dictionary is documented in :ref:`init-mem-def`. Both decimal and binary base are supported. @@ -2328,7 +2481,7 @@ def update( hard_limit: null soft_limit: null - .. versionchanged:: 3002 + .. versionchanged:: Magnesium :param disk_profile: disk profile to use :param disks: @@ -2386,6 +2539,14 @@ def update( .. versionadded:: Magnesium + :param stop_on_reboot: + If set to ``True`` the guest will stop instead of rebooting. + This is specially useful when creating a virtual machine with an installation cdrom or + an autoinstallation needing a special first boot configuration. + Defaults to ``False`` + + .. versionadded:: Aluminium + :param test: run in dry-run mode if set to True .. versionadded:: sodium @@ -2449,6 +2610,8 @@ def update( desc.find(".//os/type").get("arch"), graphics, boot, + boot_dev, + stop_on_reboot, **kwargs ) ) @@ -2469,12 +2632,26 @@ def update( def _set_nvram(node, value): node.set("template", value) - def _set_with_mib_unit(node, value): + def _set_with_byte_unit(node, value): node.text = str(value) - node.set("unit", "MiB") + node.set("unit", "bytes") + + def _get_with_unit(node): + unit = node.get("unit", "KiB") + # _handle_unit treats bytes as invalid unit for the purpose of consistency + unit = unit if unit != "bytes" else "b" + value = node.get("memory") or node.text + return _handle_unit("{}{}".format(value, unit)) if value else None + + old_mem = int(_get_with_unit(desc.find("memory")) / 1024) # Update the kernel boot parameters params_mapping = [ + { + "path": "stop_on_reboot", + "xpath": "on_reboot", + "convert": lambda v: "destroy" if v else "restart", + }, {"path": "boot:kernel", "xpath": "os/kernel"}, {"path": "boot:initrd", "xpath": "os/initrd"}, {"path": "boot:cmdline", "xpath": "os/cmdline"}, @@ -2484,14 +2661,72 @@ def update( { "path": "mem", "xpath": "memory", - "get": lambda n: int(n.text) / 1024, - "set": _set_with_mib_unit, + "convert": _handle_unit, + "get": _get_with_unit, + "set": _set_with_byte_unit, }, { "path": "mem", "xpath": "currentMemory", - "get": lambda n: int(n.text) / 1024, - "set": _set_with_mib_unit, + "convert": _handle_unit, + "get": _get_with_unit, + "set": _set_with_byte_unit, + }, + { + "path": "mem:max", + "convert": _handle_unit, + "xpath": "maxMemory", + "get": _get_with_unit, + "set": _set_with_byte_unit, + }, + { + "path": "mem:boot", + "convert": _handle_unit, + "xpath": "memory", + "get": _get_with_unit, + "set": _set_with_byte_unit, + }, + { + "path": "mem:current", + "convert": _handle_unit, + "xpath": "currentMemory", + "get": _get_with_unit, + "set": _set_with_byte_unit, + }, + { + "path": "mem:slots", + "xpath": "maxMemory", + "get": lambda n: n.get("slots"), + "set": lambda n, v: n.set("slots", str(v)), + "del": salt.utils.xmlutil.del_attribute("slots", ["unit"]), + }, + { + "path": "mem:hard_limit", + "convert": _handle_unit, + "xpath": "memtune/hard_limit", + "get": _get_with_unit, + "set": _set_with_byte_unit, + }, + { + "path": "mem:soft_limit", + "convert": _handle_unit, + "xpath": "memtune/soft_limit", + "get": _get_with_unit, + "set": _set_with_byte_unit, + }, + { + "path": "mem:swap_hard_limit", + "convert": _handle_unit, + "xpath": "memtune/swap_hard_limit", + "get": _get_with_unit, + "set": _set_with_byte_unit, + }, + { + "path": "mem:min_guarantee", + "convert": _handle_unit, + "xpath": "memtune/min_guarantee", + "get": _get_with_unit, + "set": _set_with_byte_unit, }, { "path": "boot_dev:{dev}", @@ -2577,13 +2812,24 @@ def update( } ) if mem: - commands.append( - { - "device": "mem", - "cmd": "setMemoryFlags", - "args": [mem * 1024, libvirt.VIR_DOMAIN_AFFECT_LIVE], - } - ) + if isinstance(mem, dict): + # setMemoryFlags takes memory amount in KiB + new_mem = ( + int(_handle_unit(mem.get("current")) / 1024) + if "current" in mem + else None + ) + elif isinstance(mem, int): + new_mem = int(mem * 1024) + + if old_mem != new_mem and new_mem is not None: + commands.append( + { + "device": "mem", + "cmd": "setMemoryFlags", + "args": [new_mem, libvirt.VIR_DOMAIN_AFFECT_LIVE], + } + ) # Look for removable device source changes new_disks = [] diff --git a/salt/states/virt.py b/salt/states/virt.py index df7ebb63e6..20ea1c25f1 100644 --- a/salt/states/virt.py +++ b/salt/states/virt.py @@ -289,6 +289,8 @@ def defined( boot=None, update=True, boot_dev=None, + stop_on_reboot=False, + live=True, ): """ Starts an existing guest, or defines and starts a new VM with specified arguments. @@ -297,7 +299,28 @@ def defined( :param name: name of the virtual machine to run :param cpu: number of CPUs for the virtual machine to create - :param mem: amount of memory in MiB for the new virtual machine + :param mem: Amount of memory to allocate to the virtual machine in MiB. Since Magnesium, a dictionary can be used to + contain detailed configuration which support memory allocation or tuning. Supported parameters are ``boot``, + ``current``, ``max``, ``slots``, ``hard_limit``, ``soft_limit``, ``swap_hard_limit`` and ``min_guarantee``. The + structure of the dictionary is documented in :ref:`init-mem-def`. Both decimal and binary base are supported. + Detail unit specification is documented in :ref:`virt-units`. Please note that the value for ``slots`` must be + an integer. + + .. code-block:: python + + { + 'boot': 1g, + 'current': 1g, + 'max': 1g, + 'slots': 10, + 'hard_limit': '1024' + 'soft_limit': '512m' + 'swap_hard_limit': '1g' + 'min_guarantee': '512mib' + } + + .. versionchanged:: Magnesium + :param vm_type: force virtual machine type for the new VM. The default value is taken from the host capabilities. This could be useful for example to use ``'qemu'`` type instead of the ``'kvm'`` one. @@ -357,6 +380,20 @@ def defined( .. versionadded:: Magnesium + :param stop_on_reboot: + If set to ``True`` the guest will stop instead of rebooting. + This is specially useful when creating a virtual machine with an installation cdrom or + an autoinstallation needing a special first boot configuration. + Defaults to ``False`` + + .. versionadded:: Aluminium + + :param live: + If set to ``False`` the changes will not be applied live to the running instance, but will + only apply at the next start. Note that reboot will not take those changes. + + .. versionadded:: Aluminium + .. rubric:: Example States Make sure a virtual machine called ``domain_name`` is defined: @@ -414,13 +451,14 @@ def defined( nic_profile=nic_profile, interfaces=interfaces, graphics=graphics, - live=True, + live=live, connection=connection, username=username, password=password, boot=boot, test=__opts__["test"], boot_dev=boot_dev, + stop_on_reboot=stop_on_reboot, ) ret["changes"][name] = status if not status.get("definition"): @@ -456,6 +494,7 @@ def defined( boot=boot, start=False, boot_dev=boot_dev, + stop_on_reboot=stop_on_reboot, ) ret["changes"][name] = {"definition": True} ret["comment"] = "Domain {} defined".format(name) @@ -489,6 +528,7 @@ def running( arch=None, boot=None, boot_dev=None, + stop_on_reboot=False, ): """ Starts an existing guest, or defines and starts a new VM with specified arguments. @@ -497,7 +537,23 @@ def running( :param name: name of the virtual machine to run :param cpu: number of CPUs for the virtual machine to create - :param mem: amount of memory in MiB for the new virtual machine + :param mem: Amount of memory to allocate to the virtual machine in MiB. Since Magnesium, a dictionary can be used to + contain detailed configuration which support memory allocation or tuning. Supported parameters are ``boot``, + ``current``, ``max``, ``slots``, ``hard_limit``, ``soft_limit``, ``swap_hard_limit`` and ``min_guarantee``. The + structure of the dictionary is documented in :ref:`init-mem-def`. Both decimal and binary base are supported. + Detail unit specification is documented in :ref:`virt-units`. Please note that the value for ``slots`` must be + an integer. + + To remove any parameters, pass a None object, for instance: 'soft_limit': ``None``. Please note that ``None`` + is mapped to ``null`` in sls file, pass ``null`` in sls file instead. + + .. code-block:: yaml + + - mem: + hard_limit: null + soft_limit: null + + .. versionchanged:: Magnesium :param vm_type: force virtual machine type for the new VM. The default value is taken from the host capabilities. This could be useful for example to use ``'qemu'`` type instead of the ``'kvm'`` one. @@ -608,6 +664,14 @@ def running( .. versionadded:: Magnesium + :param stop_on_reboot: + If set to ``True`` the guest will stop instead of rebooting. + This is specially useful when creating a virtual machine with an installation cdrom or + an autoinstallation needing a special first boot configuration. + Defaults to ``False`` + + .. versionadded:: Aluminium + .. rubric:: Example States Make sure an already-defined virtual machine called ``domain_name`` is running: @@ -676,6 +740,7 @@ def running( boot=boot, update=update, boot_dev=boot_dev, + stop_on_reboot=stop_on_reboot, connection=connection, username=username, password=password, diff --git a/salt/templates/virt/libvirt_domain.jinja b/salt/templates/virt/libvirt_domain.jinja index 18728a75b5..fb4c9f40d0 100644 --- a/salt/templates/virt/libvirt_domain.jinja +++ b/salt/templates/virt/libvirt_domain.jinja @@ -2,9 +2,32 @@ <domain type='{{ hypervisor }}'> <name>{{ name }}</name> <vcpu>{{ cpu }}</vcpu> - <memory unit='KiB'>{{ mem }}</memory> - <currentMemory unit='KiB'>{{ mem }}</currentMemory> - <os {{boot.os_attrib}}> + {%- 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 }}> <type arch='{{ arch }}'>{{ os_type }}</type> {% if boot %} {% if 'kernel' in boot %} @@ -27,6 +50,7 @@ <boot dev='{{ dev }}' /> {% endfor %} </os> + <on_reboot>{{ on_reboot }}</on_reboot> <devices> {% for disk in disks %} <disk type='{{ disk.type }}' device='{{ disk.device }}'> diff --git a/salt/utils/xmlutil.py b/salt/utils/xmlutil.py index 111ca155d4..d25f5c8da5 100644 --- a/salt/utils/xmlutil.py +++ b/salt/utils/xmlutil.py @@ -299,7 +299,7 @@ def change_xml(doc, data, mapping): if convert_fn: new_value = convert_fn(new_value) - if current_value != new_value: + if str(current_value) != str(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 e214e406e2..fba821ea53 100644 --- a/tests/unit/modules/test_virt.py +++ b/tests/unit/modules/test_virt.py @@ -21,7 +21,6 @@ from salt.ext import six # pylint: disable=import-error from salt.ext.six.moves import range # pylint: disable=redefined-builtin -from tests.support.helpers import dedent from tests.support.mixins import LoaderModuleMockMixin from tests.support.mock import MagicMock, patch from tests.support.unit import TestCase @@ -1856,6 +1855,25 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): virt.update("my_vm"), ) + # mem + cpu case + define_mock.reset_mock() + domain_mock.setMemoryFlags.return_value = 0 + domain_mock.setVcpusFlags.return_value = 0 + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + "mem": True, + "cpu": True, + }, + virt.update("my_vm", mem=2048, cpu=2), + ) + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual("2", setxml.find("vcpu").text) + self.assertEqual("2147483648", setxml.find("memory").text) + self.assertEqual(2048 * 1024, domain_mock.setMemoryFlags.call_args[0][0]) + # Same parameters passed than in default virt.defined state case self.assertEqual( { @@ -2001,6 +2019,50 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): with self.assertRaises(SaltInvocationError): virt.update("my_vm", boot={"efi": "Not a boolean value"}) + # Update memtune parameter case + memtune = { + "soft_limit": "0.5g", + "hard_limit": "1024", + "swap_hard_limit": "2048m", + "min_guarantee": "1 g", + } + + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("my_vm", mem=memtune), + ) + + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual( + setxml.find("memtune").find("soft_limit").text, str(int(0.5 * 1024 ** 3)) + ) + self.assertEqual(setxml.find("memtune").find("soft_limit").get("unit"), "bytes") + self.assertEqual( + setxml.find("memtune").find("hard_limit").text, str(1024 * 1024 ** 2) + ) + self.assertEqual( + setxml.find("memtune").find("swap_hard_limit").text, str(2048 * 1024 ** 2) + ) + self.assertEqual( + setxml.find("memtune").find("min_guarantee").text, str(1 * 1024 ** 3) + ) + + invalid_unit = {"soft_limit": "2HB"} + + with self.assertRaises(SaltInvocationError): + virt.update("my_vm", mem=invalid_unit) + + invalid_number = { + "soft_limit": "3.4.MB", + } + + with self.assertRaises(SaltInvocationError): + virt.update("my_vm", mem=invalid_number) + # Update memory case setmem_mock = MagicMock(return_value=0) domain_mock.setMemoryFlags = setmem_mock @@ -2015,10 +2077,43 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): virt.update("my_vm", mem=2048), ) setxml = ET.fromstring(define_mock.call_args[0][0]) - self.assertEqual(setxml.find("memory").text, "2048") - self.assertEqual(setxml.find("memory").get("unit"), "MiB") + self.assertEqual(setxml.find("memory").text, str(2048 * 1024 ** 2)) + self.assertEqual(setxml.find("memory").get("unit"), "bytes") self.assertEqual(setmem_mock.call_args[0][0], 2048 * 1024) + mem_dict = {"boot": "0.5g", "current": "2g", "max": "1g", "slots": 12} + self.assertEqual( + { + "definition": True, + "mem": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("my_vm", mem=mem_dict), + ) + + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual(setxml.find("memory").get("unit"), "bytes") + self.assertEqual(setxml.find("memory").text, str(int(0.5 * 1024 ** 3))) + self.assertEqual(setxml.find("maxMemory").text, str(1 * 1024 ** 3)) + self.assertEqual(setxml.find("currentMemory").text, str(2 * 1024 ** 3)) + + max_slot_reverse = { + "slots": "10", + "max": "3096m", + } + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("my_vm", mem=max_slot_reverse), + ) + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual(setxml.find("maxMemory").text, str(3096 * 1024 ** 2)) + self.assertEqual(setxml.find("maxMemory").attrib.get("slots"), "10") + # Update disks case devattach_mock = MagicMock(return_value=0) devdetach_mock = MagicMock(return_value=0) @@ -2533,7 +2628,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): """ Test virt.update() with existing boot parameters. """ - root_dir = os.path.join(salt.syspaths.ROOT_DIR, "srv", "salt-images") xml_boot = """ <domain type='kvm' id='8'> <name>vm_with_boot_param</name> @@ -2591,9 +2685,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): </video> </devices> </domain> - """.format( - root_dir, os.sep - ) + """ domain_mock_boot = self.set_mock_vm("vm_with_boot_param", xml_boot) domain_mock_boot.OSType = MagicMock(return_value="hvm") define_mock_boot = MagicMock(return_value=True) @@ -2694,6 +2786,218 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin): self.assertEqual(setxml.find("os").find("loader"), None) self.assertEqual(setxml.find("os").find("nvram"), None) + def test_update_memtune_params(self): + """ + Test virt.update() with memory tuning parameters. + """ + xml_with_memtune_params = """ + <domain type='kvm' id='8'> + <name>vm_with_boot_param</name> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <maxMemory slots="12" unit="bytes">1048576</maxMemory> + <vcpu placement='auto'>1</vcpu> + <memtune> + <hard_limit unit="KiB">1048576</hard_limit> + <soft_limit unit="KiB">2097152</soft_limit> + <swap_hard_limit unit="KiB">2621440</swap_hard_limit> + <min_guarantee unit='KiB'>671088</min_guarantee> + </memtune> + <os> + <type arch='x86_64' machine='pc-i440fx-2.6'>hvm</type> + </os> + </domain> + """ + domain_mock = self.set_mock_vm("vm_with_memtune_param", xml_with_memtune_params) + domain_mock.OSType = MagicMock(return_value="hvm") + define_mock = MagicMock(return_value=True) + self.mock_conn.defineXML = define_mock + + memtune_new_val = { + "boot": "0.7g", + "current": "2.5g", + "max": "3096m", + "slots": "10", + "soft_limit": "2048m", + "hard_limit": "1024", + "swap_hard_limit": "2.5g", + "min_guarantee": "1 g", + } + + domain_mock.setMemoryFlags.return_value = 0 + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + "mem": True, + }, + virt.update("vm_with_memtune_param", mem=memtune_new_val), + ) + self.assertEqual( + domain_mock.setMemoryFlags.call_args[0][0], int(2.5 * 1024 ** 2) + ) + + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual( + setxml.find("memtune").find("soft_limit").text, str(2048 * 1024) + ) + self.assertEqual( + setxml.find("memtune").find("hard_limit").text, str(1024 * 1024) + ) + self.assertEqual( + setxml.find("memtune").find("swap_hard_limit").text, + str(int(2.5 * 1024 ** 2)), + ) + self.assertEqual( + setxml.find("memtune").find("swap_hard_limit").get("unit"), "KiB", + ) + self.assertEqual( + setxml.find("memtune").find("min_guarantee").text, str(1 * 1024 ** 3) + ) + self.assertEqual( + setxml.find("memtune").find("min_guarantee").attrib.get("unit"), "bytes" + ) + self.assertEqual(setxml.find("maxMemory").text, str(3096 * 1024 ** 2)) + self.assertEqual(setxml.find("maxMemory").attrib.get("slots"), "10") + self.assertEqual(setxml.find("currentMemory").text, str(int(2.5 * 1024 ** 3))) + self.assertEqual(setxml.find("memory").text, str(int(0.7 * 1024 ** 3))) + + max_slot_reverse = { + "slots": "10", + "max": "3096m", + } + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("vm_with_memtune_param", mem=max_slot_reverse), + ) + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual(setxml.find("maxMemory").text, str(3096 * 1024 ** 2)) + self.assertEqual(setxml.find("maxMemory").get("unit"), "bytes") + self.assertEqual(setxml.find("maxMemory").attrib.get("slots"), "10") + + max_swap_none = { + "boot": "0.7g", + "current": "2.5g", + "max": None, + "slots": "10", + "soft_limit": "2048m", + "hard_limit": "1024", + "swap_hard_limit": None, + "min_guarantee": "1 g", + } + + domain_mock.setMemoryFlags.reset_mock() + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + "mem": True, + }, + virt.update("vm_with_memtune_param", mem=max_swap_none), + ) + self.assertEqual( + domain_mock.setMemoryFlags.call_args[0][0], int(2.5 * 1024 ** 2) + ) + + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual( + setxml.find("memtune").find("soft_limit").text, str(2048 * 1024) + ) + self.assertEqual( + setxml.find("memtune").find("hard_limit").text, str(1024 * 1024) + ) + self.assertEqual(setxml.find("memtune").find("swap_hard_limit"), None) + self.assertEqual( + setxml.find("memtune").find("min_guarantee").text, str(1 * 1024 ** 3) + ) + self.assertEqual( + setxml.find("memtune").find("min_guarantee").attrib.get("unit"), "bytes" + ) + self.assertEqual(setxml.find("maxMemory").text, None) + self.assertEqual(setxml.find("currentMemory").text, str(int(2.5 * 1024 ** 3))) + self.assertEqual(setxml.find("memory").text, str(int(0.7 * 1024 ** 3))) + + memtune_none = { + "soft_limit": None, + "hard_limit": None, + "swap_hard_limit": None, + "min_guarantee": None, + } + + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("vm_with_memtune_param", mem=memtune_none), + ) + + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual(setxml.find("memtune").find("soft_limit"), None) + self.assertEqual(setxml.find("memtune").find("hard_limit"), None) + self.assertEqual(setxml.find("memtune").find("swap_hard_limit"), None) + self.assertEqual(setxml.find("memtune").find("min_guarantee"), None) + + max_none = { + "max": None, + } + + self.assertEqual( + { + "definition": True, + "disk": {"attached": [], "detached": [], "updated": []}, + "interface": {"attached": [], "detached": []}, + }, + virt.update("vm_with_memtune_param", mem=max_none), + ) + + setxml = ET.fromstring(define_mock.call_args[0][0]) + self.assertEqual(setxml.find("maxMemory"), None) + self.assertEqual(setxml.find("currentMemory").text, str(int(1 * 1024 ** 2))) + self.assertEqual(setxml.find("memory").text, str(int(1 * 1024 ** 2))) + + def test_handle_unit(self): + """ + Test regex function for handling units + """ + valid_case = [ + ("2", 2097152), + ("42", 44040192), + ("5b", 5), + ("2.3Kib", 2355), + ("5.8Kb", 5800), + ("16MiB", 16777216), + ("20 GB", 20000000000), + ("16KB", 16000), + (".5k", 512), + ("2.k", 2048), + ] + + for key, val in valid_case: + self.assertEqual(virt._handle_unit(key), val) + + invalid_case = [ + ("9ib", "invalid units"), + ("8byte", "invalid units"), + ("512bytes", "invalid units"), + ("4 Kbytes", "invalid units"), + ("3.4.MB", "invalid number"), + ("", "invalid number"), + ("bytes", "invalid number"), + ("2HB", "invalid units"), + ] + + for key, val in invalid_case: + with self.assertRaises(SaltInvocationError): + virt._handle_unit(key) + def test_mixed_dict_and_list_as_profile_objects(self): """ Test virt._nic_profile with mixed dictionaries and lists as input. diff --git a/tests/unit/states/test_virt.py b/tests/unit/states/test_virt.py index 8fe892f607..1923ae5c0f 100644 --- a/tests/unit/states/test_virt.py +++ b/tests/unit/states/test_virt.py @@ -8,7 +8,6 @@ import tempfile import salt.states.virt as virt import salt.utils.files from salt.exceptions import CommandExecutionError, SaltInvocationError -from salt.ext import six from tests.support.mixins import LoaderModuleMockMixin from tests.support.mock import MagicMock, mock_open, patch from tests.support.runtests import RUNTIME_VARS @@ -346,6 +345,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): install=False, pub_key="/path/to/key.pub", priv_key="/path/to/key", + stop_on_reboot=True, connection="someconnection", username="libvirtuser", password="supersecret", @@ -371,6 +371,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): start=False, pub_key="/path/to/key.pub", priv_key="/path/to/key", + stop_on_reboot=True, connection="someconnection", username="libvirtuser", password="supersecret", @@ -484,6 +485,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): password=None, boot=None, test=False, + stop_on_reboot=False, ) # Failed definition update case @@ -554,6 +556,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): install=False, pub_key="/path/to/key.pub", priv_key="/path/to/key", + stop_on_reboot=False, connection="someconnection", username="libvirtuser", password="supersecret", @@ -596,6 +599,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): boot=None, test=True, boot_dev=None, + stop_on_reboot=False, ) # No changes case @@ -631,6 +635,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): boot=None, test=True, boot_dev=None, + stop_on_reboot=False, ) def test_running(self): @@ -708,6 +713,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): pub_key=None, priv_key=None, boot_dev=None, + stop_on_reboot=False, connection=None, username=None, password=None, @@ -770,6 +776,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): pub_key="/path/to/key.pub", priv_key="/path/to/key", boot_dev="network hd", + stop_on_reboot=True, connection="someconnection", username="libvirtuser", password="supersecret", @@ -795,6 +802,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): pub_key="/path/to/key.pub", priv_key="/path/to/key", boot_dev="network hd", + stop_on_reboot=True, connection="someconnection", username="libvirtuser", password="supersecret", @@ -940,6 +948,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): boot=None, test=False, boot_dev=None, + stop_on_reboot=False, ) # Failed definition update case @@ -1013,6 +1022,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): install=False, pub_key="/path/to/key.pub", priv_key="/path/to/key", + stop_on_reboot=True, connection="someconnection", username="libvirtuser", password="supersecret", @@ -1059,6 +1069,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): boot=None, test=True, boot_dev=None, + stop_on_reboot=False, ) start_mock.assert_not_called() @@ -1096,6 +1107,7 @@ class LibvirtTestCase(TestCase, LoaderModuleMockMixin): boot=None, test=True, boot_dev=None, + stop_on_reboot=False, ) def test_stopped(self): -- 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