diff --git a/ansible/control-host-configure.yml b/ansible/control-host-configure.yml index b1e52464d..694a25fd5 100644 --- a/ansible/control-host-configure.yml +++ b/ansible/control-host-configure.yml @@ -10,7 +10,7 @@ - import_playbook: "kayobe-target-venv.yml" - import_playbook: "wipe-disks.yml" - import_playbook: "users.yml" -- import_playbook: "dev-tools.yml" +- import_playbook: "install-packages.yml" - import_playbook: "selinux.yml" - import_playbook: "network.yml" - import_playbook: "firewall.yml" diff --git a/ansible/install-packages.yml b/ansible/install-packages.yml new file mode 100644 index 000000000..0c13378eb --- /dev/null +++ b/ansible/install-packages.yml @@ -0,0 +1,6 @@ +--- +- name: Install kayobe-packages + ansible.builtin.import_playbook: dev-tools.yml + +- name: Install kolla-packages + ansible.builtin.import_playbook: kolla-packages.yml diff --git a/ansible/kayobe-ansible-user.yml b/ansible/kayobe-ansible-user.yml index c0d826453..cf47fabc7 100644 --- a/ansible/kayobe-ansible-user.yml +++ b/ansible/kayobe-ansible-user.yml @@ -61,6 +61,7 @@ changed_when: false failed_when: false register: check_python + check_mode: false - name: Ensure Python is installed raw: "(test -e /usr/bin/apt && sudo apt {{ apt_options | select | join(' ') }} update && sudo apt install {{ apt_options | select | join(' ') }} python3-minimal) || (test -e /usr/bin/dnf && sudo dnf {{ dnf_options | select | join(' ') }} install python3)" diff --git a/ansible/overcloud-host-configure.yml b/ansible/overcloud-host-configure.yml index 7faf3ccbb..b04465806 100644 --- a/ansible/overcloud-host-configure.yml +++ b/ansible/overcloud-host-configure.yml @@ -10,7 +10,7 @@ - import_playbook: "kayobe-target-venv.yml" - import_playbook: "wipe-disks.yml" - import_playbook: "users.yml" -- import_playbook: "dev-tools.yml" +- import_playbook: "install-packages.yml" - import_playbook: "selinux.yml" - import_playbook: "network.yml" - import_playbook: "firewall.yml" @@ -28,7 +28,6 @@ - import_playbook: "kolla-ansible-user.yml" - import_playbook: "kolla-pip.yml" - import_playbook: "kolla-target-venv.yml" -- import_playbook: "kolla-packages.yml" - import_playbook: "container-engine.yml" - import_playbook: "apparmor-libvirt.yml" - import_playbook: "swift-block-devices.yml" diff --git a/ansible/physical-network.yml b/ansible/physical-network.yml index 96d0b232d..8891a62c5 100644 --- a/ansible/physical-network.yml +++ b/ansible/physical-network.yml @@ -33,6 +33,7 @@ - dellos9 - dellos10 - dell-powerconnect + - dell-sonic - junos - mellanox - nclu @@ -159,6 +160,23 @@ dell_powerconnect_switch_config: "{{ switch_config }}" dell_powerconnect_switch_interface_config: "{{ switch_interface_config }}" +- name: Ensure Dell SONiC physical switches are configured + hosts: switches_of_type_dell-sonic:&switches_in_display_mode_False + gather_facts: no + max_fail_percentage: >- + {{ physical_network_max_fail_percentage | + default(kayobe_max_fail_percentage) | + default(100) }} + roles: + - role: ssh-known-host + when: not switch_skip_keyscan | bool + + - role: dell-switch-sonic + dell_switch_sonic_type: "{{ switch_type }}" + dell_switch_sonic_config: "{{ switch_config }}" + dell_switch_sonic_interface_config: "{{ switch_interface_config }}" + dell_switch_sonic_save: "{{ switch_config_save }}" + - name: Ensure Juniper physical switches are configured hosts: switches_of_type_junos:&switches_in_display_mode_False gather_facts: no diff --git a/ansible/roles/dell-switch-sonic/README.md b/ansible/roles/dell-switch-sonic/README.md new file mode 100644 index 000000000..ce3f47f80 --- /dev/null +++ b/ansible/roles/dell-switch-sonic/README.md @@ -0,0 +1,63 @@ +Dell Switch - SONiC +=================== + +This role configures Dell switches using the `dellemc.enterprise_sonic` +Ansible collections. It provides a fairly minimal abstraction of +the configuration interface provided by the collections, allowing for +application of arbitrary switch configuration options. + +Requirements +------------ + +The switches should be configured to allow SSH access. + +Role Variables +-------------- + +`dell_switch_sonic_config` is a list of configuration lines to apply to the switch, +and defaults to an empty list. + +`dell_switch_sonic_interface_config` contains interface configuration. It is a dict +mapping switch interface names to configuration dicts. Each dict may contain +the following items: + +- `description` - a description to apply to the interface. +- `config` - a list of per-interface configuration. + +Dependencies +------------ + +None + +Example Playbook +---------------- + +The following playbook configures hosts in the `dell-switches-sonic` group. +It assumes host variables for each switch holding the host, username and +passwords. It applies global configuration for LLDP, and enables two +10G ethernet interfaces as switchports. + + --- + - name: Ensure Dell SONiC switches are configured + hosts: dell-switches-sonic + gather_facts: no + roles: + - role: dell-switch-sonic + dell_switch_sonic_config: + - "lldp enable" + - "ntp server 10.0.12.34" + dell_switch_sonic_interface_config: + Ethernet1: + description: server-1 + config: + - "no shutdown" + - "switchport" + Ethernet2: + description: server-2 + config: + - "no shutdown" + +Author Information +------------------ + +- Matt Crees () diff --git a/ansible/roles/dell-switch-sonic/defaults/main.yml b/ansible/roles/dell-switch-sonic/defaults/main.yml new file mode 100644 index 000000000..48fe43848 --- /dev/null +++ b/ansible/roles/dell-switch-sonic/defaults/main.yml @@ -0,0 +1,11 @@ +--- +# List of configuration lines to apply to the switch. +dell_switch_sonic_config: [] + +# Interface configuration. Dict mapping switch interface names to configuration +# dicts. Each dict contains a 'description' item and a 'config' item which +# should contain a list of per-interface configuration. +dell_switch_sonic_interface_config: {} + +# Whether to save the configuration. +dell_switch_sonic_save: false diff --git a/ansible/roles/dell-switch-sonic/tasks/main.yml b/ansible/roles/dell-switch-sonic/tasks/main.yml new file mode 100644 index 000000000..4f04dabf6 --- /dev/null +++ b/ansible/roles/dell-switch-sonic/tasks/main.yml @@ -0,0 +1,5 @@ +--- +- name: Ensure Dell SONiC switches are configured + dellemc.enterprise_sonic.sonic_config: # noqa fqcn[canonical] + src: "../templates/dell-sonic-config.j2" + save: "{{ dell_switch_sonic_save | bool }}" diff --git a/ansible/roles/dell-switch-sonic/templates/dell-sonic-config.j2 b/ansible/roles/dell-switch-sonic/templates/dell-sonic-config.j2 new file mode 100644 index 000000000..14a71e5af --- /dev/null +++ b/ansible/roles/dell-switch-sonic/templates/dell-sonic-config.j2 @@ -0,0 +1,16 @@ +#jinja2: trim_blocks: True,lstrip_blocks: True + +{% for line in dell_switch_sonic_config %} +{{ line }} +{% endfor %} + +{% for interface, config in dell_switch_sonic_interface_config.items() %} +interface {{ interface }} +{% if config.description is defined %} +description "{{ config.description }}" +{% endif %} +{% for line in config.config %} +{{ line }} +{% endfor %} +exit +{% endfor %} diff --git a/ansible/roles/dell-switch/templates/dellos10-config.j2 b/ansible/roles/dell-switch/templates/dellos10-config.j2 index 94c9dc068..7904f89c6 100644 --- a/ansible/roles/dell-switch/templates/dellos10-config.j2 +++ b/ansible/roles/dell-switch/templates/dellos10-config.j2 @@ -7,7 +7,7 @@ {% for interface, config in dell_switch_interface_config.items() %} interface {{ interface }} {% if config.description is defined %} -description {{ config.description }} +description "{{ config.description }}" {% endif %} {% for line in config.config %} {{ line }} diff --git a/ansible/roles/dell-switch/templates/dellos6-config.j2 b/ansible/roles/dell-switch/templates/dellos6-config.j2 index 94c9dc068..7904f89c6 100644 --- a/ansible/roles/dell-switch/templates/dellos6-config.j2 +++ b/ansible/roles/dell-switch/templates/dellos6-config.j2 @@ -7,7 +7,7 @@ {% for interface, config in dell_switch_interface_config.items() %} interface {{ interface }} {% if config.description is defined %} -description {{ config.description }} +description "{{ config.description }}" {% endif %} {% for line in config.config %} {{ line }} diff --git a/ansible/roles/dell-switch/templates/dellos9-config.j2 b/ansible/roles/dell-switch/templates/dellos9-config.j2 index 9b3cbc1fd..6108e9a0d 100644 --- a/ansible/roles/dell-switch/templates/dellos9-config.j2 +++ b/ansible/roles/dell-switch/templates/dellos9-config.j2 @@ -12,7 +12,7 @@ dell_switch_interface_config.items() %} {% if 'vlan' not in interface %} interface {{ interface }} {% if config.description is defined %} - description {{ config.description }} + description "{{ config.description }}" {% endif %} {% for line in config.config %} {{ line }} @@ -25,7 +25,7 @@ dell_switch_interface_config.items() %} {% if 'vlan' in interface %} interface {{ interface }} {% if config.description is defined %} - description {{ config.description }} + description "{{ config.description }}" {% endif %} {% for line in config.config %} {{ line }} diff --git a/ansible/roles/dev-tools/defaults/main.yml b/ansible/roles/dev-tools/defaults/main.yml index adeac39e1..87e4fdde2 100644 --- a/ansible/roles/dev-tools/defaults/main.yml +++ b/ansible/roles/dev-tools/defaults/main.yml @@ -13,6 +13,9 @@ dev_tools_packages_system: # NOTE(mgoddard): The acl package is required for the setfacl command, used by # become_user. - acl + # NOTE(rowanjohns): kernel-modules-extra-matched needs to be installed on RedHat + # hosts so that iptables can be used. + - "{{ 'kernel-modules-extra-matched' if ansible_facts.os_family == 'RedHat' and ansible_facts.distribution_major_version == '10' else '' }}" # List of packages to install. dev_tools_packages: "{{ (dev_tools_packages_default + dev_tools_packages_extra + dev_tools_packages_system) | select | list }}" diff --git a/ansible/roles/kolla/tasks/install.yml b/ansible/roles/kolla/tasks/install.yml index b754f1de7..3becbb917 100644 --- a/ansible/roles/kolla/tasks/install.yml +++ b/ansible/roles/kolla/tasks/install.yml @@ -67,9 +67,7 @@ version: "{{ item.version | default(omit) }}" state: "{{ item.state | default('present') }}" virtualenv: "{{ kolla_venv }}" - extra_args: "{{ item.extra_args | default(default_extra_args) }}" - vars: - default_extra_args: "{% if kolla_upper_constraints_file %}-c {{ kolla_upper_constraints_file }}{% endif %}" + extra_args: "{% if kolla_upper_constraints_file %}-c {{ kolla_upper_constraints_file }}{% endif %}" with_items: # Install Kolla from source. - name: "{{ kolla_source_path }}" @@ -81,10 +79,6 @@ # Install docker from PyPI since it was dropped from kolla requirements. # Skip 7.0.0 due to: https://github.com/docker/docker-py/issues/3195 - name: "docker!=7.0.0,<8" - # NOTE(wszumski): Workaround for ERROR: ResolutionImpossible when using upper constraints. - # This can be removed once a newer version of docker is released and the version in upper - # constraints has been bumped (>7.0.0). - extra_args: "{{ omit }}" install: "{{ container_engine == 'docker' }}" - name: "podman" install: "{{ container_engine == 'podman' }}" 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/seed-host-configure.yml b/ansible/seed-host-configure.yml index bdaef4754..9aafe60ab 100644 --- a/ansible/seed-host-configure.yml +++ b/ansible/seed-host-configure.yml @@ -10,7 +10,7 @@ - import_playbook: "kayobe-target-venv.yml" - import_playbook: "wipe-disks.yml" - import_playbook: "users.yml" -- import_playbook: "dev-tools.yml" +- import_playbook: "install-packages.yml" - import_playbook: "selinux.yml" - import_playbook: "network.yml" - import_playbook: "firewall.yml" diff --git a/ansible/seed-hypervisor-host-configure.yml b/ansible/seed-hypervisor-host-configure.yml index 9b7f55092..a989e0c20 100644 --- a/ansible/seed-hypervisor-host-configure.yml +++ b/ansible/seed-hypervisor-host-configure.yml @@ -10,7 +10,7 @@ - import_playbook: "kayobe-target-venv.yml" - import_playbook: "wipe-disks.yml" - import_playbook: "users.yml" -- import_playbook: "dev-tools.yml" +- import_playbook: "install-packages.yml" - import_playbook: "selinux.yml" - import_playbook: "network.yml" - import_playbook: "firewall.yml" diff --git a/doc/source/configuration/reference/network.rst b/doc/source/configuration/reference/network.rst index 00301da8b..dc79ae01f 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/doc/source/configuration/reference/physical-network.rst b/doc/source/configuration/reference/physical-network.rst index 869df5179..ff00a60c8 100644 --- a/doc/source/configuration/reference/physical-network.rst +++ b/doc/source/configuration/reference/physical-network.rst @@ -19,6 +19,7 @@ The following switch operating systems are currently supported: `__) * Cumulus Linux (via `NVIDIA User Experience command line utility (NVUE) `__) +* Dell Enterprise SONiC * Dell OS 6 * Dell OS 9 * Dell OS 10 @@ -238,6 +239,24 @@ by Ansible: * ``ansible_user`` is the SSH username. +Dell Enterprise SONiC +--------------------- + +Configuration for these devices is applied using the +``dellemc.enterprise_sonic`` Ansible module. + +``switch_type`` should be set to ``dell-sonic``. + +``switch_config_save`` may be set to ``true`` to enable saving configuration +after it has been applied. + +* ``ansible_host`` is the hostname or IP address. Optional. +* ``ansible_user`` is the SSH username. +* ``ansible_ssh_pass`` is the SSH password. +* ``switch_auth_pass`` is the 'enable' password. +* ``ansible_connection`` should be ``ansible.netcommon.network_cli``. +* ``ansible_network_os`` should be ``dellemc.enterprise_sonic.sonic``. + Dell OS6, OS9, and OS10 ----------------------- 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/releasenotes/notes/add-dell-enterprise-sonic-switch-support-f7405df8695d9d23.yaml b/releasenotes/notes/add-dell-enterprise-sonic-switch-support-f7405df8695d9d23.yaml new file mode 100644 index 000000000..917199123 --- /dev/null +++ b/releasenotes/notes/add-dell-enterprise-sonic-switch-support-f7405df8695d9d23.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Adds support for configuring Dell Enterprise SONiC switches using + ``sonic-cli``, via the ``dellemc.enterprise_sonic`` Ansible collection. + This is integrated into the ``kayobe physical network configure`` command. diff --git a/releasenotes/notes/check-python-installed-in-check-mode-0a67394bc1c37bfb.yaml b/releasenotes/notes/check-python-installed-in-check-mode-0a67394bc1c37bfb.yaml new file mode 100644 index 000000000..bd22f9ba7 --- /dev/null +++ b/releasenotes/notes/check-python-installed-in-check-mode-0a67394bc1c37bfb.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes Kayobe's Ansible user setup in check mode by ensuring that the + Python availability check runs. This allows the following task to + correctly determine whether Python needs to be installed. + `LP#2165042 `__ 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/releasenotes/notes/support-whitespace-dellos-switches-1b6a8526850d957f.yaml b/releasenotes/notes/support-whitespace-dellos-switches-1b6a8526850d957f.yaml new file mode 100644 index 000000000..f594a31a5 --- /dev/null +++ b/releasenotes/notes/support-whitespace-dellos-switches-1b6a8526850d957f.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes a bug found when trying to assign a multi-word description + to a Dell OS switch with ``kayobe physical network configure``. + `LP#2161899 `__ diff --git a/requirements.yml b/requirements.yml index fdbbed2da..ac8298809 100644 --- a/requirements.yml +++ b/requirements.yml @@ -7,6 +7,8 @@ collections: version: 5.2.0 - name: community.network version: 5.1.0 + - name: dellemc.enterprise_sonic + version: 4.1.0 - name: dellemc.os6 version: 1.0.7 - name: dellemc.os9 @@ -33,7 +35,7 @@ roles: # There are no versioned releases of this role. version: ebf31eee91056a8fcc71a304ff50b014909cf4ca - src: MichaelRigart.interfaces - version: v1.17.0 + version: v1.17.1 - src: mrlesmithjr.chrony version: v0.2.0 - src: mrlesmithjr.manage_lvm diff --git a/roles/kayobe-ci-prep/tasks/main.yml b/roles/kayobe-ci-prep/tasks/main.yml index cc813b87b..f2cb6735b 100644 --- a/roles/kayobe-ci-prep/tasks/main.yml +++ b/roles/kayobe-ci-prep/tasks/main.yml @@ -1,4 +1,13 @@ --- +# NOTE: prefer IPv4 over IPv6 in getaddrinfo() to avoid slow/failing AAAA +# lookups against hosts without real IPv6 connectivity +- name: Prefer IPv4 over IPv6 for name resolution + become: true + ansible.builtin.lineinfile: + path: /etc/gai.conf + line: "precedence ::ffff:0:0/96 100" + create: true + - name: Configure secondary storage drive #NOTE(wszumski) Some instances provide a smaller 40G root disk with a #secondary storage drive. This kolla-ansible provided role sets up the diff --git a/zuul.d/jobs.yaml b/zuul.d/jobs.yaml index 0f849424f..6ea51d33b 100644 --- a/zuul.d/jobs.yaml +++ b/zuul.d/jobs.yaml @@ -12,6 +12,7 @@ ansible_collection_kolla_src_dir: "{{ ansible_env.PWD ~ '/' ~ zuul.projects['opendev.org/openstack/ansible-collection-kolla'].src_dir }}" kayobe_src_dir: "{{ ansible_env.PWD ~ '/' ~ zuul.projects['opendev.org/openstack/kayobe'].src_dir }}" irrelevant-files: + - ^.*\.md$ - ^.*\.rst$ - ^doc/.* - ^kayobe/tests/ @@ -32,6 +33,7 @@ required-projects: - name: openstack/requirements irrelevant-files: + - ^.*\.md$ - ^.*\.rst$ - ^doc/.* - ^kayobe/tests/ @@ -53,6 +55,7 @@ vars: tox_envlist: molecule irrelevant-files: + - ^.*\.md$ - ^.*\.rst$ - ^doc/.* - ^kayobe/tests/ @@ -80,6 +83,7 @@ - name: openstack/tenks irrelevant-files: - ^\..+ + - ^.*\.md$ - ^.*\.rst$ - ^doc/.* - ^kayobe/tests/