Skip to content

Commit

Permalink
use 30 secs timeout for put vmagent log api (#2662)
Browse files Browse the repository at this point in the history 
* use 30 secs timeout for put vmagent log api

* address CR comments

* fix unit tests
  • Loading branch information
@nagworld9
nagworld9 authored Sep 9, 2022
1 parent ef996e2 commit 12f444b
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 24 deletions.
3 changes: 2 additions & 1 deletion azurelinuxagent/common/protocol/hostplugin.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,8 @@ def put_vm_log(self, content):
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)
Expand Down
40 changes: 26 additions & 14 deletions azurelinuxagent/common/utils/restutil.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -446,6 +447,7 @@ def http_request(method,
resp = _http_request(method,
host,
rel_uri,
timeout,
port=port,
data=data,
secure=secure,
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions tests/protocol/mocks.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ def http_handlers(get, post, put):
#
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
Expand Down Expand Up @@ -109,7 +109,7 @@ def http_request(method, url, data, **kwargs):
# 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
Expand Down
2 changes: 1 addition & 1 deletion tests/protocol/test_goal_state.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def test_it_should_retry_get_vm_settings_on_resource_gone_error(self):

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
Expand Down
2 changes: 1 addition & 1 deletion tests/protocol/test_wire.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ def test_send_encoded_event(self, mock_http_request, *args):

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...
Expand Down
8 changes: 4 additions & 4 deletions tests/utils/test_rest_util.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ def test_http_request_direct(self, HTTPConnection, HTTPSConnection):

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)
Expand All @@ -375,7 +375,7 @@ def test_http_request_direct_secure(self, HTTPConnection, HTTPSConnection):

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([
Expand All @@ -398,7 +398,7 @@ def test_http_request_proxy(self, HTTPConnection, HTTPSConnection):

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([
Expand Down Expand Up @@ -530,7 +530,7 @@ def test_http_request_proxy_secure(self, HTTPConnection, HTTPSConnection):

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)

Expand Down

0 comments on commit 12f444b

Please sign in to comment.