Add new network activators to bring up interfaces (#919)

Currently _bring_up_interfaces() is a no-op for any distro using
renderers. We need to be able to support bringing up a single
interfaces, a list of interfaces, and all interfaces. This should be
independent of the renderers, as the network config is often
generated independent of the mechanism used to apply it.

Additionally, I included a refactor to remove
"_supported_write_network_config". We had a confusing call chain of
apply_network_config->_write_network_config->_supported_write_network_config.
The last two have been combined.

7 files changed, 214 insertions, 43 deletions

diff --git a/cloudinit/net/activators.py b/cloudinit/net/activators.py
new file mode 100644
index 00000000..34fee3bf
--- /dev/null
+++ b/cloudinit/net/activators.py
@@ -0,0 +1,156 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+import logging
+import os
+from abc import ABC, abstractmethod
+from typing import Iterable, List, Type
+
+from cloudinit import subp
+from cloudinit import util
+from cloudinit.net.eni import available as eni_available
+from cloudinit.net.netplan import available as netplan_available
+from cloudinit.net.network_state import NetworkState
+from cloudinit.net.sysconfig import NM_CFG_FILE
+
+
+LOG = logging.getLogger(__name__)
+
+
+class NetworkActivator(ABC):
+    @staticmethod
+    @abstractmethod
+    def available() -> bool:
+        raise NotImplementedError()
+
+    @staticmethod
+    @abstractmethod
+    def bring_up_interface(device_name: str) -> bool:
+        raise NotImplementedError()
+
+    @classmethod
+    def bring_up_interfaces(cls, device_names: Iterable[str]) -> bool:
+        all_succeeded = True
+        for device in device_names:
+            if not cls.bring_up_interface(device):
+                all_succeeded = False
+        return all_succeeded
+
+    @classmethod
+    def bring_up_all_interfaces(cls, network_state: NetworkState) -> bool:
+        return cls.bring_up_interfaces(
+            [i['name'] for i in network_state.iter_interfaces()]
+        )
+
+
+class IfUpDownActivator(NetworkActivator):
+    # Note that we're not overriding bring_up_interfaces to pass something
+    # like ifup --all because it isn't supported everywhere.
+    # E.g., NetworkManager has a ifupdown plugin that requires the name
+    # of a specific connection.
+    @staticmethod
+    def available(target=None) -> bool:
+        """Return true if ifupdown can be used on this system."""
+        return eni_available(target=target)
+
+    @staticmethod
+    def bring_up_interface(device_name: str) -> bool:
+        """Bring up interface using ifup."""
+        cmd = ['ifup', device_name]
+        LOG.debug("Attempting to run bring up interface %s using command %s",
+                  device_name, cmd)
+        try:
+            (_out, err) = subp.subp(cmd)
+            if len(err):
+                LOG.warning("Running %s resulted in stderr output: %s",
+                            cmd, err)
+            return True
+        except subp.ProcessExecutionError:
+            util.logexc(LOG, "Running interface command %s failed", cmd)
+            return False
+
+
+class NetworkManagerActivator(NetworkActivator):
+    @staticmethod
+    def available(target=None) -> bool:
+        config_present = os.path.isfile(
+            subp.target_path(target, path=NM_CFG_FILE)
+        )
+        nmcli_present = subp.which('nmcli', target=target)
+        return config_present and bool(nmcli_present)
+
+    @staticmethod
+    def bring_up_interface(device_name: str) -> bool:
+        try:
+            subp.subp(['nmcli', 'connection', 'up', device_name])
+        except subp.ProcessExecutionError:
+            util.logexc(LOG, "nmcli failed to bring up {}".format(device_name))
+            return False
+        return True
+
+
+class NetplanActivator(NetworkActivator):
+    @staticmethod
+    def available(target=None) -> bool:
+        return netplan_available(target=target)
+
+    @staticmethod
+    def _apply_netplan():
+        LOG.debug('Applying current netplan config')
+        try:
+            subp.subp(['netplan', 'apply'], capture=True)
+        except subp.ProcessExecutionError:
+            util.logexc(LOG, "netplan apply failed")
+            return False
+        return True
+
+    @staticmethod
+    def bring_up_interface(device_name: str) -> bool:
+        LOG.debug("Calling 'netplan apply' rather than "
+                  "bringing up individual interfaces")
+        return NetplanActivator._apply_netplan()
+
+    @staticmethod
+    def bring_up_interfaces(device_names: Iterable[str]) -> bool:
+        LOG.debug("Calling 'netplan apply' rather than "
+                  "bringing up individual interfaces")
+        return NetplanActivator._apply_netplan()
+
+    @staticmethod
+    def bring_up_all_interfaces(network_state: NetworkState) -> bool:
+        return NetplanActivator._apply_netplan()
+
+
+# This section is mostly copied and pasted from renderers.py. An abstract
+# version to encompass both seems overkill at this point
+DEFAULT_PRIORITY = [
+    IfUpDownActivator,
+    NetworkManagerActivator,
+    NetplanActivator,
+]
+
+
+def search_activator(
+    priority=None, target=None
+) -> List[Type[NetworkActivator]]:
+    if priority is None:
+        priority = DEFAULT_PRIORITY
+
+    unknown = [i for i in priority if i not in DEFAULT_PRIORITY]
+    if unknown:
+        raise ValueError(
+            "Unknown activators provided in priority list: %s" % unknown)
+
+    return [activator for activator in priority if activator.available(target)]
+
+
+def select_activator(priority=None, target=None) -> Type[NetworkActivator]:
+    found = search_activator(priority, target)
+    if not found:
+        if priority is None:
+            priority = DEFAULT_PRIORITY
+        tmsg = ""
+        if target and target != "/":
+            tmsg = " in target=%s" % target
+        raise RuntimeError(
+            "No available network activators found%s. Searched "
+            "through list: %s" % (tmsg, priority))
+    return found[0]
diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index 53347c83..41acf963 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -4,7 +4,12 @@ import copy
 import os
 
 from . import renderer
-from .network_state import subnet_is_ipv6, NET_CONFIG_TO_V2, IPV6_DYNAMIC_TYPES
+from .network_state import (
+    NetworkState,
+    subnet_is_ipv6,
+    NET_CONFIG_TO_V2,
+    IPV6_DYNAMIC_TYPES,
+)
 
 from cloudinit import log as logging
 from cloudinit import util
@@ -256,7 +261,7 @@ class Renderer(renderer.Renderer):
                     os.path.islink(SYS_CLASS_NET + iface)]:
             subp.subp(cmd, capture=True)
 
