Repository: Azure/WALinuxAgent
Pull request #2662 (2022-09-03 00:00:18)

use 30 secs timeout for put vmagent log api
<!-- DO NOT DELETE THIS TEMPLATE -->

## Description
exercise 30 secs timeout to reduce latency failure rates on PUT VMagent log api.
Issue # <!-- if any -->
<!--
Please add an informative description that covers that changes made by the pull request. 
This checklist is used to make sure that common issues in a pull request are addressed.
This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
-->

---

### PR information
- [ ] The title of the PR is clear and informative.
- [ ] There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, [see this page](https://github.com/Azure/azure-powershell/blob/master/documentation/development-docs/cleaning-up-commits.md).
- [ ] If applicable, the PR references the bug/issue that it fixes in the description.
- [ ] New Unit tests were added for the changes made

### Quality of Code and Contribution Guidelines
- [ ] I have read the [contribution guidelines](https://github.com/Azure/WALinuxAgent/blob/master/.github/CONTRIBUTING.md).

codecov[bot]: # [Codecov](https://codecov.io/gh/Azure/WALinuxAgent/pull/2662?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) Report
> Merging [#2662](https://codecov.io/gh/Azure/WALinuxAgent/pull/2662?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (e3efacb) into [develop](https://codecov.io/gh/Azure/WALinuxAgent/commit/51b27a10ef841d0566a1eca1d965732380b123f9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) (51b27a1) will **not change** coverage.
> The diff coverage is `n/a`.

```diff
@@           Coverage Diff            @@
##           develop    #2662   +/-   ##
========================================
  Coverage    71.97%   71.97%           
========================================
  Files          103      103           
  Lines        15688    15688           
  Branches      2485     2485           
========================================
  Hits         11291    11291           
  Misses        3881     3881           
  Partials       516      516           
```


| [Impacted Files](https://codecov.io/gh/Azure/WALinuxAgent/pull/2662?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure) | Coverage Δ | |
|---|---|---|
| [azurelinuxagent/common/protocol/hostplugin.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2662/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi9wcm90b2NvbC9ob3N0cGx1Z2luLnB5) | `87.66% <ø> (ø)` | |
| [azurelinuxagent/common/utils/restutil.py](https://codecov.io/gh/Azure/WALinuxAgent/pull/2662/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure#diff-YXp1cmVsaW51eGFnZW50L2NvbW1vbi91dGlscy9yZXN0dXRpbC5weQ==) | `84.14% <ø> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Azure)


Patch:
diff --git a/azurelinuxagent/common/protocol/hostplugin.py b/azurelinuxagent/common/protocol/hostplugin.py
index 56cbafc1..55997d64 100644
--- a/azurelinuxagent/common/protocol/hostplugin.py
+++ b/azurelinuxagent/common/protocol/hostplugin.py
@@ -269,7 +269,8 @@ class HostPluginProtocol(object):
         response = restutil.http_put(url,
                                      data=content,
                                      headers=self._build_log_headers(),
-                                     redact_data=True)
+                                     redact_data=True,
+                                     timeout=30)
 
         if restutil.request_failed(response):
             error_response = restutil.read_response_error(response)
diff --git a/azurelinuxagent/common/utils/restutil.py b/azurelinuxagent/common/utils/restutil.py
index 8c2fc4e4..2180a376 100644
--- a/azurelinuxagent/common/utils/restutil.py
+++ b/azurelinuxagent/common/utils/restutil.py
@@ -309,7 +309,7 @@ def redact_sas_tokens_in_urls(url):
     return SAS_TOKEN_RETRIEVAL_REGEX.sub(r"\1" + REDACTED_TEXT + r"\3", url)
 
 
-def _http_request(method, host, rel_uri, port=None, data=None, secure=False,
+def _http_request(method, host, rel_uri, timeout, port=None, data=None, secure=False,
                   headers=None, proxy_host=None, proxy_port=None, redact_data=False):
 
     headers = {} if headers is None else headers
@@ -334,13 +334,13 @@ def _http_request(method, host, rel_uri, port=None, data=None, secure=False,
     if secure:
         conn = httpclient.HTTPSConnection(conn_host,
                                           conn_port,
-                                          timeout=10)
+                                          timeout=timeout)
         if use_proxy:
             conn.set_tunnel(host, port)
     else:
         conn = httpclient.HTTPConnection(conn_host,
                                          conn_port,
-                                         timeout=10)
+                                         timeout=timeout)
 
     payload = data
     if redact_data:
@@ -358,7 +358,8 @@ def _http_request(method, host, rel_uri, port=None, data=None, secure=False,
 
 
 def http_request(method,
-                 url, data, headers=None,
+                 url, data, timeout,
+                 headers=None,
                  use_proxy=False,
                  max_retry=None,
                  retry_codes=None,
@@ -446,6 +447,7 @@ def http_request(method,
             resp = _http_request(method,
                                  host,
                                  rel_uri,
+                                 timeout,
                                  port=port,
                                  data=data,
                                  secure=secure,
@@ -510,7 +512,8 @@ def http_get(url,
              max_retry=None,
              retry_codes=None,
              retry_delay=DELAY_IN_SECONDS,
-             return_raw_response=False):
+             return_raw_response=False,
+             timeout=10):
     """
     NOTE: This method provides some logic to handle errors in the HTTP request, including checking the HTTP status of the response
           and handling some exceptions. If return_raw_response is set to True all the error handling will be skipped and the
@@ -523,7 +526,8 @@ def http_get(url,
     if retry_codes is None:
         retry_codes = RETRY_CODES
     return http_request("GET",
-                        url, None, headers=headers,
+                        url, None, timeout,
+                        headers=headers,
                         use_proxy=use_proxy,
                         max_retry=max_retry,
                         retry_codes=retry_codes,
@@ -536,14 +540,16 @@ def http_head(url,
               use_proxy=False,
               max_retry=None,
               retry_codes=None,
-              retry_delay=DELAY_IN_SECONDS):
+              retry_delay=DELAY_IN_SECONDS,
+              timeout=10):
 
     if max_retry is None:
         max_retry = DEFAULT_RETRIES
     if retry_codes is None:
         retry_codes = RETRY_CODES
     return http_request("HEAD",
-                        url, None, headers=headers,
+                        url, None, timeout,
+                        headers=headers,
                         use_proxy=use_proxy,
                         max_retry=max_retry,
                         retry_codes=retry_codes,
@@ -556,14 +562,16 @@ def http_post(url,
               use_proxy=False,
               max_retry=None,
               retry_codes=None,
-              retry_delay=DELAY_IN_SECONDS):
+              retry_delay=DELAY_IN_SECONDS,
+              timeout=10):
 
     if max_retry is None:
         max_retry = DEFAULT_RETRIES
     if retry_codes is None:
         retry_codes = RETRY_CODES
     return http_request("POST",
-                        url, data, headers=headers,
+                        url, data, timeout,
+                        headers=headers,
                         use_proxy=use_proxy,
                         max_retry=max_retry,
                         retry_codes=retry_codes,
@@ -577,14 +585,16 @@ def http_put(url,
              max_retry=None,
              retry_codes=None,
              retry_delay=DELAY_IN_SECONDS,
-             redact_data=False):
+             redact_data=False,
+             timeout=10):
 
     if max_retry is None:
         max_retry = DEFAULT_RETRIES
     if retry_codes is None:
         retry_codes = RETRY_CODES
     return http_request("PUT",
-                        url, data, headers=headers,
+                        url, data, timeout,
+                        headers=headers,
                         use_proxy=use_proxy,
                         max_retry=max_retry,
                         retry_codes=retry_codes,
@@ -597,14 +607,16 @@ def http_delete(url,
                 use_proxy=False,
                 max_retry=None,
                 retry_codes=None,
-                retry_delay=DELAY_IN_SECONDS):
+                retry_delay=DELAY_IN_SECONDS,
+                timeout=10):
 
     if max_retry is None:
         max_retry = DEFAULT_RETRIES
     if retry_codes is None:
         retry_codes = RETRY_CODES
     return http_request("DELETE",
-                        url, None, headers=headers,
+                        url, None, timeout,
+                        headers=headers,
                         use_proxy=use_proxy,
                         max_retry=max_retry,
                         retry_codes=retry_codes,


Test patch:
diff --git a/tests/protocol/mocks.py b/tests/protocol/mocks.py
index 8b695806..b7413888 100644
--- a/tests/protocol/mocks.py
+++ b/tests/protocol/mocks.py
@@ -70,10 +70,10 @@ def mock_wire_protocol(mock_wire_data_file, http_get_handler=None, http_post_han
     #
     original_http_request = restutil.http_request
 
-    def http_request(method, url, data, **kwargs):
+    def http_request(method, url, data, timeout, **kwargs):
         # call the original resutil.http_request if the request should be mocked
         if protocol.do_not_mock(method, url):
-            return original_http_request(method, url, data, **kwargs)
+            return original_http_request(method, url, data, timeout, **kwargs)
 
         # if there is a handler for the request, use it
         handler = None
@@ -109,7 +109,7 @@ def mock_wire_protocol(mock_wire_data_file, http_get_handler=None, http_post_han
         # if there was not a response for the request then fail it or call the original resutil.http_request
         if fail_on_unknown_request:
             raise ValueError('Unknown HTTP request: {0} [{1}]'.format(url, method))
-        return original_http_request(method, url, data, **kwargs)
+        return original_http_request(method, url, data, timeout, **kwargs)
 
     #
     # functions to start/stop the mocks
diff --git a/tests/protocol/test_goal_state.py b/tests/protocol/test_goal_state.py
index 87a1db50..d853363c 100644
--- a/tests/protocol/test_goal_state.py
+++ b/tests/protocol/test_goal_state.py
@@ -68,7 +68,7 @@ class GoalStateTestCase(AgentTestCase, HttpRequestPredicates):
 
             request_headers = []  # we expect a retry with new headers and use this array to persist the headers of each request
 
-            def http_get_vm_settings(_method, _host, _relative_url, **kwargs):
+            def http_get_vm_settings(_method, _host, _relative_url, _timeout, **kwargs):
                 request_headers.append(kwargs["headers"])
                 if len(request_headers) == 1:
                     # Fail the first request with status GONE and update the mock data to return the new Container ID and RoleConfigName that should be
diff --git a/tests/protocol/test_wire.py b/tests/protocol/test_wire.py
index 0cfaa95f..b9fe23e4 100644
--- a/tests/protocol/test_wire.py
+++ b/tests/protocol/test_wire.py
@@ -405,7 +405,7 @@ class TestWireProtocol(AgentTestCase, HttpRequestPredicates):
 
         first_call = mock_http_request.call_args_list[0]
         args, kwargs = first_call
-        method, url, body_received = args  # pylint: disable=unused-variable
+        method, url, body_received, timeout = args  # pylint: disable=unused-variable
         headers = kwargs['headers']
 
         # the headers should include utf-8 encoding...
diff --git a/tests/utils/test_rest_util.py b/tests/utils/test_rest_util.py
index 02431b62..a0b00f6c 100644
--- a/tests/utils/test_rest_util.py
+++ b/tests/utils/test_rest_util.py
@@ -352,7 +352,7 @@ class TestHttpOperations(AgentTestCase):
 
         HTTPConnection.return_value = mock_conn
 
-        resp = restutil._http_request("GET", "foo", "/bar")
+        resp = restutil._http_request("GET", "foo", "/bar", 10)
 
         HTTPConnection.assert_has_calls([
             call("foo", 80, timeout=10)
@@ -375,7 +375,7 @@ class TestHttpOperations(AgentTestCase):
 
         HTTPSConnection.return_value = mock_conn
 
-        resp = restutil._http_request("GET", "foo", "/bar", secure=True)
+        resp = restutil._http_request("GET", "foo", "/bar", 10, secure=True)
 
         HTTPConnection.assert_not_called()
         HTTPSConnection.assert_has_calls([
@@ -398,7 +398,7 @@ class TestHttpOperations(AgentTestCase):
 
         HTTPConnection.return_value = mock_conn
 
-        resp = restutil._http_request("GET", "foo", "/bar",
+        resp = restutil._http_request("GET", "foo", "/bar", 10,
                             proxy_host="foo.bar", proxy_port=23333)
 
         HTTPConnection.assert_has_calls([
@@ -530,7 +530,7 @@ class TestHttpOperations(AgentTestCase):
 
         HTTPSConnection.return_value = mock_conn
 
-        resp = restutil._http_request("GET", "foo", "/bar",
+        resp = restutil._http_request("GET", "foo", "/bar", 10,
                             proxy_host="foo.bar", proxy_port=23333,
                             secure=True)