From c075e90aac914e7293c0c5ad01a05d67aec68572 Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Wed, 19 Jan 2022 13:39:11 -0500 Subject: [PATCH] sources/azure: drop unused case in _report_failure() According to the documentation in the tests: ``` We expect 3 calls to report_failure_to_fabric, because we try 3 different methods of calling report failure. The different methods are attempted in the following order: 1. Using cached ephemeral dhcp context to report failure to Azure 2. Using new ephemeral dhcp to report failure to Azure 3. Using fallback lease to report failure to Azure ``` Case 1 and 2 make sense. If networking is established, use it. Should failure occur using current network configuration, retry with fresh DHCP. Case 3 suggests that we can fall back to a lease file and retry. Given that: 1. The wireserver address has never changed to date. 2. The wireserver address should be in the DHCP lease. 3. Parsing the lease file does not improve connectivity over the prior attempts. ...we can safely remove this case without regression. Signed-off-by: Chris Patterson --- cloudinit/sources/DataSourceAzure.py | 18 ++------------- tests/unittests/sources/test_azure.py | 32 +++------------------------ 2 files changed, 5 insertions(+), 45 deletions(-) diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 7134c95a2f4..39df62a984b 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -14,6 +14,7 @@ from enum import Enum from functools import partial from time import sleep, time +from typing import Optional from xml.dom import minidom import requests @@ -1310,7 +1311,7 @@ def exc_cb(msg, exception): return return_val @azure_ds_telemetry_reporter - def _report_failure(self, description=None) -> bool: + def _report_failure(self, description: Optional[str] = None) -> bool: """Tells the Azure fabric that provisioning has failed. @param description: A description of the error encountered. @@ -1359,21 +1360,6 @@ def _report_failure(self, description=None) -> bool: logger_func=LOG.debug, ) - try: - report_diagnostic_event( - "Using fallback lease to report failure to Azure" - ) - report_failure_to_fabric( - fallback_lease_file=self.dhclient_lease_file, - description=description, - ) - return True - except Exception as e: - report_diagnostic_event( - "Failed to report failure using fallback lease: %s" % e, - logger_func=LOG.debug, - ) - return False def _report_ready(self, lease: dict) -> bool: diff --git a/tests/unittests/sources/test_azure.py b/tests/unittests/sources/test_azure.py index 02083184dea..299a9a0c428 100644 --- a/tests/unittests/sources/test_azure.py +++ b/tests/unittests/sources/test_azure.py @@ -1737,15 +1737,14 @@ def test_dsaz_report_failure_returns_false_and_does_not_propagate_exc( test_lease = {"unknown-245": test_lease_dhcp_option_245} m_ephemeral_dhcp_ctx.lease = test_lease - # We expect 3 calls to report_failure_to_fabric, - # because we try 3 different methods of calling report failure. + # We expect 2 calls to report_failure_to_fabric, + # because we try 2 different methods of calling report failure. # The different methods are attempted in the following order: # 1. Using cached ephemeral dhcp context to report failure to Azure # 2. Using new ephemeral dhcp to report failure to Azure - # 3. Using fallback lease to report failure to Azure self.m_report_failure_to_fabric.side_effect = Exception self.assertFalse(dsrc._report_failure()) - self.assertEqual(3, self.m_report_failure_to_fabric.call_count) + self.assertEqual(2, self.m_report_failure_to_fabric.call_count) def test_dsaz_report_failure_description_msg(self): dsrc = self._get_ds({"ovfcontent": construct_valid_ovf_env()}) @@ -1826,31 +1825,6 @@ def test_dsaz_report_failure_no_net_uses_new_ephemeral_dhcp_lease(self): description=mock.ANY, dhcp_opts=test_lease_dhcp_option_245 ) - def test_dsaz_report_failure_no_net_and_no_dhcp_uses_fallback_lease(self): - dsrc = self._get_ds({"ovfcontent": construct_valid_ovf_env()}) - - with mock.patch.object( - dsrc, "crawl_metadata" - ) as m_crawl_metadata, mock.patch.object( - dsrc.distro.networking, "is_up" - ) as m_dsrc_distro_networking_is_up: - # mock crawl metadata failure to cause report failure - m_crawl_metadata.side_effect = Exception - - # net is not up and cannot use cached ephemeral dhcp - m_dsrc_distro_networking_is_up.return_value = False - # ephemeral dhcp discovery failure, - # so cannot use a new ephemeral dhcp - self.m_dhcp.return_value.__enter__.side_effect = Exception - - self.assertTrue(dsrc._report_failure()) - - # ensure called with fallback lease - self.m_report_failure_to_fabric.assert_called_once_with( - description=mock.ANY, - fallback_lease_file=dsrc.dhclient_lease_file, - ) - def test_exception_fetching_fabric_data_doesnt_propagate(self): """Errors communicating with fabric should warn, but return True.""" dsrc = self._get_ds({"ovfcontent": construct_valid_ovf_env()})