-    def _render_content(self, network_state):
+    def _render_content(self, network_state: NetworkState):
 
         # if content already in netplan format, pass it back
         if network_state.version == 2:
@@ -426,4 +431,5 @@ def network_state_to_netplan(network_state, header=None):
     contents = renderer._render_content(network_state)
     return header + contents
 
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
index 8018cfb9..95b064f0 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -58,38 +58,6 @@ NET_CONFIG_TO_V2 = {
                'bridge_waitport': None}}
 
 
-def parse_net_config_data(net_config, skip_broken=True):
-    """Parses the config, returns NetworkState object
-
-    :param net_config: curtin network config dict
-    """
-    state = None
-    version = net_config.get('version')
-    config = net_config.get('config')
-    if version == 2:
-        # v2 does not have explicit 'config' key so we
-        # pass the whole net-config as-is
-        config = net_config
-
-    if version and config is not None:
-        nsi = NetworkStateInterpreter(version=version, config=config)
-        nsi.parse_config(skip_broken=skip_broken)
-        state = nsi.get_network_state()
-
-    return state
-
-
-def parse_net_config(path, skip_broken=True):
-    """Parses a curtin network configuration file and
-       return network state"""
-    ns = None
-    net_config = util.read_conf(path)
-    if 'network' in net_config:
-        ns = parse_net_config_data(net_config.get('network'),
-                                   skip_broken=skip_broken)
-    return ns
-
-
 def from_state_file(state_file):
     state = util.read_conf(state_file)
     nsi = NetworkStateInterpreter()
@@ -1088,4 +1056,31 @@ def mask_and_ipv4_to_bcast_addr(mask, ip):
     return bcast_str
 
 
+def parse_net_config_data(net_config, skip_broken=True) -> NetworkState:
+    """Parses the config, returns NetworkState object
+
+    :param net_config: curtin network config dict
+    """
+    state = None
+    version = net_config.get('version')
+    config = net_config.get('config')
+    if version == 2:
+        # v2 does not have explicit 'config' key so we
+        # pass the whole net-config as-is
+        config = net_config
+
+    if version and config is not None:
+        nsi = NetworkStateInterpreter(version=version, config=config)
+        nsi.parse_config(skip_broken=skip_broken)
+        state = nsi.get_network_state()
+
+    if not state:
+        raise RuntimeError(
+            "No valid network_state object created from network config. "
+            "Did you specify the correct version?"
+        )
+
+    return state
+
+
 # vi: ts=4 expandtab
