sources/azure: refactor http_with_retries to remove **kwargs

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 "

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):