From 9a97a3f24e196401a9c54e9c7977ef6a03c98aeb Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 23 Jun 2020 10:08:16 -0400 Subject: distros.networking: initial implementation of layout (#391) This commit introduces the initial structure for the "cloudinit.net -> cloudinit.distros.networking Hierarchy" refactor, as detailed in [0]. It also updates that section with some changes driven by this initial implementation, as well as adding a lot more specifics to it. [0] https://cloudinit.readthedocs.io/en/latest/topics/hacking.html#cloudinit-net-cloudinit-distros-networking-hierarchy --- cloudinit/distros/networking.py | 137 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 cloudinit/distros/networking.py (limited to 'cloudinit/distros/networking.py') diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py new file mode 100644 index 00000000..f9842889 --- /dev/null +++ b/cloudinit/distros/networking.py @@ -0,0 +1,137 @@ +import abc + +from cloudinit import net + + +# Type aliases (https://docs.python.org/3/library/typing.html#type-aliases), +# used to make the signatures of methods a little clearer +DeviceName = str +NetworkConfig = dict + + +class Networking(metaclass=abc.ABCMeta): + """The root of the Networking hierarchy in cloud-init. + + This is part of an ongoing refactor in the cloud-init codebase, for more + details see "``cloudinit.net`` -> ``cloudinit.distros.networking`` + Hierarchy" in HACKING.rst for full details. + """ + + def _get_current_rename_info(self) -> dict: + return net._get_current_rename_info() + + def _rename_interfaces(self, renames: list, *, current_info=None) -> None: + return net._rename_interfaces(renames, current_info=current_info) + + def apply_network_config_names(self, netcfg: NetworkConfig) -> None: + return net.apply_network_config_names(netcfg) + + def device_devid(self, devname: DeviceName): + return net.device_devid(devname) + + def device_driver(self, devname: DeviceName): + return net.device_driver(devname) + + def extract_physdevs(self, netcfg: NetworkConfig) -> list: + return net.extract_physdevs(netcfg) + + def find_fallback_nic(self, *, blacklist_drivers=None): + return net.find_fallback_nic(blacklist_drivers=blacklist_drivers) + + def generate_fallback_config( + self, *, blacklist_drivers=None, config_driver: bool = False + ): + return net.generate_fallback_config( + blacklist_drivers=blacklist_drivers, config_driver=config_driver + ) + + def get_devicelist(self) -> list: + return net.get_devicelist() + + def get_ib_hwaddrs_by_interface(self) -> dict: + return net.get_ib_hwaddrs_by_interface() + + def get_ib_interface_hwaddr( + self, devname: DeviceName, ethernet_format: bool + ): + return net.get_ib_interface_hwaddr(devname, ethernet_format) + + def get_interface_mac(self, devname: DeviceName): + return net.get_interface_mac(devname) + + def get_interfaces(self) -> list: + return net.get_interfaces() + + def get_interfaces_by_mac(self) -> dict: + return net.get_interfaces_by_mac() + + def get_master(self, devname: DeviceName): + return net.get_master(devname) + + def interface_has_own_mac( + self, devname: DeviceName, *, strict: bool = False + ) -> bool: + return net.interface_has_own_mac(devname, strict=strict) + + def is_bond(self, devname: DeviceName) -> bool: + return net.is_bond(devname) + + def is_bridge(self, devname: DeviceName) -> bool: + return net.is_bridge(devname) + + def is_connected(self, devname: DeviceName) -> bool: + return net.is_connected(devname) + + def is_physical(self, devname: DeviceName) -> bool: + return net.is_physical(devname) + + def is_present(self, devname: DeviceName) -> bool: + return net.is_present(devname) + + def is_renamed(self, devname: DeviceName) -> bool: + return net.is_renamed(devname) + + def is_up(self, devname: DeviceName) -> bool: + return net.is_up(devname) + + def is_vlan(self, devname: DeviceName) -> bool: + return net.is_vlan(devname) + + def is_wireless(self, devname: DeviceName) -> bool: + return net.is_wireless(devname) + + def master_is_bridge_or_bond(self, devname: DeviceName) -> bool: + return net.master_is_bridge_or_bond(devname) + + def wait_for_physdevs( + self, netcfg: NetworkConfig, *, strict: bool = True + ) -> None: + return net.wait_for_physdevs(netcfg, strict=strict) + + +class BSDNetworking(Networking): + """Implementation of networking functionality shared across BSDs.""" + + pass + + +class LinuxNetworking(Networking): + """Implementation of networking functionality common to Linux distros.""" + + def get_dev_features(self, devname: DeviceName) -> str: + return net.get_dev_features(devname) + + def has_netfail_standby_feature(self, devname: DeviceName) -> bool: + return net.has_netfail_standby_feature(devname) + + def is_netfailover(self, devname: DeviceName) -> bool: + return net.is_netfailover(devname) + + def is_netfail_master(self, devname: DeviceName) -> bool: + return net.is_netfail_master(devname) + + def is_netfail_primary(self, devname: DeviceName) -> bool: + return net.is_netfail_primary(devname) + + def is_netfail_standby(self, devname: DeviceName) -> bool: + return net.is_netfail_standby(devname) -- cgit v1.2.3 From 31b540524fd1dd5ae79bc703f581339fff1883e9 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Wed, 24 Jun 2020 15:22:45 -0400 Subject: net/networking: remove unused functions/methods (#453) Namely, is_connected, is_wireless and is_present. None of these are used in the cloud-init codebase, so remove the dead code (instead of refactoring it). --- HACKING.rst | 3 --- cloudinit/distros/networking.py | 9 --------- cloudinit/net/__init__.py | 22 ---------------------- cloudinit/net/tests/test_init.py | 26 -------------------------- 4 files changed, 60 deletions(-) (limited to 'cloudinit/distros/networking.py') diff --git a/HACKING.rst b/HACKING.rst index 4e5f56d7..d98e1d40 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -486,13 +486,10 @@ categories: * ``interface_has_own_mac`` * ``is_bond`` * ``is_bridge`` - * ``is_connected`` * ``is_physical`` - * ``is_present`` * ``is_renamed`` * ``is_up`` * ``is_vlan`` - * ``is_wireless`` * ``wait_for_physdevs`` * those that directly access ``/sys`` (via helpers) but may be diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index f9842889..eecdccc6 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -79,15 +79,9 @@ class Networking(metaclass=abc.ABCMeta): def is_bridge(self, devname: DeviceName) -> bool: return net.is_bridge(devname) - def is_connected(self, devname: DeviceName) -> bool: - return net.is_connected(devname) - def is_physical(self, devname: DeviceName) -> bool: return net.is_physical(devname) - def is_present(self, devname: DeviceName) -> bool: - return net.is_present(devname) - def is_renamed(self, devname: DeviceName) -> bool: return net.is_renamed(devname) @@ -97,9 +91,6 @@ class Networking(metaclass=abc.ABCMeta): def is_vlan(self, devname: DeviceName) -> bool: return net.is_vlan(devname) - def is_wireless(self, devname: DeviceName) -> bool: - return net.is_wireless(devname) - def master_is_bridge_or_bond(self, devname: DeviceName) -> bool: return net.master_is_bridge_or_bond(devname) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index b40cb154..fd5a1b3e 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -99,10 +99,6 @@ def is_up(devname): return read_sys_net_safe(devname, "operstate", translate=translate) -def is_wireless(devname): - return os.path.exists(sys_dev_path(devname, "wireless")) - - def is_bridge(devname): return os.path.exists(sys_dev_path(devname, "bridge")) @@ -266,28 +262,10 @@ def is_vlan(devname): return 'DEVTYPE=vlan' in uevent.splitlines() -def is_connected(devname): - # is_connected isn't really as simple as that. 2 is - # 'physically connected'. 3 is 'not connected'. but a wlan interface will - # always show 3. - iflink = read_sys_net_safe(devname, "iflink") - if iflink == "2": - return True - if not is_wireless(devname): - return False - LOG.debug("'%s' is wireless, basing 'connected' on carrier", devname) - return read_sys_net_safe(devname, "carrier", - translate={'0': False, '1': True}) - - def is_physical(devname): return os.path.exists(sys_dev_path(devname, "device")) -def is_present(devname): - return os.path.exists(sys_dev_path(devname)) - - def device_driver(devname): """Return the device driver for net device named 'devname'.""" driver = None diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index 1c0a16a8..e36d4387 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -143,12 +143,6 @@ class TestReadSysNet(CiTestCase): write_file(os.path.join(self.sysdir, 'eth0', 'operstate'), state) self.assertFalse(net.is_up('eth0')) - def test_is_wireless(self): - """is_wireless is True when /sys/net/devname/wireless exists.""" - self.assertFalse(net.is_wireless('eth0')) - ensure_file(os.path.join(self.sysdir, 'eth0', 'wireless')) - self.assertTrue(net.is_wireless('eth0')) - def test_is_bridge(self): """is_bridge is True when /sys/net/devname/bridge exists.""" self.assertFalse(net.is_bridge('eth0')) @@ -204,32 +198,12 @@ class TestReadSysNet(CiTestCase): write_file(os.path.join(self.sysdir, 'eth0', 'uevent'), content) self.assertTrue(net.is_vlan('eth0')) - def test_is_connected_when_physically_connected(self): - """is_connected is True when /sys/net/devname/iflink reports 2.""" - self.assertFalse(net.is_connected('eth0')) - write_file(os.path.join(self.sysdir, 'eth0', 'iflink'), "2") - self.assertTrue(net.is_connected('eth0')) - - def test_is_connected_when_wireless_and_carrier_active(self): - """is_connected is True if wireless /sys/net/devname/carrier is 1.""" - self.assertFalse(net.is_connected('eth0')) - ensure_file(os.path.join(self.sysdir, 'eth0', 'wireless')) - self.assertFalse(net.is_connected('eth0')) - write_file(os.path.join(self.sysdir, 'eth0', 'carrier'), "1") - self.assertTrue(net.is_connected('eth0')) - def test_is_physical(self): """is_physical is True when /sys/net/devname/device exists.""" self.assertFalse(net.is_physical('eth0')) ensure_file(os.path.join(self.sysdir, 'eth0', 'device')) self.assertTrue(net.is_physical('eth0')) - def test_is_present(self): - """is_present is True when /sys/net/devname exists.""" - self.assertFalse(net.is_present('eth0')) - ensure_file(os.path.join(self.sysdir, 'eth0', 'device')) - self.assertTrue(net.is_present('eth0')) - class TestGenerateFallbackConfig(CiTestCase): -- cgit v1.2.3 From 882f1a5f2d5bafd08e6900a2782c3affa67c9d86 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 30 Jun 2020 14:19:38 -0400 Subject: networking: refactor is_physical from cloudinit.net (#457) As the first refactor PR, this also includes the initial structure for tests. LP: #1884619 --- cloudinit/distros/networking.py | 16 ++- cloudinit/distros/tests/test_networking.py | 42 ++++++ cloudinit/net/__init__.py | 4 - cloudinit/net/tests/test_init.py | 6 - cloudinit/sources/DataSourceDigitalOcean.py | 2 +- cloudinit/sources/DataSourceOpenNebula.py | 30 +++-- cloudinit/sources/helpers/digitalocean.py | 12 +- tests/unittests/test_datasource/test_opennebula.py | 149 +++++++++++++-------- 8 files changed, 181 insertions(+), 80 deletions(-) create mode 100644 cloudinit/distros/tests/test_networking.py (limited to 'cloudinit/distros/networking.py') diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index eecdccc6..e421a2ce 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -1,4 +1,5 @@ import abc +import os from cloudinit import net @@ -79,8 +80,15 @@ class Networking(metaclass=abc.ABCMeta): def is_bridge(self, devname: DeviceName) -> bool: return net.is_bridge(devname) + @abc.abstractmethod def is_physical(self, devname: DeviceName) -> bool: - return net.is_physical(devname) + """ + Is ``devname`` a physical network device? + + Examples of non-physical network devices: bonds, bridges, tunnels, + loopback devices. + """ + pass def is_renamed(self, devname: DeviceName) -> bool: return net.is_renamed(devname) @@ -103,7 +111,8 @@ class Networking(metaclass=abc.ABCMeta): class BSDNetworking(Networking): """Implementation of networking functionality shared across BSDs.""" - pass + def is_physical(self, devname: DeviceName) -> bool: + raise NotImplementedError() class LinuxNetworking(Networking): @@ -126,3 +135,6 @@ class LinuxNetworking(Networking): def is_netfail_standby(self, devname: DeviceName) -> bool: return net.is_netfail_standby(devname) + + def is_physical(self, devname: DeviceName) -> bool: + return os.path.exists(net.sys_dev_path(devname, "device")) diff --git a/cloudinit/distros/tests/test_networking.py b/cloudinit/distros/tests/test_networking.py new file mode 100644 index 00000000..2acb12f4 --- /dev/null +++ b/cloudinit/distros/tests/test_networking.py @@ -0,0 +1,42 @@ +from unittest import mock + +import pytest + +from cloudinit.distros.networking import BSDNetworking, LinuxNetworking + + +@pytest.yield_fixture +def sys_class_net(tmpdir): + sys_class_net_path = tmpdir.join("sys/class/net") + sys_class_net_path.ensure_dir() + with mock.patch( + "cloudinit.net.get_sys_class_path", + return_value=sys_class_net_path.strpath + "/", + ): + yield sys_class_net_path + + +class TestBSDNetworkingIsPhysical: + def test_raises_notimplementederror(self): + with pytest.raises(NotImplementedError): + BSDNetworking().is_physical("eth0") + + +class TestLinuxNetworkingIsPhysical: + def test_returns_false_by_default(self, sys_class_net): + assert not LinuxNetworking().is_physical("eth0") + + def test_returns_false_if_devname_exists_but_not_physical( + self, sys_class_net + ): + devname = "eth0" + sys_class_net.join(devname).mkdir() + assert not LinuxNetworking().is_physical(devname) + + def test_returns_true_if_device_is_physical(self, sys_class_net): + devname = "eth0" + device_dir = sys_class_net.join(devname) + device_dir.mkdir() + device_dir.join("device").write("") + + assert LinuxNetworking().is_physical(devname) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index fd5a1b3e..9d8c7ba9 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -262,10 +262,6 @@ def is_vlan(devname): return 'DEVTYPE=vlan' in uevent.splitlines() -def is_physical(devname): - return os.path.exists(sys_dev_path(devname, "device")) - - def device_driver(devname): """Return the device driver for net device named 'devname'.""" driver = None diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index e36d4387..eb458c39 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -198,12 +198,6 @@ class TestReadSysNet(CiTestCase): write_file(os.path.join(self.sysdir, 'eth0', 'uevent'), content) self.assertTrue(net.is_vlan('eth0')) - def test_is_physical(self): - """is_physical is True when /sys/net/devname/device exists.""" - self.assertFalse(net.is_physical('eth0')) - ensure_file(os.path.join(self.sysdir, 'eth0', 'device')) - self.assertTrue(net.is_physical('eth0')) - class TestGenerateFallbackConfig(CiTestCase): diff --git a/cloudinit/sources/DataSourceDigitalOcean.py b/cloudinit/sources/DataSourceDigitalOcean.py index e0ef665e..5040ce5b 100644 --- a/cloudinit/sources/DataSourceDigitalOcean.py +++ b/cloudinit/sources/DataSourceDigitalOcean.py @@ -58,7 +58,7 @@ class DataSourceDigitalOcean(sources.DataSource): ipv4LL_nic = None if self.use_ip4LL: - ipv4LL_nic = do_helper.assign_ipv4_link_local() + ipv4LL_nic = do_helper.assign_ipv4_link_local(self.distro) md = do_helper.read_metadata( self.metadata_address, timeout=self.timeout, diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py index c7fdc2a5..12b1f94f 100644 --- a/cloudinit/sources/DataSourceOpenNebula.py +++ b/cloudinit/sources/DataSourceOpenNebula.py @@ -13,6 +13,7 @@ # This file is part of cloud-init. See LICENSE file for license information. import collections +import functools import os import pwd import re @@ -60,10 +61,19 @@ class DataSourceOpenNebula(sources.DataSource): for cdev in candidates: try: if os.path.isdir(self.seed_dir): - results = read_context_disk_dir(cdev, asuser=parseuser) + results = read_context_disk_dir( + cdev, self.distro, asuser=parseuser + ) elif cdev.startswith("/dev"): - results = util.mount_cb(cdev, read_context_disk_dir, - data=parseuser) + # util.mount_cb only handles passing a single argument + # through to the wrapped function, so we have to partially + # apply the function to pass in `distro`. See LP: #1884979 + partially_applied_func = functools.partial( + read_context_disk_dir, + asuser=parseuser, + distro=self.distro, + ) + results = util.mount_cb(cdev, partially_applied_func) except NonContextDiskDir: continue except BrokenContextDiskDir as exc: @@ -129,10 +139,10 @@ class BrokenContextDiskDir(Exception): class OpenNebulaNetwork(object): - def __init__(self, context, system_nics_by_mac=None): + def __init__(self, context, distro, system_nics_by_mac=None): self.context = context if system_nics_by_mac is None: - system_nics_by_mac = get_physical_nics_by_mac() + system_nics_by_mac = get_physical_nics_by_mac(distro) self.ifaces = collections.OrderedDict( [k for k in sorted(system_nics_by_mac.items(), key=lambda k: net.natural_sort_key(k[1]))]) @@ -367,7 +377,7 @@ def parse_shell_config(content, keylist=None, bash=None, asuser=None, return ret -def read_context_disk_dir(source_dir, asuser=None): +def read_context_disk_dir(source_dir, distro, asuser=None): """ read_context_disk_dir(source_dir): read source_dir and return a tuple with metadata dict and user-data @@ -450,15 +460,17 @@ def read_context_disk_dir(source_dir, asuser=None): # http://docs.opennebula.org/5.4/operation/references/template.html#context-section ipaddr_keys = [k for k in context if re.match(r'^ETH\d+_IP.*$', k)] if ipaddr_keys: - onet = OpenNebulaNetwork(context) + onet = OpenNebulaNetwork(context, distro) results['network-interfaces'] = onet.gen_conf() return results -def get_physical_nics_by_mac(): +def get_physical_nics_by_mac(distro): devs = net.get_interfaces_by_mac() - return dict([(m, n) for m, n in devs.items() if net.is_physical(n)]) + return dict( + [(m, n) for m, n in devs.items() if distro.networking.is_physical(n)] + ) # Legacy: Must be present in case we load an old pkl object diff --git a/cloudinit/sources/helpers/digitalocean.py b/cloudinit/sources/helpers/digitalocean.py index f5bbe46a..b545c4d6 100644 --- a/cloudinit/sources/helpers/digitalocean.py +++ b/cloudinit/sources/helpers/digitalocean.py @@ -16,7 +16,7 @@ NIC_MAP = {'public': 'eth0', 'private': 'eth1'} LOG = logging.getLogger(__name__) -def assign_ipv4_link_local(nic=None): +def assign_ipv4_link_local(distro, nic=None): """Bring up NIC using an address using link-local (ip4LL) IPs. On DigitalOcean, the link-local domain is per-droplet routed, so there is no risk of collisions. However, to be more safe, the ip4LL @@ -24,7 +24,7 @@ def assign_ipv4_link_local(nic=None): """ if not nic: - nic = get_link_local_nic() + nic = get_link_local_nic(distro) LOG.debug("selected interface '%s' for reading metadata", nic) if not nic: @@ -54,8 +54,12 @@ def assign_ipv4_link_local(nic=None): return nic -def get_link_local_nic(): - nics = [f for f in cloudnet.get_devicelist() if cloudnet.is_physical(f)] +def get_link_local_nic(distro): + nics = [ + f + for f in cloudnet.get_devicelist() + if distro.networking.is_physical(f) + ] if not nics: return None return min(nics, key=lambda d: cloudnet.read_sys_net_int(d, 'ifindex')) diff --git a/tests/unittests/test_datasource/test_opennebula.py b/tests/unittests/test_datasource/test_opennebula.py index 7c859c8a..80841182 100644 --- a/tests/unittests/test_datasource/test_opennebula.py +++ b/tests/unittests/test_datasource/test_opennebula.py @@ -132,18 +132,18 @@ class TestOpenNebulaDataSource(CiTestCase): def test_seed_dir_non_contextdisk(self): self.assertRaises(ds.NonContextDiskDir, ds.read_context_disk_dir, - self.seed_dir) + self.seed_dir, mock.Mock()) def test_seed_dir_empty1_context(self): populate_dir(self.seed_dir, {'context.sh': ''}) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertIsNone(results['userdata']) self.assertEqual(results['metadata'], {}) def test_seed_dir_empty2_context(self): populate_context_dir(self.seed_dir, {}) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertIsNone(results['userdata']) self.assertEqual(results['metadata'], {}) @@ -153,11 +153,11 @@ class TestOpenNebulaDataSource(CiTestCase): self.assertRaises(ds.BrokenContextDiskDir, ds.read_context_disk_dir, - self.seed_dir) + self.seed_dir, mock.Mock()) def test_context_parser(self): populate_context_dir(self.seed_dir, TEST_VARS) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('metadata' in results) self.assertEqual(TEST_VARS, results['metadata']) @@ -168,7 +168,7 @@ class TestOpenNebulaDataSource(CiTestCase): for k in ('SSH_KEY', 'SSH_PUBLIC_KEY'): my_d = os.path.join(self.tmp, "%s-%i" % (k, c)) populate_context_dir(my_d, {k: '\n'.join(public_keys)}) - results = ds.read_context_disk_dir(my_d) + results = ds.read_context_disk_dir(my_d, mock.Mock()) self.assertTrue('metadata' in results) self.assertTrue('public-keys' in results['metadata']) @@ -182,7 +182,7 @@ class TestOpenNebulaDataSource(CiTestCase): my_d = os.path.join(self.tmp, k) populate_context_dir(my_d, {k: USER_DATA, 'USERDATA_ENCODING': ''}) - results = ds.read_context_disk_dir(my_d) + results = ds.read_context_disk_dir(my_d, mock.Mock()) self.assertTrue('userdata' in results) self.assertEqual(USER_DATA, results['userdata']) @@ -192,7 +192,7 @@ class TestOpenNebulaDataSource(CiTestCase): for k in ('USER_DATA', 'USERDATA'): my_d = os.path.join(self.tmp, k) populate_context_dir(my_d, {k: b64userdata}) - results = ds.read_context_disk_dir(my_d) + results = ds.read_context_disk_dir(my_d, mock.Mock()) self.assertTrue('userdata' in results) self.assertEqual(b64userdata, results['userdata']) @@ -202,7 +202,7 @@ class TestOpenNebulaDataSource(CiTestCase): my_d = os.path.join(self.tmp, k) populate_context_dir(my_d, {k: util.b64e(USER_DATA), 'USERDATA_ENCODING': 'base64'}) - results = ds.read_context_disk_dir(my_d) + results = ds.read_context_disk_dir(my_d, mock.Mock()) self.assertTrue('userdata' in results) self.assertEqual(USER_DATA, results['userdata']) @@ -214,7 +214,7 @@ class TestOpenNebulaDataSource(CiTestCase): for k in ('HOSTNAME', 'PUBLIC_IP', 'IP_PUBLIC', 'ETH0_IP'): my_d = os.path.join(self.tmp, k) populate_context_dir(my_d, {k: PUBLIC_IP}) - results = ds.read_context_disk_dir(my_d) + results = ds.read_context_disk_dir(my_d, mock.Mock()) self.assertTrue('metadata' in results) self.assertTrue('local-hostname' in results['metadata']) @@ -229,7 +229,7 @@ class TestOpenNebulaDataSource(CiTestCase): # without ETH0_MAC # for Older OpenNebula? populate_context_dir(self.seed_dir, {'ETH0_IP': IP_BY_MACADDR}) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('network-interfaces' in results) self.assertTrue( @@ -239,7 +239,7 @@ class TestOpenNebulaDataSource(CiTestCase): # ETH0_IP and ETH0_MAC populate_context_dir( self.seed_dir, {'ETH0_IP': IP_BY_MACADDR, 'ETH0_MAC': MACADDR}) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('network-interfaces' in results) self.assertTrue( @@ -251,7 +251,7 @@ class TestOpenNebulaDataSource(CiTestCase): # "AR = [ TYPE = ETHER ]" populate_context_dir( self.seed_dir, {'ETH0_IP': '', 'ETH0_MAC': MACADDR}) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('network-interfaces' in results) self.assertTrue( @@ -265,7 +265,7 @@ class TestOpenNebulaDataSource(CiTestCase): 'ETH0_MAC': MACADDR, 'ETH0_MASK': '255.255.0.0' }) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('network-interfaces' in results) self.assertTrue( @@ -279,7 +279,7 @@ class TestOpenNebulaDataSource(CiTestCase): 'ETH0_MAC': MACADDR, 'ETH0_MASK': '' }) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('network-interfaces' in results) self.assertTrue( @@ -292,7 +292,7 @@ class TestOpenNebulaDataSource(CiTestCase): 'ETH0_IP6': IP6_GLOBAL, 'ETH0_MAC': MACADDR, }) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('network-interfaces' in results) self.assertTrue( @@ -305,7 +305,7 @@ class TestOpenNebulaDataSource(CiTestCase): 'ETH0_IP6_ULA': IP6_ULA, 'ETH0_MAC': MACADDR, }) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('network-interfaces' in results) self.assertTrue( @@ -319,7 +319,7 @@ class TestOpenNebulaDataSource(CiTestCase): 'ETH0_IP6_PREFIX_LENGTH': IP6_PREFIX, 'ETH0_MAC': MACADDR, }) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('network-interfaces' in results) self.assertTrue( @@ -333,7 +333,7 @@ class TestOpenNebulaDataSource(CiTestCase): 'ETH0_IP6_PREFIX_LENGTH': '', 'ETH0_MAC': MACADDR, }) - results = ds.read_context_disk_dir(self.seed_dir) + results = ds.read_context_disk_dir(self.seed_dir, mock.Mock()) self.assertTrue('network-interfaces' in results) self.assertTrue( @@ -370,7 +370,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): expected = { '02:00:0a:12:01:01': 'ETH0', '02:00:0a:12:0f:0f': 'ETH1', } - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(expected, net.context_devname) def test_get_nameservers(self): @@ -385,21 +385,21 @@ class TestOpenNebulaNetwork(unittest.TestCase): expected = { 'addresses': ['1.2.3.6', '1.2.3.7', '1.2.3.8'], 'search': ['example.com', 'example.org']} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_nameservers('eth0') self.assertEqual(expected, val) def test_get_mtu(self): """Verify get_mtu('device') correctly returns MTU size.""" context = {'ETH0_MTU': '1280'} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_mtu('eth0') self.assertEqual('1280', val) def test_get_ip(self): """Verify get_ip('device') correctly returns IPv4 address.""" context = {'ETH0_IP': PUBLIC_IP} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_ip('eth0', MACADDR) self.assertEqual(PUBLIC_IP, val) @@ -410,7 +410,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): string. """ context = {'ETH0_IP': ''} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_ip('eth0', MACADDR) self.assertEqual(IP_BY_MACADDR, val) @@ -423,7 +423,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'ETH0_IP6': IP6_GLOBAL, 'ETH0_IP6_ULA': '', } expected = [IP6_GLOBAL] - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_ip6('eth0') self.assertEqual(expected, val) @@ -436,7 +436,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'ETH0_IP6': '', 'ETH0_IP6_ULA': IP6_ULA, } expected = [IP6_ULA] - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_ip6('eth0') self.assertEqual(expected, val) @@ -449,7 +449,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'ETH0_IP6': IP6_GLOBAL, 'ETH0_IP6_ULA': IP6_ULA, } expected = [IP6_GLOBAL, IP6_ULA] - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_ip6('eth0') self.assertEqual(expected, val) @@ -458,7 +458,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): Verify get_ip6_prefix('device') correctly returns IPv6 prefix. """ context = {'ETH0_IP6_PREFIX_LENGTH': IP6_PREFIX} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_ip6_prefix('eth0') self.assertEqual(IP6_PREFIX, val) @@ -469,7 +469,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): string. """ context = {'ETH0_IP6_PREFIX_LENGTH': ''} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_ip6_prefix('eth0') self.assertEqual('64', val) @@ -479,7 +479,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): address. """ context = {'ETH0_GATEWAY': '1.2.3.5'} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_gateway('eth0') self.assertEqual('1.2.3.5', val) @@ -489,7 +489,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): address. """ context = {'ETH0_GATEWAY6': IP6_GW} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_gateway6('eth0') self.assertEqual(IP6_GW, val) @@ -498,7 +498,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): Verify get_mask('device') correctly returns IPv4 subnet mask. """ context = {'ETH0_MASK': '255.255.0.0'} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_mask('eth0') self.assertEqual('255.255.0.0', val) @@ -508,7 +508,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): It returns default value '255.255.255.0' if ETH0_MASK has empty string. """ context = {'ETH0_MASK': ''} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_mask('eth0') self.assertEqual('255.255.255.0', val) @@ -517,7 +517,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): Verify get_network('device') correctly returns IPv4 network address. """ context = {'ETH0_NETWORK': '1.2.3.0'} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_network('eth0', MACADDR) self.assertEqual('1.2.3.0', val) @@ -528,7 +528,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): empty string. """ context = {'ETH0_NETWORK': ''} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_network('eth0', MACADDR) self.assertEqual('10.18.1.0', val) @@ -537,7 +537,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): Verify get_field('device', 'name') returns *context* value. """ context = {'ETH9_DUMMY': 'DUMMY_VALUE'} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_field('eth9', 'dummy') self.assertEqual('DUMMY_VALUE', val) @@ -547,7 +547,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): value. """ context = {'ETH9_DUMMY': 'DUMMY_VALUE'} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_field('eth9', 'dummy', 'DEFAULT_VALUE') self.assertEqual('DUMMY_VALUE', val) @@ -557,7 +557,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): value if context value is empty string. """ context = {'ETH9_DUMMY': ''} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_field('eth9', 'dummy', 'DEFAULT_VALUE') self.assertEqual('DEFAULT_VALUE', val) @@ -567,7 +567,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): empty string. """ context = {'ETH9_DUMMY': ''} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_field('eth9', 'dummy') self.assertEqual(None, val) @@ -577,7 +577,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): None. """ context = {'ETH9_DUMMY': None} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) val = net.get_field('eth9', 'dummy') self.assertEqual(None, val) @@ -597,7 +597,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'match': {'macaddress': MACADDR}, 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) # set ETH0_GATEWAY @@ -613,7 +613,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'match': {'macaddress': MACADDR}, 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) @mock.patch(DS_PATH + ".get_physical_nics_by_mac") @@ -632,7 +632,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'match': {'macaddress': MACADDR}, 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) # set ETH0_GATEWAY6 @@ -648,7 +648,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'match': {'macaddress': MACADDR}, 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) @mock.patch(DS_PATH + ".get_physical_nics_by_mac") @@ -669,7 +669,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'match': {'macaddress': MACADDR}, 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) # set ETH0_IP6, ETH0_IP6_ULA, ETH0_IP6_PREFIX_LENGTH @@ -689,7 +689,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): IP6_GLOBAL + '/' + IP6_PREFIX, IP6_ULA + '/' + IP6_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) @mock.patch(DS_PATH + ".get_physical_nics_by_mac") @@ -710,7 +710,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'match': {'macaddress': MACADDR}, 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) # set DNS, ETH0_DNS, ETH0_SEARCH_DOMAIN @@ -730,7 +730,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'match': {'macaddress': MACADDR}, 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) @mock.patch(DS_PATH + ".get_physical_nics_by_mac") @@ -749,7 +749,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'match': {'macaddress': MACADDR}, 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) # set ETH0_MTU @@ -765,14 +765,14 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'match': {'macaddress': MACADDR}, 'addresses': [IP_BY_MACADDR + '/' + IP4_PREFIX]}}} m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork(context) + net = ds.OpenNebulaNetwork(context, mock.Mock()) self.assertEqual(net.gen_conf(), expected) @mock.patch(DS_PATH + ".get_physical_nics_by_mac") def test_eth0(self, m_get_phys_by_mac): for nic in self.system_nics: m_get_phys_by_mac.return_value = {MACADDR: nic} - net = ds.OpenNebulaNetwork({}) + net = ds.OpenNebulaNetwork({}, mock.Mock()) expected = { 'version': 2, 'ethernets': { @@ -782,6 +782,14 @@ class TestOpenNebulaNetwork(unittest.TestCase): self.assertEqual(net.gen_conf(), expected) + @mock.patch(DS_PATH + ".get_physical_nics_by_mac") + def test_distro_passed_through(self, m_get_physical_nics_by_mac): + ds.OpenNebulaNetwork({}, mock.sentinel.distro) + self.assertEqual( + [mock.call(mock.sentinel.distro)], + m_get_physical_nics_by_mac.call_args_list, + ) + def test_eth0_override(self): self.maxDiff = None context = { @@ -800,7 +808,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'ETH0_SEARCH_DOMAIN': '', } for nic in self.system_nics: - net = ds.OpenNebulaNetwork(context, + net = ds.OpenNebulaNetwork(context, mock.Mock(), system_nics_by_mac={MACADDR: nic}) expected = { 'version': 2, @@ -832,7 +840,7 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'ETH0_SEARCH_DOMAIN': 'example.com example.org', } for nic in self.system_nics: - net = ds.OpenNebulaNetwork(context, + net = ds.OpenNebulaNetwork(context, mock.Mock(), system_nics_by_mac={MACADDR: nic}) expected = { @@ -886,7 +894,10 @@ class TestOpenNebulaNetwork(unittest.TestCase): 'ETH3_SEARCH_DOMAIN': 'third.example.com third.example.org', } net = ds.OpenNebulaNetwork( - context, system_nics_by_mac={MAC_1: 'enp0s25', MAC_2: 'enp1s2'}) + context, + mock.Mock(), + system_nics_by_mac={MAC_1: 'enp0s25', MAC_2: 'enp1s2'} + ) expected = { 'version': 2, @@ -926,6 +937,36 @@ class TestParseShellConfig: assert ret == {"foo": "bar", "xx": "foo"} +class TestGetPhysicalNicsByMac: + @pytest.mark.parametrize( + "interfaces_by_mac,physical_devs,expected_return", + [ + # No interfaces => empty return + ({}, [], {}), + # Only virtual interface => empty return + ({"mac1": "virtual0"}, [], {}), + # Only physical interface => it is returned + ({"mac2": "physical0"}, ["physical0"], {"mac2": "physical0"}), + # Combination of physical and virtual => only physical returned + ( + {"mac3": "physical1", "mac4": "virtual1"}, + ["physical1"], + {"mac3": "physical1"}, + ), + ], + ) + def test(self, interfaces_by_mac, physical_devs, expected_return): + distro = mock.Mock() + distro.networking.is_physical.side_effect = ( + lambda devname: devname in physical_devs + ) + with mock.patch( + DS_PATH + ".net.get_interfaces_by_mac", + return_value=interfaces_by_mac, + ): + assert expected_return == ds.get_physical_nics_by_mac(distro) + + def populate_context_dir(path, variables): data = "# Context variables generated by OpenNebula\n" for k, v in variables.items(): -- cgit v1.2.3 From 25289087e44c9c74543248519e37ca1f11b8a711 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 14 Jul 2020 13:31:13 -0400 Subject: networking: refactor wait_for_physdevs from cloudinit.net (#466) * Refactor `cloudinit.net.wait_for_physdevs` to `cloudinit.distros.networking.Networking.wait_for_physdevs` * Split the Linux-specific `udevadm_settle` call out to a separate abstract `Networking.settle` method; implement it on `LinuxNetworking` and add a `NotImplementedError` implementation to `BSDNetworking` * Modify `wait_for_physdevs`s one callsite to use the new location LP: #1884626 --- cloudinit/distros/networking.py | 79 ++++++++++++++- cloudinit/distros/tests/test_networking.py | 152 ++++++++++++++++++++++++++++- cloudinit/net/__init__.py | 38 -------- cloudinit/net/tests/test_init.py | 74 -------------- cloudinit/stages.py | 2 +- 5 files changed, 229 insertions(+), 116 deletions(-) (limited to 'cloudinit/distros/networking.py') diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index e421a2ce..75e69df5 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -1,7 +1,11 @@ import abc +import logging import os -from cloudinit import net +from cloudinit import net, util + + +LOG = logging.getLogger(__name__) # Type aliases (https://docs.python.org/3/library/typing.html#type-aliases), @@ -102,10 +106,72 @@ class Networking(metaclass=abc.ABCMeta): def master_is_bridge_or_bond(self, devname: DeviceName) -> bool: return net.master_is_bridge_or_bond(devname) + @abc.abstractmethod + def settle(self, *, exists=None) -> None: + """Wait for device population in the system to complete. + + :param exists: + An optional optimisation. If given, only perform as much of the + settle process as is required for the given DeviceName to be + present in the system. (This may include skipping the settle + process entirely, if the device already exists.) + :type exists: Optional[DeviceName] + """ + pass + def wait_for_physdevs( self, netcfg: NetworkConfig, *, strict: bool = True ) -> None: - return net.wait_for_physdevs(netcfg, strict=strict) + """Wait for all the physical devices in `netcfg` to exist on the system + + Specifically, this will call `self.settle` 5 times, and check after + each one if the physical devices are now present in the system. + + :param netcfg: + The NetworkConfig from which to extract physical devices to wait + for. + :param strict: + Raise a `RuntimeError` if any physical devices are not present + after waiting. + """ + physdevs = self.extract_physdevs(netcfg) + + # set of expected iface names and mac addrs + expected_ifaces = dict([(iface[0], iface[1]) for iface in physdevs]) + expected_macs = set(expected_ifaces.keys()) + + # set of current macs + present_macs = self.get_interfaces_by_mac().keys() + + # compare the set of expected mac address values to + # the current macs present; we only check MAC as cloud-init + # has not yet renamed interfaces and the netcfg may include + # such renames. + for _ in range(0, 5): + if expected_macs.issubset(present_macs): + LOG.debug("net: all expected physical devices present") + return + + missing = expected_macs.difference(present_macs) + LOG.debug("net: waiting for expected net devices: %s", missing) + for mac in missing: + # trigger a settle, unless this interface exists + devname = expected_ifaces[mac] + msg = "Waiting for settle or {} exists".format(devname) + util.log_time( + LOG.debug, + msg, + func=self.settle, + kwargs={"exists": devname}, + ) + + # update present_macs after settles + present_macs = self.get_interfaces_by_mac().keys() + + msg = "Not all expected physical devices present: %s" % missing + LOG.warning(msg) + if strict: + raise RuntimeError(msg) class BSDNetworking(Networking): @@ -114,6 +180,10 @@ class BSDNetworking(Networking): def is_physical(self, devname: DeviceName) -> bool: raise NotImplementedError() + def settle(self, *, exists=None) -> None: + """BSD has no equivalent to `udevadm settle`; noop.""" + pass + class LinuxNetworking(Networking): """Implementation of networking functionality common to Linux distros.""" @@ -138,3 +208,8 @@ class LinuxNetworking(Networking): def is_physical(self, devname: DeviceName) -> bool: return os.path.exists(net.sys_dev_path(devname, "device")) + + def settle(self, *, exists=None) -> None: + if exists is not None: + exists = net.sys_dev_path(exists) + util.udevadm_settle(exists=exists) diff --git a/cloudinit/distros/tests/test_networking.py b/cloudinit/distros/tests/test_networking.py index 2acb12f4..b9a63842 100644 --- a/cloudinit/distros/tests/test_networking.py +++ b/cloudinit/distros/tests/test_networking.py @@ -2,7 +2,39 @@ from unittest import mock import pytest -from cloudinit.distros.networking import BSDNetworking, LinuxNetworking +from cloudinit import net +from cloudinit.distros.networking import ( + BSDNetworking, + LinuxNetworking, + Networking, +) + +# See https://docs.pytest.org/en/stable/example +# /parametrize.html#parametrizing-conditional-raising +from contextlib import ExitStack as does_not_raise + + +@pytest.yield_fixture +def generic_networking_cls(): + """Returns a direct Networking subclass which errors on /sys usage. + + This enables the direct testing of functionality only present on the + ``Networking`` super-class, and provides a check on accidentally using /sys + in that context. + """ + + class TestNetworking(Networking): + def is_physical(self, *args, **kwargs): + raise NotImplementedError + + def settle(self, *args, **kwargs): + raise NotImplementedError + + error = AssertionError("Unexpectedly used /sys in generic networking code") + with mock.patch( + "cloudinit.net.get_sys_class_path", side_effect=error, + ): + yield TestNetworking @pytest.yield_fixture @@ -40,3 +72,121 @@ class TestLinuxNetworkingIsPhysical: device_dir.join("device").write("") assert LinuxNetworking().is_physical(devname) + + +class TestBSDNetworkingSettle: + def test_settle_doesnt_error(self): + # This also implicitly tests that it doesn't use subp.subp + BSDNetworking().settle() + + +@pytest.mark.usefixtures("sys_class_net") +@mock.patch("cloudinit.distros.networking.util.udevadm_settle", autospec=True) +class TestLinuxNetworkingSettle: + def test_no_arguments(self, m_udevadm_settle): + LinuxNetworking().settle() + + assert [mock.call(exists=None)] == m_udevadm_settle.call_args_list + + def test_exists_argument(self, m_udevadm_settle): + LinuxNetworking().settle(exists="ens3") + + expected_path = net.sys_dev_path("ens3") + assert [ + mock.call(exists=expected_path) + ] == m_udevadm_settle.call_args_list + + +class TestNetworkingWaitForPhysDevs: + @pytest.fixture + def wait_for_physdevs_netcfg(self): + """This config is shared across all the tests in this class.""" + + def ethernet(mac, name, driver=None, device_id=None): + v2_cfg = {"set-name": name, "match": {"macaddress": mac}} + if driver: + v2_cfg["match"].update({"driver": driver}) + if device_id: + v2_cfg["match"].update({"device_id": device_id}) + + return v2_cfg + + physdevs = [ + ["aa:bb:cc:dd:ee:ff", "eth0", "virtio", "0x1000"], + ["00:11:22:33:44:55", "ens3", "e1000", "0x1643"], + ] + netcfg = { + "version": 2, + "ethernets": {args[1]: ethernet(*args) for args in physdevs}, + } + return netcfg + + def test_skips_settle_if_all_present( + self, generic_networking_cls, wait_for_physdevs_netcfg, + ): + networking = generic_networking_cls() + with mock.patch.object( + networking, "get_interfaces_by_mac" + ) as m_get_interfaces_by_mac: + m_get_interfaces_by_mac.side_effect = iter( + [{"aa:bb:cc:dd:ee:ff": "eth0", "00:11:22:33:44:55": "ens3"}] + ) + with mock.patch.object( + networking, "settle", autospec=True + ) as m_settle: + networking.wait_for_physdevs(wait_for_physdevs_netcfg) + assert 0 == m_settle.call_count + + def test_calls_udev_settle_on_missing( + self, generic_networking_cls, wait_for_physdevs_netcfg, + ): + networking = generic_networking_cls() + with mock.patch.object( + networking, "get_interfaces_by_mac" + ) as m_get_interfaces_by_mac: + m_get_interfaces_by_mac.side_effect = iter( + [ + { + "aa:bb:cc:dd:ee:ff": "eth0" + }, # first call ens3 is missing + { + "aa:bb:cc:dd:ee:ff": "eth0", + "00:11:22:33:44:55": "ens3", + }, # second call has both + ] + ) + with mock.patch.object( + networking, "settle", autospec=True + ) as m_settle: + networking.wait_for_physdevs(wait_for_physdevs_netcfg) + m_settle.assert_called_with(exists="ens3") + + @pytest.mark.parametrize( + "strict,expectation", + [(True, pytest.raises(RuntimeError)), (False, does_not_raise())], + ) + def test_retrying_and_strict_behaviour( + self, + strict, + expectation, + generic_networking_cls, + wait_for_physdevs_netcfg, + ): + networking = generic_networking_cls() + with mock.patch.object( + networking, "get_interfaces_by_mac" + ) as m_get_interfaces_by_mac: + m_get_interfaces_by_mac.return_value = {} + + with mock.patch.object( + networking, "settle", autospec=True + ) as m_settle: + with expectation: + networking.wait_for_physdevs( + wait_for_physdevs_netcfg, strict=strict + ) + + assert ( + 5 * len(wait_for_physdevs_netcfg["ethernets"]) + == m_settle.call_count + ) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 9d8c7ba9..322af77b 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -10,7 +10,6 @@ import ipaddress import logging import os import re -from functools import partial from cloudinit import subp from cloudinit import util @@ -494,43 +493,6 @@ def extract_physdevs(netcfg): raise RuntimeError('Unknown network config version: %s' % version) -def wait_for_physdevs(netcfg, strict=True): - physdevs = extract_physdevs(netcfg) - - # set of expected iface names and mac addrs - expected_ifaces = dict([(iface[0], iface[1]) for iface in physdevs]) - expected_macs = set(expected_ifaces.keys()) - - # set of current macs - present_macs = get_interfaces_by_mac().keys() - - # compare the set of expected mac address values to - # the current macs present; we only check MAC as cloud-init - # has not yet renamed interfaces and the netcfg may include - # such renames. - for _ in range(0, 5): - if expected_macs.issubset(present_macs): - LOG.debug('net: all expected physical devices present') - return - - missing = expected_macs.difference(present_macs) - LOG.debug('net: waiting for expected net devices: %s', missing) - for mac in missing: - # trigger a settle, unless this interface exists - syspath = sys_dev_path(expected_ifaces[mac]) - settle = partial(util.udevadm_settle, exists=syspath) - msg = 'Waiting for udev events to settle or %s exists' % syspath - util.log_time(LOG.debug, msg, func=settle) - - # update present_macs after settles - present_macs = get_interfaces_by_mac().keys() - - msg = 'Not all expected physical devices present: %s' % missing - LOG.warning(msg) - if strict: - raise RuntimeError(msg) - - def apply_network_config_names(netcfg, strict_present=True, strict_busy=True): """read the network config and rename devices accordingly. if strict_present is false, then do not raise exception if no devices diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index eb458c39..311ab6f8 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -963,80 +963,6 @@ class TestExtractPhysdevs(CiTestCase): net.extract_physdevs({'version': 3, 'awesome_config': []}) -class TestWaitForPhysdevs(CiTestCase): - - def setUp(self): - super(TestWaitForPhysdevs, self).setUp() - self.add_patch('cloudinit.net.get_interfaces_by_mac', - 'm_get_iface_mac') - self.add_patch('cloudinit.util.udevadm_settle', 'm_udev_settle') - - def test_wait_for_physdevs_skips_settle_if_all_present(self): - physdevs = [ - ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], - ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], - ] - netcfg = { - 'version': 2, - 'ethernets': {args[1]: _mk_v2_phys(*args) - for args in physdevs}, - } - self.m_get_iface_mac.side_effect = iter([ - {'aa:bb:cc:dd:ee:ff': 'eth0', - '00:11:22:33:44:55': 'ens3'}, - ]) - net.wait_for_physdevs(netcfg) - self.assertEqual(0, self.m_udev_settle.call_count) - - def test_wait_for_physdevs_calls_udev_settle_on_missing(self): - physdevs = [ - ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], - ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], - ] - netcfg = { - 'version': 2, - 'ethernets': {args[1]: _mk_v2_phys(*args) - for args in physdevs}, - } - self.m_get_iface_mac.side_effect = iter([ - {'aa:bb:cc:dd:ee:ff': 'eth0'}, # first call ens3 is missing - {'aa:bb:cc:dd:ee:ff': 'eth0', - '00:11:22:33:44:55': 'ens3'}, # second call has both - ]) - net.wait_for_physdevs(netcfg) - self.m_udev_settle.assert_called_with(exists=net.sys_dev_path('ens3')) - - def test_wait_for_physdevs_raise_runtime_error_if_missing_and_strict(self): - physdevs = [ - ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], - ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], - ] - netcfg = { - 'version': 2, - 'ethernets': {args[1]: _mk_v2_phys(*args) - for args in physdevs}, - } - self.m_get_iface_mac.return_value = {} - with self.assertRaises(RuntimeError): - net.wait_for_physdevs(netcfg) - - self.assertEqual(5 * len(physdevs), self.m_udev_settle.call_count) - - def test_wait_for_physdevs_no_raise_if_not_strict(self): - physdevs = [ - ['aa:bb:cc:dd:ee:ff', 'eth0', 'virtio', '0x1000'], - ['00:11:22:33:44:55', 'ens3', 'e1000', '0x1643'], - ] - netcfg = { - 'version': 2, - 'ethernets': {args[1]: _mk_v2_phys(*args) - for args in physdevs}, - } - self.m_get_iface_mac.return_value = {} - net.wait_for_physdevs(netcfg, strict=False) - self.assertEqual(5 * len(physdevs), self.m_udev_settle.call_count) - - class TestNetFailOver(CiTestCase): def setUp(self): diff --git a/cloudinit/stages.py b/cloudinit/stages.py index db8ba64c..69e6b7e1 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -696,7 +696,7 @@ class Init(object): netcfg, src = self._find_networking_config() # ensure all physical devices in config are present - net.wait_for_physdevs(netcfg) + self.distro.networking.wait_for_physdevs(netcfg) # apply renames from config self._apply_netcfg_names(netcfg) -- cgit v1.2.3 From 4fe576516d65feda17ba78e9265a8e494a195e7b Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Wed, 15 Jul 2020 10:26:12 -0400 Subject: cloudinit: remove global disable of pylint W0107 and fix errors (#489) * cloudinit: remove global disable of pylint W0107 and fix errors This includes removing a test class which contained no tests but wasn't detected as empty because of an errant pass statement. * .pylintrc: update disable comment to match arguments --- .pylintrc | 4 +--- cloudinit/config/cc_set_hostname.py | 1 - cloudinit/distros/networking.py | 3 --- cloudinit/distros/ubuntu.py | 2 -- cloudinit/net/cmdline.py | 2 -- cloudinit/net/dhcp.py | 2 -- cloudinit/reporting/handlers.py | 1 - cloudinit/sources/DataSourceAzure.py | 1 - cloudinit/sources/__init__.py | 2 -- cloudinit/sources/helpers/netlink.py | 1 - cloudinit/sources/helpers/vmware/imc/config_file.py | 1 - cloudinit/sources/helpers/vmware/imc/config_namespace.py | 1 - cloudinit/sources/helpers/vmware/imc/config_source.py | 1 - cloudinit/stages.py | 1 - tests/cloud_tests/platforms/azurecloud/instance.py | 2 -- tests/cloud_tests/platforms/images.py | 1 - tests/cloud_tests/platforms/snapshots.py | 1 - tests/cloud_tests/testcases/base.py | 1 - tests/unittests/test_datasource/test_ibmcloud.py | 7 ------- tests/unittests/test_reporting.py | 1 - 20 files changed, 1 insertion(+), 35 deletions(-) (limited to 'cloudinit/distros/networking.py') diff --git a/.pylintrc b/.pylintrc index f04603b9..94a81d0e 100644 --- a/.pylintrc +++ b/.pylintrc @@ -7,7 +7,6 @@ jobs=4 [MESSAGES CONTROL] # Errors and warings with some filtered: -# W0107(unnecessary-pass) # W0201(attribute-defined-outside-init) # W0212(protected-access) # W0221(arguments-differ) @@ -19,7 +18,6 @@ jobs=4 # W0602(global-variable-not-assigned) # W0603(global-statement) # W0611(unused-import) -# W0612(unused-variable) # W0613(unused-argument) # W0621(redefined-outer-name) # W0622(redefined-builtin) @@ -27,7 +25,7 @@ jobs=4 # W0703(broad-except) # W1401(anomalous-backslash-in-string) -disable=C, F, I, R, W0107, W0201, W0212, W0221, W0222, W0223, W0231, W0311, W0511, W0602, W0603, W0611, W0613, W0621, W0622, W0631, W0703, W1401 +disable=C, F, I, R, W0201, W0212, W0221, W0222, W0223, W0231, W0311, W0511, W0602, W0603, W0611, W0613, W0621, W0622, W0631, W0703, W1401 [REPORTS] diff --git a/cloudinit/config/cc_set_hostname.py b/cloudinit/config/cc_set_hostname.py index 10d6d197..c13020d8 100644 --- a/cloudinit/config/cc_set_hostname.py +++ b/cloudinit/config/cc_set_hostname.py @@ -55,7 +55,6 @@ class SetHostnameError(Exception): This may happen if we attempt to set the hostname early in cloud-init's init-local timeframe as certain services may not be running yet. """ - pass def handle(name, cfg, cloud, log, _args): diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index 75e69df5..10ed249d 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -92,7 +92,6 @@ class Networking(metaclass=abc.ABCMeta): Examples of non-physical network devices: bonds, bridges, tunnels, loopback devices. """ - pass def is_renamed(self, devname: DeviceName) -> bool: return net.is_renamed(devname) @@ -117,7 +116,6 @@ class Networking(metaclass=abc.ABCMeta): process entirely, if the device already exists.) :type exists: Optional[DeviceName] """ - pass def wait_for_physdevs( self, netcfg: NetworkConfig, *, strict: bool = True @@ -182,7 +180,6 @@ class BSDNetworking(Networking): def settle(self, *, exists=None) -> None: """BSD has no equivalent to `udevadm settle`; noop.""" - pass class LinuxNetworking(Networking): diff --git a/cloudinit/distros/ubuntu.py b/cloudinit/distros/ubuntu.py index 23be3bdd..b4c4b0c3 100644 --- a/cloudinit/distros/ubuntu.py +++ b/cloudinit/distros/ubuntu.py @@ -49,7 +49,5 @@ class Distro(debian.Distro): copy.deepcopy(PREFERRED_NTP_CLIENTS)) return self._preferred_ntp_clients - pass - # vi: ts=4 expandtab diff --git a/cloudinit/net/cmdline.py b/cloudinit/net/cmdline.py index 0e83685d..7ca7262b 100755 --- a/cloudinit/net/cmdline.py +++ b/cloudinit/net/cmdline.py @@ -29,12 +29,10 @@ class InitramfsNetworkConfigSource(metaclass=abc.ABCMeta): @abc.abstractmethod def is_applicable(self) -> bool: """Is this initramfs config source applicable to the current system?""" - pass @abc.abstractmethod def render_config(self) -> dict: """Render a v1 network config from the initramfs configuration""" - pass class KlibcNetworkConfigSource(InitramfsNetworkConfigSource): diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index 9e004688..d1d1255e 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -31,12 +31,10 @@ class InvalidDHCPLeaseFileError(Exception): Current uses are DataSourceAzure and DataSourceEc2 during ephemeral boot to scrape metadata. """ - pass class NoDHCPLeaseError(Exception): """Raised when unable to get a DHCP lease.""" - pass class EphemeralDHCPv4(object): diff --git a/cloudinit/reporting/handlers.py b/cloudinit/reporting/handlers.py index 6b9127b6..1986ebdd 100755 --- a/cloudinit/reporting/handlers.py +++ b/cloudinit/reporting/handlers.py @@ -35,7 +35,6 @@ class ReportingHandler(metaclass=abc.ABCMeta): def flush(self): """Ensure ReportingHandler has published all events""" - pass class LogHandler(ReportingHandler): diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 5e25b956..8b34d047 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -688,7 +688,6 @@ class DataSourceAzure(sources.DataSource): except UrlError: # Teardown our EphemeralDHCPv4 context on failure as we retry self._ephemeral_dhcp_ctx.clean_network() - pass finally: if nl_sock: nl_sock.close() diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 923e3cea..c4d60fff 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -78,7 +78,6 @@ class DataSourceNotFoundException(Exception): class InvalidMetaDataException(Exception): """Raised when metadata is broken, unavailable or disabled.""" - pass def process_instance_metadata(metadata, key_path='', sensitive_keys=()): @@ -511,7 +510,6 @@ class DataSource(metaclass=abc.ABCMeta): (e.g. 'ssh-rsa') and key_value is the key itself (e.g. 'AAAAB3NzaC1y...'). """ - pass def _remap_device(self, short_name): # LP: #611137 diff --git a/cloudinit/sources/helpers/netlink.py b/cloudinit/sources/helpers/netlink.py index d377ae3d..a74a3588 100644 --- a/cloudinit/sources/helpers/netlink.py +++ b/cloudinit/sources/helpers/netlink.py @@ -55,7 +55,6 @@ NetlinkHeader = namedtuple('NetlinkHeader', ['length', 'type', 'flags', 'seq', class NetlinkCreateSocketError(RuntimeError): '''Raised if netlink socket fails during create or bind.''' - pass def create_bound_netlink_socket(): diff --git a/cloudinit/sources/helpers/vmware/imc/config_file.py b/cloudinit/sources/helpers/vmware/imc/config_file.py index 602af078..fc034c95 100644 --- a/cloudinit/sources/helpers/vmware/imc/config_file.py +++ b/cloudinit/sources/helpers/vmware/imc/config_file.py @@ -22,7 +22,6 @@ class ConfigFile(ConfigSource, dict): def __init__(self, filename): self._loadConfigFile(filename) - pass def _insertKey(self, key, val): """ diff --git a/cloudinit/sources/helpers/vmware/imc/config_namespace.py b/cloudinit/sources/helpers/vmware/imc/config_namespace.py index 2f29edd4..5899d8f7 100644 --- a/cloudinit/sources/helpers/vmware/imc/config_namespace.py +++ b/cloudinit/sources/helpers/vmware/imc/config_namespace.py @@ -10,6 +10,5 @@ from .config_source import ConfigSource class ConfigNamespace(ConfigSource): """Specifies the Config Namespace.""" - pass # vi: ts=4 expandtab diff --git a/cloudinit/sources/helpers/vmware/imc/config_source.py b/cloudinit/sources/helpers/vmware/imc/config_source.py index 2f8ea546..7ec06a9c 100644 --- a/cloudinit/sources/helpers/vmware/imc/config_source.py +++ b/cloudinit/sources/helpers/vmware/imc/config_source.py @@ -8,6 +8,5 @@ class ConfigSource(object): """Specifies a source for the Config Content.""" - pass # vi: ts=4 expandtab diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 69e6b7e1..765f4aab 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -947,7 +947,6 @@ def _pkl_load(fname): except Exception as e: if os.path.isfile(fname): LOG.warning("failed loading pickle in %s: %s", fname, e) - pass # This is allowed so just return nothing successfully loaded... if not pickle_contents: diff --git a/tests/cloud_tests/platforms/azurecloud/instance.py b/tests/cloud_tests/platforms/azurecloud/instance.py index f1e28a96..a136cf0d 100644 --- a/tests/cloud_tests/platforms/azurecloud/instance.py +++ b/tests/cloud_tests/platforms/azurecloud/instance.py @@ -80,7 +80,6 @@ class AzureCloudInstance(Instance): except CloudError: LOG.debug(('image not found, launching instance with base image, ' 'image_id=%s'), self.image_id) - pass vm_params = { 'name': self.vm_name, @@ -169,7 +168,6 @@ class AzureCloudInstance(Instance): sleep(15) else: LOG.warning('Could not find console log: %s', e) - pass LOG.debug('stopping instance %s', self.image_id) vm_deallocate = \ diff --git a/tests/cloud_tests/platforms/images.py b/tests/cloud_tests/platforms/images.py index 557a5cf6..f047de2e 100644 --- a/tests/cloud_tests/platforms/images.py +++ b/tests/cloud_tests/platforms/images.py @@ -52,6 +52,5 @@ class Image(TargetBase): def destroy(self): """Clean up data associated with image.""" - pass # vi: ts=4 expandtab diff --git a/tests/cloud_tests/platforms/snapshots.py b/tests/cloud_tests/platforms/snapshots.py index 94328982..0f5f8bb6 100644 --- a/tests/cloud_tests/platforms/snapshots.py +++ b/tests/cloud_tests/platforms/snapshots.py @@ -40,6 +40,5 @@ class Snapshot(object): def destroy(self): """Clean up snapshot data.""" - pass # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py index 2e7c6686..4448e0b5 100644 --- a/tests/cloud_tests/testcases/base.py +++ b/tests/cloud_tests/testcases/base.py @@ -34,7 +34,6 @@ class CloudTestCase(unittest.TestCase): @classmethod def maybeSkipTest(cls): """Present to allow subclasses to override and raise a skipTest.""" - pass def assertPackageInstalled(self, name, version=None): """Check dpkg-query --show output for matching package name. diff --git a/tests/unittests/test_datasource/test_ibmcloud.py b/tests/unittests/test_datasource/test_ibmcloud.py index 0b54f585..9013ae9f 100644 --- a/tests/unittests/test_datasource/test_ibmcloud.py +++ b/tests/unittests/test_datasource/test_ibmcloud.py @@ -15,13 +15,6 @@ mock = test_helpers.mock D_PATH = "cloudinit.sources.DataSourceIBMCloud." -class TestIBMCloud(test_helpers.CiTestCase): - """Test the datasource.""" - def setUp(self): - super(TestIBMCloud, self).setUp() - pass - - @mock.patch(D_PATH + "_is_xen", return_value=True) @mock.patch(D_PATH + "_is_ibm_provisioning") @mock.patch(D_PATH + "util.blkid") diff --git a/tests/unittests/test_reporting.py b/tests/unittests/test_reporting.py index 6814030e..9f11fd5c 100644 --- a/tests/unittests/test_reporting.py +++ b/tests/unittests/test_reporting.py @@ -349,7 +349,6 @@ class TestReportingEventStack(TestCase): with parent: with child: pass - pass self.assertEqual(report_start.call_count, 0) self.assertEqual(report_finish.call_count, 0) -- cgit v1.2.3