diff --git a/cloudinit/net/ephemeral.py b/cloudinit/net/ephemeral.py index 3b5f9b96a2e..6efebed058d 100644 --- a/cloudinit/net/ephemeral.py +++ b/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 diff --git a/tests/unittests/net/test_ephemeral.py b/tests/unittests/net/test_ephemeral.py index 99fbcb0bf6b..77ac95c912f 100644 --- a/tests/unittests/net/test_ephemeral.py +++ b/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