Skip to content

Commit

Permalink
sources/azure: drop unused case in _report_failure() (#1200)
Browse files Browse the repository at this point in the history
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 <cpatterson@microsoft.com>
  • Loading branch information
cjp256 authored Jan 20, 2022
1 parent 109c2bb commit 6f0e881
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 45 deletions.
18 changes: 2 additions & 16 deletions cloudinit/sources/DataSourceAzure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1315,7 +1316,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.
Expand Down Expand Up @@ -1364,21 +1365,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:
Expand Down
32 changes: 3 additions & 29 deletions tests/unittests/sources/test_azure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()})
Expand Down Expand Up @@ -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()})
Expand Down

0 comments on commit 6f0e881

Please sign in to comment.