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): """