cloudinit.net refactor: apply_network_config_names

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()

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.")

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()

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()

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)
 

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.
 

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)
 

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"))