Comments and minor refactors. Additionally, move the wait_on_network …

…call into the activators.

cloudinit/cmd/main.py

@@ -48,6 +48,8 @@
     CLOUD_CONFIG,
 )
 
+Reason = str
+
 # Welcome message template
 WELCOME_MSG_TPL = (
     "Cloud-init v. {version} running '{action}' at "
@@ -337,9 +339,9 @@ def _should_bring_up_interfaces(init, args):
 
 def _should_wait_via_cloud_config(
     raw_config: Optional[Union[str, bytes]]
-) -> bool:
+) -> Tuple[bool, Reason]:
     if not raw_config:
-        return False
+        return False, "no configuration found"
 
     # If our header is anything other than #cloud-config, wait
     possible_header: Union[bytes, str] = raw_config.strip()[:13]
@@ -349,12 +351,12 @@ def _should_wait_via_cloud_config(
         try:
             decoded_header = possible_header.decode("utf-8")
         except UnicodeDecodeError:
-            return True
+            return True, "Binary user data found"
     if not decoded_header.startswith("#cloud-config"):
-        return True
+        return True, "non-cloud-config user data found"
 
     try:
-        userdata_yaml = yaml.safe_load(raw_config)
+        parsed_yaml = yaml.safe_load(raw_config)
     except Exception as e:
         log_with_downgradable_level(
             logger=LOG,
@@ -363,19 +365,25 @@ def _should_wait_via_cloud_config(
             msg="Unexpected failure parsing userdata: %s",
             args=e,
         )
-        return True
+        return True, "failed to parse user data as yaml"
 
     # These all have the potential to require network access, so we should wait
-    if "write_files" in userdata_yaml:
-        return any("source" in item for item in userdata_yaml["write_files"])
-    return bool(
-        userdata_yaml.get("bootcmd")
-        or userdata_yaml.get("random_seed", {}).get("command")
-        or userdata_yaml.get("mounts")
-    )
-
-
-def _should_wait_on_network(datasource: Optional[sources.DataSource]) -> bool:
+    if "write_files" in parsed_yaml and any(
+        "source" in item for item in parsed_yaml["write_files"]
+    ):
+        return True, "write_files with source found"
+    if parsed_yaml.get("bootcmd"):
+        return True, "bootcmd found"
+    if parsed_yaml.get("random_seed", {}).get("command"):
+        return True, "random_seed command found"
+    if parsed_yaml.get("mounts"):
+        return True, "mounts found"
+    return False, "cloud-config does not contain network requiring elements"
+
+
+def _should_wait_on_network(
+    datasource: Optional[sources.DataSource],
+) -> Tuple[bool, Reason]:
     """Determine if we should wait on network connectivity for cloud-init.
 
     We need to wait if:
@@ -386,14 +394,30 @@ def _should_wait_on_network(datasource: Optional[sources.DataSource]) -> bool:
     - We have user data that requires network access
     """
     if not datasource:
-        return True
-    return any(
-        _should_wait_via_cloud_config(config)
-        for config in [
-            datasource.get_userdata_raw(),
-            datasource.get_vendordata_raw(),
-            datasource.get_vendordata2_raw(),
-        ]
+        return True, "no datasource found"
+    user_should_wait, user_reason = _should_wait_via_cloud_config(
+        datasource.get_userdata_raw()
+    )
+    if user_should_wait:
+        return True, f"{user_reason} in user data"
+    vendor_should_wait, vendor_reason = _should_wait_via_cloud_config(
+        datasource.get_vendordata_raw()
+    )
+    if vendor_should_wait:
+        return True, f"{vendor_reason} in vendor data"
+    vendor2_should_wait, vendor2_reason = _should_wait_via_cloud_config(
+        datasource.get_vendordata2_raw()
+    )
+    if vendor2_should_wait:
+        return True, f"{vendor2_reason} in vendor data2"
+
+    return (
+        False,
+        (
+            f"user data: {user_reason}, "
+            f"vendor data: {vendor_reason}, "
+            f"vendor data2: {vendor2_reason}"
+        ),
     )
 
 
@@ -548,16 +572,19 @@ def main_init(name, args):
     init.apply_network_config(bring_up=bring_up_interfaces)
 
     if mode == sources.DSMODE_LOCAL:
-        if _should_wait_on_network(init.datasource):
+        should_wait, reason = _should_wait_on_network(init.datasource)
+        if should_wait:
             LOG.debug(
                 "Network connectivity determined necessary for "
-                "cloud-init's network stage"
+                "cloud-init's network stage. Reason: %s",
+                reason,
             )
             util.write_file(init.paths.get_runpath(".wait-on-network"), "")
         else:
             LOG.debug(
                 "Network connectivity determined unnecessary for "
-                "cloud-init's network stage"
+                "cloud-init's network stage. %s",
+                reason,
             )
 
         if init.datasource.dsmode != mode:

cloudinit/distros/__init__.py

@@ -349,15 +349,16 @@ def dhcp_client(self) -> dhcp.DhcpClient:
         raise dhcp.NoDHCPLeaseMissingDhclientError()
 
     @property
-    def network_activator(self) -> Optional[Type[activators.NetworkActivator]]:
-        """Return the configured network activator for this environment."""
+    def network_activator(self) -> Type[activators.NetworkActivator]:
+        """Return the configured network activator for this environment.
+
+        :returns: The network activator class to use
+        "raises": NoActivatorException if no activator is found
+        """
         priority = util.get_cfg_by_path(
             self._cfg, ("network", "activators"), None
         )
-        try:
-            return activators.select_activator(priority=priority)
-        except activators.NoActivatorException:
-            return None
+        return activators.select_activator(priority=priority)
 
     @property
     def network_renderer(self) -> Renderer:
@@ -460,8 +461,9 @@ def apply_network_config(self, netconfig, bring_up=False) -> bool:
         # Now try to bring them up
         if bring_up:
             LOG.debug("Bringing up newly configured network interfaces")
-            network_activator = self.network_activator
-            if not network_activator:
+            try:
+                network_activator = self.network_activator
+            except activators.NoActivatorException:
                 LOG.warning(
                     "No network activator found, not bringing up "
                     "network interfaces"

cloudinit/distros/debian.py

@@ -15,7 +15,6 @@
 from cloudinit.distros.package_management.apt import Apt
 from cloudinit.distros.package_management.package_manager import PackageManager
 from cloudinit.distros.parsers.hostname import HostnameConf
-from cloudinit.net import netplan
 from cloudinit.net.netplan import CLOUDINIT_NETPLAN_FILE
 
 LOG = logging.getLogger(__name__)
@@ -221,10 +220,6 @@ def set_keymap(self, layout: str, model: str, variant: str, options: str):
         # be needed
         self.manage_service("restart", "console-setup")
 
-    def wait_for_network(self):
-        """Ensure that cloud-init's network service has network connectivity"""
-        netplan.wait_for_network()
-
 
 def _maybe_remove_legacy_eth0(path="/etc/network/interfaces.d/eth0.cfg"):
     """Ubuntu cloud images previously included a 'eth0.cfg' that had

cloudinit/distros/ubuntu.py

@@ -10,11 +10,15 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
 import copy
+import logging
 
 from cloudinit.distros import PREFERRED_NTP_CLIENTS, debian
 from cloudinit.distros.package_management.snap import Snap
+from cloudinit.net import activators
 from cloudinit.net.netplan import CLOUDINIT_NETPLAN_FILE
 
+LOG = logging.getLogger(__name__)
+
 
 class Distro(debian.Distro):
     def __init__(self, name, cfg, paths):
@@ -49,3 +53,12 @@ def preferred_ntp_clients(self):
         if not self._preferred_ntp_clients:
             self._preferred_ntp_clients = copy.deepcopy(PREFERRED_NTP_CLIENTS)
         return self._preferred_ntp_clients
+
+    def wait_for_network(self) -> None:
+        """Ensure that cloud-init's network service has network connectivity"""
+        try:
+            self.network_activator.wait_for_network()
+        except activators.NoActivatorException:
+            LOG.error("Failed to wait for network. No network activator found")
+        except Exception as e:
+            LOG.error("Failed to wait for network: %s", e)

cloudinit/net/activators.py

@@ -5,12 +5,9 @@
 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 import eni, netplan, network_manager, networkd
 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
-from cloudinit.net.networkd import available as networkd_available
 
 LOG = logging.getLogger(__name__)
 
@@ -88,6 +85,11 @@ def bring_up_all_interfaces(cls, network_state: NetworkState) -> bool:
             [i["name"] for i in network_state.iter_interfaces()]
         )
 
+    @staticmethod
+    def wait_for_network() -> None:
+        """Wait for network to come up."""
+        raise NotImplementedError()
+
 
 class IfUpDownActivator(NetworkActivator):
     # Note that we're not overriding bring_up_interfaces to pass something
@@ -97,7 +99,7 @@ class IfUpDownActivator(NetworkActivator):
     @staticmethod
     def available(target: Optional[str] = None) -> bool:
         """Return true if ifupdown can be used on this system."""
-        return eni_available(target=target)
+        return eni.available(target=target)
 
     @staticmethod
     def bring_up_interface(device_name: str) -> bool:
@@ -149,7 +151,7 @@ class NetworkManagerActivator(NetworkActivator):
     @staticmethod
     def available(target=None) -> bool:
         """Return true if NetworkManager can be used on this system."""
-        return nm_available(target=target)
+        return network_manager.available(target=target)
 
     @staticmethod
     def bring_up_interface(device_name: str) -> bool:
@@ -215,7 +217,7 @@ class NetplanActivator(NetworkActivator):
     @staticmethod
     def available(target=None) -> bool:
         """Return true if netplan can be used on this system."""
-        return netplan_available(target=target)
+        return netplan.available(target=target)
 
     @staticmethod
     def bring_up_interface(device_name: str) -> bool:
@@ -269,12 +271,21 @@ def bring_down_interface(device_name: str) -> bool:
             NetplanActivator.NETPLAN_CMD, "all", warn_on_stderr=False
         )
 
+    @staticmethod
+    def wait_for_network() -> None:
+        """On networkd systems, wait for systemd-networkd-wait-online"""
+        # At the moment, this is only supported using the networkd renderer.
+        if network_manager.available():
+            LOG.debug("NetworkManager is enabled, skipping networkd wait")
+            return
+        NetworkdActivator.wait_for_network()
+
 
 class NetworkdActivator(NetworkActivator):
     @staticmethod
     def available(target=None) -> bool:
         """Return true if ifupdown can be used on this system."""
-        return networkd_available(target=target)
+        return networkd.available(target=target)
 
     @staticmethod
     def bring_up_interface(device_name: str) -> bool:
@@ -296,6 +307,31 @@ def bring_down_interface(device_name: str) -> bool:
             partial(Iproute2.link_down, device_name)
         )
 
+    @staticmethod
+    def wait_for_network() -> None:
+        """Wait for systemd-networkd-wait-online."""
+        wait_online_def: str = subp.subp(
+            ["systemctl", "cat", "systemd-networkd-wait-online.service"]
+        ).stdout
+
+        # We need to extract the ExecStart= lines from the service definition.
+        # If we come across an ExecStart= line that is empty, that clears any
+        # previously found commands, which we should expect from the drop-in.
+        # Since the service is a oneshot, we can have multiple ExecStart= lines
+        # and systemd runs them in parallel. We'll run them serially since
+        # there's really no gain for us in running them in parallel.
+        wait_commands: List[List[str]] = []
+        for line in wait_online_def.splitlines():
+            if line.startswith("ExecStart="):
+                command_str = line.split("=", 1)[1].strip()
+                if not command_str:
+                    wait_commands.clear()
+                else:
+                    wait_commands.append(command_str.split())
+
+        for command in wait_commands:
+            subp.subp(command)
+
 
 # This section is mostly copied and pasted from renderers.py. An abstract
 # version to encompass both seems overkill at this point
@@ -318,35 +354,36 @@ def bring_down_interface(device_name: str) -> bool:
 
 def search_activator(
     priority: List[str], target: Union[str, None]
-) -> List[Type[NetworkActivator]]:
+) -> Optional[Type[NetworkActivator]]:
+    """Returns the first available activator from the priority list or None."""
     unknown = [i for i in priority if i not in DEFAULT_PRIORITY]
     if unknown:
         raise ValueError(
-            "Unknown activators provided in priority list: %s" % unknown
+            f"Unknown activators provided in priority list: {unknown}"
         )
     activator_classes = [NAME_TO_ACTIVATOR[name] for name in priority]
-    return [
-        activator_cls
-        for activator_cls in activator_classes
-        if activator_cls.available(target)
-    ]
+    return next(
+        (
+            activator_cls
+            for activator_cls in activator_classes
+            if activator_cls.available(target)
+        ),
+        None,
+    )
 
 
 def select_activator(
     priority: Optional[List[str]] = None, target: Optional[str] = None
 ) -> Type[NetworkActivator]:
     if priority is None:
         priority = DEFAULT_PRIORITY
-    found = search_activator(priority, target)
-    if not found:
-        tmsg = ""
-        if target and target != "/":
-            tmsg = " in target=%s" % target
+    selected = search_activator(priority, target)
+    if not selected:
+        tmsg = f" in target={target}" if target and target != "/" else ""
         raise NoActivatorException(
-            "No available network activators found%s. Searched "
-            "through list: %s" % (tmsg, priority)
+            f"No available network activators found{tmsg}. "
+            f"Searched through list: {priority}"
         )
-    selected = found[0]
     LOG.debug(
         "Using selected activator: %s from priority: %s", selected, priority
     )