From 38a5a91752700ed15ff855959c0e9cdf9bfea32f Mon Sep 17 00:00:00 2001 From: Jonathan Gangi Date: Tue, 22 Jul 2025 17:21:49 -0300 Subject: [PATCH 1/2] CLOUDDST-28603: Add retry on ErrataClient._call_et This commit adds a retry on `ErrataClientBase._call_et` in order to re-attempt failed requests with the Errata Tool. Refers to CLOUDDST-28603 Signed-off-by: Jonathan Gangi --- requirements.in | 1 + requirements.txt | 5 ++++ .../backend/errata_source/errata_client.py | 13 +++++++++ test-requirements.txt | 4 +++ tests/errata/test_errata_client.py | 29 +++++++++++++++++++ tests/errata/test_errata_client_logging.py | 1 + 6 files changed, 53 insertions(+) diff --git a/requirements.in b/requirements.in index eebecb71..bb541431 100644 --- a/requirements.in +++ b/requirements.in @@ -11,3 +11,4 @@ python-dateutil pytz; python_version < '3.9' PyYAML requests +tenacity diff --git a/requirements.txt b/requirements.txt index eda8cbe7..a292f9d1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -572,9 +572,14 @@ six==1.17.0 \ # koji # more-executors # python-dateutil +tenacity==9.1.2 \ + --hash=sha256:1169d376c297e7de388d18b4481760d478b0e99a777cad3a9c86e556f4b697cb \ + --hash=sha256:f77bf36710d8b73a50b2dd155c97b870017ad21afe6ab300326b0371b3b05138 + # via -r requirements.in typing-extensions==4.14.1 \ --hash=sha256:38b39f4aeeab64884ce9f74c94263ef78f3c22467c8724005483154c26648d36 \ --hash=sha256:d1e1e3b58374dc93031d6eda2420a48ea44a36c2b4766a4fdeb3710755731d76 + # via referencing urllib3==2.5.0 \ --hash=sha256:3fc47733c7e419d4bc3f6b3dc2b4f890bb743906a30d56ba4a5bfa4bbff92760 \ diff --git a/src/pushsource/_impl/backend/errata_source/errata_client.py b/src/pushsource/_impl/backend/errata_source/errata_client.py index c7578c51..83ff1603 100644 --- a/src/pushsource/_impl/backend/errata_source/errata_client.py +++ b/src/pushsource/_impl/backend/errata_source/errata_client.py @@ -14,6 +14,13 @@ from more_executors.futures import f_zip, f_map import requests import requests_gssapi +from tenacity import ( + retry, + stop_after_attempt, + retry_if_exception, + retry_if_exception_type, + wait_exponential, +) from ...compat_attr import attr @@ -111,6 +118,12 @@ def get_raw_f(self, advisory_id): ) return f_map(all_responses, lambda tup: ErrataRaw(*tup)) + @retry( + retry=retry_if_exception_type(requests.exceptions.HTTPError) + & retry_if_exception(lambda exc: exc.response.status_code >= 500), + stop=stop_after_attempt(5), + wait=wait_exponential(multiplier=2, min=1, max=10), + ) def _call_et(self, method, advisory_id): # These APIs have had performance issues occasionally, so let's set up some # detailed structured logs which can be used to check the performance. diff --git a/test-requirements.txt b/test-requirements.txt index 9565af7e..673c753b 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -996,6 +996,10 @@ stevedore==5.4.1 \ --hash=sha256:3135b5ae50fe12816ef291baff420acb727fcd356106e3e9cbfa9e5985cd6f4b \ --hash=sha256:d10a31c7b86cba16c1f6e8d15416955fc797052351a56af15e608ad20811fcfe # via bandit +tenacity==9.1.2 \ + --hash=sha256:1169d376c297e7de388d18b4481760d478b0e99a777cad3a9c86e556f4b697cb \ + --hash=sha256:f77bf36710d8b73a50b2dd155c97b870017ad21afe6ab300326b0371b3b05138 + # via -r requirements.in tomli==2.2.1 \ --hash=sha256:023aa114dd824ade0100497eb2318602af309e5a55595f76b626d6d9f3b7b0a6 \ --hash=sha256:02abe224de6ae62c19f090f68da4e27b10af2b93213d36cf44e6e1c5abd19fdd \ diff --git a/tests/errata/test_errata_client.py b/tests/errata/test_errata_client.py index b0712930..b8d9af42 100644 --- a/tests/errata/test_errata_client.py +++ b/tests/errata/test_errata_client.py @@ -113,3 +113,32 @@ def test_get_advisory_data(caplog): "GET https://errata.example.com/api/v1/erratum/RHSA-123456789 200", "Errata Tool completed call /api/v1/erratum/{id}(RHSA-123456789)", ] + + +def test_get_advisory_data_retry(caplog): + caplog.set_level(logging.DEBUG) + + client = ErrataHTTPClient( + 1, "https://errata.example.com/", "/path/to/keytab", "pub-errata@IPA.REDHAT.COM" + ) + client._call_et.retry.sleep = mock.MagicMock() + client._tls.session = requests.Session() + + with requests_mock.Mocker() as m: + m.get( + "https://errata.example.com/api/v1/erratum/RHSA-123456789", + [{"status_code": 504}, {"json": {"errata": "data"}, "status_code": 200}], + ) + + data = client.get_advisory_data("RHSA-123456789") + + assert data == {"errata": "data"} + assert m.call_count == 2 + assert caplog.messages == [ + "Calling Errata Tool /api/v1/erratum/{id}(RHSA-123456789)", + "GET https://errata.example.com/api/v1/erratum/RHSA-123456789 504", + "Failed to call Errata Tool /api/v1/erratum/{id}(RHSA-123456789)", + "Calling Errata Tool /api/v1/erratum/{id}(RHSA-123456789)", + "GET https://errata.example.com/api/v1/erratum/RHSA-123456789 200", + "Errata Tool completed call /api/v1/erratum/{id}(RHSA-123456789)", + ] diff --git a/tests/errata/test_errata_client_logging.py b/tests/errata/test_errata_client_logging.py index ca71ed83..1b5528ff 100644 --- a/tests/errata/test_errata_client_logging.py +++ b/tests/errata/test_errata_client_logging.py @@ -41,6 +41,7 @@ def test_errata_client_debug_logs(caplog): # disable retry just for this test max_attempts=1, ) + client._call_et.retry.sleep = mock.MagicMock() with patch( "pushsource._impl.backend.errata_source.errata_client.xmlrpc_client.ServerProxy" From 36d308f30919e476deb62bacb588e3b89a914bd6 Mon Sep 17 00:00:00 2001 From: Jonathan Gangi Date: Wed, 6 Aug 2025 17:04:48 -0300 Subject: [PATCH 2/2] Errata: Use retry from requests library This commit updates the `errata_client` module to use the retry mechanism from `requests` library during the `_errata_service` session definition. Refers to CLOUDDST-28603 Signed-off-by: Jonathan Gangi --- requirements.in | 1 - requirements.txt | 5 --- .../backend/errata_source/errata_client.py | 26 ++++++------- test-requirements.txt | 4 -- tests/errata/fake_errata_tool.py | 2 + tests/errata/test_errata_client.py | 37 ++++--------------- tests/errata/test_errata_client_logging.py | 1 - 7 files changed, 22 insertions(+), 54 deletions(-) diff --git a/requirements.in b/requirements.in index bb541431..eebecb71 100644 --- a/requirements.in +++ b/requirements.in @@ -11,4 +11,3 @@ python-dateutil pytz; python_version < '3.9' PyYAML requests -tenacity diff --git a/requirements.txt b/requirements.txt index a292f9d1..eda8cbe7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -572,14 +572,9 @@ six==1.17.0 \ # koji # more-executors # python-dateutil -tenacity==9.1.2 \ - --hash=sha256:1169d376c297e7de388d18b4481760d478b0e99a777cad3a9c86e556f4b697cb \ - --hash=sha256:f77bf36710d8b73a50b2dd155c97b870017ad21afe6ab300326b0371b3b05138 - # via -r requirements.in typing-extensions==4.14.1 \ --hash=sha256:38b39f4aeeab64884ce9f74c94263ef78f3c22467c8724005483154c26648d36 \ --hash=sha256:d1e1e3b58374dc93031d6eda2420a48ea44a36c2b4766a4fdeb3710755731d76 - # via referencing urllib3==2.5.0 \ --hash=sha256:3fc47733c7e419d4bc3f6b3dc2b4f890bb743906a30d56ba4a5bfa4bbff92760 \ diff --git a/src/pushsource/_impl/backend/errata_source/errata_client.py b/src/pushsource/_impl/backend/errata_source/errata_client.py index 83ff1603..636c4f95 100644 --- a/src/pushsource/_impl/backend/errata_source/errata_client.py +++ b/src/pushsource/_impl/backend/errata_source/errata_client.py @@ -7,20 +7,15 @@ import threading import xmlrpc.client as xmlrpc_client # nosec B411 from urllib.parse import urljoin +from urllib3.util.retry import Retry import warnings import gssapi from more_executors import Executors from more_executors.futures import f_zip, f_map import requests +import requests.adapters import requests_gssapi -from tenacity import ( - retry, - stop_after_attempt, - retry_if_exception, - retry_if_exception_type, - wait_exponential, -) from ...compat_attr import attr @@ -28,6 +23,14 @@ USE_XMLRPC_CLIENT = os.environ.get("PUSHSOURCE_ERRATA_USE_XMLRPC_API") == "1" +ERRATA_RETRY_STRATEGY = Retry( + total=5, + status_forcelist=range(500, 600), + backoff_factor=2, + allowed_methods=frozenset(["GET", "POST"]), + raise_on_status=False, +) + def get_errata_client( threads, @@ -118,12 +121,6 @@ def get_raw_f(self, advisory_id): ) return f_map(all_responses, lambda tup: ErrataRaw(*tup)) - @retry( - retry=retry_if_exception_type(requests.exceptions.HTTPError) - & retry_if_exception(lambda exc: exc.response.status_code >= 500), - stop=stop_after_attempt(5), - wait=wait_exponential(multiplier=2, min=1, max=10), - ) def _call_et(self, method, advisory_id): # These APIs have had performance issues occasionally, so let's set up some # detailed structured logs which can be used to check the performance. @@ -302,6 +299,9 @@ def _errata_service(self): session = requests.Session() session.auth = requests_gssapi.HTTPSPNEGOAuth(creds=creds) + adapter = requests.adapters.HTTPAdapter(max_retries=ERRATA_RETRY_STRATEGY) + session.mount("https://", adapter) + session.mount("http://", adapter) self._tls.session = session return self._tls.session diff --git a/test-requirements.txt b/test-requirements.txt index 673c753b..9565af7e 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -996,10 +996,6 @@ stevedore==5.4.1 \ --hash=sha256:3135b5ae50fe12816ef291baff420acb727fcd356106e3e9cbfa9e5985cd6f4b \ --hash=sha256:d10a31c7b86cba16c1f6e8d15416955fc797052351a56af15e608ad20811fcfe # via bandit -tenacity==9.1.2 \ - --hash=sha256:1169d376c297e7de388d18b4481760d478b0e99a777cad3a9c86e556f4b697cb \ - --hash=sha256:f77bf36710d8b73a50b2dd155c97b870017ad21afe6ab300326b0371b3b05138 - # via -r requirements.in tomli==2.2.1 \ --hash=sha256:023aa114dd824ade0100497eb2318602af309e5a55595f76b626d6d9f3b7b0a6 \ --hash=sha256:02abe224de6ae62c19f090f68da4e27b10af2b93213d36cf44e6e1c5abd19fdd \ diff --git a/tests/errata/fake_errata_tool.py b/tests/errata/fake_errata_tool.py index d10722c1..bf3e40ca 100644 --- a/tests/errata/fake_errata_tool.py +++ b/tests/errata/fake_errata_tool.py @@ -4,6 +4,7 @@ import yaml import json from xmlrpc.client import Fault # nosec B411 +from urllib3.util.retry import Retry import requests @@ -54,6 +55,7 @@ class FakeErrataToolProxy(object): def __init__(self, controller): self._ctrl = controller self.auth = None + self.mount = Mock(spec=Retry) self.url_map = [ (".*/api/v1/erratum/(.*)", self.get_advisory_data), ( diff --git a/tests/errata/test_errata_client.py b/tests/errata/test_errata_client.py index b8d9af42..0144034e 100644 --- a/tests/errata/test_errata_client.py +++ b/tests/errata/test_errata_client.py @@ -7,6 +7,7 @@ import requests from pushsource._impl.backend.errata_source.errata_client import ( + ERRATA_RETRY_STRATEGY, ErrataHTTPClient, get_errata_client, ) @@ -35,11 +36,14 @@ def test_init_env_vars(): assert client.principal == "pub-errata@IPA.REDHAT.COM" +@mock.patch("requests.adapters.HTTPAdapter") @mock.patch("gssapi.Name") @mock.patch("gssapi.Credentials.acquire") @mock.patch("requests.Session") @mock.patch("requests_gssapi.HTTPSPNEGOAuth") -def test_get_session(mock_auth, mock_session, mock_acquire, mock_name, caplog): +def test_get_session( + mock_auth, mock_session, mock_acquire, mock_name, mock_adapter, caplog +): caplog.set_level(logging.DEBUG) client = ErrataHTTPClient( @@ -57,6 +61,8 @@ def test_get_session(mock_auth, mock_session, mock_acquire, mock_name, caplog): ) mock_session.assert_called_once_with() mock_auth.assert_called_once_with(creds=mock_acquire.return_value.creds) + mock_adapter.assert_called_once_with(max_retries=ERRATA_RETRY_STRATEGY) + assert session.mount.call_count == 2 assert session == mock_session.return_value assert client._tls.session == mock_session.return_value @@ -113,32 +119,3 @@ def test_get_advisory_data(caplog): "GET https://errata.example.com/api/v1/erratum/RHSA-123456789 200", "Errata Tool completed call /api/v1/erratum/{id}(RHSA-123456789)", ] - - -def test_get_advisory_data_retry(caplog): - caplog.set_level(logging.DEBUG) - - client = ErrataHTTPClient( - 1, "https://errata.example.com/", "/path/to/keytab", "pub-errata@IPA.REDHAT.COM" - ) - client._call_et.retry.sleep = mock.MagicMock() - client._tls.session = requests.Session() - - with requests_mock.Mocker() as m: - m.get( - "https://errata.example.com/api/v1/erratum/RHSA-123456789", - [{"status_code": 504}, {"json": {"errata": "data"}, "status_code": 200}], - ) - - data = client.get_advisory_data("RHSA-123456789") - - assert data == {"errata": "data"} - assert m.call_count == 2 - assert caplog.messages == [ - "Calling Errata Tool /api/v1/erratum/{id}(RHSA-123456789)", - "GET https://errata.example.com/api/v1/erratum/RHSA-123456789 504", - "Failed to call Errata Tool /api/v1/erratum/{id}(RHSA-123456789)", - "Calling Errata Tool /api/v1/erratum/{id}(RHSA-123456789)", - "GET https://errata.example.com/api/v1/erratum/RHSA-123456789 200", - "Errata Tool completed call /api/v1/erratum/{id}(RHSA-123456789)", - ] diff --git a/tests/errata/test_errata_client_logging.py b/tests/errata/test_errata_client_logging.py index 1b5528ff..ca71ed83 100644 --- a/tests/errata/test_errata_client_logging.py +++ b/tests/errata/test_errata_client_logging.py @@ -41,7 +41,6 @@ def test_errata_client_debug_logs(caplog): # disable retry just for this test max_attempts=1, ) - client._call_et.retry.sleep = mock.MagicMock() with patch( "pushsource._impl.backend.errata_source.errata_client.xmlrpc_client.ServerProxy"