diff --git a/cloudinit/net/renderer.py b/cloudinit/net/renderer.py
index 2a61a7a8..27447bc2 100644
--- a/cloudinit/net/renderer.py
+++ b/cloudinit/net/renderer.py
@@ -28,6 +28,8 @@ filter_by_physical = filter_by_type('physical')
 
 
 class Renderer(object):
+    def __init__(self, config=None):
+        pass
 
     @staticmethod
     def _render_persistent_net(network_state):
diff --git a/cloudinit/net/renderers.py b/cloudinit/net/renderers.py
index c3931a98..822b45de 100644
--- a/cloudinit/net/renderers.py
+++ b/cloudinit/net/renderers.py
@@ -1,10 +1,13 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
+from typing import List, Tuple, Type
+
 from . import eni
 from . import freebsd
 from . import netbsd
 from . import netplan
 from . import networkd
+from . import renderer
 from . import RendererNotFoundError
 from . import openbsd
 from . import sysconfig
@@ -23,7 +26,9 @@ DEFAULT_PRIORITY = ["eni", "sysconfig", "netplan", "freebsd",
                     "netbsd", "openbsd", "networkd"]
 
 
-def search(priority=None, target=None, first=False):
+def search(
+    priority=None, target=None, first=False
+) -> List[Tuple[str, Type[renderer.Renderer]]]:
     if priority is None:
         priority = DEFAULT_PRIORITY
 
@@ -40,13 +45,13 @@ def search(priority=None, target=None, first=False):
         if render_mod.available(target):
             cur = (name, render_mod.Renderer)
             if first:
-                return cur
+                return [cur]
             found.append(cur)
 
     return found
 
 
-def select(priority=None, target=None):
+def select(priority=None, target=None) -> Tuple[str, Type[renderer.Renderer]]:
     found = search(priority, target=target, first=True)
     if not found:
         if priority is None:
@@ -57,6 +62,6 @@ def select(priority=None, target=None):
         raise RendererNotFoundError(
             "No available network renderers found%s. Searched "
             "through list: %s" % (tmsg, priority))
-    return found
+    return found[0]
 
 # vi: ts=4 expandtab
diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index 3a433c99..8031cd3a 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -18,8 +18,8 @@ from .network_state import (
     is_ipv6_addr, net_prefix_to_ipv4_mask, subnet_is_ipv6, IPV6_DYNAMIC_TYPES)
 
 LOG = logging.getLogger(__name__)
-NM_CFG_FILE = "/etc/NetworkManager/NetworkManager.conf"
 KNOWN_DISTROS = ['almalinux', 'centos', 'fedora', 'rhel', 'rocky', 'suse']
+NM_CFG_FILE = "/etc/NetworkManager/NetworkManager.conf"
 
 
 def _make_header(sep='#'):
@@ -931,7 +931,9 @@ class Renderer(renderer.Renderer):
             netrules_path = subp.target_path(target, self.netrules_path)
             util.write_file(netrules_path, netrules_content, file_mode)
         if available_nm(target=target):
-            enable_ifcfg_rh(subp.target_path(target, path=NM_CFG_FILE))
+            enable_ifcfg_rh(subp.target_path(
+                target, path=NM_CFG_FILE
+            ))
 
         sysconfig_path = subp.target_path(target, templates.get('control'))
         # Distros configuring /etc/sysconfig/network as a file e.g. Centos
@@ -978,7 +980,10 @@ def available_sysconfig(target=None):
 
 
 def available_nm(target=None):
-    if not os.path.isfile(subp.target_path(target, path=NM_CFG_FILE)):
+    if not os.path.isfile(subp.target_path(
+        target,
+        path=NM_CFG_FILE
+    )):
         return False
     return True
 
diff --git a/cloudinit/net/tests/test_network_state.py b/cloudinit/net/tests/test_network_state.py
index fc4724a1..84e8308a 100644
--- a/cloudinit/net/tests/test_network_state.py
+++ b/cloudinit/net/tests/test_network_state.py
@@ -67,11 +67,13 @@ class TestNetworkStateParseConfig(CiTestCase):
 
     def test_missing_version_returns_none(self):
         ncfg = {}
-        self.assertEqual(None, network_state.parse_net_config_data(ncfg))
+        with self.assertRaises(RuntimeError):
+            network_state.parse_net_config_data(ncfg)
 
     def test_unknown_versions_returns_none(self):
         ncfg = {'version': 13.2}
-        self.assertEqual(None, network_state.parse_net_config_data(ncfg))
+        with self.assertRaises(RuntimeError):
+            network_state.parse_net_config_data(ncfg)
 
     def test_version_2_passes_self_as_config(self):
         ncfg = {'version': 2, 'otherconfig': {}, 'somemore': [1, 2, 3]}