sources/azure: consolidate DHCP variants to EphemeralDHCPv4WithReporting

cloudinit/sources/DataSourceAzure.py

@@ -23,7 +23,6 @@
 from cloudinit import net, sources, ssh_util, subp, util
 from cloudinit.event import EventScope, EventType
 from cloudinit.net import device_driver
-from cloudinit.net.dhcp import EphemeralDHCPv4
 from cloudinit.reporting import events
 from cloudinit.sources.helpers import netlink
 from cloudinit.sources.helpers.azure import (
@@ -32,7 +31,6 @@
     azure_ds_reporter,
     azure_ds_telemetry_reporter,
     build_minimal_ovf,
-    dhcp_log_cb,
     get_boot_telemetry,
     get_metadata_from_fabric,
     get_system_info,
@@ -946,20 +944,10 @@ def _check_if_nic_is_primary(self, ifname):
         # VM provisioning if there is any DHCP failure when trying to determine
         # the primary NIC.
         try:
-            with events.ReportEventStack(
-                name="obtain-dhcp-lease",
-                description=(
-                    "obtain dhcp lease for %s when attempting to "
-                    "determine primary NIC during reprovision of "
-                    "a pre-provisioned VM"
-                )
-                % ifname,
-                parent=azure_ds_reporter,
-            ):
-                dhcp_ctx = EphemeralDHCPv4(
-                    iface=ifname, dhcp_log_func=dhcp_log_cb
-                )
-                dhcp_ctx.obtain_lease()
+            dhcp_ctx = EphemeralDHCPv4WithReporting(
+                azure_ds_reporter, iface=ifname
+            )
+            dhcp_ctx.obtain_lease()
         except Exception as e:
             report_diagnostic_event(
                 "Giving up. Failed to obtain dhcp lease "
@@ -1234,15 +1222,10 @@ def exc_cb(msg, exception):
                 if not is_ephemeral_ctx_present:
                     # Save our EphemeralDHCPv4 context to avoid repeated dhcp
                     # later when we report ready
-                    with events.ReportEventStack(
-                        name="obtain-dhcp-lease",
-                        description="obtain dhcp lease",
-                        parent=azure_ds_reporter,
-                    ):
-                        self._ephemeral_dhcp_ctx = EphemeralDHCPv4(
-                            dhcp_log_func=dhcp_log_cb
-                        )
-                        lease = self._ephemeral_dhcp_ctx.obtain_lease()
+                    self._ephemeral_dhcp_ctx = EphemeralDHCPv4WithReporting(
+                        azure_ds_reporter
+                    )
+                    lease = self._ephemeral_dhcp_ctx.obtain_lease()
 
                 if vnet_switched:
                     dhcp_attempts += 1

cloudinit/sources/helpers/azure.py

@@ -1243,11 +1243,12 @@ def dhcp_log_cb(out, err):
     )
 
 
-class EphemeralDHCPv4WithReporting:
-    def __init__(self, reporter, nic=None):
+class EphemeralDHCPv4WithReporting(EphemeralDHCPv4):
+    def __init__(self, reporter, iface=None):
         self.reporter = reporter
-        self.ephemeralDHCPv4 = EphemeralDHCPv4(
-            iface=nic, dhcp_log_func=dhcp_log_cb
+
+        super(EphemeralDHCPv4WithReporting, self).__init__(
+            iface=iface, dhcp_log_func=dhcp_log_cb
         )
 
     def __enter__(self):
@@ -1256,10 +1257,7 @@ def __enter__(self):
             description="obtain dhcp lease",
             parent=self.reporter,
         ):
-            return self.ephemeralDHCPv4.__enter__()
-
-    def __exit__(self, excp_type, excp_value, excp_traceback):
-        self.ephemeralDHCPv4.__exit__(excp_type, excp_value, excp_traceback)
+            return super(EphemeralDHCPv4WithReporting, self).__enter__()
 
 
 # vi: ts=4 expandtab

tests/unittests/sources/test_azure.py

@@ -465,7 +465,7 @@ def setUp(self):
         )
 
     @mock.patch(MOCKPATH + "readurl")
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4", autospec=True)
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     @mock.patch(MOCKPATH + "net.is_up", autospec=True)
     def test_get_metadata_does_not_dhcp_if_network_is_up(
         self, m_net_is_up, m_dhcp, m_readurl
@@ -487,7 +487,7 @@ def test_get_metadata_does_not_dhcp_if_network_is_up(
         )
 
     @mock.patch(MOCKPATH + "readurl", autospec=True)
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     @mock.patch(MOCKPATH + "net.is_up")
     def test_get_metadata_uses_instance_url(
         self, m_net_is_up, m_dhcp, m_readurl
@@ -512,7 +512,7 @@ def test_get_metadata_uses_instance_url(
         )
 
     @mock.patch(MOCKPATH + "readurl", autospec=True)
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     @mock.patch(MOCKPATH + "net.is_up")
     def test_get_network_metadata_uses_network_url(
         self, m_net_is_up, m_dhcp, m_readurl
@@ -538,7 +538,7 @@ def test_get_network_metadata_uses_network_url(
         )
 
     @mock.patch(MOCKPATH + "readurl", autospec=True)
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     @mock.patch(MOCKPATH + "net.is_up")
     def test_get_default_metadata_uses_instance_url(
         self, m_net_is_up, m_dhcp, m_readurl
@@ -561,7 +561,7 @@ def test_get_default_metadata_uses_instance_url(
         )
 
     @mock.patch(MOCKPATH + "readurl", autospec=True)
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     @mock.patch(MOCKPATH + "net.is_up")
     def test_get_metadata_uses_extended_url(
         self, m_net_is_up, m_dhcp, m_readurl
@@ -698,6 +698,15 @@ def setUp(self):
         self.patches.enter_context(
             mock.patch.object(dsaz, "_get_random_seed", return_value="wild")
         )
+
+        self.m_dhcp = self.patches.enter_context(
+            mock.patch.object(
+                dsaz,
+                "EphemeralDHCPv4WithReporting",
+                autospec=True,
+            )
+        )
+
         self.m_get_metadata_from_imds = self.patches.enter_context(
             mock.patch.object(
                 dsaz,
@@ -799,8 +808,6 @@ def _load_possible_azure_ds(seed_dir, cache_dir):
             return_value={"public-keys": []}
         )
         self.m_report_failure_to_fabric = mock.MagicMock(autospec=True)
-        self.m_ephemeral_dhcpv4 = mock.MagicMock()
-        self.m_ephemeral_dhcpv4_with_reporting = mock.MagicMock()
         self.m_list_possible_azure_ds = mock.MagicMock(
             side_effect=_load_possible_azure_ds
         )
@@ -834,12 +841,6 @@ def _dmi_mocks(key):
                     "report_failure_to_fabric",
                     self.m_report_failure_to_fabric,
                 ),
-                (dsaz, "EphemeralDHCPv4", self.m_ephemeral_dhcpv4),
-                (
-                    dsaz,
-                    "EphemeralDHCPv4WithReporting",
-                    self.m_ephemeral_dhcpv4_with_reporting,
-                ),
                 (dsaz, "get_boot_telemetry", mock.MagicMock()),
                 (dsaz, "get_system_info", mock.MagicMock()),
                 (dsaz.subp, "which", lambda x: True),
@@ -1138,16 +1139,13 @@ def test_crawl_metadata_call_imds_once_no_reprovision(self):
         dsrc.crawl_metadata()
         self.assertEqual(1, self.m_get_metadata_from_imds.call_count)
 
-    @mock.patch(
-        "cloudinit.sources.DataSourceAzure.EphemeralDHCPv4WithReporting"
-    )
     @mock.patch("cloudinit.sources.DataSourceAzure.util.write_file")
     @mock.patch(
         "cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready"
     )
     @mock.patch("cloudinit.sources.DataSourceAzure.DataSourceAzure._poll_imds")
     def test_crawl_metadata_call_imds_twice_with_reprovision(
-        self, poll_imds_func, m_report_ready, m_write, m_dhcp
+        self, poll_imds_func, m_report_ready, m_write
     ):
         """If reprovisioning, imds metadata will be fetched twice"""
         ovfenv = construct_valid_ovf_env(
@@ -1160,16 +1158,13 @@ def test_crawl_metadata_call_imds_twice_with_reprovision(
         dsrc.crawl_metadata()
         self.assertEqual(2, self.m_get_metadata_from_imds.call_count)
 
-    @mock.patch(
-        "cloudinit.sources.DataSourceAzure.EphemeralDHCPv4WithReporting"
-    )
     @mock.patch("cloudinit.sources.DataSourceAzure.util.write_file")
     @mock.patch(
         "cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready"
     )
     @mock.patch("cloudinit.sources.DataSourceAzure.DataSourceAzure._poll_imds")
     def test_crawl_metadata_on_reprovision_reports_ready(
-        self, poll_imds_func, m_report_ready, m_write, m_dhcp
+        self, poll_imds_func, m_report_ready, m_write
     ):
         """If reprovisioning, report ready at the end"""
         ovfenv = construct_valid_ovf_env(
@@ -1182,9 +1177,6 @@ def test_crawl_metadata_on_reprovision_reports_ready(
         dsrc.crawl_metadata()
         self.assertEqual(1, m_report_ready.call_count)
 
-    @mock.patch(
-        "cloudinit.sources.DataSourceAzure.EphemeralDHCPv4WithReporting"
-    )
     @mock.patch("cloudinit.sources.DataSourceAzure.util.write_file")
     @mock.patch(
         "cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready"
@@ -1195,7 +1187,7 @@ def test_crawl_metadata_on_reprovision_reports_ready(
         "_wait_for_all_nics_ready"
     )
     def test_crawl_metadata_waits_for_nic_on_savable_vms(
-        self, detect_nics, poll_imds_func, report_ready_func, m_write, m_dhcp
+        self, detect_nics, poll_imds_func, report_ready_func, m_write
     ):
         """If reprovisioning, report ready at the end"""
         ovfenv = construct_valid_ovf_env(
@@ -1212,9 +1204,6 @@ def test_crawl_metadata_waits_for_nic_on_savable_vms(
         self.assertEqual(1, report_ready_func.call_count)
         self.assertEqual(1, detect_nics.call_count)
 
-    @mock.patch(
-        "cloudinit.sources.DataSourceAzure.EphemeralDHCPv4WithReporting"
-    )
     @mock.patch("cloudinit.sources.DataSourceAzure.util.write_file")
     @mock.patch(
         "cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready"
@@ -1232,7 +1221,6 @@ def test_detect_nics_when_marker_present(
         poll_imds_func,
         report_ready_func,
         m_write,
-        m_dhcp,
     ):
         """If reprovisioning, wait for nic attach if marker present"""
 
@@ -1286,9 +1274,7 @@ def test_crawl_metadata_on_reprovision_reports_ready_using_lease(
                 "subnet-mask": "255.255.255.0",
                 "unknown-245": "624c3620",
             }
-            self.m_ephemeral_dhcpv4_with_reporting.return_value.__enter__.return_value = (  # noqa: E501
-                lease
-            )
+            self.m_dhcp.return_value.__enter__.return_value = lease
             m_media_switch.return_value = None
 
             reprovision_ovfenv = construct_valid_ovf_env()
@@ -1793,9 +1779,7 @@ def test_dsaz_report_failure_no_net_uses_new_ephemeral_dhcp_lease(self):
             # setup ephemeral dhcp lease discovery mock
             test_lease_dhcp_option_245 = "test_lease_dhcp_option_245"
             test_lease = {"unknown-245": test_lease_dhcp_option_245}
-            self.m_ephemeral_dhcpv4_with_reporting.return_value.__enter__.return_value = (  # noqa: E501
-                test_lease
-            )
+            self.m_dhcp.return_value.__enter__.return_value = test_lease
 
             self.assertTrue(dsrc._report_failure())
 
@@ -1820,9 +1804,7 @@ def test_dsaz_report_failure_no_net_and_no_dhcp_uses_fallback_lease(self):
             m_dsrc_distro_networking_is_up.return_value = False
             # ephemeral dhcp discovery failure,
             # so cannot use a new ephemeral dhcp
-            self.m_ephemeral_dhcpv4_with_reporting.return_value.__enter__.side_effect = (  # noqa: E501
-                Exception
-            )
+            self.m_dhcp.return_value.__enter__.side_effect = Exception
 
             self.assertTrue(dsrc._report_failure())
 
@@ -2867,7 +2849,7 @@ def test_nic_detach_writes_marker(self, m_writefile, m_detach):
 
     @mock.patch(MOCKPATH + "util.write_file", autospec=True)
     @mock.patch(MOCKPATH + "DataSourceAzure.fallback_interface")
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     @mock.patch(MOCKPATH + "DataSourceAzure._report_ready")
     @mock.patch(MOCKPATH + "DataSourceAzure._wait_for_nic_detach")
     def test_detect_nic_attach_reports_ready_and_waits_for_detach(
@@ -2887,7 +2869,7 @@ def test_detect_nic_attach_reports_ready_and_waits_for_detach(
 
     @mock.patch("os.path.isfile")
     @mock.patch(MOCKPATH + "DataSourceAzure.fallback_interface")
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     @mock.patch(MOCKPATH + "DataSourceAzure._report_ready")
     @mock.patch(MOCKPATH + "DataSourceAzure._wait_for_nic_detach")
     def test_detect_nic_attach_skips_report_ready_when_marker_present(
@@ -2928,7 +2910,7 @@ def test_detect_nic_attach_skips_nic_detach_when_marker_present(
     @mock.patch("cloudinit.sources.helpers.netlink.wait_for_nic_attach_event")
     @mock.patch("cloudinit.sources.net.find_fallback_nic")
     @mock.patch(MOCKPATH + "get_metadata_from_imds")
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     @mock.patch(MOCKPATH + "DataSourceAzure._wait_for_nic_detach")
     @mock.patch("os.path.isfile")
     def test_wait_for_nic_attach_if_no_fallback_interface(
@@ -2972,7 +2954,7 @@ def test_wait_for_nic_attach_if_no_fallback_interface(
     @mock.patch("cloudinit.sources.helpers.netlink.wait_for_nic_attach_event")
     @mock.patch("cloudinit.sources.net.find_fallback_nic")
     @mock.patch(MOCKPATH + "DataSourceAzure.get_imds_data_with_api_fallback")
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     @mock.patch(MOCKPATH + "DataSourceAzure._wait_for_nic_detach")
     @mock.patch("os.path.isfile")
     def test_wait_for_nic_attach_multinic_attach(
@@ -3036,7 +3018,7 @@ def network_metadata_ret(ifname, retries, type, exc_cb, infinite):
         self.assertEqual(2, m_link_up.call_count)
 
     @mock.patch(MOCKPATH + "DataSourceAzure.get_imds_data_with_api_fallback")
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     def test_check_if_nic_is_primary_retries_on_failures(
         self, m_dhcpv4, m_imds
     ):
@@ -3218,7 +3200,7 @@ def setUp(self):
         dsaz.BUILTIN_DS_CONFIG["data_dir"] = self.waagent_d
 
     @mock.patch("time.sleep", mock.MagicMock())
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     def test_poll_imds_re_dhcp_on_timeout(
         self,
         m_dhcpv4,
@@ -3293,7 +3275,7 @@ def test_poll_imds_skips_dhcp_if_ctx_present(
         self.assertEqual(0, m_media_switch.call_count)
 
     @mock.patch("os.path.isfile")
-    @mock.patch(MOCKPATH + "EphemeralDHCPv4")
+    @mock.patch(MOCKPATH + "EphemeralDHCPv4WithReporting", autospec=True)
     def test_poll_imds_does_dhcp_on_retries_if_ctx_present(
         self,
         m_ephemeral_dhcpv4,