ephemeral: Handle link up failure for both ipv4 and ipv6 (#4547)

EphemeralIPv{4,6} failure is not always an error, therefore do not log
this event as an error in the context manager. Allow call sites to
determine log level.

Fixes #4540

cloudinit/net/ephemeral.py

@@ -8,12 +8,12 @@
 from typing import Any, Callable, Dict, List, Optional
 
 import cloudinit.net as net
-from cloudinit import subp
 from cloudinit.net.dhcp import (
     IscDhclient,
     NoDHCPLeaseError,
     maybe_perform_dhcp_discovery,
 )
+from cloudinit.subp import ProcessExecutionError
 
 LOG = logging.getLogger(__name__)
 
@@ -106,11 +106,7 @@ def __enter__(self):
                 self._bringup_static_routes()
             elif self.router:
                 self._bringup_router()
-        except subp.ProcessExecutionError:
-            LOG.error(
-                "Error bringing up EphemeralIPv4Network. "
-                "Datasource setup cannot continue"
-            )
+        except ProcessExecutionError:
             self.__exit__(None, None, None)
             raise
 
@@ -130,7 +126,7 @@ def _bringup_device(self):
         )
         try:
             self.distro.net_ops.add_addr(self.interface, cidr, self.broadcast)
-        except subp.ProcessExecutionError as e:
+        except ProcessExecutionError as e:
             if "File exists" not in str(e.stderr):
                 raise
             LOG.debug(
@@ -274,11 +270,9 @@ def __exit__(self, excp_type, excp_value, excp_traceback):
 
     def clean_network(self):
         """Exit _ephipv4 context to teardown of ip configuration performed."""
-        if self.lease:
-            self.lease = None
-        if not self._ephipv4:
-            return
-        self._ephipv4.__exit__(None, None, None)
+        self.lease = None
+        if self._ephipv4:
+            self._ephipv4.__exit__(None, None, None)
 
     def obtain_lease(self):
         """Perform dhcp discovery in a sandboxed environment if possible.
@@ -350,7 +344,13 @@ def get_first_option_value(
 
 
 class EphemeralIPNetwork:
-    """Marries together IPv4 and IPv6 ephemeral context managers"""
+    """Combined ephemeral context manager for IPv4 and IPv6
+
+    Either ipv4 or ipv6 ephemeral network may fail to initialize, but if either
+    succeeds, then this context manager will not raise exception. This allows
+    either ipv4 or ipv6 ephemeral network to succeed, but requires that error
+    handling for networks unavailable be done within the context.
+    """
 
     def __init__(
         self,
@@ -367,21 +367,50 @@ def __init__(
         self.distro = distro
 
     def __enter__(self):
-        # ipv6 dualstack might succeed when dhcp4 fails
-        # therefore catch exception unless only v4 is used
+        if not (self.ipv4 or self.ipv6):
+            # no ephemeral network requested, but this object still needs to
+            # function as a context manager
+            return self
         try:
+            exceptions = []
+            ephemeral_obtained = False
             if self.ipv4:
-                self.stack.enter_context(
-                    EphemeralDHCPv4(self.distro, self.interface)
-                )
+                try:
+                    self.stack.enter_context(
+                        EphemeralDHCPv4(
+                            self.distro,
+                            self.interface,
+                        )
+                    )
+                    ephemeral_obtained = True
+                except (ProcessExecutionError, NoDHCPLeaseError) as e:
+                    LOG.info("Failed to bring up %s for ipv4.", self)
+                    exceptions.append(e)
+
             if self.ipv6:
-                self.stack.enter_context(
-                    EphemeralIPv6Network(self.distro, self.interface)
+                try:
+                    self.stack.enter_context(
+                        EphemeralIPv6Network(
+                            self.distro,
+                            self.interface,
+                        )
+                    )
+                    ephemeral_obtained = True
+                except ProcessExecutionError as e:
+                    LOG.info("Failed to bring up %s for ipv6.", self)
+                    exceptions.append(e)
+            if not ephemeral_obtained:
+                # Ephemeral network setup failed in linkup for both ipv4 and
+                # ipv6. Raise only the first exception found.
+                LOG.error(
+                    "Failed to bring up EphemeralIPNetwork. "
+                    "Datasource setup cannot continue"
                 )
-        # v6 link local might be usable
-        # caller may want to log network state
+                raise exceptions[0]
         except NoDHCPLeaseError as e:
             if self.ipv6:
+                # ipv6 dualstack might succeed when dhcp4 fails, so catch
+                # NoDHCPLeaseError unless only v4 is used
                 self.state_msg = "using link-local ipv6"
             else:
                 raise e

tests/unittests/net/test_ephemeral.py

@@ -5,6 +5,8 @@
 import pytest
 
 from cloudinit.net.ephemeral import EphemeralIPNetwork
+from cloudinit.subp import ProcessExecutionError
+from tests.unittests.helpers import does_not_raise
 from tests.unittests.util import MockDistro
 
 M_PATH = "cloudinit.net.ephemeral."
@@ -51,3 +53,52 @@ def test_stack_order(
             expected_call_args_list
             == m_exit_stack.return_value.enter_context.call_args_list
         )
+
+    @pytest.mark.parametrize(
+        "m_v4, m_v6, m_context, m_side_effects",
+        [
+            pytest.param(
+                False, True, does_not_raise(), [None, None], id="v6_only"
+            ),
+            pytest.param(
+                True, False, does_not_raise(), [None, None], id="v4_only"
+            ),
+            pytest.param(
+                True,
+                True,
+                does_not_raise(),
+                [ProcessExecutionError, None],
+                id="v4_error",
+            ),
+            pytest.param(
+                True,
+                True,
+                does_not_raise(),
+                [None, ProcessExecutionError],
+                id="v6_error",
+            ),
+            pytest.param(
+                True,
+                True,
+                pytest.raises(ProcessExecutionError),
+                [
+                    ProcessExecutionError,
+                    ProcessExecutionError,
+                ],
+                id="v4_v6_error",
+            ),
+        ],
+    )
+    def test_interface_init_failures(
+        self, m_v4, m_v6, m_context, m_side_effects, mocker
+    ):
+        mocker.patch(
+            "cloudinit.net.ephemeral.EphemeralDHCPv4"
+        ).return_value.__enter__.side_effect = m_side_effects[0]
+        mocker.patch(
+            "cloudinit.net.ephemeral.EphemeralIPv6Network"
+        ).return_value.__enter__.side_effect = m_side_effects[1]
+        distro = MockDistro()
+        with m_context:
+            with EphemeralIPNetwork(distro, "eth0", ipv4=m_v4, ipv6=m_v6):
+                pass