From 70d76516276b1d6f13eba65f926c427fb193de86 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 25 Jun 2020 09:35:22 -0400 Subject: [PATCH 1/9] stages: update call of wait_for_physdevs to distro.networking --- cloudinit/stages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/stages.py b/cloudinit/stages.py index db8ba64c496..69e6b7e1365 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -696,7 +696,7 @@ def apply_network_config(self, bring_up): 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) From 0d4a1e3004082a086f57b9d07408062e47d22361 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 25 Jun 2020 09:36:21 -0400 Subject: [PATCH 2/9] initial (naive) refactor of wait_for_physdevs TODO: * refactor Linux-specific behaviour out of base class implementation * docstrings --- cloudinit/distros/networking.py | 44 +++++++- cloudinit/distros/tests/test_networking.py | 122 ++++++++++++++++++++- cloudinit/net/__init__.py | 38 ------- cloudinit/net/tests/test_init.py | 74 ------------- 4 files changed, 163 insertions(+), 115 deletions(-) diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index e421a2ce572..a89f489c851 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -1,7 +1,12 @@ import abc +import logging import os +from functools import partial -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), @@ -105,7 +110,42 @@ def master_is_bridge_or_bond(self, devname: DeviceName) -> bool: def wait_for_physdevs( self, netcfg: NetworkConfig, *, strict: bool = True ) -> None: - return net.wait_for_physdevs(netcfg, strict=strict) + 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 + syspath = net.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 = 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): diff --git a/cloudinit/distros/tests/test_networking.py b/cloudinit/distros/tests/test_networking.py index 2acb12f4fdf..8585982ad44 100644 --- a/cloudinit/distros/tests/test_networking.py +++ b/cloudinit/distros/tests/test_networking.py @@ -2,7 +2,31 @@ 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.fixture +def generic_networking_cls(): + """Returns a direct Networking subclass with abstract methods implemented. + + This enables the direct testing of functionality only present on the + ``Networking`` super-class. + """ + + class TestNetworking(Networking): + def is_physical(self, *args, **kwargs): + raise NotImplementedError + + return TestNetworking @pytest.yield_fixture @@ -40,3 +64,99 @@ def test_returns_true_if_device_is_physical(self, sys_class_net): device_dir.join("device").write("") assert LinuxNetworking().is_physical(devname) + + +@mock.patch("cloudinit.util.udevadm_settle", autospec=True) +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, + m_udevadm_settle, + 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"}] + ) + networking.wait_for_physdevs(wait_for_physdevs_netcfg) + assert 0 == m_udevadm_settle.call_count + + def test_calls_udev_settle_on_missing( + self, + m_udevadm_settle, + 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 + ] + ) + networking.wait_for_physdevs(wait_for_physdevs_netcfg) + m_udevadm_settle.assert_called_with( + exists=net.sys_dev_path("ens3") + ) + + @pytest.mark.parametrize( + "strict,expectation", + [(True, pytest.raises(RuntimeError)), (False, does_not_raise())], + ) + def test_retrying_and_strict_behaviour( + self, + m_udevadm_settle, + 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 expectation: + networking.wait_for_physdevs( + wait_for_physdevs_netcfg, strict=strict + ) + + assert ( + 5 * len(wait_for_physdevs_netcfg["ethernets"]) + == m_udevadm_settle.call_count + ) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 9d8c7ba9038..322af77b02a 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -10,7 +10,6 @@ import logging import os import re -from functools import partial from cloudinit import subp from cloudinit import util @@ -494,43 +493,6 @@ def _version_2(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 eb458c39ce4..311ab6f8ecc 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -963,80 +963,6 @@ def test_runtime_error_on_unknown_netcfg_version(self): 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): From 5a9a66aeac881dba1c743e1e0b11afc51ec9a587 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 25 Jun 2020 10:09:22 -0400 Subject: [PATCH 3/9] networking: start refactoring to a separate settle method --- cloudinit/distros/networking.py | 23 ++++++--- cloudinit/distros/tests/test_networking.py | 58 ++++++++++++---------- 2 files changed, 49 insertions(+), 32 deletions(-) diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index a89f489c851..e9f9ea6d0ed 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -1,7 +1,6 @@ import abc import logging import os -from functools import partial from cloudinit import net, util @@ -107,6 +106,10 @@ def is_vlan(self, devname: DeviceName) -> bool: 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: + pass + def wait_for_physdevs( self, netcfg: NetworkConfig, *, strict: bool = True ) -> None: @@ -132,12 +135,14 @@ def wait_for_physdevs( LOG.debug("net: waiting for expected net devices: %s", missing) for mac in missing: # trigger a settle, unless this interface exists - syspath = net.sys_dev_path(expected_ifaces[mac]) - settle = partial(util.udevadm_settle, exists=syspath) - msg = ( - "Waiting for udev events to settle or %s exists" % syspath + devname = expected_ifaces[mac] + msg = "Waiting for settle or {} exists".format(devname) + util.log_time( + LOG.debug, + msg, + func=self.settle, + kwargs={"exists": devname}, ) - util.log_time(LOG.debug, msg, func=settle) # update present_macs after settles present_macs = self.get_interfaces_by_mac().keys() @@ -154,6 +159,9 @@ class BSDNetworking(Networking): def is_physical(self, devname: DeviceName) -> bool: raise NotImplementedError() + def settle(self, *, exists=None) -> None: + raise NotImplementedError() + class LinuxNetworking(Networking): """Implementation of networking functionality common to Linux distros.""" @@ -178,3 +186,6 @@ def is_netfail_standby(self, devname: DeviceName) -> bool: def is_physical(self, devname: DeviceName) -> bool: return os.path.exists(net.sys_dev_path(devname, "device")) + + def settle(self, *, exists=None) -> None: + raise NotImplementedError() diff --git a/cloudinit/distros/tests/test_networking.py b/cloudinit/distros/tests/test_networking.py index 8585982ad44..3807f1e4e8c 100644 --- a/cloudinit/distros/tests/test_networking.py +++ b/cloudinit/distros/tests/test_networking.py @@ -2,7 +2,6 @@ import pytest -from cloudinit import net from cloudinit.distros.networking import ( BSDNetworking, LinuxNetworking, @@ -14,19 +13,27 @@ from contextlib import ExitStack as does_not_raise -@pytest.fixture +@pytest.yield_fixture def generic_networking_cls(): - """Returns a direct Networking subclass with abstract methods implemented. + """Returns a direct Networking subclass which errors on /sys usage. This enables the direct testing of functionality only present on the - ``Networking`` super-class. + ``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 - return TestNetworking + 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 @@ -66,7 +73,6 @@ def test_returns_true_if_device_is_physical(self, sys_class_net): assert LinuxNetworking().is_physical(devname) -@mock.patch("cloudinit.util.udevadm_settle", autospec=True) class TestNetworkingWaitForPhysDevs: @pytest.fixture def wait_for_physdevs_netcfg(self): @@ -92,10 +98,7 @@ def ethernet(mac, name, driver=None, device_id=None): return netcfg def test_skips_settle_if_all_present( - self, - m_udevadm_settle, - generic_networking_cls, - wait_for_physdevs_netcfg, + self, generic_networking_cls, wait_for_physdevs_netcfg, ): networking = generic_networking_cls() with mock.patch.object( @@ -104,14 +107,14 @@ def test_skips_settle_if_all_present( m_get_interfaces_by_mac.side_effect = iter( [{"aa:bb:cc:dd:ee:ff": "eth0", "00:11:22:33:44:55": "ens3"}] ) - networking.wait_for_physdevs(wait_for_physdevs_netcfg) - assert 0 == m_udevadm_settle.call_count + 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, - m_udevadm_settle, - generic_networking_cls, - wait_for_physdevs_netcfg, + self, generic_networking_cls, wait_for_physdevs_netcfg, ): networking = generic_networking_cls() with mock.patch.object( @@ -128,10 +131,11 @@ def test_calls_udev_settle_on_missing( }, # second call has both ] ) - networking.wait_for_physdevs(wait_for_physdevs_netcfg) - m_udevadm_settle.assert_called_with( - exists=net.sys_dev_path("ens3") - ) + 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", @@ -139,7 +143,6 @@ def test_calls_udev_settle_on_missing( ) def test_retrying_and_strict_behaviour( self, - m_udevadm_settle, strict, expectation, generic_networking_cls, @@ -151,12 +154,15 @@ def test_retrying_and_strict_behaviour( ) as m_get_interfaces_by_mac: m_get_interfaces_by_mac.return_value = {} - with expectation: - networking.wait_for_physdevs( - wait_for_physdevs_netcfg, strict=strict - ) + 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_udevadm_settle.call_count + == m_settle.call_count ) From 3cd445212467d68e84122b84a631de88486ea146 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 25 Jun 2020 10:29:21 -0400 Subject: [PATCH 4/9] networking: implement LinuxNetworking.settle --- cloudinit/distros/networking.py | 4 +++- cloudinit/distros/tests/test_networking.py | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index e9f9ea6d0ed..44465f855b0 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -188,4 +188,6 @@ def is_physical(self, devname: DeviceName) -> bool: return os.path.exists(net.sys_dev_path(devname, "device")) def settle(self, *, exists=None) -> None: - raise NotImplementedError() + 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 3807f1e4e8c..b0f607deca1 100644 --- a/cloudinit/distros/tests/test_networking.py +++ b/cloudinit/distros/tests/test_networking.py @@ -2,6 +2,7 @@ import pytest +from cloudinit import net from cloudinit.distros.networking import ( BSDNetworking, LinuxNetworking, @@ -73,6 +74,23 @@ def test_returns_true_if_device_is_physical(self, sys_class_net): assert LinuxNetworking().is_physical(devname) +@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): From d51573156a34184e62528ccdfa836b21a9fecd43 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 25 Jun 2020 10:31:02 -0400 Subject: [PATCH 5/9] test_networking: add test for current BSD settle behaviour --- cloudinit/distros/tests/test_networking.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cloudinit/distros/tests/test_networking.py b/cloudinit/distros/tests/test_networking.py index b0f607deca1..3fde219b271 100644 --- a/cloudinit/distros/tests/test_networking.py +++ b/cloudinit/distros/tests/test_networking.py @@ -74,6 +74,12 @@ def test_returns_true_if_device_is_physical(self, sys_class_net): assert LinuxNetworking().is_physical(devname) +class TestBSDNetworkingSettle: + def test_settle_raises_notimplementederror(self): + with pytest.raises(NotImplementedError): + BSDNetworking().settle() + + @pytest.mark.usefixtures("sys_class_net") @mock.patch("cloudinit.distros.networking.util.udevadm_settle", autospec=True) class TestLinuxNetworkingSettle: From 561eb57529812d74b81432c64be49a0ab0ba6c03 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 25 Jun 2020 13:16:16 -0400 Subject: [PATCH 6/9] Add docstrings --- cloudinit/distros/networking.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index 44465f855b0..e7933bcb3e8 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -108,11 +108,29 @@ def master_is_bridge_or_bond(self, devname: DeviceName) -> bool: @abc.abstractmethod def settle(self, *, exists=None) -> None: + """Wait for device population in the system to complete. + + :param exists: + if given, skip the settle process if the given DeviceName is + already present in the system + """ pass def wait_for_physdevs( self, netcfg: NetworkConfig, *, strict: bool = True ) -> None: + """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 From f34e5f5df4d932c93a8aac28ac4cee6141df350b Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 2 Jul 2020 15:12:54 -0400 Subject: [PATCH 7/9] Modify BSDNetworking.settle() to pass --- cloudinit/distros/networking.py | 3 ++- cloudinit/distros/tests/test_networking.py | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index e7933bcb3e8..e892f459361 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -178,7 +178,8 @@ def is_physical(self, devname: DeviceName) -> bool: raise NotImplementedError() def settle(self, *, exists=None) -> None: - raise NotImplementedError() + """BSD has no equivalent to `udevadm settle`; noop.""" + pass class LinuxNetworking(Networking): diff --git a/cloudinit/distros/tests/test_networking.py b/cloudinit/distros/tests/test_networking.py index 3fde219b271..b9a638420a1 100644 --- a/cloudinit/distros/tests/test_networking.py +++ b/cloudinit/distros/tests/test_networking.py @@ -75,9 +75,9 @@ def test_returns_true_if_device_is_physical(self, sys_class_net): class TestBSDNetworkingSettle: - def test_settle_raises_notimplementederror(self): - with pytest.raises(NotImplementedError): - BSDNetworking().settle() + 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") From 643fc9f68d0c30d146abefa5d19d9b689b746ac7 Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Thu, 2 Jul 2020 15:23:58 -0400 Subject: [PATCH 8/9] networking: reword Networking.settle docstring --- cloudinit/distros/networking.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index e892f459361..fa101f1147e 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -111,8 +111,10 @@ def settle(self, *, exists=None) -> None: """Wait for device population in the system to complete. :param exists: - if given, skip the settle process if the given DeviceName is - already present in the system + 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.) """ pass From 6df1b3a62e9ffe7cc0e3dbfdb2d3cb52499a11dd Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 14 Jul 2020 12:29:26 -0400 Subject: [PATCH 9/9] networking: add exists type to settle docstring --- cloudinit/distros/networking.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index fa101f1147e..75e69df5c04 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -115,6 +115,7 @@ def settle(self, *, exists=None) -> None: 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