tests: Prefer permanent MAC for finding link info, fallback to current MAC

library/network_connections.py

@@ -95,6 +95,7 @@
 
 DEFAULT_ACTIVATION_TIMEOUT = 90
 DEFAULT_TIMEOUT = 10
+NULL_MAC = "00:00:00:00:00:00"
 
 
 class CheckMode:
@@ -166,6 +167,20 @@
         c = SysUtil._sysctl_read("/sys/class/net/" + ifname + "/address")
         return Util.mac_norm(c.strip())
 
+    @staticmethod
+    def _link_read_bond_port_perm_hwaddr(ifname):
+        filename = os.path.join(
+            "/sys/class/net",
+            ifname,
+            # wokeignore:rule=slave
+            "bonding_slave",
+            "perm_hwaddr",
+        )
+        if not os.path.exists(filename):
+            return None
+        c = SysUtil._sysctl_read(filename)
+        return Util.mac_norm(c.strip())
+
     @staticmethod
     def _link_read_permaddress(ifname):
         return ethtool.get_perm_addr(ifname)
@@ -186,6 +201,13 @@
                 "ifname": ifname,
                 "address": SysUtil._link_read_address(ifname),
                 "perm-address": SysUtil._link_read_permaddress(ifname),
+                # When an interface is added as a port of a bonding device, its MAC
+                # address might change, we need to retrieve and preserve the original
+                # MAC address to ensure the user-provided interface name and MAC match
+                # correctly.
+                "bond-port-perm-hwaddr": SysUtil._link_read_bond_port_perm_hwaddr(
+                    ifname
+                ),
             }
         return links
 
@@ -222,31 +244,29 @@
         return linkinfos
 
     @classmethod
-    def link_info_find(cls, refresh=False, mac=None, ifname=None):
-        if mac is not None:
+    def link_info_find(cls, mac=None, ifname=None):
+        if mac:
             mac = Util.mac_norm(mac)
-        for linkinfo in cls.link_infos(refresh).values():
-            perm_address = linkinfo.get("perm-address", None)
-            current_address = linkinfo.get("address", None)
 
-            # Match by perm-address (prioritized)
-            if mac is not None and perm_address not in [None, "00:00:00:00:00:00"]:
-                if mac == perm_address:
-                    return linkinfo
+        result = None
 
-            # Fallback to match by address
-            if mac is not None and (perm_address in [None, "00:00:00:00:00:00"]):
-                if mac == current_address:
-                    matched_by_address = linkinfo  # Save for potential fallback
+        for linkinfo in cls.link_infos().values():
+            perm_address = linkinfo.get("perm-address", NULL_MAC)
+            current_address = linkinfo.get("address", NULL_MAC)
+            bond_port_perm_hwaddr = linkinfo.get("bond-port-perm-hwaddr", NULL_MAC)
 
-            if ifname is not None and ifname == linkinfo.get("ifname", None):
-                return linkinfo
+            if ifname and ifname == linkinfo["ifname"]:
+                result = linkinfo
+                break
 
-        # Return fallback match by address if no perm-address match found
-        if "matched_by_address" in locals():
-            return matched_by_address
+            if mac:
+                if mac == perm_address:
+                    result = linkinfo
+                    break
+                elif mac in (current_address, bond_port_perm_hwaddr):
+                    result = linkinfo
 
-        return None
+        return result
 
 
 ###############################################################################
@@ -2182,13 +2202,23 @@
                                     "infiniband interface exists"
                                     % (connection["interface_name"]),
                                 )
-                if li_mac and li_ifname and li_mac != li_ifname:
-                    self.log_fatal(
-                        idx,
-                        "profile specifies interface_name '%s' and mac '%s' but no "
-                        "such interface exists"
-                        % (connection["interface_name"], connection["mac"]),
-                    )
+                    elif connection["mac"]:
+                        perm_address = li_ifname.get("perm-address", NULL_MAC)
+                        current_address = li_ifname.get("address", NULL_MAC)
+                        bond_port_perm_hwaddr = li_ifname.get(
+                            "bond-port-perm-hwaddr", NULL_MAC
+                        )
+                        if (perm_address not in (NULL_MAC, connection["mac"])) or (
+                            perm_address == NULL_MAC
+                            and connection["mac"]
+                            not in (current_address, bond_port_perm_hwaddr)
+                        ):
+                            self.log_fatal(
+                                idx,
+                                "profile specifies interface_name '%s' and mac '%s' "
+                                "but no such interface exists"
+                                % (connection["interface_name"], connection["mac"]),
+                            )
 
     def start_transaction(self):
         """Hook before making changes"""

tests/ensure_provider_tests.py

