diff --git a/.github/workflows/autopkgtest.yml b/.github/workflows/autopkgtest.yml index c67de0804..a142991b4 100644 --- a/.github/workflows/autopkgtest.yml +++ b/.github/workflows/autopkgtest.yml @@ -47,6 +47,7 @@ jobs: pull-lp-source netplan.io cp -r netplan.io-*/debian . rm -r debian/patches/ # clear any distro patches + sed -i 's|iproute2,|iproute2, ethtool,|' debian/control # add ethtool as a dependency of netplan.io temporarily sed -i 's| python3-gi,| python3-gi, python3-packaging,|' debian/tests/control # needed for the 'routing' test (nm_version) sed -i 's|bytecompile=-1|bytecompile=-1 -Dtesting=false|' debian/rules # drop after v1.1 TAG=$(git describe --tags $(git rev-list --tags --max-count=1)) # find latest (stable) tag diff --git a/.github/workflows/debci.yml b/.github/workflows/debci.yml index 25b1efa98..4eeae32a4 100644 --- a/.github/workflows/debci.yml +++ b/.github/workflows/debci.yml @@ -50,6 +50,7 @@ jobs: dget -u "https://deb.debian.org/debian/pool/main/n/netplan.io/netplan.io_$V.dsc" cp -r netplan.io-*/debian . rm -r debian/patches/ # clear any distro patches + sed -i 's|iproute2,|iproute2, ethtool,|' debian/control # add ethtool as a dependency of netplan.io temporarily TAG=$(git describe --tags $(git rev-list --tags --max-count=1)) # find latest (stable) tag REV=$(git rev-parse --short HEAD) # get current git revision VER="$TAG+git~$REV" diff --git a/netplan_cli/cli/commands/apply.py b/netplan_cli/cli/commands/apply.py index a7b254065..f6b94cdcb 100644 --- a/netplan_cli/cli/commands/apply.py +++ b/netplan_cli/cli/commands/apply.py @@ -27,7 +27,6 @@ import shutil import tempfile import filecmp -import netifaces import time from .. import utils @@ -117,7 +116,7 @@ def command_apply(self, run_generate=True, sync=False, exit_on_error=True, state old_ovs_glob.remove(ovs_cleanup_service) old_files_ovs = bool(old_ovs_glob) old_nm_glob = glob.glob('/run/NetworkManager/system-connections/netplan-*') - nm_ifaces = utils.nm_interfaces(old_nm_glob, netifaces.interfaces()) + nm_ifaces = utils.nm_interfaces(old_nm_glob, utils.get_interfaces()) old_files_nm = bool(old_nm_glob) restart_networkd = False @@ -151,7 +150,7 @@ def command_apply(self, run_generate=True, sync=False, exit_on_error=True, state if comp.left_only or comp.right_only or comp.diff_files: restart_networkd = True - devices = netifaces.interfaces() + devices = utils.get_interfaces() restart_ovs_glob = glob.glob('/run/systemd/system/netplan-ovs-*') # Ignore netplan-ovs-cleanup.service, as it can always be there @@ -203,7 +202,7 @@ def command_apply(self, run_generate=True, sync=False, exit_on_error=True, state logging.debug('no netplan generated NM configuration exists') # Refresh devices now; restarting a backend might have made something appear. - devices = netifaces.interfaces() + devices = utils.get_interfaces() # evaluate config for extra steps we need to take (like renaming) # for now, only applies to non-virtual (real) devices. @@ -223,7 +222,7 @@ def command_apply(self, run_generate=True, sync=False, exit_on_error=True, state # the interface name, if it was already renamed once (e.g. during boot), # because of the NamePolicy=keep default: # https://www.freedesktop.org/software/systemd/man/systemd.net-naming-scheme.html - devices = netifaces.interfaces() + devices = utils.get_interfaces() for device in devices: logging.debug('netplan triggering .link rules for %s', device) try: @@ -239,7 +238,7 @@ def command_apply(self, run_generate=True, sync=False, exit_on_error=True, state except subprocess.CalledProcessError: logging.debug('Ignoring device without syspath: %s', device) - devices_after_udev = netifaces.interfaces() + devices_after_udev = utils.get_interfaces() # apply some more changes manually for iface, settings in changes.items(): # rename non-critical network interfaces @@ -331,23 +330,6 @@ def command_apply(self, run_generate=True, sync=False, exit_on_error=True, state if 'lo' in nm_interfaces and loopback_connection: utils.nm_bring_interface_up(loopback_connection) - @staticmethod - def is_composite_member(composites, phy): - """ - Is this physical interface a member of a 'composite' virtual - interface? (bond, bridge) - """ - for composite in composites: - for _, settings in composite.items(): - if not type(settings) is dict: - continue - members = settings.get('interfaces', []) - for iface in members: - if iface == phy: - return True - - return False - @staticmethod def clear_virtual_links(prev_links, curr_links, devices=[]): """ @@ -382,7 +364,6 @@ def process_link_changes(interfaces, config_manager: ConfigManager): # pragma: """ changes = {} - composite_interfaces = [config_manager.bridges, config_manager.bonds] # Find physical interfaces which need a rename # But do not rename virtual interfaces @@ -392,11 +373,6 @@ def process_link_changes(interfaces, config_manager: ConfigManager): # pragma: continue # Skip if no new name needs to be set if not netdef._has_match: continue # Skip if no match for current name is given - if NetplanApply.is_composite_member(composite_interfaces, netdef.id): - logging.debug('Skipping composite member {}'.format(netdef.id)) - # do not rename members of virtual devices. MAC addresses - # may be the same for all interface members. - continue # Find current name of the interface, according to match conditions and globs (name, mac, driver) current_iface_name = utils.find_matching_iface(interfaces, netdef) if not current_iface_name: diff --git a/netplan_cli/cli/sriov.py b/netplan_cli/cli/sriov.py index b0e0c6be0..b12fef36c 100644 --- a/netplan_cli/cli/sriov.py +++ b/netplan_cli/cli/sriov.py @@ -27,8 +27,6 @@ from ..configmanager import ConfigurationError import netplan -import netifaces - # PCIDevice class originates from mlnx_switchdev_mode/sriovify.py # Copyright 2019 Canonical Ltd, Apache License, Version 2.0 @@ -223,7 +221,7 @@ def _get_interface_name_for_netdef(netdef: netplan.NetDefinition) -> Optional[st Try to match a netdef with the real system network interface. Throws ConfigurationError if there is more than one match. """ - interfaces: List[str] = netifaces.interfaces() + interfaces: List[str] = utils.get_interfaces() if netdef._has_match: # now here it's a bit tricky set_name: str = netdef.set_name @@ -455,7 +453,7 @@ def apply_sriov_config(config_manager, rootdir='/'): them and perform all other necessary setup. """ config_manager.parse() - interfaces = netifaces.interfaces() + interfaces = utils.get_interfaces() np_state = config_manager.np_state # for sr-iov devices, we identify VFs by them having a link: field @@ -487,7 +485,7 @@ def apply_sriov_config(config_manager, rootdir='/'): # also, since the VF number changed, the interfaces list also # changed, so we need to refresh it - interfaces = netifaces.interfaces() + interfaces = utils.get_interfaces() # now in theory we should have all the new VFs set up and existing; # this is needed because we will have to now match the defined VF diff --git a/netplan_cli/cli/utils.py b/netplan_cli/cli/utils.py index c9f9ce201..105e998fa 100644 --- a/netplan_cli/cli/utils.py +++ b/netplan_cli/cli/utils.py @@ -20,9 +20,10 @@ import logging import argparse import subprocess -import netifaces import fnmatch import re +import json +from typing import Optional from ..configmanager import ConfigurationError from netplan import NetDefinition, NetplanException @@ -196,10 +197,43 @@ def get_interface_driver_name(interface, only_down=False): # pragma: nocover (c return driver_name -def get_interface_macaddress(interface): - # return an empty list (and string) if no LL data can be found - link = netifaces.ifaddresses(interface).get(netifaces.AF_LINK, [{}])[0] - return link.get('addr', '') +def _get_permanent_macaddress(interface: str) -> Optional[str]: + mac = None + try: + out = subprocess.check_output(['ethtool', '-P', interface]).decode('utf-8') + split = out.split(': ') + if len(split) == 2 and is_valid_macaddress(split[1].strip()): + mac = split[1].strip() + except Exception: + return mac + + return mac + + +def _get_macaddress(interface: str) -> Optional[str]: + try: + with open(f'/sys/class/net/{interface}/address') as f: + return f.read().strip() + except Exception: + return None + + +def get_interfaces() -> list[str]: + try: + out = subprocess.check_output(['ip', '--json', 'link']).decode('utf-8') + out_json = json.loads(out) + return [iface['ifname'] for iface in out_json] + except Exception: + return [] + + +def get_interface_macaddress(interface: str) -> Optional[str]: + mac = _get_permanent_macaddress(interface) + + if not mac: + mac = _get_macaddress(interface) + + return mac def find_matching_iface(interfaces: list, netdef): diff --git a/tests/cli/test_units.py b/tests/cli/test_units.py index cbdd08a26..c50cf8e78 100644 --- a/tests/cli/test_units.py +++ b/tests/cli/test_units.py @@ -42,21 +42,6 @@ def setUp(self): def tearDown(self): shutil.rmtree(self.tmproot) - def test_is_composite_member(self): - res = NetplanApply.is_composite_member([{'br0': {'interfaces': ['eth0']}}], 'eth0') - self.assertTrue(res) - - def test_is_composite_member_false(self): - res = NetplanApply.is_composite_member([ - {'br0': {'interfaces': ['eth42']}}, - {'bond0': {'interfaces': ['eth1']}} - ], 'eth0') - self.assertFalse(res) - - def test_is_composite_member_with_renderer(self): - res = NetplanApply.is_composite_member([{'renderer': 'networkd', 'br0': {'interfaces': ['eth0']}}], 'eth0') - self.assertTrue(res) - @patch('subprocess.check_call') def test_clear_virtual_links(self, mock): # simulate as if 'tun3' would have already been delete another way, diff --git a/tests/test_sriov.py b/tests/test_sriov.py index 8b625f3e7..f0dcaa04b 100644 --- a/tests/test_sriov.py +++ b/tests/test_sriov.py @@ -135,7 +135,7 @@ def _prepare_sysfs_dir_structure(self, pf=('enp2', '0000:00:1f.0'), for i in range(len(vfs)): os.symlink(os.path.join('../../..', vfs[i][1]), os.path.join(pf_dev_path, 'virtfn'+str(i))) - @patch('netifaces.interfaces') + @patch('netplan_cli.cli.utils.get_interfaces') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') def test_get_vf_count_vfs_and_pfs(self, gim, gidn, ifaces): @@ -207,7 +207,7 @@ def test_get_vf_count_vfs_and_pfs(self, gim, gidn, ifaces): {'enp1': 'enp1', 'enp2': 'enp2', 'enp3': 'enp3', 'enpx': 'enp5', 'enp8': 'enp8', 'enp10': 'enp10'}) - @patch('netifaces.interfaces') + @patch('netplan_cli.cli.utils.get_interfaces') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') def test_get_physical_functions(self, gim, gidn, ifaces): @@ -268,7 +268,7 @@ def test_get_physical_functions(self, gim, gidn, ifaces): {'enp1': 'enp1', 'enp2': 'enp2', 'enp3': 'enp3', 'enpx': 'enp5', 'enp8': 'enp8', 'enp10': 'enp10'}) - @patch('netifaces.interfaces') + @patch('netplan_cli.cli.utils.get_interfaces') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') def test_get_vf_number_per_pf(self, gim, gidn, ifaces): @@ -327,7 +327,7 @@ def test_get_vf_number_per_pf(self, gim, gidn, ifaces): vf_counts, {'enp1': 2, 'enp2': 2, 'enp3': 1, 'enp5': 1, 'enp8': 7}) - @patch('netifaces.interfaces') + @patch('netplan_cli.cli.utils.get_interfaces') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') def test_get_virtual_functions(self, gim, gidn, ifaces): @@ -386,7 +386,7 @@ def test_get_virtual_functions(self, gim, gidn, ifaces): {'enp1s16f1', 'enp1s16f2', 'enp2s16f1', 'enp2s16f2', 'enp3s16f1', 'enpxs16f1'}) - @patch('netifaces.interfaces') + @patch('netplan_cli.cli.utils.get_interfaces') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') def test_get_vf_count_vfs_and_pfs_set_name(self, gim, gidn, ifaces): @@ -433,7 +433,7 @@ def test_get_vf_count_vfs_and_pfs_set_name(self, gim, gidn, ifaces): pfs, {'enp1': 'pf1', 'enp8': 'enp8'}) - @patch('netifaces.interfaces') + @patch('netplan_cli.cli.utils.get_interfaces') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') def test_get_vf_count_vfs_and_pfs_many_match(self, gim, gidn, ifaces): @@ -464,7 +464,7 @@ def test_get_vf_count_vfs_and_pfs_many_match(self, gim, gidn, ifaces): self.assertIn('matched more than one interface for a PF device: enpx', str(e.exception)) - @patch('netifaces.interfaces') + @patch('netplan_cli.cli.utils.get_interfaces') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') def test_get_vf_count_vfs_and_pfs_not_enough_explicit(self, gim, gidn, ifaces): @@ -646,7 +646,6 @@ def test_apply_vlan_filter_for_vf_failed_ip_link_set(self, check_call): self.assertIn('failed setting SR-IOV VLAN filter for vlan vlan10', str(e.exception)) - @patch('netifaces.interfaces') @patch('netplan_cli.cli.sriov._get_vf_number_per_pf') @patch('netplan_cli.cli.sriov._get_virtual_functions') @patch('netplan_cli.cli.sriov._get_physical_functions') @@ -655,8 +654,9 @@ def test_apply_vlan_filter_for_vf_failed_ip_link_set(self, check_call): @patch('netplan_cli.cli.sriov.apply_vlan_filter_for_vf') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') - def test_apply_sriov_config(self, gim, gidn, apply_vlan, quirks, - set_numvfs, get_phys, get_virt, get_num, netifs): + @patch('netplan_cli.cli.utils.get_interfaces') + def test_apply_sriov_config(self, netifs, gim, gidn, apply_vlan, quirks, + set_numvfs, get_phys, get_virt, get_num): # set up the environment with open(os.path.join(self.workdir.name, "etc/netplan/test.yaml"), 'w') as fd: print('''network: @@ -711,7 +711,6 @@ def test_apply_sriov_config(self, gim, gidn, apply_vlan, quirks, # only one had a hardware vlan apply_vlan.assert_called_once_with('enp2', 'enp2s16f1', 'vf1.15', 15) - @patch('netifaces.interfaces') @patch('netplan_cli.cli.sriov._get_vf_number_per_pf') @patch('netplan_cli.cli.sriov._get_virtual_functions') @patch('netplan_cli.cli.sriov._get_physical_functions') @@ -720,8 +719,9 @@ def test_apply_sriov_config(self, gim, gidn, apply_vlan, quirks, @patch('netplan_cli.cli.sriov.apply_vlan_filter_for_vf') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') - def test_apply_sriov_config_invalid_vlan(self, gim, gidn, apply_vlan, quirks, - set_numvfs, get_phys, get_virt, get_num, netifs): + @patch('netplan_cli.cli.utils.get_interfaces') + def test_apply_sriov_config_invalid_vlan(self, netifs, gim, gidn, apply_vlan, quirks, + set_numvfs, get_phys, get_virt, get_num): # set up the environment with open(os.path.join(self.workdir.name, "etc/netplan/test.yaml"), 'w') as fd: print('''network: @@ -783,7 +783,6 @@ def test_apply_sriov_invalid_link_no_vf(self): 'either not a VF or has no matches', logs.output[0]) - @patch('netifaces.interfaces') @patch('netplan_cli.cli.sriov._get_vf_number_per_pf') @patch('netplan_cli.cli.sriov._get_virtual_functions') @patch('netplan_cli.cli.sriov._get_physical_functions') @@ -792,8 +791,9 @@ def test_apply_sriov_invalid_link_no_vf(self): @patch('netplan_cli.cli.sriov.apply_vlan_filter_for_vf') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') - def test_apply_sriov_config_too_many_vlans(self, gim, gidn, apply_vlan, quirks, - set_numvfs, get_phys, get_virt, get_num, netifs): + @patch('netplan_cli.cli.utils.get_interfaces') + def test_apply_sriov_config_too_many_vlans(self, netifs, gim, gidn, apply_vlan, quirks, + set_numvfs, get_phys, get_virt, get_num): # set up the environment with open(os.path.join(self.workdir.name, "etc/netplan/test.yaml"), 'w') as fd: print('''network: @@ -842,7 +842,6 @@ def test_apply_sriov_config_too_many_vlans(self, gim, gidn, apply_vlan, quirks, str(e.exception)) self.assertEqual(apply_vlan.call_count, 1) - @patch('netifaces.interfaces') @patch('netplan_cli.cli.sriov._get_vf_number_per_pf') @patch('netplan_cli.cli.sriov._get_virtual_functions') @patch('netplan_cli.cli.sriov._get_physical_functions') @@ -851,8 +850,9 @@ def test_apply_sriov_config_too_many_vlans(self, gim, gidn, apply_vlan, quirks, @patch('netplan_cli.cli.sriov.apply_vlan_filter_for_vf') @patch('netplan_cli.cli.utils.get_interface_driver_name') @patch('netplan_cli.cli.utils.get_interface_macaddress') - def test_apply_sriov_config_many_match(self, gim, gidn, apply_vlan, quirks, - set_numvfs, get_phys, get_virt, get_num, netifs): + @patch('netplan_cli.cli.utils.get_interfaces') + def test_apply_sriov_config_many_match(self, netifs, gim, gidn, apply_vlan, quirks, + set_numvfs, get_phys, get_virt, get_num): # set up the environment with open(os.path.join(self.workdir.name, "etc/netplan/test.yaml"), 'w') as fd: print('''network: @@ -919,7 +919,7 @@ def test_unit_class_PCIDevice(self): self.assertTrue(pcidev.is_vf) @patch('netplan_cli.cli.utils.get_interface_macaddress') - @patch('netifaces.interfaces') + @patch('netplan_cli.cli.utils.get_interfaces') @patch('netplan_cli.cli.sriov._get_vf_number_per_pf') @patch('netplan_cli.cli.sriov._get_virtual_functions') @patch('netplan_cli.cli.sriov._get_physical_functions') @@ -1022,7 +1022,7 @@ def driver_mock_open(*args, **kwargs): @patch('netplan_cli.cli.sriov.unbind_vfs') @patch('netplan_cli.cli.utils.get_interface_macaddress') - @patch('netifaces.interfaces') + @patch('netplan_cli.cli.utils.get_interfaces') @patch('netplan_cli.cli.sriov._get_vf_number_per_pf') @patch('netplan_cli.cli.sriov._get_virtual_functions') @patch('netplan_cli.cli.sriov._get_physical_functions') diff --git a/tests/test_utils.py b/tests/test_utils.py index c7ab0601e..48ef12e98 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -21,7 +21,6 @@ import unittest import tempfile import glob -import netifaces import netplan from contextlib import redirect_stdout @@ -235,15 +234,57 @@ def test_find_matching_iface_name_and_drivers(self, gim, gidn): iface = utils.find_matching_iface(DEVICES, state['netplan-id']) self.assertEqual(iface, 'ens4') - @patch('netifaces.ifaddresses') - def test_interface_macaddress(self, ifaddr): - ifaddr.side_effect = lambda _: {netifaces.AF_LINK: [{'addr': '00:01:02:03:04:05'}]} + @patch('netplan_cli.cli.utils._get_macaddress') + @patch('netplan_cli.cli.utils._get_permanent_macaddress') + def test_interface_macaddress(self, getpm, getm): + getpm.return_value = None + getm.return_value = '00:01:02:03:04:05' self.assertEqual(utils.get_interface_macaddress('eth42'), '00:01:02:03:04:05') - @patch('netifaces.ifaddresses') - def test_interface_macaddress_empty(self, ifaddr): - ifaddr.side_effect = lambda _: {} - self.assertEqual(utils.get_interface_macaddress('eth42'), '') + @patch('builtins.open') + @patch('subprocess.check_output') + def test_interface_macaddress_empty(self, subp, o): + subp.side_effect = Exception + o.side_effect = Exception + self.assertEqual(utils.get_interface_macaddress('eth42'), None) + + @patch('builtins.open') + @patch('subprocess.check_output') + def test_interface_macaddress_empty_not_set(self, subp, o): + subp.return_value = b'Permanent address: not set' + o.side_effect = Exception + self.assertEqual(utils.get_interface_macaddress('eth42'), None) + + @patch('subprocess.check_output') + def test_interface_permanent_macaddress(self, subp): + subp.return_value = b'Permanent address: 00:01:02:03:04:05' + self.assertEqual(utils.get_interface_macaddress('eth42'), '00:01:02:03:04:05') + + @patch('subprocess.check_output') + def test_interface_permanent_macaddress_not_set(self, subp): + subp.return_value = b'Permanent address: not set' + self.assertEqual(utils._get_permanent_macaddress('eth42'), None) + + @patch('builtins.open') + @patch('subprocess.check_output') + def test_interface_nonpermanent_macaddress(self, subp, o): + file = io.StringIO('00:01:02:03:04:05') + subp.side_effect = Exception + o.return_value = file + self.assertEqual(utils.get_interface_macaddress('eth42'), '00:01:02:03:04:05') + + @patch('builtins.open') + @patch('subprocess.check_output') + def test_interface_nonpermanent_macaddress_not_set(self, subp, o): + file = io.StringIO('00:01:02:03:04:05') + subp.return_value = b'Permanent address: not set' + o.return_value = file + self.assertEqual(utils.get_interface_macaddress('eth42'), '00:01:02:03:04:05') + + @patch('subprocess.check_output') + def test_get_interfaces_empty(self, subp): + subp.side_effect = Exception + self.assertListEqual(utils.get_interfaces(), []) def test_systemctl(self): self.mock_systemctl = MockCmd('systemctl')