Merge 313dfef134 into 877f20f278

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)

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",

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