From c8a5d4d311d00225f4e425b1f3628ded99a61a5b Mon Sep 17 00:00:00 2001 From: Nejc Habjan Date: Fri, 26 Jan 2024 11:23:58 +0100 Subject: [PATCH] galaxy: make retryable HTTP status codes configurable --- .../fragments/82246-extend-http-retry-codes.yml | 3 +++ lib/ansible/config/base.yml | 12 ++++++++++++ lib/ansible/galaxy/api.py | 13 ++++++------- test/units/galaxy/test_api.py | 10 +++++++++- 4 files changed, 30 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/82246-extend-http-retry-codes.yml diff --git a/changelogs/fragments/82246-extend-http-retry-codes.yml b/changelogs/fragments/82246-extend-http-retry-codes.yml new file mode 100644 index 00000000000..94c370aeaf7 --- /dev/null +++ b/changelogs/fragments/82246-extend-http-retry-codes.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - Make HTTP status codes to retry during galaxy API calls configurable (https://github.com/ansible/ansible/pull/82246). diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index 079cb55c0d7..7fcc5f7f263 100644 --- a/lib/ansible/config/base.yml +++ b/lib/ansible/config/base.yml @@ -1510,6 +1510,18 @@ GALAXY_REQUIRED_VALID_SIGNATURE_COUNT: - The number of signatures that must be successful during GPG signature verification while installing or verifying collections. - This should be a positive integer or all to indicate all signatures must successfully validate the collection. - Prepend + to the value to fail if no valid signatures are found for the collection. +GALAXY_RETRY_HTTP_ERROR_CODES: + type: list + default: + - "429" # Too many requests + - "520" # Galaxy rate limit error code (Cloudflare unknown error) + - "502" # Common error from galaxy that may represent any number of transient backend issues + env: + - name: ANSIBLE_GALAXY_RETRY_HTTP_ERROR_CODES + ini: + - section: galaxy + key: retry_http_error_codes + description: List of HTTP Status error codes to retry on. HOST_KEY_CHECKING: # NOTE: constant not in use by ssh/paramiko plugins anymore, but they do support the same configuration sources # TODO: check non ssh connection plugins for use/migration diff --git a/lib/ansible/galaxy/api.py b/lib/ansible/galaxy/api.py index ad340415867..7bc3fcbd102 100644 --- a/lib/ansible/galaxy/api.py +++ b/lib/ansible/galaxy/api.py @@ -15,7 +15,6 @@ import tarfile import time import threading -from http import HTTPStatus from http.client import BadStatusLine, IncompleteRead from urllib.error import HTTPError, URLError from urllib.parse import quote as urlquote, urlencode, urlparse, parse_qs, urljoin @@ -35,11 +34,6 @@ from ansible.utils.path import makedirs_safe display = Display() _CACHE_LOCK = threading.Lock() COLLECTION_PAGE_SIZE = 100 -RETRY_HTTP_ERROR_CODES = { # TODO: Allow user-configuration - HTTPStatus.TOO_MANY_REQUESTS, - 520, # Galaxy rate limit error code (Cloudflare unknown error) - HTTPStatus.BAD_GATEWAY, # Common error from galaxy that may represent any number of transient backend issues -} def cache_lock(func): @@ -54,7 +48,12 @@ def should_retry_error(exception): # Note: cloud.redhat.com masks rate limit errors with 403 (Forbidden) error codes. # Since 403 could reflect the actual problem (such as an expired token), we should # not retry by default. - if isinstance(exception, GalaxyError) and exception.http_code in RETRY_HTTP_ERROR_CODES: + try: + retry_codes = [int(code) for code in C.GALAXY_RETRY_HTTP_ERROR_CODES] + except ValueError as e: + raise AnsibleError("Invalid value for HTTP retry code: %s. Only integer values are supported." % e) + + if isinstance(exception, GalaxyError) and exception.http_code in retry_codes: return True if isinstance(exception, AnsibleError) and (orig_exc := getattr(exception, 'orig_exc', None)): diff --git a/test/units/galaxy/test_api.py b/test/units/galaxy/test_api.py index a6c62328cca..712633ab80e 100644 --- a/test/units/galaxy/test_api.py +++ b/test/units/galaxy/test_api.py @@ -20,7 +20,7 @@ import ansible.constants as C from ansible import context from ansible.errors import AnsibleError from ansible.galaxy import api as galaxy_api -from ansible.galaxy.api import CollectionVersionMetadata, GalaxyAPI, GalaxyError +from ansible.galaxy.api import CollectionVersionMetadata, GalaxyAPI, GalaxyError, should_retry_error from ansible.galaxy.token import BasicAuthToken, GalaxyToken, KeycloakToken from ansible.module_utils.common.text.converters import to_native, to_text import urllib.error @@ -1353,3 +1353,11 @@ def test_clear_cache(cache_dir): def test_cache_id(url, expected): actual = galaxy_api.get_cache_id(url) assert actual == expected + + +def test_should_retry_error_with_invalid_code(monkeypatch): + expected = "Invalid value for HTTP retry code" + monkeypatch.setattr(C, 'GALAXY_RETRY_HTTP_ERROR_CODES', ["429", "invalid"]) + + with pytest.raises(AnsibleError, match=expected): + should_retry_error(Exception())