From ca332c6c904e2c4149b8267f5c604fe551718cb4 Mon Sep 17 00:00:00 2001 From: Olivier Raggi Date: Thu, 28 May 2026 14:04:53 +0000 Subject: [PATCH] nmcli: preserve existing bond mode when omitted on existing connections Remove hardcoded bond mode default from nmcli module and preserve the existing bond mode during updates when mode is not explicitly supplied. Include regression coverage and a changelog fragment for issue #9201. Co-Authored-By: GitHub Copilot --- .../9201-nmcli-preserve-bond-mode.yml | 2 + plugins/modules/nmcli.py | 7 +-- tests/unit/plugins/modules/test_nmcli.py | 49 +++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/9201-nmcli-preserve-bond-mode.yml diff --git a/changelogs/fragments/9201-nmcli-preserve-bond-mode.yml b/changelogs/fragments/9201-nmcli-preserve-bond-mode.yml new file mode 100644 index 0000000000..88a834bbad --- /dev/null +++ b/changelogs/fragments/9201-nmcli-preserve-bond-mode.yml @@ -0,0 +1,2 @@ +bugfixes: + - nmcli - preserve the existing bond mode when `mode` is omitted on an existing connection, avoiding unintended changes to `balance-rr` and preventing broken bond configurations (https://github.com/ansible-collections/community.general/issues/9201) diff --git a/plugins/modules/nmcli.py b/plugins/modules/nmcli.py index 4c8862f7f2..847897261c 100644 --- a/plugins/modules/nmcli.py +++ b/plugins/modules/nmcli.py @@ -120,10 +120,12 @@ options: - loopback mode: description: - - This is the type of device or network connection that you wish to create for a bond or bridge. + - Bond mode to use for a bond connection. + - When creating a new bond, NetworkManager's default mode V(balance-rr) is used if this option is not provided. + - When omitted for an existing bond connection, the module preserves the current bond mode. + - This option only applies when C(type) is set to C(bond). type: str choices: [802.3ad, active-backup, balance-alb, balance-rr, balance-tlb, balance-xor, broadcast] - default: balance-rr transport_mode: description: - This option sets the connection type of Infiniband IPoIB devices. @@ -2818,7 +2820,6 @@ def create_module() -> AnsibleModule: # Bond Specific vars mode=dict( type="str", - default="balance-rr", choices=[ "802.3ad", "active-backup", diff --git a/tests/unit/plugins/modules/test_nmcli.py b/tests/unit/plugins/modules/test_nmcli.py index c73af9ab15..8ce1791ed7 100644 --- a/tests/unit/plugins/modules/test_nmcli.py +++ b/tests/unit/plugins/modules/test_nmcli.py @@ -621,6 +621,34 @@ ipv6.ignore-auto-routes: no bond.options: mode=active-backup,primary=non_existent_primary,xmit_hash_policy=layer3+4 """ +TESTCASE_BOND_MODE_UNSET = [ + { + "type": "bond", + "conn_name": "non_existent_nw_device", + "ifname": "bond_non_existant", + "dns4_search": ["example1.com", "example2.com"], + "state": "present", + "_ansible_check_mode": False, + } +] + +TESTCASE_BOND_MODE_UNSET_OUTPUT = """\ +connection.id: non_existent_nw_device +connection.interface-name: bond_non_existant +connection.autoconnect: yes +ipv4.method: manual +ipv4.dns-search: example1.com,example2.com +ipv4.ignore-auto-dns: no +ipv4.ignore-auto-routes: no +ipv4.never-default: no +ipv4.may-fail: yes +ipv6.method: auto +ipv6.ignore-auto-dns: no +ipv6.ignore-auto-routes: no +802-3-ethernet.mtu: auto +bond.options: mode=802.3ad +""" + TESTCASE_BOND_ARP = [ { "type": "bond", @@ -1817,6 +1845,11 @@ def mocked_bond_connection_unchanged(mocker): mocker_set(mocker, connection_exists=True, execute_return=(0, TESTCASE_BOND_SHOW_OUTPUT, "")) +@pytest.fixture +def mocked_bond_mode_unset_connection_unchanged(mocker): + mocker_set(mocker, connection_exists=True, execute_return=(0, TESTCASE_BOND_MODE_UNSET_OUTPUT, "")) + + @pytest.fixture def mocked_bond_arp_connection_unchanged(mocker): mocker_set(mocker, connection_exists=True, execute_return=(0, TESTCASE_BOND_ARP_SHOW_OUTPUT, "")) @@ -2277,6 +2310,22 @@ def test_bond_connection_unchanged(mocked_bond_connection_unchanged, capfd): assert not results["changed"] +@pytest.mark.parametrize("patch_ansible_module", TESTCASE_BOND_MODE_UNSET, indirect=["patch_ansible_module"]) +def test_bond_mode_omitted_does_not_change_existing_connection(mocked_bond_mode_unset_connection_unchanged): + """ + Regression test for unspecified bond mode on existing connections. + """ + module = nmcli.create_module() + nmcli_module = nmcli.Nmcli(module) + + changed, diff = nmcli_module.is_connection_changed() + assert not changed + assert "mode" not in diff["before"] + assert "mode" not in diff["after"] + assert diff["before"]["ipv4.dns-search"] == ["example1.com", "example2.com"] + assert diff["after"]["ipv4.dns-search"] == ["example1.com", "example2.com"] + + @pytest.mark.parametrize("patch_ansible_module", TESTCASE_BOND_ARP, indirect=["patch_ansible_module"]) def test_bond_arp_connection_unchanged(mocked_bond_arp_connection_unchanged): """