diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py index 1c0440bafa0..a451dfa18a6 100644 --- a/cloudinit/cmd/main.py +++ b/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: diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index d9dfaf57bf8..db64b733b5b 100644 --- a/cloudinit/distros/__init__.py +++ b/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" diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py index 0c3f659a3a9..cef6e1fb8c0 100644 --- a/cloudinit/distros/debian.py +++ b/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 diff --git a/cloudinit/distros/ubuntu.py b/cloudinit/distros/ubuntu.py index fbee6e89737..6f432e19661 100644 --- a/cloudinit/distros/ubuntu.py +++ b/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) diff --git a/cloudinit/net/activators.py b/cloudinit/net/activators.py index 36e1cedcc1f..6c907755b66 100644 --- a/cloudinit/net/activators.py +++ b/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,18 +354,22 @@ 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( @@ -337,16 +377,13 @@ def select_activator( ) -> 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 ) diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py index 10b661170f6..50af17694d5 100644 --- a/cloudinit/net/netplan.py +++ b/cloudinit/net/netplan.py @@ -13,7 +13,6 @@ IPV6_DYNAMIC_TYPES, SYS_CLASS_NET, get_devicelist, - network_manager, renderer, should_add_gateway_onlink_flag, subnet_is_ipv6, @@ -571,32 +570,3 @@ def available(target=None): if not subp.which(p, search=search, target=target): return False return True - - -def wait_for_network() -> None: - """On networkd systems, manually 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 - 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) diff --git a/systemd/cloud-init-network.service.tmpl b/systemd/cloud-init-network.service.tmpl index 6c431e1f91a..1269b416206 100644 --- a/systemd/cloud-init-network.service.tmpl +++ b/systemd/cloud-init-network.service.tmpl @@ -9,7 +9,7 @@ Wants=cloud-init-local.service Wants=sshd-keygen.service Wants=sshd.service After=cloud-init-local.service -{% if variant not in ["debian", "ubuntu"] %} +{% if variant not in ["ubuntu"] %} After=systemd-networkd-wait-online.service {% endif %} {% if variant in ["ubuntu", "unknown", "debian"] %} diff --git a/tests/integration_tests/util.py b/tests/integration_tests/util.py index 4f728f39a2d..19e11936550 100644 --- a/tests/integration_tests/util.py +++ b/tests/integration_tests/util.py @@ -15,11 +15,7 @@ from cloudinit.subp import subp from tests.integration_tests.integration_settings import PLATFORM -from tests.integration_tests.releases import ( - CURRENT_RELEASE, - NETWORK_SERVICE_FILE, - NOBLE, -) +from tests.integration_tests.releases import CURRENT_RELEASE, NOBLE LOG = logging.getLogger("integration_testing.util") diff --git a/tests/unittests/cmd/test_main.py b/tests/unittests/cmd/test_main.py index 508e969acb6..4b4eaa76915 100644 --- a/tests/unittests/cmd/test_main.py +++ b/tests/unittests/cmd/test_main.py @@ -244,7 +244,7 @@ def test_should_wait_on_network(self, ds, userdata, expected): ds.get_userdata_raw = mock.Mock(return_value=userdata) ds.get_vendordata_raw = mock.Mock(return_value=None) ds.get_vendordata2_raw = mock.Mock(return_value=None) - assert main._should_wait_on_network(ds) is expected + assert main._should_wait_on_network(ds)[0] is expected # Here we rotate our configs to ensure that any of userdata, # vendordata, or vendordata2 can be the one that causes us to wait. @@ -259,14 +259,14 @@ def test_should_wait_on_network(self, ds, userdata, expected): ds.get_vendordata2_raw, ds.get_userdata_raw, ) - assert main._should_wait_on_network(ds) is expected + assert main._should_wait_on_network(ds)[0] is expected @pytest.mark.parametrize( "distro,should_wait,expected_add_wait", [ ("ubuntu", True, True), ("ubuntu", False, False), - ("debian", True, True), + ("debian", True, False), ("debian", False, False), ("centos", True, False), ("rhel", False, False), @@ -293,6 +293,7 @@ def fake_subp(*args, **kwargs): return SubpResult(FAKE_SERVICE_FILE, "") return SubpResult("", "") + mocker.patch("cloudinit.net.netplan.available", return_value=True) m_nm = mocker.patch( "cloudinit.net.network_manager.available", return_value=False )