@@ -74,11 +74,13 @@
     },
     "playbooks/tests_ignore_auto_dns.yml": {},
     "playbooks/tests_bond_options.yml": {},
+    "playbooks/tests_bond_port_match_by_mac.yml": {},
     "playbooks/tests_eth_dns_support.yml": {},
     "playbooks/tests_dummy.yml": {},  # wokeignore:rule=dummy
     "playbooks/tests_infiniband.yml": {},
     "playbooks/tests_ipv6_disabled.yml": {},
     "playbooks/tests_ipv6_dns_search.yml": {},
+    "playbooks/tests_mac_address_match.yml": {},
     "playbooks/tests_provider.yml": {
         MINIMUM_VERSION: "'1.20.0'",
         "comment": "# NetworKmanager 1.20.0 added support for forgetting profiles",

tests/playbooks/tests_bond_port_match_by_mac.yml

@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: BSD-3-Clause
+---
+- name: Play for creating the connection to match the port device
+    based on the perm_hwaddr
+  hosts: all
+  vars:
+    controller_profile: bond0
+    controller_device: nm-bond
+    port1_profile: bond0.0
+    dhcp_interface1: test1
+    port2_profile: bond0.1
+    dhcp_interface2: test2
+    profile: test2conn
+    interface: test2
+  tasks:
+    - name: Test creating the connection to match the port device
+        based on the perm_hwaddr
+      tags:
+        - tests::bond:create
+      block:
+        - name: Include the task 'run_test.yml'
+          include_tasks: tasks/run_test.yml
+          vars:
+            lsr_description: Given two DHCP-enabled network interfaces,
+              when creating a bond profile with them,
+              then we can still create the connection to match the port device
+              based on the perm_hwaddr
+            lsr_setup:
+              - tasks/create_test_interfaces_with_dhcp.yml
+              - tasks/assert_dhcp_device_present.yml
+            lsr_test:
+              - tasks/create_bond_profile.yml
+              - tasks/create_bond_port_match_by_mac.yml
+            lsr_assert:
+              - tasks/assert_controller_device_present.yml
+              - tasks/assert_profile_present.yml
+            lsr_cleanup:
+              - tasks/cleanup_bond_profile+device.yml
+              - tasks/remove_test_interfaces_with_dhcp.yml
+              - tasks/check_network_dns.yml

tests/playbooks/tests_mac_address_match.yml

@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: BSD-3-Clause
+---
+- name: Play for testing MAC address match on device
+  hosts: all
+  vars:
+    # This test requires an Ethernet interface whose permanent
+    # MAC address matches its current MAC address. Ensure that the
+    # specified interface meets this condition.
+    #
+    # Two VLAN profiles are defined to test deterministic behavior when fetching
+    # link information from sysfs. The issue being tested arises when `os.listdir(path)`
+    # returns interfaces in an arbitrary order, potentially listing a VLAN device
+    # before its parent interface. This can cause intermittent "no such interface
+    # exists" errors when applying configurations repeatedly.
+    #
+    # - `vlan_profile1` (e.g., `eth1.3732`) is named with the VLAN ID appended
+    #   after the parent interface, following the standard `<parent>.<vlan_id>` format.
+    # - `vlan_profile2` (e.g., `120-vlan`) has a fixed name, designed to test a scenario
+    #   where lexicographic sorting causes the VLAN to appear before its parent interface.
+    interface: "{{ lookup('env', 'MAC_ADDR_MATCH_INTERFACE') | default('eth1', true) }}"
+    profile: "{{ interface }}"
+    vlan_profile1: "{{ interface }}.3732"
+    vlan_profile2: "120-vlan"
+    lsr_fail_debug:
+      - __network_connections_result
+  tags:
+    - "tests::match"
+  tasks:
+    - name: Show playbook name
+      debug:
+        msg: "this is: playbooks/tests_mac_address_match.yml"
+      tags:
+        - always
+
+    - name: Install ethtool (test dependency)
+      package:
+        name: ethtool
+        state: present
+        use: "{{ (__network_is_ostree | d(false)) |
+          ternary('ansible.posix.rhel_rpm_ostree', omit) }}"
+
+    - name: Retrieve MAC address using ethtool
+      command: ethtool -P {{ interface }}
+      register: mac_address_result
+      changed_when: false
+      failed_when: mac_address_result.rc != 0
+
+    - name: Set the MAC address variable
+      set_fact:
+        mac: "{{ mac_address_result.stdout_lines[-1].split(' ')[-1] }}"
+
+    - name: Display the retrieved MAC address
+      debug:
+        msg: "Retrieved MAC address for {{ interface }}: {{ mac }}"
+
+    - name: Test the MAC address match
+      tags:
+        - tests::match:match
+      block:
+        - name: Include the task 'run_test.yml'
+          include_tasks: tasks/run_test.yml
+          vars:
+            lsr_description: Test MAC address match on device
+            lsr_setup:
+              - tasks/find+remove_profile.yml
+              - tasks/assert_profile_absent.yml
+            lsr_test:
+              - tasks/create_mac_address_match.yml
+              - tasks/create_mac_address_match.yml
+            lsr_assert:
+              - tasks/assert_profile_present.yml
+              - tasks/assert_network_connections_succeeded.yml
+            lsr_cleanup:
+              - tasks/cleanup_vlan_and_parent_profile+device.yml
+              - tasks/check_network_dns.yml

