diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index 196ee0311f4..4bb8b8dbd9a 100755 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -330,34 +330,24 @@ def get_ip_from_lease_value(fallback_lease_value): @azure_ds_telemetry_reporter -def http_with_retries(url, **kwargs) -> url_helper.UrlResponse: - """Wrapper around url_helper.readurl() with custom telemetry logging - that url_helper.readurl() does not provide. +def http_with_retries( + url: str, *, headers: dict, data: Optional[str] = None +) -> url_helper.UrlResponse: + """Readurl wrapper for querying wireserver. + + Retries up to 40 minutes: + 240 attempts * (5s timeout + 5s sleep) """ max_readurl_attempts = 240 - default_readurl_timeout = 5 + readurl_timeout = 5 sleep_duration_between_retries = 5 periodic_logging_attempts = 12 - if "timeout" not in kwargs: - kwargs["timeout"] = default_readurl_timeout - - # remove kwargs that cause url_helper.readurl to retry, - # since we are already implementing our own retry logic. - if kwargs.pop("retries", None): - LOG.warning( - "Ignoring retries kwarg passed in for " - "communication with Azure endpoint." - ) - if kwargs.pop("infinite", None): - LOG.warning( - "Ignoring infinite kwarg passed in for communication " - "with Azure endpoint." - ) - for attempt in range(1, max_readurl_attempts + 1): try: - ret = url_helper.readurl(url, **kwargs) + ret = url_helper.readurl( + url, headers=headers, data=data, timeout=readurl_timeout + ) report_diagnostic_event( "Successful HTTP request with Azure endpoint %s after " diff --git a/tests/unittests/sources/test_azure_helper.py b/tests/unittests/sources/test_azure_helper.py index 326b41cc413..4279dc4f643 100644 --- a/tests/unittests/sources/test_azure_helper.py +++ b/tests/unittests/sources/test_azure_helper.py @@ -1,6 +1,5 @@ # This file is part of cloud-init. See LICENSE file for license information. -import copy import os import re import unittest @@ -358,7 +357,7 @@ def setUp(self): def test_http_with_retries(self): self.m_readurl.return_value = "TestResp" self.assertEqual( - azure_helper.http_with_retries("testurl"), + azure_helper.http_with_retries("testurl", headers={}), self.m_readurl.return_value, ) self.assertEqual(self.m_readurl.call_count, 1) @@ -367,7 +366,10 @@ def test_http_with_retries_propagates_readurl_exc_and_logs_exc(self): self.m_readurl.side_effect = SentinelException self.assertRaises( - SentinelException, azure_helper.http_with_retries, "testurl" + SentinelException, + azure_helper.http_with_retries, + "testurl", + headers={}, ) self.assertEqual(self.m_readurl.call_count, self.max_readurl_attempts) @@ -394,7 +396,7 @@ def test_http_with_retries_delayed_success_due_to_temporary_readurl_exc( ] * self.periodic_logging_attempts + ["TestResp"] self.m_readurl.return_value = "TestResp" - response = azure_helper.http_with_retries("testurl") + response = azure_helper.http_with_retries("testurl", headers={}) self.assertEqual(response, self.m_readurl.return_value) self.assertEqual( self.m_readurl.call_count, self.periodic_logging_attempts + 1 @@ -412,7 +414,7 @@ def test_http_with_retries_long_delay_logs_periodic_failure_msg(self): ] * self.periodic_logging_attempts + ["TestResp"] self.m_readurl.return_value = "TestResp" - azure_helper.http_with_retries("testurl") + azure_helper.http_with_retries("testurl", headers={}) self.assertEqual( self.m_readurl.call_count, self.periodic_logging_attempts + 1 @@ -440,7 +442,7 @@ def test_http_with_retries_short_delay_does_not_log_periodic_failure_msg( ) + ["TestResp"] self.m_readurl.return_value = "TestResp" - azure_helper.http_with_retries("testurl") + azure_helper.http_with_retries("testurl", headers={}) self.assertEqual( self.m_readurl.call_count, self.periodic_logging_attempts ) @@ -465,49 +467,9 @@ def test_http_with_retries_calls_url_helper_readurl_with_args_kwargs(self): kwargs = { "headers": mock.MagicMock(), "data": mock.MagicMock(), - # timeout kwarg should not be modified or deleted if present - "timeout": mock.MagicMock(), - } - azure_helper.http_with_retries(testurl, **kwargs) - self.m_readurl.assert_called_once_with(testurl, **kwargs) - - def test_http_with_retries_adds_timeout_kwarg_if_not_present(self): - testurl = mock.MagicMock() - kwargs = {"headers": mock.MagicMock(), "data": mock.MagicMock()} - expected_kwargs = copy.deepcopy(kwargs) - expected_kwargs["timeout"] = self.default_readurl_timeout - - azure_helper.http_with_retries(testurl, **kwargs) - self.m_readurl.assert_called_once_with(testurl, **expected_kwargs) - - def test_http_with_retries_deletes_retries_kwargs_passed_in(self): - """http_with_retries already implements retry logic, - so url_helper.readurl should not have retries. - http_with_retries should delete kwargs that - cause url_helper.readurl to retry. - """ - testurl = mock.MagicMock() - kwargs = { - "headers": mock.MagicMock(), - "data": mock.MagicMock(), - "timeout": mock.MagicMock(), - "retries": mock.MagicMock(), - "infinite": mock.MagicMock(), } - expected_kwargs = copy.deepcopy(kwargs) - expected_kwargs.pop("retries", None) - expected_kwargs.pop("infinite", None) - azure_helper.http_with_retries(testurl, **kwargs) - self.m_readurl.assert_called_once_with(testurl, **expected_kwargs) - self.assertIn( - "retries kwarg passed in for communication with Azure endpoint.", - self.logs.getvalue(), - ) - self.assertIn( - "infinite kwarg passed in for communication with Azure endpoint.", - self.logs.getvalue(), - ) + self.m_readurl.assert_called_once_with(testurl, **kwargs, timeout=5) class TestOpenSSLManager(CiTestCase):