From dcf224e04b34a9f1b38cb33497feaea5a4394691 Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Mon, 24 Jan 2022 08:58:31 -0500 Subject: [PATCH 1/3] net: introduce find_candidate_nics() find_fallback_nic_on_linux(), etc. provides valuable filtering of network interfaces in an effort to determine the best candidate for the fallback interface. Expose this logic with a new set of methods for finding the candidate network interfaces. These methods can be used by data sources which cannot rely on the fallback interface being the correct choice. Note that the MAC address filtering is now part of find_candidate_nics_on_linux(). This should be consistent behavior as find_fallback_nic_on_linux() never selected an interface without a MAC. find_fallback_nic_on_linux() continues to prefer eth0, but we make no such distinction in the candidate search. Signed-off-by: Chris Patterson --- cloudinit/net/__init__.py | 174 +++++++++++++++++++++---------- tests/unittests/net/test_init.py | 158 ++++++++++++++++++++++++++++ 2 files changed, 278 insertions(+), 54 deletions(-) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 3270e1f760a..26f7594ef34 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -11,7 +11,7 @@ import logging import os import re -from typing import Any, Dict +from typing import Any, Dict, List, Optional from cloudinit import subp, util from cloudinit.net.network_state import ipv4_mask_to_net_prefix @@ -389,8 +389,23 @@ def is_disabled_cfg(cfg): return cfg.get("config") == "disabled" -def find_fallback_nic(blacklist_drivers=None): - """Return the name of the 'fallback' network device.""" +def find_candidate_nics(blacklist_drivers: Optional[List] = None) -> List[str]: + """Get the list of network interfaces viable for networking. + + @return List of interfaces, sorted naturally. + """ + if util.is_FreeBSD() or util.is_DragonFlyBSD(): + return find_candidate_nics_on_freebsd(blacklist_drivers) + elif util.is_NetBSD() or util.is_OpenBSD(): + return find_candidate_nics_on_netbsd_or_openbsd(blacklist_drivers) + else: + return find_candidate_nics_on_linux(blacklist_drivers) + + +def find_fallback_nic( + blacklist_drivers: Optional[List] = None, +) -> Optional[str]: + """Get the name of the 'fallback' network device.""" if util.is_FreeBSD() or util.is_DragonFlyBSD(): return find_fallback_nic_on_freebsd(blacklist_drivers) elif util.is_NetBSD() or util.is_OpenBSD(): @@ -399,37 +414,73 @@ def find_fallback_nic(blacklist_drivers=None): return find_fallback_nic_on_linux(blacklist_drivers) -def find_fallback_nic_on_netbsd_or_openbsd(blacklist_drivers=None): - values = list( - sorted(get_interfaces_by_mac().values(), key=natural_sort_key) - ) - if values: - return values[0] +def find_candidate_nics_on_netbsd_or_openbsd( + blacklist_drivers: Optional[List] = None, +) -> List[str]: + """Get the names of the candidate network devices on NetBSD/OpenBSD. + + @param blacklist_drivers: currently ignored + @return list of sorted interfaces + """ + return sorted(get_interfaces_by_mac().values(), key=natural_sort_key) -def find_fallback_nic_on_freebsd(blacklist_drivers=None): - """Return the name of the 'fallback' network device on FreeBSD. +def find_fallback_nic_on_netbsd_or_openbsd( + blacklist_drivers: Optional[List] = None, +) -> Optional[str]: + """Get the 'fallback' network device name on NetBSD/OpenBSD. @param blacklist_drivers: currently ignored @return default interface, or None + """ + names = find_candidate_nics_on_netbsd_or_openbsd(blacklist_drivers) + if names: + return names[0] + + return None + +def find_candidate_nics_on_freebsd( + blacklist_drivers: Optional[List] = None, +) -> List[str]: + """Get the names of the candidate network devices on FreeBSD. - we'll use the first interface from ``ifconfig -l -u ether`` + @param blacklist_drivers: Currently ignored. + @return List of sorted interfaces. """ stdout, _stderr = subp.subp(["ifconfig", "-l", "-u", "ether"]) values = stdout.split() if values: - return values[0] + return values + # On FreeBSD <= 10, 'ifconfig -l' ignores the interfaces with DOWN # status - values = list(get_interfaces_by_mac().values()) - values.sort() - if values: - return values[0] + return sorted(get_interfaces_by_mac().values(), key=natural_sort_key) + + +def find_fallback_nic_on_freebsd( + blacklist_drivers: Optional[List] = None, +) -> Optional[str]: + """Get the 'fallback' network device name on FreeBSD. + + @param blacklist_drivers: Currently ignored. + @return List of sorted interfaces. + """ + names = find_candidate_nics_on_freebsd(blacklist_drivers) + if names: + return names[0] + return None + + +def find_candidate_nics_on_linux( + blacklist_drivers: Optional[List] = None, +) -> List[str]: + """Get the names of the candidate network devices on Linux. -def find_fallback_nic_on_linux(blacklist_drivers=None): - """Return the name of the 'fallback' network device on Linux.""" + @param blacklist_drivers: Filter out NICs with these drivers. + @return List of sorted interfaces. + """ if not blacklist_drivers: blacklist_drivers = [] @@ -449,36 +500,39 @@ def find_fallback_nic_on_linux(blacklist_drivers=None): msg = "Waiting for udev events to settle" util.log_time(LOG.debug, msg, func=util.udevadm_settle) - # get list of interfaces that could have connections - invalid_interfaces = set(["lo"]) - potential_interfaces = set( - [ - device - for device in get_devicelist() - if device_driver(device) not in blacklist_drivers - ] - ) - potential_interfaces = potential_interfaces.difference(invalid_interfaces) # sort into interfaces with carrier, interfaces which could have carrier, # and ignore interfaces that are definitely disconnected connected = [] possibly_connected = [] - for interface in potential_interfaces: + for interface in get_devicelist(): + if interface == "lo": + continue + driver = device_driver(interface) + if driver in blacklist_drivers: + LOG.debug( + "Ignoring interface with %s driver: %s", driver, interface + ) + continue + if not read_sys_net_safe(interface, "address"): + LOG.debug("Ignoring interface without mac: %s", interface) + continue if interface.startswith("veth"): + LOG.debug("Ignoring veth interface: %s", interface) continue if is_bridge(interface): - # skip any bridges + LOG.debug("Ignoring bridge interface: %s", interface) continue if is_bond(interface): - # skip any bonds + LOG.debug("Ignoring bond interface: %s", interface) continue if is_netfailover(interface): - # ignore netfailover primary/standby interfaces + LOG.debug("Ignoring failover interface: %s", interface) continue carrier = read_sys_net_int(interface, "carrier") if carrier: connected.append(interface) continue + LOG.debug("Interface has no carrier: %s", interface) # check if nic is dormant or down, as this may make a nick appear to # not have a carrier even though it could acquire one when brought # online by dhclient @@ -491,24 +545,36 @@ def find_fallback_nic_on_linux(blacklist_drivers=None): possibly_connected.append(interface) continue - # don't bother with interfaces that might not be connected if there are - # some that definitely are - if connected: - potential_interfaces = connected - else: - potential_interfaces = possibly_connected - - # if eth0 exists use it above anything else, otherwise get the interface - # that we can read 'first' (using the sorted definition of first). - names = list(sorted(potential_interfaces, key=natural_sort_key)) - if DEFAULT_PRIMARY_INTERFACE in names: - names.remove(DEFAULT_PRIMARY_INTERFACE) - names.insert(0, DEFAULT_PRIMARY_INTERFACE) - - # pick the first that has a mac-address - for name in names: - if read_sys_net_safe(name, "address"): - return name + LOG.debug("Interface ignored: %s", interface) + + # Order the NICs: + # 1. DEFAULT_PRIMARY_INTERFACE, if connected. + # 2. Remaining connected interfaces, naturally sorted. + # 3. DEFAULT_PRIMARY_INTERFACE, if possibly connected. + # 4. Remaining possibly connected interfaces, naturally sorted. + sorted_interfaces = [] + for interfaces in [connected, possibly_connected]: + interfaces = sorted(interfaces, key=natural_sort_key) + if DEFAULT_PRIMARY_INTERFACE in interfaces: + interfaces.remove(DEFAULT_PRIMARY_INTERFACE) + interfaces.insert(0, DEFAULT_PRIMARY_INTERFACE) + sorted_interfaces += interfaces + + return sorted_interfaces + + +def find_fallback_nic_on_linux( + blacklist_drivers: Optional[List] = None, +) -> Optional[str]: + """Get the 'fallback' network device name on Linux. + + @param blacklist_drivers: Ignore devices with these drivers. + @return List of sorted interfaces. + """ + names = find_candidate_nics_on_linux(blacklist_drivers) + if names: + return names[0] + return None @@ -872,7 +938,7 @@ def get_interfaces_by_mac(blacklist_drivers=None) -> dict: ) -def get_interfaces_by_mac_on_freebsd(blacklist_drivers=None) -> dict(): +def get_interfaces_by_mac_on_freebsd(blacklist_drivers=None) -> dict: (out, _) = subp.subp(["ifconfig", "-a", "ether"]) # flatten each interface block in a single line @@ -900,7 +966,7 @@ def find_mac(flat_list): return results -def get_interfaces_by_mac_on_netbsd(blacklist_drivers=None) -> dict(): +def get_interfaces_by_mac_on_netbsd(blacklist_drivers=None) -> dict: ret = {} re_field_match = ( r"(?P\w+).*address:\s" @@ -916,7 +982,7 @@ def get_interfaces_by_mac_on_netbsd(blacklist_drivers=None) -> dict(): return ret -def get_interfaces_by_mac_on_openbsd(blacklist_drivers=None) -> dict(): +def get_interfaces_by_mac_on_openbsd(blacklist_drivers=None) -> dict: ret = {} re_field_match = ( r"(?P\w+).*lladdr\s" diff --git a/tests/unittests/net/test_init.py b/tests/unittests/net/test_init.py index 18b3fe5934a..8a9f02a3e23 100644 --- a/tests/unittests/net/test_init.py +++ b/tests/unittests/net/test_init.py @@ -5,6 +5,7 @@ import ipaddress import os import textwrap +from typing import Optional from unittest import mock import httpretty @@ -390,6 +391,163 @@ def test_generate_fallback_finds_first_connected_eth_with_mac(self): self.assertEqual("eth1", net.find_fallback_nic()) +class TestNetFindCandidateNics: + def create_fake_interface( + self, + name: str, + address: Optional[str] = "aa:bb:cc:aa:bb:cc", + carrier: bool = True, + bonding: bool = False, + dormant: bool = False, + driver: str = "fakenic", + bridge: bool = False, + failover_standby: bool = False, + operstate: Optional[str] = None, + ): + interface_path = self.sys_path / name + interface_path.mkdir(parents=True) + + if address is not None: + (interface_path / "address").write_text(str(address)) + + if carrier: + (interface_path / "carrier").write_text("1") + else: + (interface_path / "carrier").write_text("0") + + if bonding: + (interface_path / "bonding").write_text("1") + + if bridge: + (interface_path / "bridge").write_text("1") + + if dormant: + (interface_path / "dormant").write_text("1") + else: + (interface_path / "dormant").write_text("0") + + if operstate: + (interface_path / "operstate").write_text(operstate) + + device_path = interface_path / "device" + device_path.mkdir() + if failover_standby: + driver = "virtio_net" + (interface_path / "master").symlink_to(os.path.join("..", name)) + (device_path / "features").write_text("1" * 64) + + if driver: + (device_path / driver).write_text(driver) + (device_path / "driver").symlink_to(driver) + + @pytest.fixture(autouse=True) + def setup(self, monkeypatch, tmp_path): + self.sys_path = tmp_path / "sys" + monkeypatch.setattr( + net, "get_sys_class_path", lambda: str(self.sys_path) + "/" + ) + monkeypatch.setattr( + net.util, + "is_container", + lambda: False, + ) + monkeypatch.setattr(net.util, "udevadm_settle", lambda: None) + + def test_ignored_interfaces(self): + self.create_fake_interface( + name="ethNoCarrierDormantOperstateIgnored", + carrier=False, + ) + self.create_fake_interface( + name="ethWithoutMacIgnored", + address=None, + ) + self.create_fake_interface(name="vethIgnored", carrier=1) + self.create_fake_interface( + name="bondIgnored", + bonding=True, + ) + self.create_fake_interface( + name="bridgeIgnored", + bridge=True, + ) + self.create_fake_interface( + name="failOverIgnored", + failover_standby=True, + ) + self.create_fake_interface( + name="TestingOperStateIgnored", + carrier=False, + operstate="testing", + ) + self.create_fake_interface( + name="blacklistedDriverIgnored", + driver="bad", + ) + + assert ( + net.find_candidate_nics_on_linux(blacklist_drivers=["bad"]) == [] + ) + + def test_carrier_preferred(self): + self.create_fake_interface(name="eth0", carrier=False, dormant=True) + self.create_fake_interface(name="eth1") + + assert net.find_candidate_nics_on_linux() == ["eth1", "eth0"] + + def test_natural_sort(self): + self.create_fake_interface(name="a") + self.create_fake_interface(name="a1") + self.create_fake_interface(name="a2") + self.create_fake_interface(name="a10") + self.create_fake_interface(name="b1") + + assert net.find_candidate_nics_on_linux() == [ + "a", + "a1", + "a2", + "a10", + "b1", + ] + + def test_eth0_preferred_with_carrier(self): + self.create_fake_interface(name="abc0") + self.create_fake_interface(name="eth0") + + assert net.find_candidate_nics_on_linux() == ["eth0", "abc0"] + + @pytest.mark.parametrize("dormant", [False, True]) + @pytest.mark.parametrize( + "operstate", ["dormant", "down", "lowerlayerdown", "unknown"] + ) + def test_eth0_preferred_after_carrier(self, dormant, operstate): + self.create_fake_interface(name="xeth10") + self.create_fake_interface(name="eth", carrier=False, dormant=True) + self.create_fake_interface( + name="eth0", + carrier=False, + dormant=dormant, + operstate=operstate, + ) + self.create_fake_interface(name="eth1", carrier=False, dormant=True) + self.create_fake_interface( + name="eth2", + carrier=False, + operstate=operstate, + ) + + assert net.find_candidate_nics_on_linux() == [ + "xeth10", + "eth0", + "eth", + "eth1", + "eth2", + ] + + def test_no_nics(self): + assert net.find_candidate_nics_on_linux() == [] + + class TestGetDeviceList(CiTestCase): def setUp(self): super(TestGetDeviceList, self).setUp() From 00255736dbd7e848c79728545f8b730dcfb5ff14 Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Fri, 4 Mar 2022 11:41:53 -0500 Subject: [PATCH 2/3] fix type info Signed-off-by: Chris Patterson --- cloudinit/net/__init__.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 26f7594ef34..dad7b364705 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -389,7 +389,9 @@ def is_disabled_cfg(cfg): return cfg.get("config") == "disabled" -def find_candidate_nics(blacklist_drivers: Optional[List] = None) -> List[str]: +def find_candidate_nics( + blacklist_drivers: Optional[List[str]] = None, +) -> List[str]: """Get the list of network interfaces viable for networking. @return List of interfaces, sorted naturally. @@ -403,7 +405,7 @@ def find_candidate_nics(blacklist_drivers: Optional[List] = None) -> List[str]: def find_fallback_nic( - blacklist_drivers: Optional[List] = None, + blacklist_drivers: Optional[List[str]] = None, ) -> Optional[str]: """Get the name of the 'fallback' network device.""" if util.is_FreeBSD() or util.is_DragonFlyBSD(): @@ -415,7 +417,7 @@ def find_fallback_nic( def find_candidate_nics_on_netbsd_or_openbsd( - blacklist_drivers: Optional[List] = None, + blacklist_drivers: Optional[List[str]] = None, ) -> List[str]: """Get the names of the candidate network devices on NetBSD/OpenBSD. @@ -426,7 +428,7 @@ def find_candidate_nics_on_netbsd_or_openbsd( def find_fallback_nic_on_netbsd_or_openbsd( - blacklist_drivers: Optional[List] = None, + blacklist_drivers: Optional[List[str]] = None, ) -> Optional[str]: """Get the 'fallback' network device name on NetBSD/OpenBSD. @@ -441,7 +443,7 @@ def find_fallback_nic_on_netbsd_or_openbsd( def find_candidate_nics_on_freebsd( - blacklist_drivers: Optional[List] = None, + blacklist_drivers: Optional[List[str]] = None, ) -> List[str]: """Get the names of the candidate network devices on FreeBSD. @@ -459,7 +461,7 @@ def find_candidate_nics_on_freebsd( def find_fallback_nic_on_freebsd( - blacklist_drivers: Optional[List] = None, + blacklist_drivers: Optional[List[str]] = None, ) -> Optional[str]: """Get the 'fallback' network device name on FreeBSD. @@ -474,7 +476,7 @@ def find_fallback_nic_on_freebsd( def find_candidate_nics_on_linux( - blacklist_drivers: Optional[List] = None, + blacklist_drivers: Optional[List[str]] = None, ) -> List[str]: """Get the names of the candidate network devices on Linux. @@ -564,7 +566,7 @@ def find_candidate_nics_on_linux( def find_fallback_nic_on_linux( - blacklist_drivers: Optional[List] = None, + blacklist_drivers: Optional[List[str]] = None, ) -> Optional[str]: """Get the 'fallback' network device name on Linux. From 1d84f8ef740226600e47553bfb6e702c5c7d50e2 Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Tue, 8 Mar 2022 10:58:40 -0500 Subject: [PATCH 3/3] fix for python3.6 w/o tmp_path fixture Signed-off-by: Chris Patterson --- tests/unittests/net/test_init.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unittests/net/test_init.py b/tests/unittests/net/test_init.py index 8a9f02a3e23..f3ad5292b85 100644 --- a/tests/unittests/net/test_init.py +++ b/tests/unittests/net/test_init.py @@ -5,6 +5,7 @@ import ipaddress import os import textwrap +from pathlib import Path from typing import Optional from unittest import mock @@ -441,8 +442,8 @@ def create_fake_interface( (device_path / "driver").symlink_to(driver) @pytest.fixture(autouse=True) - def setup(self, monkeypatch, tmp_path): - self.sys_path = tmp_path / "sys" + def setup(self, monkeypatch, tmpdir): + self.sys_path = Path(tmpdir) / "sys" monkeypatch.setattr( net, "get_sys_class_path", lambda: str(self.sys_path) + "/" )