tests/tasks/assert_network_connections_succeeded.yml

@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: BSD-3-Clause
+---
+- name: Assert that configuring network connections is succeeded
+  assert:
+    that:
+      - __network_connections_result.failed == false
+    msg: Configuring network connections is failed with the error
+      "{{ __network_connections_result.stderr }}"

tests/tasks/cleanup_vlan_and_parent_profile+device.yml

@@ -0,0 +1,26 @@
+# SPDX-License-Identifier: BSD-3-Clause
+---
+- name: Clean up the test devices and the connection profiles
+  tags:
+    - "tests::cleanup"
+  block:
+    - name: Import network role
+      import_role:
+        name: linux-system-roles.network
+      vars:
+        network_connections:
+          - name: "{{ profile }}"
+            persistent_state: absent
+            state: down
+          - name: "{{ vlan_profile1 }}"
+            persistent_state: absent
+            state: down
+          - name: "{{ vlan_profile2 }}"
+            persistent_state: absent
+            state: down
+      failed_when: false
+    - name: Delete the device '{{ interface }}'
+      command: ip link del {{ interface }}
+      failed_when: false
+      changed_when: false
+...

tests/tasks/create_bond_port_match_by_mac.yml

@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: BSD-3-Clause
+---
+- name: Retrieve perm_hwaddr using ethtool
+  # wokeignore:rule=slave
+  command: cat /sys/class/net/{{ interface }}/bonding_slave/perm_hwaddr
+  register: mac_address_result
+  changed_when: false
+  failed_when: mac_address_result.rc != 0
+- name: Set the MAC address variable
+  set_fact:
+    mac: "{{ mac_address_result.stdout_lines[-1].split(' ')[-1] }}"
+- name: Display the retrieved MAC address
+  debug:
+    msg: "Retrieved MAC address for {{ interface }}: {{ mac }}"
+- name: Test matching the port device based on the perm_hwaddr
+  import_role:
+    name: linux-system-roles.network
+  vars:
+    network_connections:
+      - name: "{{ profile }}"
+        state: up
+        type: ethernet
+        interface_name: "{{ interface }}"
+        mac: "{{ mac }}"
+...

tests/tasks/create_mac_address_match.yml

@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: BSD-3-Clause
+---
+- name: Include network role
+  include_role:
+    name: linux-system-roles.network
+  vars:
+    network_connections:
+      - name: "{{ interface }}"
+        state: up
+        persistent_state: present
+        autoconnect: true
+        type: ethernet
+        interface_name: "{{ interface }}"
+        mac: "{{ mac }}"
+        ip:
+          dhcp4: false
+          auto6: false
+
+      - name: "{{ vlan_profile1 }}"
+        state: up
+        persistent_state: present
+        type: vlan
+        parent: "{{ interface }}"
+        vlan:
+          id: 3732
+        autoconnect: true
+        ip:
+          auto_gateway: false
+          gateway4: 10.10.0.1
+          address: 10.10.0.6/24
+          dhcp4: false
+          auto6: false
+
+      - name: "{{ vlan_profile2 }}"
+        state: up
+        persistent_state: present
+        type: vlan
+        parent: "{{ interface }}"
+        vlan:
+          id: 120
+        autoconnect: true
+        ip:
+          auto_gateway: false
+          gateway4: 10.10.120.1
+          address: 10.10.120.120/24
+          dhcp4: false
+          auto6: false
+- name: Show result
+  debug:
+    var: __network_connections_result
+...

tests/tasks/find+remove_profile.yml

@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: BSD-3-Clause
+---
+- name: Get connection profile for '{{ interface }}'
+  command: "nmcli -g GENERAL.CONNECTION device show {{ interface }}"
+  register: connection_name
+  changed_when: false
+
+- name: Bring down and delete the connection profile for '{{ interface }}'
+  include_role:
+    name: linux-system-roles.network
+  vars:
+    network_connections:
+      - name: "{{ connection_name.stdout }}"
+        persistent_state: absent
+        state: down