diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index f2d9de107ed..cf32b68b73f 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -253,9 +253,6 @@ def apply_network_config(self, netconfig, bring_up=False) -> bool: LOG.debug("Not bringing up newly configured network interfaces") return False - def apply_network_config_names(self, netconfig): - net.apply_network_config_names(netconfig) - @abc.abstractmethod def apply_locale(self, locale, out_fn=None): raise NotImplementedError() diff --git a/cloudinit/distros/bsd.py b/cloudinit/distros/bsd.py index 1b4498b31a9..bab222b5998 100644 --- a/cloudinit/distros/bsd.py +++ b/cloudinit/distros/bsd.py @@ -133,6 +133,3 @@ def set_timezone(self, tz): def apply_locale(self, locale, out_fn=None): LOG.debug("Cannot set the locale.") - - def apply_network_config_names(self, netconfig): - LOG.debug("Cannot rename network interface.") diff --git a/cloudinit/distros/freebsd.py b/cloudinit/distros/freebsd.py index 513abdc205c..fa5c6616d52 100644 --- a/cloudinit/distros/freebsd.py +++ b/cloudinit/distros/freebsd.py @@ -13,6 +13,8 @@ from cloudinit import subp, util from cloudinit.settings import PER_INSTANCE +from .networking import FreeBSDNetworking + LOG = logging.getLogger(__name__) @@ -23,6 +25,7 @@ class Distro(cloudinit.distros.bsd.BSD): (N.B. DragonFlyBSD inherits from this class.) """ + networking_cls = FreeBSDNetworking usr_lib_exec = "/usr/local/lib" login_conf_fn = "/etc/login.conf" login_conf_fn_bak = "/etc/login.conf.orig" @@ -153,13 +156,6 @@ def apply_locale(self, locale, out_fn=None): LOG, "Failed to restore %s backup", self.login_conf_fn ) - def apply_network_config_names(self, netconfig): - # This is handled by the freebsd network renderer. It writes in - # /etc/rc.conf a line with the following format: - # ifconfig_OLDNAME_name=NEWNAME - # FreeBSD network script will rename the interface automatically. - pass - def _get_pkg_cmd_environ(self): """Return environment vars used in *BSD package_command operations""" e = os.environ.copy() diff --git a/cloudinit/distros/netbsd.py b/cloudinit/distros/netbsd.py index 9c38ae5143e..c0d6390f68c 100644 --- a/cloudinit/distros/netbsd.py +++ b/cloudinit/distros/netbsd.py @@ -133,9 +133,6 @@ def unlock_passwd(self, name): def apply_locale(self, locale, out_fn=None): LOG.debug("Cannot set the locale.") - def apply_network_config_names(self, netconfig): - LOG.debug("NetBSD cannot rename network interface.") - def _get_pkg_cmd_environ(self): """Return env vars used in NetBSD package_command operations""" os_release = platform.release() diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index b24b6233bf9..f14d678dc68 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -19,7 +19,7 @@ class Networking(metaclass=abc.ABCMeta): 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. + Hierarchy" in CONTRIBUTING.rst for full details. """ def __init__(self): @@ -31,8 +31,9 @@ def _get_current_rename_info(self) -> dict: def _rename_interfaces(self, renames: list, *, current_info=None) -> None: return net._rename_interfaces(renames, current_info=current_info) + @abc.abstractmethod def apply_network_config_names(self, netcfg: NetworkConfig) -> None: - return net.apply_network_config_names(netcfg) + """Read the network config and rename devices accordingly.""" def device_devid(self, devname: DeviceName): return net.device_devid(devname) @@ -184,6 +185,9 @@ def try_set_link_up(self, devname: DeviceName) -> bool: class BSDNetworking(Networking): """Implementation of networking functionality shared across BSDs.""" + def apply_network_config_names(self, netcfg: NetworkConfig) -> None: + LOG.debug("Cannot rename network interface.") + def is_physical(self, devname: DeviceName) -> bool: raise NotImplementedError() @@ -194,9 +198,33 @@ def try_set_link_up(self, devname: DeviceName) -> bool: raise NotImplementedError() +class FreeBSDNetworking(BSDNetworking): + def apply_network_config_names(self, netcfg: NetworkConfig) -> None: + # This is handled by the freebsd network renderer. It writes in + # /etc/rc.conf a line with the following format: + # ifconfig_OLDNAME_name=NEWNAME + # FreeBSD network script will rename the interface automatically. + pass + + class LinuxNetworking(Networking): """Implementation of networking functionality common to Linux distros.""" + def apply_network_config_names(self, netcfg: NetworkConfig) -> None: + """Read the network config and rename devices accordingly. + + Renames are only attempted for interfaces of type 'physical'. It is + expected that the network system will create other devices with the + correct name in place. + """ + + try: + self._rename_interfaces(self.extract_physdevs(netcfg)) + except RuntimeError as e: + raise RuntimeError( + "Failed to apply network config names: %s" % e + ) from e + def get_dev_features(self, devname: DeviceName) -> str: return net.get_dev_features(devname) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index f02bae2ed0b..5581333f09e 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -660,24 +660,6 @@ def _version_2(netcfg): raise RuntimeError("Unknown network config version: %s" % version) -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 - match. if strict_busy is false, then do not raise exception if the - device cannot be renamed because it is currently configured. - - renames are only attempted for interfaces of type 'physical'. It is - expected that the network system will create other devices with the - correct name in place.""" - - try: - _rename_interfaces(extract_physdevs(netcfg)) - except RuntimeError as e: - raise RuntimeError( - "Failed to apply network config names: %s" % e - ) from e - - def interface_has_own_mac(ifname, strict=False): """return True if the provided interface has its own address. diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 89b38147da0..3dece33633d 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -852,7 +852,7 @@ def _find_networking_config(self): def _apply_netcfg_names(self, netcfg): try: LOG.debug("applying net config names for %s", netcfg) - self.distro.apply_network_config_names(netcfg) + self.distro.networking.apply_network_config_names(netcfg) except Exception as e: LOG.warning("Failed to rename devices: %s", e) diff --git a/tests/unittests/distros/test_networking.py b/tests/unittests/distros/test_networking.py index 274647cbb23..f56b34ad97e 100644 --- a/tests/unittests/distros/test_networking.py +++ b/tests/unittests/distros/test_networking.py @@ -1,11 +1,14 @@ # See https://docs.pytest.org/en/stable/example # /parametrize.html#parametrizing-conditional-raising + +import textwrap from contextlib import ExitStack as does_not_raise from unittest import mock import pytest from cloudinit import net +from cloudinit import safeyaml as yaml from cloudinit.distros.networking import ( BSDNetworking, LinuxNetworking, @@ -23,6 +26,9 @@ def generic_networking_cls(): """ class TestNetworking(Networking): + def apply_network_config_names(self, *args, **kwargs): + raise NotImplementedError + def is_physical(self, *args, **kwargs): raise NotImplementedError @@ -229,3 +235,115 @@ def test_retrying_and_strict_behaviour( 5 * len(wait_for_physdevs_netcfg["ethernets"]) == m_settle.call_count ) + + +class TestLinuxNetworkingApplyNetworkCfgNames: + V1_CONFIG = textwrap.dedent( + """\ + version: 1 + config: + - type: physical + name: interface0 + mac_address: "52:54:00:12:34:00" + subnets: + - type: static + address: 10.0.2.15 + netmask: 255.255.255.0 + gateway: 10.0.2.2 + """ + ) + V2_CONFIG = textwrap.dedent( + """\ + version: 2 + ethernets: + interface0: + match: + macaddress: "52:54:00:12:34:00" + addresses: + - 10.0.2.15/24 + gateway4: 10.0.2.2 + set-name: interface0 + """ + ) + + V2_CONFIG_NO_SETNAME = textwrap.dedent( + """\ + version: 2 + ethernets: + interface0: + match: + macaddress: "52:54:00:12:34:00" + addresses: + - 10.0.2.15/24 + gateway4: 10.0.2.2 + """ + ) + + V2_CONFIG_NO_MAC = textwrap.dedent( + """\ + version: 2 + ethernets: + interface0: + match: + driver: virtio-net + addresses: + - 10.0.2.15/24 + gateway4: 10.0.2.2 + set-name: interface0 + """ + ) + + @pytest.mark.parametrize( + ["config_attr"], + [ + pytest.param("V1_CONFIG", id="v1"), + pytest.param("V2_CONFIG", id="v2"), + ], + ) + @mock.patch("cloudinit.net.device_devid") + @mock.patch("cloudinit.net.device_driver") + def test_apply_renames( + self, + m_device_driver, + m_device_devid, + config_attr: str, + ): + networking = LinuxNetworking() + m_device_driver.return_value = "virtio_net" + m_device_devid.return_value = "0x15d8" + netcfg = yaml.load(getattr(self, config_attr)) + + with mock.patch.object( + networking, "_rename_interfaces" + ) as m_rename_interfaces: + networking.apply_network_config_names(netcfg) + + assert ( + mock.call( + [["52:54:00:12:34:00", "interface0", "virtio_net", "0x15d8"]] + ) + == m_rename_interfaces.call_args_list[-1] + ) + + @pytest.mark.parametrize( + ["config_attr"], + [ + pytest.param("V2_CONFIG_NO_SETNAME", id="without_setname"), + pytest.param("V2_CONFIG_NO_MAC", id="without_mac"), + ], + ) + def test_apply_v2_renames_skips_without_setname_or_mac( + self, config_attr: str + ): + networking = LinuxNetworking() + netcfg = yaml.load(getattr(self, config_attr)) + with mock.patch.object( + networking, "_rename_interfaces" + ) as m_rename_interfaces: + networking.apply_network_config_names(netcfg) + m_rename_interfaces.assert_called_with([]) + + def test_apply_v2_renames_raises_runtime_error_on_unknown_version(self): + networking = LinuxNetworking() + with pytest.raises(RuntimeError): + networking.apply_network_config_names(yaml.load("version: 3")) diff --git a/tests/unittests/net/test_init.py b/tests/unittests/net/test_init.py index d03186e9da7..768cc1129f4 100644 --- a/tests/unittests/net/test_init.py +++ b/tests/unittests/net/test_init.py @@ -4,7 +4,6 @@ import errno import ipaddress import os -import textwrap from pathlib import Path from typing import Optional from unittest import mock @@ -14,7 +13,6 @@ import requests import cloudinit.net as net -from cloudinit import safeyaml as yaml from cloudinit.subp import ProcessExecutionError from cloudinit.util import ensure_file, write_file from tests.unittests.helpers import CiTestCase, HttprettyTestCase @@ -1196,105 +1194,6 @@ def test_ephemeral_ipv4_network_with_rfc3442_static_routes(self, m_subp): m_subp.assert_has_calls(expected_setup_calls + expected_teardown_calls) -class TestApplyNetworkCfgNames(CiTestCase): - V1_CONFIG = textwrap.dedent( - """\ - version: 1 - config: - - type: physical - name: interface0 - mac_address: "52:54:00:12:34:00" - subnets: - - type: static - address: 10.0.2.15 - netmask: 255.255.255.0 - gateway: 10.0.2.2 - """ - ) - V2_CONFIG = textwrap.dedent( - """\ - version: 2 - ethernets: - interface0: - match: - macaddress: "52:54:00:12:34:00" - addresses: - - 10.0.2.15/24 - gateway4: 10.0.2.2 - set-name: interface0 - """ - ) - - V2_CONFIG_NO_SETNAME = textwrap.dedent( - """\ - version: 2 - ethernets: - interface0: - match: - macaddress: "52:54:00:12:34:00" - addresses: - - 10.0.2.15/24 - gateway4: 10.0.2.2 - """ - ) - - V2_CONFIG_NO_MAC = textwrap.dedent( - """\ - version: 2 - ethernets: - interface0: - match: - driver: virtio-net - addresses: - - 10.0.2.15/24 - gateway4: 10.0.2.2 - set-name: interface0 - """ - ) - - @mock.patch("cloudinit.net.device_devid") - @mock.patch("cloudinit.net.device_driver") - @mock.patch("cloudinit.net._rename_interfaces") - def test_apply_v1_renames( - self, m_rename_interfaces, m_device_driver, m_device_devid - ): - m_device_driver.return_value = "virtio_net" - m_device_devid.return_value = "0x15d8" - - net.apply_network_config_names(yaml.load(self.V1_CONFIG)) - - call = ["52:54:00:12:34:00", "interface0", "virtio_net", "0x15d8"] - m_rename_interfaces.assert_called_with([call]) - - @mock.patch("cloudinit.net.device_devid") - @mock.patch("cloudinit.net.device_driver") - @mock.patch("cloudinit.net._rename_interfaces") - def test_apply_v2_renames( - self, m_rename_interfaces, m_device_driver, m_device_devid - ): - m_device_driver.return_value = "virtio_net" - m_device_devid.return_value = "0x15d8" - - net.apply_network_config_names(yaml.load(self.V2_CONFIG)) - - call = ["52:54:00:12:34:00", "interface0", "virtio_net", "0x15d8"] - m_rename_interfaces.assert_called_with([call]) - - @mock.patch("cloudinit.net._rename_interfaces") - def test_apply_v2_renames_skips_without_setname(self, m_rename_interfaces): - net.apply_network_config_names(yaml.load(self.V2_CONFIG_NO_SETNAME)) - m_rename_interfaces.assert_called_with([]) - - @mock.patch("cloudinit.net._rename_interfaces") - def test_apply_v2_renames_skips_without_mac(self, m_rename_interfaces): - net.apply_network_config_names(yaml.load(self.V2_CONFIG_NO_MAC)) - m_rename_interfaces.assert_called_with([]) - - def test_apply_v2_renames_raises_runtime_error_on_unknown_version(self): - with self.assertRaises(RuntimeError): - net.apply_network_config_names(yaml.load("version: 3")) - - class TestHasURLConnectivity(HttprettyTestCase): def setUp(self): super(TestHasURLConnectivity, self).setUp() diff --git a/tests/unittests/test_stages.py b/tests/unittests/test_stages.py index 3214410b669..38bf231d7aa 100644 --- a/tests/unittests/test_stages.py +++ b/tests/unittests/test_stages.py @@ -360,14 +360,16 @@ def fake_network_config(): self.init._find_networking_config = fake_network_config self.init.apply_network_config(True) - self.init.distro.apply_network_config_names.assert_called_with(net_cfg) + networking = self.init.distro.networking + networking.apply_network_config_names.assert_called_with(net_cfg) self.init.distro.apply_network_config.assert_called_with( net_cfg, bring_up=True ) @mock.patch("cloudinit.distros.ubuntu.Distro") def test_apply_network_on_same_instance_id(self, m_ubuntu): - """Only call distro.apply_network_config_names on same instance id.""" + """Only call distro.networking.apply_network_config_names on same + instance id.""" self.init.is_new_instance = self._real_is_new_instance old_instance_id = os.path.join( self.init.paths.get_cpath("data"), "instance-id" @@ -391,7 +393,8 @@ def fake_network_config(): self.init._find_networking_config = fake_network_config self.init.apply_network_config(True) - self.init.distro.apply_network_config_names.assert_called_with(net_cfg) + networking = self.init.distro.networking + networking.apply_network_config_names.assert_called_with(net_cfg) self.init.distro.apply_network_config.assert_not_called() assert ( "No network config applied. Neither a new instance nor datasource " @@ -439,9 +442,10 @@ def test_apply_network_allowed_when_default_boot(self, m_ubuntu, m_macs): net_cfg = self._apply_network_setup(m_macs) self.init.apply_network_config(True) + networking = self.init.distro.networking assert ( mock.call(net_cfg) - == self.init.distro.apply_network_config_names.call_args_list[-1] + == networking.apply_network_config_names.call_args_list[-1] ) assert ( mock.call(net_cfg, bring_up=True) @@ -479,7 +483,8 @@ def test_apply_network_allowed_with_userdata_overrides( net_cfg = self._apply_network_setup(m_macs) self.init._cfg = {"updates": {"network": {"when": ["boot"]}}} self.init.apply_network_config(True) - self.init.distro.apply_network_config_names.assert_called_with(net_cfg) + networking = self.init.distro.networking + networking.apply_network_config_names.assert_called_with(net_cfg) self.init.distro.apply_network_config.assert_called_with( net_cfg, bring_up=True ) diff --git a/tests/unittests/util.py b/tests/unittests/util.py index 79a6e1d0d5d..f57a3d25802 100644 --- a/tests/unittests/util.py +++ b/tests/unittests/util.py @@ -84,9 +84,6 @@ def generate_fallback_config(self): def apply_network_config(self, netconfig, bring_up=False) -> bool: return False - def apply_network_config_names(self, netconfig): - pass - def apply_locale(self, locale, out_fn=None): pass