Netops refactor

cloudinit/distros/networking.py

@@ -4,6 +4,7 @@
 
 from cloudinit import net, subp, util
 from cloudinit.distros.parsers import ifconfig
+from cloudinit.net.netops.iproute2 import Iproute2
 
 LOG = logging.getLogger(__name__)
 
@@ -299,5 +300,5 @@ def try_set_link_up(self, devname: DeviceName) -> bool:
         """Try setting the link to up explicitly and return if it is up.
         Not guaranteed to bring the interface up. The caller is expected to
         add wait times before retrying."""
-        subp.subp(["ip", "link", "set", devname, "up"])
+        Iproute2.link_up(devname)
         return self.is_up(devname)

cloudinit/net/__init__.py

@@ -15,6 +15,7 @@
 from urllib.parse import urlparse
 
 from cloudinit import subp, util
+from cloudinit.net.netops.iproute2 import Iproute2
 from cloudinit.url_helper import UrlError, readurl
 
 LOG = logging.getLogger(__name__)
@@ -723,15 +724,6 @@ def _rename_interfaces(
     def update_byname(bymac):
         return dict((data["name"], data) for data in cur_info.values())
 
-    def rename(cur, new):
-        subp.subp(["ip", "link", "set", cur, "name", new], capture=True)
-
-    def down(name):
-        subp.subp(["ip", "link", "set", name, "down"], capture=True)
-
-    def up(name):
-        subp.subp(["ip", "link", "set", name, "up"], capture=True)
-
     ops = []
     errors = []
     ups = []
@@ -835,7 +827,11 @@ def find_entry(mac, driver, device_id):
         cur_byname = update_byname(cur_info)
         ops += cur_ops
 
-    opmap = {"rename": rename, "down": down, "up": up}
+    opmap = {
+        "rename": Iproute2.link_rename,
+        "down": Iproute2.link_down,
+        "up": Iproute2.link_up,
+    }
 
     if len(ops) + len(ups) == 0:
         if len(errors):

cloudinit/net/activators.py

@@ -1,10 +1,12 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 import logging
 from abc import ABC, abstractmethod
-from typing import Dict, Iterable, List, Optional, Type, Union
+from functools import partial
+from typing import Callable, Dict, Iterable, List, Optional, Type, Union
 
 from cloudinit import subp, util
 from cloudinit.net.eni import available as eni_available
+from cloudinit.net.netops.iproute2 import Iproute2
 from cloudinit.net.netplan import available as netplan_available
 from cloudinit.net.network_manager import available as nm_available
 from cloudinit.net.network_state import NetworkState
@@ -17,15 +19,24 @@ class NoActivatorException(Exception):
     pass
 
 
-def _alter_interface(cmd, device_name) -> bool:
-    LOG.debug("Attempting command %s for device %s", cmd, device_name)
+def _alter_interface(cmd: list, device_name: str) -> bool:
+    """Attempt to alter an interface using a command list"""
+    return _alter_interface_callable(partial(subp.subp, cmd))
+
+
+def _alter_interface_callable(callable: Callable) -> bool:
+    """Attempt to alter an interface using a callable
+
+    this function standardizes logging and response to failure for
+    various activators
+    """
     try:
-        (_out, err) = subp.subp(cmd)
+        _out, err = callable()
         if len(err):
-            LOG.warning("Running %s resulted in stderr output: %s", cmd, err)
+            LOG.warning("Received stderr output: %s", err)
         return True
-    except subp.ProcessExecutionError:
-        util.logexc(LOG, "Running interface command %s failed", cmd)
+    except subp.ProcessExecutionError as e:
+        util.logexc(LOG, "Running interface command %s failed", e.cmd)
         return False
 
 
@@ -231,8 +242,9 @@ def available(target=None) -> bool:
     @staticmethod
     def bring_up_interface(device_name: str) -> bool:
         """Return True is successful, otherwise return False"""
-        cmd = ["ip", "link", "set", "up", device_name]
-        return _alter_interface(cmd, device_name)
+        return _alter_interface_callable(
+            partial(Iproute2.link_up, device_name)
+        )
 
     @staticmethod
     def bring_up_all_interfaces(network_state: NetworkState) -> bool:
@@ -243,8 +255,9 @@ def bring_up_all_interfaces(network_state: NetworkState) -> bool:
     @staticmethod
     def bring_down_interface(device_name: str) -> bool:
         """Return True is successful, otherwise return False"""
-        cmd = ["ip", "link", "set", "down", device_name]
-        return _alter_interface(cmd, device_name)
+        return _alter_interface_callable(
+            partial(Iproute2.link_down, device_name)
+        )
 
 
 # This section is mostly copied and pasted from renderers.py. An abstract

cloudinit/net/netops/__init__.py

@@ -1,13 +1,19 @@
 from typing import Optional
 
+from cloudinit.subp import SubpResult
+
 
 class NetOps:
     @staticmethod
-    def link_up(interface: str):
+    def link_up(interface: str) -> SubpResult:
+        pass
+
+    @staticmethod
+    def link_down(interface: str) -> SubpResult:
         pass
 
     @staticmethod
-    def link_down(interface: str):
+    def link_rename(current_name: str, new_name: str):
         pass
 
     @staticmethod
@@ -39,9 +45,15 @@ def get_default_route() -> str:
         pass
 
     @staticmethod
-    def add_addr(interface: str, address: str, broadcast: str):
+    def add_addr(
+        interface: str, address: str, broadcast: Optional[str] = None
+    ):
         pass
 
     @staticmethod
     def del_addr(interface: str, address: str):
         pass
+
+    @staticmethod
+    def flush_addr(interface: str):
+        pass

cloudinit/net/netops/bsd_netops.py

@@ -6,12 +6,12 @@
 
 class BsdNetOps(netops.NetOps):
     @staticmethod
-    def link_up(interface: str):
-        subp.subp(["ifconfig", interface, "up"])
+    def link_up(interface: str) -> subp.SubpResult:
+        return subp.subp(["ifconfig", interface, "up"])
 
     @staticmethod
-    def link_down(interface: str):
-        subp.subp(["ifconfig", interface, "down"])
+    def link_down(interface: str) -> subp.SubpResult:
+        return subp.subp(["ifconfig", interface, "down"])
 
     @staticmethod
     def add_route(
@@ -50,14 +50,18 @@ def get_default_route() -> str:
         return std.splitlines()[-1].strip()
 
     @staticmethod
-    def add_addr(interface: str, address: str, broadcast: str):
+    def add_addr(
+        interface: str, address: str, broadcast: Optional[str] = None
+    ):
+        broadcast_args = []
+        if broadcast:
+            broadcast_args = ["broadcast", broadcast]
         subp.subp(
             [
                 "ifconfig",
                 interface,
                 address,
-                "broadcast",
-                broadcast,
+                *broadcast_args,
                 "alias",
             ],
         )

cloudinit/net/netops/iproute2.py

@@ -1,26 +1,36 @@
 from typing import Optional
 
-import cloudinit.net.netops as netops
 from cloudinit import subp
+from cloudinit.net.netops import NetOps
 
 
-class Iproute2(netops.NetOps):
+class Iproute2(NetOps):
     @staticmethod
-    def link_up(interface: str, family: Optional[str] = None):
-        subp.subp(
-            ["ip"]
-            + (["-family", family] if family else [])
-            + ["link", "set", "dev", interface, "up"]
+    def link_up(
+        interface: str, family: Optional[str] = None
+    ) -> subp.SubpResult:
+        family_args = []
+        if family:
+            family_args = ["-family", family]
+        return subp.subp(
+            ["ip", *family_args, "link", "set", "dev", interface, "up"]
         )
 
     @staticmethod
-    def link_down(interface: str, family: Optional[str] = None):
-        subp.subp(
-            ["ip"]
-            + (["-family", family] if family else [])
-            + ["link", "set", "dev", interface, "down"]
+    def link_down(
+        interface: str, family: Optional[str] = None
+    ) -> subp.SubpResult:
+        family_args = []
+        if family:
+            family_args = ["-family", family]
+        return subp.subp(
+            ["ip", *family_args, "link", "set", "dev", interface, "down"]
         )
 
+    @staticmethod
+    def link_rename(current_name: str, new_name: str):
+        subp.subp(["ip", "link", "set", current_name, "name", new_name])
+
     @staticmethod
     def add_route(
         interface: str,
@@ -29,22 +39,42 @@ def add_route(
         gateway: Optional[str] = None,
         source_address: Optional[str] = None,
     ):
+        gateway_args = []
+        source_args = []
+        if gateway and gateway != "0.0.0.0":
+            gateway_args = ["via", gateway]
+        if source_address:
+            source_args = ["src", source_address]
         subp.subp(
-            ["ip", "-4", "route", "add", route]
-            + (["via", gateway] if gateway and gateway != "0.0.0.0" else [])
-            + [
+            [
+                "ip",
+                "-4",
+                "route",
+                "add",
+                route,
+                *gateway_args,
                 "dev",
                 interface,
+                *source_args,
             ]
-            + (["src", source_address] if source_address else []),
         )
 
     @staticmethod
     def append_route(interface: str, address: str, gateway: str):
+        gateway_args = []
+        if gateway and gateway != "0.0.0.0":
+            gateway_args = ["via", gateway]
         subp.subp(
-            ["ip", "-4", "route", "append", address]
-            + (["via", gateway] if gateway and gateway != "0.0.0.0" else [])
-            + ["dev", interface]
+            [
+                "ip",
+                "-4",
+                "route",
+                "append",
+                address,
+                *gateway_args,
+                "dev",
+                interface,
+            ]
         )
 
     @staticmethod
@@ -55,11 +85,24 @@ def del_route(
         gateway: Optional[str] = None,
         source_address: Optional[str] = None,
     ):
+        gateway_args = []
+        source_args = []
+        if gateway and gateway != "0.0.0.0":
+            gateway_args = ["via", gateway]
+        if source_address:
+            source_args = ["src", source_address]
         subp.subp(
-            ["ip", "-4", "route", "del", address]
-            + (["via", gateway] if gateway and gateway != "0.0.0.0" else [])
-            + ["dev", interface]
-            + (["src", source_address] if source_address else [])
+            [
+                "ip",
+                "-4",
+                "route",
+                "del",
+                address,
+                *gateway_args,
+                "dev",
+                interface,
+                *source_args,
+            ]
         )
 
     @staticmethod
@@ -69,7 +112,12 @@ def get_default_route() -> str:
         ).stdout
 
     @staticmethod
-    def add_addr(interface: str, address: str, broadcast: str):
+    def add_addr(
+        interface: str, address: str, broadcast: Optional[str] = None
+    ):
+        broadcast_args = []
+        if broadcast:
+            broadcast_args = ["broadcast", broadcast]
         subp.subp(
             [
                 "ip",
@@ -78,8 +126,7 @@ def add_addr(interface: str, address: str, broadcast: str):
                 "addr",
                 "add",
                 address,
-                "broadcast",
-                broadcast,
+                *broadcast_args,
                 "dev",
                 interface,
             ],
@@ -91,3 +138,7 @@ def del_addr(interface: str, address: str):
         subp.subp(
             ["ip", "-family", "inet", "addr", "del", address, "dev", interface]
         )
+
+    @staticmethod
+    def flush_addr(interface: str):
+        subp.subp(["ip", "flush", "dev", interface])

cloudinit/sources/DataSourceDigitalOcean.py

@@ -94,7 +94,7 @@ def _get_data(self):
         self.userdata_raw = md.get("user_data", None)
 
         if ipv4LL_nic:
-            do_helper.del_ipv4_link_local(ipv4LL_nic)
+            do_helper.del_ipv4_link_local(self.distro, ipv4LL_nic)
 
         return True