diff --git a/ansible/dump-config.yml b/ansible/dump-config.yml index f740a063a..348d73601 100644 --- a/ansible/dump-config.yml +++ b/ansible/dump-config.yml @@ -28,14 +28,18 @@ - name: Write host config to file delegate_to: localhost copy: - content: "{{ hostvars[inventory_hostname] | to_nice_yaml }}" + content: >- + {{ hostvars[inventory_hostname] | kayobe_config_dump + | to_nice_yaml }} dest: "{{ dump_path }}/{{ inventory_hostname }}.yml" when: dump_var_name is not defined - name: Write host variable to file delegate_to: localhost copy: - content: "{{ hostvars[inventory_hostname][dump_var_name] | to_nice_yaml }}" + content: >- + {{ hostvars[inventory_hostname] + | kayobe_config_dump(dump_var_name) | to_nice_yaml }} dest: "{{ dump_path }}/{{ inventory_hostname }}.yml" when: dump_var_name is defined diff --git a/ansible/filter_plugins/config_dump.py b/ansible/filter_plugins/config_dump.py new file mode 100644 index 000000000..b0f701123 --- /dev/null +++ b/ansible/filter_plugins/config_dump.py @@ -0,0 +1,22 @@ +# Copyright (c) 2026 StackHPC Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from kayobe.plugins.filter import config_dump + + +class FilterModule(object): + """Configuration dump filters.""" + + def filters(self): + return config_dump.get_filters() diff --git a/ansible/inventory/group_vars/all/globals b/ansible/inventory/group_vars/all/globals index 715fe2758..085ef0e10 100644 --- a/ansible/inventory/group_vars/all/globals +++ b/ansible/inventory/group_vars/all/globals @@ -93,3 +93,8 @@ kayobe_ansible_setup_gather_subset: "{{ omit }}" # This allows us to install packages and create arbitrary directories that our # user would not normally have permission to create. Default is true. kayobe_control_host_become: true + +# Global variable to choose whether or not to reboot hosts when required by +# host configuration playbooks, including VGPU and SELinux playbooks. Default +# is false. +kayobe_do_reboot: false diff --git a/ansible/inventory/group_vars/all/vgpu b/ansible/inventory/group_vars/all/vgpu index 43c828fc0..ffc7cc5e3 100644 --- a/ansible/inventory/group_vars/all/vgpu +++ b/ansible/inventory/group_vars/all/vgpu @@ -10,7 +10,7 @@ vgpu_driver_url: "" # Flag to control whether the vGPU playbook should automatically reboot the # hypervisor. Note: this is necessary for the driver to be loaded correctly. # Caution should be used when changing this option. -vgpu_do_reboot: true +vgpu_do_reboot: "{{ kayobe_do_reboot | bool }}" # Time to wait when rebooting the host before failing. vgpu_reboot_timeout: 600 diff --git a/ansible/roles/kolla-ansible/defaults/main.yml b/ansible/roles/kolla-ansible/defaults/main.yml index 0624d626e..ad5b08b89 100644 --- a/ansible/roles/kolla-ansible/defaults/main.yml +++ b/ansible/roles/kolla-ansible/defaults/main.yml @@ -26,7 +26,7 @@ kolla_ansible_venv_extra_requirements: [] # tested code. Changes to this limit should be tested. It is possible to only # install ansible-core by setting kolla_ansible_venv_ansible to None. kolla_ansible_venv_ansible: -kolla_ansible_venv_ansible_core: 'ansible-core>=2.19,<2.21' +kolla_ansible_venv_ansible_core: 'ansible-core>=2.20,<2.21' # Path to a requirements.yml file for Ansible collections. kolla_ansible_requirements_yml: "{{ kolla_ansible_venv }}/share/kolla-ansible/requirements.yml" diff --git a/ansible/roles/kolla-openstack/molecule/default/create.yml b/ansible/roles/kolla-openstack/molecule/default/create.yml index 64257aa47..574ce1f0f 100644 --- a/ansible/roles/kolla-openstack/molecule/default/create.yml +++ b/ansible/roles/kolla-openstack/molecule/default/create.yml @@ -23,15 +23,12 @@ register: docker_images - name: Build an Ansible compatible image - docker_image: + community.docker.docker_image_build: name: "molecule_local/{{ item.item.image }}" - source: build - build: - path: "{{ molecule_ephemeral_directory }}" - dockerfile: "{{ item.item.dockerfile | default(item.invocation.module_args.dest) }}" - network: host - force_source: "{{ item.item.force | default(true) }}" - force_tag: "{{ item.item.force | default(true) }}" + path: "{{ molecule_ephemeral_directory }}" + dockerfile: "{{ item.item.dockerfile | default(item.dest) }}" + network: host + rebuild: "{{ 'always' if item.item.force | default(true) else 'never' }}" with_items: "{{ platforms.results }}" when: platforms.changed or docker_images.results | map(attribute='images') | select('equalto', []) | list | count >= 0 diff --git a/ansible/roles/kolla-openstack/molecule/enable-everything/create.yml b/ansible/roles/kolla-openstack/molecule/enable-everything/create.yml index bbb81a13f..2b476b09b 100644 --- a/ansible/roles/kolla-openstack/molecule/enable-everything/create.yml +++ b/ansible/roles/kolla-openstack/molecule/enable-everything/create.yml @@ -24,15 +24,12 @@ register: docker_images - name: Build an Ansible compatible image - docker_image: + community.docker.docker_image_build: name: "molecule_local/{{ item.item.image }}" - source: build - build: - path: "{{ molecule_ephemeral_directory }}" - dockerfile: "{{ item.item.dockerfile | default(item.invocation.module_args.dest) }}" - network: host - force_source: "{{ item.item.force | default(true) }}" - force_tag: "{{ item.item.force | default(true) }}" + path: "{{ molecule_ephemeral_directory }}" + dockerfile: "{{ item.item.dockerfile | default(item.dest) }}" + network: host + rebuild: "{{ 'always' if item.item.force | default(true) else 'never' }}" with_items: "{{ platforms.results }}" when: platforms.changed or docker_images.results | map(attribute='images') | select('equalto', []) | list | count >= 0 diff --git a/ansible/roles/network-nmstate/library/nmstate_apply.py b/ansible/roles/network-nmstate/library/nmstate_apply.py index f04b89399..a932a9386 100644 --- a/ansible/roles/network-nmstate/library/nmstate_apply.py +++ b/ansible/roles/network-nmstate/library/nmstate_apply.py @@ -14,6 +14,7 @@ # under the License. import importlib +import json from ansible.module_utils.basic import AnsibleModule @@ -25,7 +26,8 @@ short_description: Apply network state using nmstate description: - "This module allows applying a network state using nmstate library. - Provides idempotency by comparing desired and current states." + Provides idempotency by comparing desired and current states. + Supports check and diff modes." options: state: description: @@ -39,7 +41,7 @@ default: False type: bool requirements: - - libnmstate + - libnmstate (nmstate 2.x) """ EXAMPLES = """ @@ -66,7 +68,15 @@ state: description: Current network state after applying desired state type: dict - returned: always + returned: when not in check mode +differences: + description: Computed differences between the current and desired states + type: dict + returned: when changed +diff: + description: Prepared diff of the computed differences + type: dict + returned: when changed previous_state: description: Network state before applying (when debug=true) type: dict @@ -78,6 +88,14 @@ """ +def _is_empty(value): + if isinstance(value, dict): + return all(_is_empty(item) for item in value.values()) + if isinstance(value, (list, tuple)): + return all(_is_empty(item) for item in value) + return value is None + + def run_module(): argument_spec = dict( state=dict(required=True, type="dict"), @@ -86,7 +104,7 @@ def run_module(): module = AnsibleModule( argument_spec=argument_spec, - supports_check_mode=False, + supports_check_mode=True, ) try: @@ -101,26 +119,54 @@ def run_module(): ) % repr(e) ) - previous_state = libnmstate.show() + if not hasattr(libnmstate, "generate_differences"): + module.fail_json( + msg=( + "The installed libnmstate does not provide " + "generate_differences(). The nmstate_apply module requires " + "nmstate 2.x (for example python3-libnmstate 2.x)." + ) + ) + + current_state = libnmstate.show() desired_state = module.params["state"] debug = module.params["debug"] - result = {"changed": False} - + differences = libnmstate.generate_differences( + desired_state, current_state) + changed = not _is_empty(differences) + + result = {"changed": changed} + if changed: + # "prepared" is a special Ansible diff key whose content is + # printed as-is, keeping --diff output to the changed properties. + if module._diff: + result["diff"] = { + "prepared": json.dumps( + differences, indent=1, sort_keys=True), + } + result["differences"] = differences + + if debug: + result["previous_state"] = current_state + result["desired_state"] = desired_state + + if module.check_mode: + module.exit_json(**result) + return + + # generate_differences() compares against the runtime state, not + # the config persisted by NetworkManager, so apply unconditionally + # to re-persist the config and prevent drift after a reboot. try: libnmstate.apply(desired_state) except Exception as e: - module.fail_json(msg="Failed to apply nmstate state: %s" % repr(e)) - - current_state = libnmstate.show() - - if current_state != previous_state: - result["changed"] = True - if debug: - result["previous_state"] = previous_state - result["desired_state"] = desired_state + # A failed apply must not claim changed: nmstate verifies the + # applied state and rolls back on failure. + module.fail_json(msg="Failed to apply nmstate state: %s" % repr(e), + differences=differences) - result["state"] = current_state + result["state"] = libnmstate.show() module.exit_json(**result) diff --git a/ansible/roles/network-nmstate/tasks/main.yml b/ansible/roles/network-nmstate/tasks/main.yml index 3a3732ae3..cd862a4fa 100644 --- a/ansible/roles/network-nmstate/tasks/main.yml +++ b/ansible/roles/network-nmstate/tasks/main.yml @@ -50,6 +50,7 @@ name: "{{ network_nmstate_packages }}" state: present become: true + register: network_nmstate_package_result when: - network_nmstate_install_packages | bool - network_nmstate_packages | length > 0 @@ -97,6 +98,9 @@ become: true vars: ansible_python_interpreter: "{{ ansible_facts.python.executable }}" + when: >- + not (ansible_check_mode and + network_nmstate_package_result is changed) - name: Initialise nmstate firewalld interface-zone map set_fact: @@ -132,31 +136,72 @@ # TODO(gkoper): replace temporary nmcli profile zone sync with native # zone handling in the nmstate filter/module path. - - name: Gather NetworkManager connection firewalld zones for nmstate interfaces - command: - argv: - - nmcli - - -g - - connection.zone - - connection - - show - - "{{ item.interface }}" - changed_when: false - loop: "{{ network_nmstate_zone_items }}" - register: network_nmstate_nm_zone_result - - - name: Ensure NetworkManager connection firewalld zones are set for nmstate interfaces - command: - argv: - - nmcli - - connection - - modify - - "{{ item.item.interface }}" - - connection.zone - - "{{ item.item.zone }}" - loop: "{{ network_nmstate_nm_zone_result.results }}" - when: - - (item.stdout | default('') | trim) != item.item.zone + - name: Sync NetworkManager connection firewalld zones for nmstate interfaces + block: + # Cloud-init can create a profile whose ID is prefixed with + # "cloud-init", while its active device is still the nmstate interface. + # `nmcli connection show` accepts a profile UUID, so get the active + # profile UUID from the device before reading or updating its zone. + - name: Gather NetworkManager connection profiles for nmstate interfaces + command: + argv: + - nmcli + - -g + - GENERAL.CON-UUID + - device + - show + - "{{ zone_item.interface }}" + changed_when: false + loop: "{{ network_nmstate_zone_items }}" + loop_control: + loop_var: zone_item + label: "{{ zone_item.interface }}" + register: network_nmstate_nm_connection_result + + - name: Validate NetworkManager connections for nmstate interfaces + ansible.builtin.assert: + that: + - (connection_item.stdout | trim) not in ['', '--'] + fail_msg: >- + No active NetworkManager connection found for nmstate interface + {{ connection_item.zone_item.interface }} + loop: "{{ network_nmstate_nm_connection_result.results }}" + loop_control: + loop_var: connection_item + label: "{{ connection_item.zone_item.interface }}" + + - name: Gather NetworkManager connection firewalld zones for nmstate interfaces + command: + argv: + - nmcli + - -g + - connection.zone + - connection + - show + - "{{ connection_item.stdout | trim }}" + changed_when: false + loop: "{{ network_nmstate_nm_connection_result.results }}" + loop_control: + loop_var: connection_item + label: "{{ connection_item.zone_item.interface }}" + register: network_nmstate_nm_zone_result + + - name: Ensure NetworkManager connection firewalld zones are set for nmstate interfaces + command: + argv: + - nmcli + - connection + - modify + - "{{ zone_result.connection_item.stdout | trim }}" + - connection.zone + - "{{ zone_result.connection_item.zone_item.zone }}" + loop: "{{ network_nmstate_nm_zone_result.results }}" + loop_control: + loop_var: zone_result + label: "{{ zone_result.connection_item.zone_item.interface }}" + when: + - (zone_result.stdout | default('') | trim) != zone_result.connection_item.zone_item.zone + when: not ansible_check_mode # Keep permanent firewalld configuration in sync first. Runtime state is # refreshed separately below from permanent config. diff --git a/ansible/roles/selinux/defaults/main.yml b/ansible/roles/selinux/defaults/main.yml index 8966e67f3..347dabd61 100644 --- a/ansible/roles/selinux/defaults/main.yml +++ b/ansible/roles/selinux/defaults/main.yml @@ -6,7 +6,7 @@ selinux_policy: targeted selinux_state: permissive # Whether to reboot to apply SELinux config changes. -selinux_do_reboot: false +selinux_do_reboot: "{{ kayobe_do_reboot | bool }}" # Number of seconds to wait for hosts to become accessible via SSH after being # rebooted. diff --git a/ansible/vgpu.yml b/ansible/vgpu.yml index 58a424eb7..9db39df3e 100644 --- a/ansible/vgpu.yml +++ b/ansible/vgpu.yml @@ -1,4 +1,24 @@ --- +- name: Precheck that reboot is enabled for VGPU configuration + hosts: iommu:vgpu + max_fail_percentage: >- + {{ vgpu_max_fail_percentage | + default(host_configure_max_fail_percentage) | + default(kayobe_max_fail_percentage) | + default(100) }} + tags: + - reboot + - iommu + - vgpu + tasks: + - name: Abort VGPU configuration because reboot is disabled + ansible.builtin.fail: + msg: > + VGPU configuration requires a reboot, but vgpu_do_reboot is + false. Please run again with vgpu_do_reboot set to true to reboot. + when: + - not vgpu_do_reboot | bool + - name: Configure IOMMU hosts: iommu max_fail_percentage: >- @@ -12,6 +32,8 @@ tasks: - import_role: name: stackhpc.linux.iommu + vars: + iommu_do_reboot: "{{ vgpu_do_reboot | bool }}" handlers: - name: Register that a reboot is required set_fact: diff --git a/doc/source/configuration/reference/network.rst b/doc/source/configuration/reference/network.rst index 4c0fe56e7..32d259557 100644 --- a/doc/source/configuration/reference/network.rst +++ b/doc/source/configuration/reference/network.rst @@ -139,6 +139,13 @@ Set the engine in ``${KAYOBE_CONFIG_PATH}/globals.yml``: The nmstate engine is only supported on Rocky Linux. For Ubuntu Noble, use the ``default`` engine (default). +.. note:: + + The nmstate engine supports Ansible check and diff modes. Running for + example ``kayobe overcloud host configure --check --diff`` reports + whether network changes are required and shows the proposed changes + without modifying the host. + Nmstate Engine Features ------------------------- diff --git a/etc/kayobe/globals.yml b/etc/kayobe/globals.yml index c17ec7c27..216bc21e7 100644 --- a/etc/kayobe/globals.yml +++ b/etc/kayobe/globals.yml @@ -85,6 +85,11 @@ # user would not normally have permission to create. Default is true. #kayobe_control_host_become: +# Global variable to choose whether or not to reboot hosts when required by +# host configuration playbooks, including VGPU and SELinux playbooks. Default +# is false. +#kayobe_do_reboot: + ############################################################################### # Networking configuration. diff --git a/kayobe/plugins/action/merge_configs.py b/kayobe/plugins/action/merge_configs.py index 0a93fbfe7..a75ba18c7 100644 --- a/kayobe/plugins/action/merge_configs.py +++ b/kayobe/plugins/action/merge_configs.py @@ -221,9 +221,10 @@ def run(self, tmp=None, task_vars=None): templar=self._templar, shared_loader_obj=self._shared_loader_obj) copy_result = copy_action.run(task_vars=task_vars) - copy_result['invocation']['module_args'].update({ - 'src': result_file, 'sources': sources, - 'whitespace': whitespace}) + if 'invocation' in copy_result: + copy_result['invocation']['module_args'].update({ + 'src': result_file, 'sources': sources, + 'whitespace': whitespace}) result.update(copy_result) finally: shutil.rmtree(local_tempdir) diff --git a/kayobe/plugins/action/merge_yaml.py b/kayobe/plugins/action/merge_yaml.py index c35b182b4..3d122232f 100644 --- a/kayobe/plugins/action/merge_yaml.py +++ b/kayobe/plugins/action/merge_yaml.py @@ -165,9 +165,10 @@ def run(self, tmp=None, task_vars=None): templar=self._templar, shared_loader_obj=self._shared_loader_obj) copy_result = copy_action.run(task_vars=task_vars) - copy_result['invocation']['module_args'].update({ - 'src': result_file, 'sources': sources, - 'extend_lists': extend_lists}) + if 'invocation' in copy_result: + copy_result['invocation']['module_args'].update({ + 'src': result_file, 'sources': sources, + 'extend_lists': extend_lists}) result.update(copy_result) finally: shutil.rmtree(local_tempdir) diff --git a/kayobe/plugins/filter/config_dump.py b/kayobe/plugins/filter/config_dump.py new file mode 100644 index 000000000..2a939acce --- /dev/null +++ b/kayobe/plugins/filter/config_dump.py @@ -0,0 +1,122 @@ +# Copyright (c) 2026 StackHPC Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import jinja2 + +from ansible import errors +from ansible.parsing.dataloader import DataLoader +from ansible.template import Templar +from ansible.utils.display import Display + +try: + from ansible.module_utils._internal._datatag._tags import Deprecated +except ImportError: # pragma: no cover + # Older ansible-core versions do not tag top-level facts as deprecated. + Deprecated = None + +display = Display() + + +def _make_serializable(value): + """Recursively convert values that cannot be serialized to YAML. + + :param value: value to convert. + :returns: a value that can be serialized to YAML. + """ + if isinstance(value, dict): + return {k: _make_serializable(v) for k, v in value.items()} + if isinstance(value, (list, tuple)): + return [_make_serializable(v) for v in value] + if value is None or isinstance(value, (str, bytes, int, float, bool)): + return value + return str(value) + + +def _resolve(templar, name, value): + """Resolve a raw Ansible variable in the current templating context. + + Top-level facts are tagged as deprecated by Ansible. They are already + resolved, but may have been overridden by a user-provided template, so + attempt to resolve them. If that fails, emit a warning and preserve the + raw value. + + :param templar: an Ansible Templar with the current context's variables. + :param name: name of the variable being resolved. + :param value: raw variable value to resolve. + :returns: the resolved value, or the raw value as a string if it could not + be resolved, for example because it references an undefined + variable. + """ + if Deprecated is not None and Deprecated.is_tagged_on(value): + value = Deprecated.untag(value) + try: + return templar.template(value, fail_on_undefined=False) + except Exception: + display.warning( + "Failed to resolve variable '%s'. It references a deprecated " + "top-level fact; use ansible_facts instead." % name) + return value + try: + return templar.template(value, fail_on_undefined=False) + except Exception: + # Preserve variables that cannot be resolved, such as those that + # reference omitted values. Casting to a plain string ensures that the + # value is not templated again by the caller. + return "%s" % value + + +@jinja2.pass_context +def kayobe_config_dump(context, hostvars, var_name=None): + """Return the resolved Ansible variables for a host. + + This is used by the ``kayobe configuration dump`` command. It is not + possible to simply serialize the result of ``hostvars[host]`` because in + ansible-core >= 2.19 ``HostVarsVars`` templates variables in a context that + does not include the ``hostvars`` variable, and fails on undefined + variables. Instead, use the raw variables and template them in the current + context. + + :param context: a Jinja2 Context object. + :param hostvars: Ansible host variables for a host, typically + ``hostvars[inventory_hostname]``. + :param var_name: optional name of a single variable to return. If not + specified, a dict of all variables is returned. + :returns: a dict mapping variable names to resolved values, or a single + resolved value if ``var_name`` is specified. + :raises: ansible.errors.AnsibleFilterError + """ + templar = Templar(loader=DataLoader(), variables=context.get_all()) + # HostVarsVars exposes its raw variables via _vars. There is no public API + # for accessing them without templating. + variables = getattr(hostvars, '_vars', hostvars) + + if var_name is not None: + if var_name not in variables: + inventory_hostname = context.get('inventory_hostname') + raise errors.AnsibleFilterError( + "Variable '%s' not found for host '%s'" % + (var_name, inventory_hostname)) + return _make_serializable( + _resolve(templar, var_name, variables[var_name])) + + return { + name: _make_serializable(_resolve(templar, name, value)) + for name, value in variables.items() + } + + +def get_filters(): + return { + 'kayobe_config_dump': kayobe_config_dump, + } diff --git a/kayobe/tests/unit/plugins/filter/test_config_dump.py b/kayobe/tests/unit/plugins/filter/test_config_dump.py new file mode 100644 index 000000000..e786f72c0 --- /dev/null +++ b/kayobe/tests/unit/plugins/filter/test_config_dump.py @@ -0,0 +1,147 @@ +# Copyright (c) 2026 StackHPC Ltd. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import unittest +from unittest import mock + +import jinja2 + +from ansible._internal._datatag._tags import TrustedAsTemplate +from ansible import errors +from ansible.module_utils._internal._datatag._tags import Deprecated +from ansible.module_utils._internal._datatag import AnsibleTagHelper +from ansible.parsing.dataloader import DataLoader +from kayobe.plugins.filter import config_dump + + +class FakeHostVarsVars(object): + """A minimal stand-in for Ansible's HostVarsVars.""" + + def __init__(self, variables): + self._vars = variables + + +def _trust(value): + """Mark a variable as a trusted template, as the inventory does.""" + if isinstance(value, str): + return AnsibleTagHelper.tag(value, TrustedAsTemplate()) + return value + + +def _deprecate(value): + """Mark a value as deprecated, as top-level facts are.""" + return Deprecated(msg='deprecated', version='2.24').tag(value) + + +class TestConfigDump(unittest.TestCase): + + maxDiff = None + + def setUp(self): + # Bandit complains about Jinja2 autoescaping without nosec. + self.env = jinja2.Environment() # nosec + self.hostvars_vars = DataLoader().load(""" +inventory_hostname: host1 +plain: hello +# A variable referencing hostvars, as Kayobe's network interface variables +# do. +self_ref: >- + {{ 'a' if 'ansible_dummy1' in + hostvars[inventory_hostname] else [] }} +# A variable referencing an undefined variable. +undefined_nested: "{{ ansible_facts.architecture }}" +# A variable resolving to the omit sentinel. +omit_var: "{{ omit }}" +a_dict: + key: value +a_list: + - a + - b +""") + for key in ("self_ref", "undefined_nested", "omit_var"): + self.hostvars_vars[key] = _trust(self.hostvars_vars[key]) + # The templating context must provide the hostvars variable, but the + # host's variables themselves do not include it. + context_vars = dict(self.hostvars_vars) + context_vars["hostvars"] = {"host1": self.hostvars_vars} + self.context = self._make_context(context_vars) + self.hostvars = FakeHostVarsVars(self.hostvars_vars) + + def _make_context(self, parent): + return self.env.context_class( + self.env, parent=parent, name='dummy', blocks={}) + + def test_kayobe_config_dump(self): + result = config_dump.kayobe_config_dump(self.context, self.hostvars) + self.assertEqual("hello", result["plain"]) + # Variables referencing hostvars are resolved. + self.assertEqual([], result["self_ref"]) + # Variables referencing undefined variables are preserved. + self.assertEqual("{{ ansible_facts.architecture }}", + result["undefined_nested"]) + # Omitted variables are preserved. + self.assertEqual("{{ omit }}", result["omit_var"]) + self.assertEqual({"key": "value"}, result["a_dict"]) + self.assertEqual(["a", "b"], result["a_list"]) + + def test_kayobe_config_dump_var_name(self): + result = config_dump.kayobe_config_dump( + self.context, self.hostvars, "self_ref") + self.assertEqual([], result) + + def test_kayobe_config_dump_var_name_missing(self): + self.assertRaises( + errors.AnsibleFilterError, + config_dump.kayobe_config_dump, + self.context, self.hostvars, "does_not_exist") + + def test_kayobe_config_dump_plain_dict(self): + variables = {"plain": "hello"} + result = config_dump.kayobe_config_dump(self.context, variables) + self.assertEqual("hello", result["plain"]) + + def test_kayobe_config_dump_top_level_facts(self): + variables = { + "inventory_hostname": "host1", + # A top-level fact, as injected when inject_facts_as_vars is + # enabled. It is deprecated, but is still templated, for example + # because a user may have overridden it. + "ansible_hostname": _trust( + _deprecate("{{ 'resolved-host1' }}")), + } + context_vars = dict(variables) + context_vars["hostvars"] = {"host1": variables} + context = self._make_context(context_vars) + hostvars = FakeHostVarsVars(variables) + + result = config_dump.kayobe_config_dump(context, hostvars) + + self.assertEqual("resolved-host1", result["ansible_hostname"]) + + @mock.patch.object(config_dump.display, 'warning') + def test_kayobe_config_dump_deprecated_fact_unresolved(self, mock_warning): + variables = { + "inventory_hostname": "host1", + # A deprecated top-level fact that cannot be templated. + "ansible_hostname": _trust(_deprecate("{{ omit }}")), + } + context_vars = dict(variables) + context_vars["hostvars"] = {"host1": variables} + context = self._make_context(context_vars) + hostvars = FakeHostVarsVars(variables) + + result = config_dump.kayobe_config_dump(context, hostvars) + + self.assertTrue(mock_warning.called) + self.assertEqual("{{ omit }}", result["ansible_hostname"]) diff --git a/kayobe/tests/unit/test_nmstate_apply.py b/kayobe/tests/unit/test_nmstate_apply.py index 0573ea18e..87feea8bc 100644 --- a/kayobe/tests/unit/test_nmstate_apply.py +++ b/kayobe/tests/unit/test_nmstate_apply.py @@ -13,6 +13,7 @@ # under the License. import importlib.util +import json from pathlib import Path import unittest from unittest import mock @@ -37,8 +38,10 @@ def __init__(self, payload): class FakeModule: - def __init__(self, params): + def __init__(self, params, check_mode=False, diff_mode=True): self.params = params + self.check_mode = check_mode + self._diff = diff_mode def fail_json(self, **kwargs): raise ModuleFailed(kwargs) @@ -47,8 +50,19 @@ def exit_json(self, **kwargs): raise ModuleExited(kwargs) +def make_fake_libnmstate(differences=None, states=None): + fake_libnmstate = mock.Mock() + if differences is not None: + fake_libnmstate.generate_differences.return_value = differences + if states is not None: + fake_libnmstate.show.side_effect = states + return fake_libnmstate + + class TestNMStateApply(unittest.TestCase): + maxDiff = None + def _load_module(self): spec = importlib.util.spec_from_file_location( "kayobe_nmstate_apply_module", @@ -60,6 +74,17 @@ def _load_module(self): spec.loader.exec_module(module) return module + def _run_module(self, module, fake_module, fake_libnmstate): + with mock.patch.object( + module, "AnsibleModule", return_value=fake_module + ): + with mock.patch.object( + module.importlib, + "import_module", + return_value=fake_libnmstate, + ): + module.run_module() + def test_import_failure(self): module = self._load_module() fake_module = FakeModule({"state": {}, "debug": False}) @@ -79,55 +104,192 @@ def test_import_failure(self): self.assertIn("Failed to import libnmstate module", message) self.assertIn("python3-libnmstate", message) + def test_missing_differences_api(self): + module = self._load_module() + fake_module = FakeModule({"state": {}, "debug": False}) + + # Simulate a libnmstate without generate_differences (for + # example nmstate 1.x). + fake_libnmstate = mock.Mock(spec=["show"]) + + with self.assertRaises(ModuleFailed) as context: + self._run_module(module, fake_module, fake_libnmstate) + + message = context.exception.payload["msg"] + self.assertIn("generate_differences", message) + self.assertIn("nmstate 2.x", message) + fake_libnmstate.show.assert_not_called() + def test_apply_failure(self): module = self._load_module() - fake_module = FakeModule({"state": {"interfaces": []}, "debug": False}) + desired_state = {"interfaces": [{"name": "eth0", "state": "up"}]} + fake_module = FakeModule({"state": desired_state, "debug": False}) - fake_libnmstate = mock.Mock() - fake_libnmstate.show.return_value = {"interfaces": []} + current_state = {"interfaces": [{"name": "eth0", "state": "down"}]} + differences = {"interfaces": [{"name": "eth0", "state": "up"}]} + fake_libnmstate = make_fake_libnmstate( + differences=differences, + states=[current_state], + ) fake_libnmstate.apply.side_effect = RuntimeError("apply failed") - with mock.patch.object( - module, "AnsibleModule", return_value=fake_module - ): - with mock.patch.object( - module.importlib, - "import_module", - return_value=fake_libnmstate, - ): - with self.assertRaises(ModuleFailed) as context: - module.run_module() + with self.assertRaises(ModuleFailed) as context: + self._run_module(module, fake_module, fake_libnmstate) + + payload = context.exception.payload + self.assertIn("Failed to apply nmstate state", payload["msg"]) + # A failed apply must not claim changed (nmstate rolls back). + self.assertEqual(payload["differences"], differences) + self.assertNotIn("diff", payload) + self.assertNotIn("changed", payload) + + def test_changed_without_diff_mode(self): + module = self._load_module() + desired_state = {"interfaces": [{"name": "eth0", "state": "up"}]} + fake_module = FakeModule( + {"state": desired_state, "debug": False}, + check_mode=True, diff_mode=False) + + current_state = {"interfaces": [{"name": "eth0", "state": "down"}]} + differences = {"interfaces": [{"name": "eth0", "state": "up"}]} + fake_libnmstate = make_fake_libnmstate( + differences=differences, + states=[current_state], + ) + + with self.assertRaises(ModuleExited) as context: + self._run_module(module, fake_module, fake_libnmstate) + + payload = context.exception.payload + self.assertTrue(payload["changed"]) + self.assertEqual(payload["differences"], differences) + self.assertNotIn("diff", payload) + fake_libnmstate.apply.assert_not_called() + + def test_apply_no_changes(self): + module = self._load_module() + desired_state = {"interfaces": [{"name": "eth0", "state": "up"}]} + fake_module = FakeModule({"state": desired_state, "debug": False}) - self.assertIn( - "Failed to apply nmstate state", - context.exception.payload["msg"], + current_state = {"interfaces": [{"name": "eth0", "state": "up"}]} + # Real shape returned by the library when nothing differs. + fake_libnmstate = make_fake_libnmstate( + differences={"interfaces": []}, + states=[current_state, current_state], ) - def test_apply_success_debug_output(self): + with self.assertRaises(ModuleExited) as context: + self._run_module(module, fake_module, fake_libnmstate) + + payload = context.exception.payload + self.assertFalse(payload["changed"]) + self.assertNotIn("diff", payload) + self.assertNotIn("differences", payload) + self.assertEqual(payload["state"], current_state) + # Apply is intentional even without changes (re-persists config). + fake_libnmstate.apply.assert_called_once_with(desired_state) + + def test_apply_changed_with_diff_and_debug(self): module = self._load_module() desired_state = {"interfaces": [{"name": "eth0", "state": "up"}]} fake_module = FakeModule({"state": desired_state, "debug": True}) previous_state = {"interfaces": [{"name": "eth0", "state": "down"}]} current_state = {"interfaces": [{"name": "eth0", "state": "up"}]} + differences = {"interfaces": [{"name": "eth0", "state": "up"}]} - fake_libnmstate = mock.Mock() - fake_libnmstate.show.side_effect = [previous_state, current_state] + fake_libnmstate = make_fake_libnmstate( + differences=differences, + states=[previous_state, current_state], + ) - with mock.patch.object( - module, "AnsibleModule", return_value=fake_module - ): - with mock.patch.object( - module.importlib, - "import_module", - return_value=fake_libnmstate, - ): - with self.assertRaises(ModuleExited) as context: - module.run_module() + with self.assertRaises(ModuleExited) as context: + self._run_module(module, fake_module, fake_libnmstate) payload = context.exception.payload self.assertTrue(payload["changed"]) self.assertEqual(payload["state"], current_state) self.assertEqual(payload["previous_state"], previous_state) self.assertEqual(payload["desired_state"], desired_state) + self.assertEqual(payload["differences"], differences) + self.assertEqual( + payload["diff"], + { + "prepared": json.dumps( + differences, indent=1, sort_keys=True), + }, + ) fake_libnmstate.apply.assert_called_once_with(desired_state) + + def test_check_mode_with_changes(self): + module = self._load_module() + desired_state = {"interfaces": [{"name": "eth0", "state": "up"}]} + fake_module = FakeModule( + {"state": desired_state, "debug": True}, check_mode=True) + + current_state = {"interfaces": [{"name": "eth0", "state": "down"}]} + differences = {"interfaces": [{"name": "eth0", "state": "up"}]} + fake_libnmstate = make_fake_libnmstate( + differences=differences, + states=[current_state], + ) + + with self.assertRaises(ModuleExited) as context: + self._run_module(module, fake_module, fake_libnmstate) + + payload = context.exception.payload + self.assertTrue(payload["changed"]) + self.assertEqual(payload["differences"], differences) + self.assertEqual( + payload["diff"], + { + "prepared": json.dumps( + differences, indent=1, sort_keys=True), + }, + ) + # Debug output is available in check mode as well. + self.assertEqual(payload["previous_state"], current_state) + self.assertEqual(payload["desired_state"], desired_state) + self.assertNotIn("state", payload) + fake_libnmstate.apply.assert_not_called() + + def test_check_mode_no_changes(self): + module = self._load_module() + desired_state = {"interfaces": [{"name": "eth0", "state": "up"}]} + fake_module = FakeModule( + {"state": desired_state, "debug": False}, check_mode=True) + + current_state = {"interfaces": [{"name": "eth0", "state": "up"}]} + fake_libnmstate = make_fake_libnmstate( + differences={"interfaces": []}, + states=[current_state], + ) + + with self.assertRaises(ModuleExited) as context: + self._run_module(module, fake_module, fake_libnmstate) + + payload = context.exception.payload + self.assertFalse(payload["changed"]) + self.assertNotIn("diff", payload) + self.assertNotIn("differences", payload) + fake_libnmstate.apply.assert_not_called() + + def test_is_empty(self): + module = self._load_module() + + # Empty or fully elided differences mean no changes. + self.assertTrue(module._is_empty({})) + self.assertTrue(module._is_empty({"interfaces": []})) + self.assertTrue(module._is_empty({"routes": {"config": []}})) + self.assertTrue(module._is_empty( + {"dns-resolver": {"config": {}}})) + self.assertTrue(module._is_empty(None)) + + # Any populated structure or scalar means changes. + self.assertFalse(module._is_empty( + {"interfaces": [{"name": "eth0"}]})) + self.assertFalse(module._is_empty( + {"routes": {"config": [{"destination": "0.0.0.0/0"}]}})) + self.assertFalse(module._is_empty({"hostname": "host01"})) + self.assertFalse(module._is_empty(False)) + self.assertFalse(module._is_empty(0)) diff --git a/playbooks/kayobe-infra-vm-base/overrides.yml.j2 b/playbooks/kayobe-infra-vm-base/overrides.yml.j2 index c6d5cc075..2df774af1 100644 --- a/playbooks/kayobe-infra-vm-base/overrides.yml.j2 +++ b/playbooks/kayobe-infra-vm-base/overrides.yml.j2 @@ -34,7 +34,7 @@ infra_vm_memory_mb: "{{ 1 * 1024 }}" {% if infra_vm_use_cirros | default(true) %} # Use cirros rather than distribution cloud image for the VM. infra_vm_bootstrap_user: cirros -infra_vm_root_image: /opt/cache/files/cirros-0.5.3-x86_64-disk.img +infra_vm_root_image: https://download.cirros-cloud.net/0.5.3/cirros-0.5.3-x86_64-disk.img # Cirros doesn't load cdom drivers by default. vm_configdrive_device: disk diff --git a/playbooks/kayobe-overcloud-upgrade-base/run.yml b/playbooks/kayobe-overcloud-upgrade-base/run.yml index 123d5686b..318432e67 100644 --- a/playbooks/kayobe-overcloud-upgrade-base/run.yml +++ b/playbooks/kayobe-overcloud-upgrade-base/run.yml @@ -76,12 +76,14 @@ cmd: dev/overcloud-test-vm.sh &> {{ logs_dir }}/ansible/overcloud-test-vm-pre-upgrade chdir: "{{ previous_kayobe_src_dir }}" executable: /bin/bash + when: ansible_facts.os_family != 'Debian' - name: Perform testing of the baremetal machines in the overcloud prior to upgrade shell: cmd: dev/overcloud-test-baremetal.sh &> {{ logs_dir }}/ansible/overcloud-test-bm-pre-upgrade chdir: "{{ previous_kayobe_src_dir }}" executable: /bin/bash + when: ansible_facts.os_family != 'Debian' # Upgrade Kayobe, and use it to perform an upgrade of the control plane. diff --git a/playbooks/kayobe-seed-vm-base/overrides.yml.j2 b/playbooks/kayobe-seed-vm-base/overrides.yml.j2 index 83be19105..137370220 100644 --- a/playbooks/kayobe-seed-vm-base/overrides.yml.j2 +++ b/playbooks/kayobe-seed-vm-base/overrides.yml.j2 @@ -37,7 +37,7 @@ seed_vm_memory_mb: "{{ 1 * 1024 }}" {% if seed_vm_use_cirros | default(true) %} # Use cirros rather than distribution cloud image for the VM. seed_bootstrap_user: cirros -seed_vm_root_image: /opt/cache/files/cirros-0.5.3-x86_64-disk.img +seed_vm_root_image: https://download.cirros-cloud.net/0.5.3/cirros-0.5.3-x86_64-disk.img # Cirros doesn't load cdom drivers by default. seed_vm_configdrive_device: disk diff --git a/releasenotes/notes/bump-ansible-14-bf0e2fe83a32efbb.yaml b/releasenotes/notes/bump-ansible-14-bf0e2fe83a32efbb.yaml new file mode 100644 index 000000000..0d23fe455 --- /dev/null +++ b/releasenotes/notes/bump-ansible-14-bf0e2fe83a32efbb.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + Updates the maximum supported version of Ansible from 13 (ansible-core + 2.20) to 14 (ansible-core 2.21). The minimum supported version is updated + from 12.x to 13.x. This is true for both Kayobe and Kolla Ansible. diff --git a/releasenotes/notes/fix-config-dump-ansible-219-3c98df89eecf4969.yaml b/releasenotes/notes/fix-config-dump-ansible-219-3c98df89eecf4969.yaml new file mode 100644 index 000000000..889f819d5 --- /dev/null +++ b/releasenotes/notes/fix-config-dump-ansible-219-3c98df89eecf4969.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes an issue where ``kayobe configuration dump`` would fail with + ansible-core 2.19 or later. Variables that reference ``hostvars`` or + undefined variables are now resolved correctly, and variables that cannot + be resolved are included in the dump in their raw form. `LP#2166964 + `__ diff --git a/releasenotes/notes/fix-nmstate-cloud-init-profile-3ea31ce33527ad63.yaml b/releasenotes/notes/fix-nmstate-cloud-init-profile-3ea31ce33527ad63.yaml new file mode 100644 index 000000000..7ec31646a --- /dev/null +++ b/releasenotes/notes/fix-nmstate-cloud-init-profile-3ea31ce33527ad63.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + Fixes the nmstate network engine when cloud-init creates a + NetworkManager connection whose profile ID differs from the interface + name (`LP#2165006 `__). + Firewalld zone synchronisation now resolves the active profile from the + interface before reading or updating its connection zone. diff --git a/releasenotes/notes/nmstate-engine-check-diff-mode-9a2c7d41e8f3b056.yaml b/releasenotes/notes/nmstate-engine-check-diff-mode-9a2c7d41e8f3b056.yaml new file mode 100644 index 000000000..8b6fe5913 --- /dev/null +++ b/releasenotes/notes/nmstate-engine-check-diff-mode-9a2c7d41e8f3b056.yaml @@ -0,0 +1,17 @@ +--- +features: + - | + The nmstate network engine now supports Ansible check and diff modes. + Running for example ``kayobe overcloud host configure --check --diff`` + reports whether network changes are required and shows the computed + differences without modifying the host. +fixes: + - | + The nmstate network engine reported ``changed`` on every run for hosts + whose runtime state contains volatile data (for example Linux bridges, + whose bridge ageing timers such as ``gc-timer`` change over time), + because it compared full ``libnmstate.show()`` snapshots taken before + and after applying the state. Change detection now uses + ``libnmstate.generate_differences()``, which only compares the desired + state against the runtime state. + `LP#2164642 `__ diff --git a/requirements.txt b/requirements.txt index 67091f5ae..4785c1154 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,6 @@ pbr>=2.0 # Apache-2.0 Jinja2>3 # BSD -ansible>=12,<14 # GPLv3 +ansible>=13,<15 # GPLv3 cliff>=3.1.0 # Apache netaddr!=0.7.16,>=0.7.13 # BSD PyYAML>=3.10.0 # MIT diff --git a/requirements.yml b/requirements.yml index cd5c7f029..5c63e4dff 100644 --- a/requirements.yml +++ b/requirements.yml @@ -13,12 +13,14 @@ collections: version: 1.0.4 - name: dellemc.os10 version: 1.2.7 + - name: junipernetworks.junos + version: 11.1.1 - name: nvidia.nvue version: 1.2.9 - name: openstack.cloud version: '<3' - name: stackhpc.linux - version: 1.5.2 + version: 1.6.0 - name: stackhpc.network version: 1.0.0 - name: stackhpc.openstack diff --git a/zuul.d/jobs.yaml b/zuul.d/jobs.yaml index fb5f7682a..2ace4b0a5 100644 --- a/zuul.d/jobs.yaml +++ b/zuul.d/jobs.yaml @@ -170,7 +170,7 @@ Configures the primary VM as an overcloud controller. pre-run: playbooks/kayobe-overcloud-base/pre.yml run: playbooks/kayobe-overcloud-base/run.yml - timeout: 7200 + timeout: 10800 - job: name: kayobe-overcloud-centos10s