sources/azure: consolidate DHCP variants to EphemeralDHCPv4WithReporting (#1190)

- Update EphemeralDHCPv4WithReporting to subclass EphemeralDHCPv4 for
  consistency (non-functional change).

- Replace all usage of EphemeralDHCPv4 with EphemeralDHCPv4WithReporting.

- Converging to one DHCP class exposed an issue with ExitStack patches
  being mixed with decorators.  Specifically, it appeared that tests
  that did not enable azure.EphemeralDHCPv4WithReporting mocks had it
  applied anyways from previous tests.

  Presumably ExitStack was overwriting the actual value with the
  mock provided by the decorator?  For now, remove some mock patches
  that trigger failures, but future work should move towards a
  consistent approach to prevent undetected effects.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>

1 files changed, 27 insertions, 45 deletions

diff --git a/tests/unittests/sources/test_azure.py b/tests/unittests/sources/test_azure.py
index ff7f6479..df1c289b 100644
--- a/tests/unittests/sources/test_azure.py
+++ b/tests/unittests/sources/test_azure.py
@@ -465,7 +465,7 @@ class TestGetMetadataFromIMDS(HttprettyTestCase):
         )
 
     @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 @@ class TestGetMetadataFromIMDS(HttprettyTestCase):
         )
 
     @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 @@ class TestGetMetadataFromIMDS(HttprettyTestCase):
         )
 
     @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 @@ class TestGetMetadataFromIMDS(HttprettyTestCase):
         )
 
     @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 @@ class TestGetMetadataFromIMDS(HttprettyTestCase):
         )
 
     @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 @@ class TestAzureDataSource(CiTestCase):
         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 @@ scbus-1 on xpt0 bus 0
             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 @@ scbus-1 on xpt0 bus 0
                     "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 @@ scbus-1 on xpt0 bus 0
         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 @@ scbus-1 on xpt0 bus 0
         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 @@ scbus-1 on xpt0 bus 0
         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 @@ scbus-1 on xpt0 bus 0
         "_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 @@ scbus-1 on xpt0 bus 0
         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 @@ scbus-1 on xpt0 bus 0
         poll_imds_func,
         report_ready_func,
         m_write,
-        m_dhcp,
     ):
         """If reprovisioning, wait for nic attach if marker present"""
 
@@ -1286,9 +1274,7 @@ scbus-1 on xpt0 bus 0
                 "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 @@ scbus-1 on xpt0 bus 0
             # 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 @@ scbus-1 on xpt0 bus 0
             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 @@ class TestPreprovisioningHotAttachNics(CiTestCase):
 
     @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 @@ class TestPreprovisioningHotAttachNics(CiTestCase):
 
     @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 @@ class TestPreprovisioningHotAttachNics(CiTestCase):
     @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 @@ class TestPreprovisioningHotAttachNics(CiTestCase):
     @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 @@ class TestPreprovisioningHotAttachNics(CiTestCase):
         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 @@ class TestPreprovisioningPollIMDS(CiTestCase):
         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 @@ class TestPreprovisioningPollIMDS(CiTestCase):
         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,