From 0273712c90d6facfc0fbf8d6def352f9810902a3 Mon Sep 17 00:00:00 2001
From: sxt1001 <shixuantong1@huawei.com>
Date: Mon, 3 Apr 2023 23:52:15 +0800
Subject: [PATCH] Cleanup ephemeral IP routes on exception (#2100)

If an exception occurs during EphemeralIPv4Network setup, any routes
that were setup need to be torn down. This wasn't happening, and this
commit adds the teardown.
---
 cloudinit/net/__init__.py        | 43 +++++++++++--------
 tests/unittests/net/test_init.py | 73 ++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+), 17 deletions(-)

diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py
index 3297a31..fc9668e 100644
--- a/cloudinit/net/__init__.py
+++ b/cloudinit/net/__init__.py
@@ -1370,23 +1370,32 @@ class EphemeralIPv4Network(object):
                 )
                 return
 
-        self._bringup_device()
-
-        # rfc3442 requires us to ignore the router config *if* classless static
-        # routes are provided.
-        #
-        # https://tools.ietf.org/html/rfc3442
-        #
-        # If the DHCP server returns both a Classless Static Routes option and
-        # a Router option, the DHCP client MUST ignore the Router option.
-        #
-        # Similarly, if the DHCP server returns both a Classless Static Routes
-        # option and a Static Routes option, the DHCP client MUST ignore the
-        # Static Routes option.
-        if self.static_routes:
-            self._bringup_static_routes()
-        elif self.router:
-            self._bringup_router()
+        try:
+            self._bringup_device()
+
+            # rfc3442 requires us to ignore the router config *if*
+            # classless static routes are provided.
+            #
+            # https://tools.ietf.org/html/rfc3442
+            #
+            # If the DHCP server returns both a Classless Static Routes
+            # option and a Router option, the DHCP client MUST ignore
+            # the Router option.
+            #
+            # Similarly, if the DHCP server returns both a Classless
+            # Static Routes option and a Static Routes option, the DHCP
+            # client MUST ignore the Static Routes option.
+            if self.static_routes:
+                self._bringup_static_routes()
+            elif self.router:
+                self._bringup_router()
+        except subp.ProcessExecutionError:
+            LOG.error(
+                "Error bringing up EphemeralIPv4Network. "
+                "Datasource setup cannot continue"
+            )
+            self.__exit__(None, None, None)
+            raise
 
     def __exit__(self, excp_type, excp_value, excp_traceback):
         """Teardown anything we set up."""
diff --git a/tests/unittests/net/test_init.py b/tests/unittests/net/test_init.py
index 768cc11..5da1232 100644
--- a/tests/unittests/net/test_init.py
+++ b/tests/unittests/net/test_init.py
@@ -13,6 +13,7 @@ import pytest
 import requests
 
 import cloudinit.net as net
+from cloudinit import subp
 from cloudinit.subp import ProcessExecutionError
 from cloudinit.util import ensure_file, write_file
 from tests.unittests.helpers import CiTestCase, HttprettyTestCase
@@ -853,6 +854,78 @@ class TestEphemeralIPV4Network(CiTestCase):
             self.assertEqual(expected_setup_calls, m_subp.call_args_list)
         m_subp.assert_has_calls(expected_teardown_calls)
 
+    def test_teardown_on_enter_exception(self, m_subp):
+        """Ensure ephemeral teardown happens.
+
+        Even though we're using a context manager, we need to handle any
+        exceptions raised in __enter__ manually and do the appropriate
+        teardown.
+        """
+
+        def side_effect(args, **kwargs):
+            if args[3] == "append" and args[4] == "3.3.3.3/32":
+                raise subp.ProcessExecutionError("oh no!")
+
+        m_subp.side_effect = side_effect
+
+        with pytest.raises(subp.ProcessExecutionError):
+            with net.EphemeralIPv4Network(
+                interface="eth0",
+                ip="1.1.1.1",
+                prefix_or_mask="255.255.255.0",
+                broadcast="1.1.1.255",
+                static_routes=[
+                    ("2.2.2.2/32", "9.9.9.9"),
+                    ("3.3.3.3/32", "8.8.8.8"),
+                ],
+            ):
+                pass
+
+        expected_teardown_calls = [
+            mock.call(
+                [
+                    "ip",
+                    "-4",
+                    "route",
+                    "del",
+                    "2.2.2.2/32",
+                    "via",
+                    "9.9.9.9",
+                    "dev",
+                    "eth0",
+                ],
+                capture=True,
+            ),
+            mock.call(
+                [
+                    "ip",
+                    "-family",
+                    "inet",
+                    "link",
+                    "set",
+                    "dev",
+                    "eth0",
+                    "down",
+                ],
+                capture=True,
+            ),
+            mock.call(
+                [
+                    "ip",
+                    "-family",
+                    "inet",
+                    "addr",
+                    "del",
+                    "1.1.1.1/24",
+                    "dev",
+                    "eth0",
+                ],
+                capture=True,
+            ),
+        ]
+        for teardown in expected_teardown_calls:
+            assert teardown in m_subp.call_args_list
+
     @mock.patch("cloudinit.net.readurl")
     def test_ephemeral_ipv4_no_network_if_url_connectivity(
         self, m_readurl, m_subp
-- 
2.33.0