openstack/kayobe
Code Issues Proposed changes

Add check and diff mode support to nmstate engine

Browse Source 
The nmstate_apply module declared supports_check_mode=False, so
"kayobe overcloud host configure --check --diff" skipped the network
state application task and reported nothing about the desired network
changes.

Closes-Bug: #2164642
Generated-By: Qwen3.8-27B-UD-Q4_K_XL(initial review Opus5)
Change-Id: Id9d4d2643b0faf04d83553e289a40d2de8f00c53
Signed-off-by: Grzegorz Koper <grzegorzk@stackhpc.com>
This commit is contained in:
Grzegorz Koper committed 2026-09-04 12:49:02 +02:00
1 parent 39661017d8
commit 2b7c7ad4ae
5 files changed
+282 -46
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

ansible/roles/network-nmstate/library/nmstate_apply.py
+62 -16
View File
@@ -14,6 +14,7 @@
# under the License.
import importlib
import json
from ansible.module_utils.basic import AnsibleModule
@@ -25,7 +26,8 @@ author: "StackHPC"
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 @@ options:
default: False
type: bool
requirements:
- libnmstate
- libnmstate (nmstate 2.x)
"""
EXAMPLES = """
@@ -66,7 +68,15 @@ changed:
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 @@ desired_state:
"""
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))
# 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)
current_state = libnmstate.show()
if current_state != previous_state:
result["changed"] = True
if debug:
result["previous_state"] = previous_state
result["desired_state"] = desired_state
result["state"] = current_state
result["state"] = libnmstate.show()
module.exit_json(**result)
ansible/roles/network-nmstate/tasks/main.yml
+4
View File
@@ -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:
doc/source/configuration/reference/network.rst
+7
View File
@@ -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
-------------------------
kayobe/tests/unit/test_nmstate_apply.py
+192 -30
View File
@@ -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 @@ class ModuleExited(Exception):
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 @@ class FakeModule:
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 @@ class TestNMStateApply(unittest.TestCase):
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 @@ class TestNMStateApply(unittest.TestCase):
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)
self.assertIn(
"Failed to apply nmstate state",
context.exception.payload["msg"],
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],
)
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.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})
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],
)
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))
releasenotes/notes/nmstate-engine-check-diff-mode-9a2c7d41e8f3b056.yaml
+17
View File
@@ -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 <https://bugs.launchpad.net/kayobe/+bug/2164642>`__