Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Prioritize find link info by permanent MAC address, with fallback to current address #751

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 18 additions & 20 deletions library/network_connections.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@

DEFAULT_ACTIVATION_TIMEOUT = 90
DEFAULT_TIMEOUT = 10
NULL_MAC = "00:00:00:00:00:00"


class CheckMode:
Expand Down Expand Up @@ -173,7 +174,7 @@
@staticmethod
def _link_infos_fetch():
links = {}
for ifname in os.listdir("/sys/class/net/"):
for ifname in sorted(os.listdir("/sys/class/net/")):

Check warning on line 177 in library/network_connections.py

View check run for this annotation

Codecov / codecov/patch

library/network_connections.py#L177

Added line #L177 was not covered by tests
richm marked this conversation as resolved.
Show resolved Hide resolved
liangwen12year marked this conversation as resolved.
Show resolved Hide resolved
if not os.path.islink("/sys/class/net/" + ifname):
# /sys/class/net may contain certain entries
# that are not interface names, like
Expand Down Expand Up @@ -222,31 +223,28 @@
return linkinfos

@classmethod
def link_info_find(cls, refresh=False, mac=None, ifname=None):
liangwen12year marked this conversation as resolved.
Show resolved Hide resolved
if mac is not None:
def link_info_find(cls, mac=None, ifname=None):
if mac:

Check warning on line 227 in library/network_connections.py

View check run for this annotation

Codecov / codecov/patch

library/network_connections.py#L227

Added line #L227 was not covered by tests
richm marked this conversation as resolved.
Show resolved Hide resolved
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

Check warning on line 230 in library/network_connections.py

View check run for this annotation

Codecov / codecov/patch

library/network_connections.py#L230

Added line #L230 was not covered by tests

# 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)

Check warning on line 234 in library/network_connections.py

View check run for this annotation

Codecov / codecov/patch

library/network_connections.py#L232-L234

Added lines #L232 - L234 were not covered by tests

if ifname is not None and ifname == linkinfo.get("ifname", None):
return linkinfo
if ifname and ifname == linkinfo["ifname"]:
result = linkinfo
break

Check warning on line 238 in library/network_connections.py

View check run for this annotation

Codecov / codecov/patch

library/network_connections.py#L236-L238

Added lines #L236 - L238 were not covered by tests

# Return fallback match by address if no perm-address match found
if "matched_by_address" in locals():
return matched_by_address
if mac:
if perm_address != NULL_MAC and mac == perm_address:
result = linkinfo
break
elif perm_address == NULL_MAC and mac == current_address:
result = linkinfo

Check warning on line 245 in library/network_connections.py

View check run for this annotation

Codecov / codecov/patch

library/network_connections.py#L240-L245

Added lines #L240 - L245 were not covered by tests

return None
return result

Check warning on line 247 in library/network_connections.py

View check run for this annotation

Codecov / codecov/patch

library/network_connections.py#L247

Added line #L247 was not covered by tests


###############################################################################
Expand Down
1 change: 1 addition & 0 deletions tests/ensure_provider_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
"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",
Expand Down
75 changes: 75 additions & 0 deletions tests/playbooks/tests_mac_address_match.yml
Original file line number Diff line number Diff line change
@@ -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
liangwen12year marked this conversation as resolved.
Show resolved Hide resolved
- 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
8 changes: 8 additions & 0 deletions tests/tasks/assert_network_connections_succeeded.yml
Original file line number Diff line number Diff line change
@@ -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 }}"
26 changes: 26 additions & 0 deletions tests/tasks/cleanup_vlan_and_parent_profile+device.yml
Original file line number Diff line number Diff line change
@@ -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
...
51 changes: 51 additions & 0 deletions tests/tasks/create_mac_address_match.yml
Original file line number Diff line number Diff line change
@@ -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
...
15 changes: 15 additions & 0 deletions tests/tasks/find+remove_profile.yml
Original file line number Diff line number Diff line change
@@ -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
23 changes: 23 additions & 0 deletions tests/tests_mac_address_match_nm.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# SPDX-License-Identifier: BSD-3-Clause
# This file was generated by ensure_provider_tests.py
---
# set network provider and gather facts
# yamllint disable rule:line-length
- name: Run playbook 'playbooks/tests_mac_address_match.yml' with nm as provider
hosts: all
tasks:
- name: Include the task 'el_repo_setup.yml'
include_tasks: tasks/el_repo_setup.yml
- name: Set network provider to 'nm'
set_fact:
network_provider: nm
tags:
- always


# The test requires or should run with NetworkManager, therefore it cannot run
# on RHEL/CentOS 6
- name: Import the playbook 'playbooks/tests_mac_address_match.yml'
import_playbook: playbooks/tests_mac_address_match.yml
when:
- ansible_distribution_major_version != '6'
Loading