From f9f2c6a6de0f1b7f549189080206539d7d137997 Mon Sep 17 00:00:00 2001 From: Lukas Holecek Date: Oct 12 2021 15:52:01 +0000 Subject: Optimize cache usage There is no need to store whole Koji build metadata in cache. Only task ID and source URL are needed. Similar with task request metadata where only the build target is needed. --- diff --git a/greenwave/product_versions.py b/greenwave/product_versions.py index af119a5..5294793 100644 --- a/greenwave/product_versions.py +++ b/greenwave/product_versions.py @@ -8,7 +8,12 @@ import re import socket import xmlrpc.client -from greenwave.resources import retrieve_koji_build, retrieve_koji_task_request +from werkzeug.exceptions import NotFound + +from greenwave.resources import ( + retrieve_koji_task_id_and_source, + retrieve_koji_build_target, +) log = logging.getLogger(__name__) @@ -49,16 +54,19 @@ def _guess_koji_build_product_version( try: if not koji_task_id: log.debug('Getting Koji task ID for build %r', subject_identifier) - build = retrieve_koji_build(subject_identifier, koji_base_url) or {} - koji_task_id = build.get('task_id') + try: + koji_task_id, _ = retrieve_koji_task_id_and_source( + subject_identifier, koji_base_url + ) + except NotFound: + koji_task_id = None + if not koji_task_id: return None - task_request = retrieve_koji_task_request(koji_task_id, koji_base_url) - if isinstance(task_request, list) and len(task_request) > 1: - target = task_request[1] - if isinstance(target, str): - return _guess_product_version(target, koji_build=True) + target = retrieve_koji_build_target(koji_task_id, koji_base_url) + if target: + return _guess_product_version(target, koji_build=True) return None except (xmlrpc.client.ProtocolError, socket.error) as err: diff --git a/greenwave/resources.py b/greenwave/resources.py index e4b4f50..803d5e9 100644 --- a/greenwave/resources.py +++ b/greenwave/resources.py @@ -146,42 +146,48 @@ class NoSourceException(RuntimeError): @cached -def retrieve_koji_task_request(nvr, koji_url): +def retrieve_koji_build_target(nvr, koji_url): log.debug('Getting Koji task request ID %r', nvr) proxy = get_server_proxy(koji_url, _requests_timeout()) - return proxy.getTaskRequest(nvr) + task_request = proxy.getTaskRequest(nvr) + if isinstance(task_request, list) and len(task_request) > 1: + target = task_request[1] + if isinstance(target, str): + return target + return None @cached -def retrieve_koji_build(nvr, koji_url): +def retrieve_koji_task_id_and_source(nvr, koji_url): log.debug('Getting Koji build %r', nvr) proxy = get_server_proxy(koji_url, _requests_timeout()) - return proxy.getBuild(nvr) + build = proxy.getBuild(nvr) + if not build: + raise NotFound( + 'Failed to find Koji build for "{}" at "{}"'.format(nvr, koji_url) + ) + task_id = build.get("task_id") -def retrieve_scm_from_koji(nvr): - """ Retrieve cached rev and namespace from koji using the nvr """ - koji_url = current_app.config['KOJI_BASE_URL'] try: - build = retrieve_koji_build(nvr, koji_url) - except (xmlrpc.client.ProtocolError, socket.error) as err: - raise ConnectionError('Could not reach Koji: {}'.format(err)) - return retrieve_scm_from_koji_build(nvr, build, koji_url) + source = build["extra"]["source"]["original_url"] + except (TypeError, KeyError, AttributeError): + source = build.get("source") + return (task_id, source) -def retrieve_scm_from_koji_build(nvr, build, koji_url): - if not build: - raise NotFound('Failed to find Koji build for "{}" at "{}"'.format(nvr, koji_url)) - source = None +def retrieve_scm_from_koji(nvr): + """Retrieve cached rev and namespace from koji using the nvr""" + koji_url = current_app.config["KOJI_BASE_URL"] try: - source = build['extra']['source']['original_url'] - except (TypeError, KeyError, AttributeError): - pass - finally: - if not source: - source = build.get('source') + _, source = retrieve_koji_task_id_and_source(nvr, koji_url) + except (xmlrpc.client.ProtocolError, socket.error) as err: + raise ConnectionError("Could not reach Koji: {}".format(err)) + return retrieve_scm_from_koji_build(nvr, source, koji_url) + +def retrieve_scm_from_koji_build(nvr, source, koji_url): if not source: raise NoSourceException( 'Failed to retrieve SCM URL from Koji build "{}" at "{}" ' diff --git a/greenwave/tests/test_retrieve_gating_yaml.py b/greenwave/tests/test_retrieve_gating_yaml.py index a68aeeb..2f03073 100644 --- a/greenwave/tests/test_retrieve_gating_yaml.py +++ b/greenwave/tests/test_retrieve_gating_yaml.py @@ -7,19 +7,16 @@ import pytest import mock from werkzeug.exceptions import BadGateway, NotFound -import greenwave.app_factory from greenwave.resources import ( - retrieve_scm_from_koji_build, retrieve_scm_from_koji, retrieve_yaml_remote_rule, - NoSourceException + NoSourceException, + retrieve_scm_from_koji, + retrieve_yaml_remote_rule, ) -from greenwave.app_factory import create_app -KOJI_URL = 'https://koji.fedoraproject.org/kojihub' - -def test_retrieve_scm_from_rpm_build(): +def test_retrieve_scm_from_rpm_build(app, koji_proxy): nvr = 'nethack-3.6.1-3.fc29' - build = { + koji_proxy.getBuild.return_value = { 'nvr': nvr, 'extra': { 'source': { @@ -30,60 +27,60 @@ def test_retrieve_scm_from_rpm_build(): # also check, that there's no fallback to source 'source': 'git+https://src.fedoraproject.org/rpms/nethack.git#master' } - namespace, pkg_name, rev = retrieve_scm_from_koji_build(nvr, build, KOJI_URL) + namespace, pkg_name, rev = retrieve_scm_from_koji(nvr) assert namespace == 'rpms' assert rev == '0c1a84e0e8a152897003bd7e27b3f407ff6ba040' assert pkg_name == 'nethack' -def test_retrieve_scm_from_rpm_build_fallback_to_source(): +def test_retrieve_scm_from_rpm_build_fallback_to_source(app, koji_proxy): nvr = 'nethack-3.6.1-3.fc29' - build = { + koji_proxy.getBuild.return_value = { 'nvr': nvr, 'source': 'git+https://src.fedoraproject.org/rpms/nethack.git#' '0c1a84e0e8a152897003bd7e27b3f407ff6ba040' } - namespace, pkg_name, rev = retrieve_scm_from_koji_build(nvr, build, KOJI_URL) + namespace, pkg_name, rev = retrieve_scm_from_koji(nvr) assert namespace == 'rpms' assert rev == '0c1a84e0e8a152897003bd7e27b3f407ff6ba040' assert pkg_name == 'nethack' -def test_retrieve_scm_from_container_build(): +def test_retrieve_scm_from_container_build(app, koji_proxy): nvr = 'golang-github-openshift-prometheus-alert-buffer-container-v3.10.0-0.34.0.0' - build = { + koji_proxy.getBuild.return_value = { 'nvr': nvr, 'source': 'git://pkgs.devel.redhat.com/containers/' 'golang-github-openshift-prometheus-alert-buffer#' '46af2f8efbfb0a4e7e7d5676f4efb997f72d4b8c' } - namespace, pkg_name, rev = retrieve_scm_from_koji_build(nvr, build, KOJI_URL) + namespace, pkg_name, rev = retrieve_scm_from_koji(nvr) assert namespace == 'containers' assert rev == '46af2f8efbfb0a4e7e7d5676f4efb997f72d4b8c' assert pkg_name == 'golang-github-openshift-prometheus-alert-buffer' -def test_retrieve_scm_from_nonexistent_build(): +def test_retrieve_scm_from_nonexistent_build(app, koji_proxy): nvr = 'foo-1.2.3-1.fc29' - build = {} - expected_error = 'Failed to find Koji build for "{}" at "{}"'.format(nvr, KOJI_URL) + koji_proxy.getBuild.return_value = {} + expected_error = 'Failed to find Koji build for "{}" at "{}"'.format( + nvr, app.config["KOJI_BASE_URL"] + ) with pytest.raises(NotFound, match=expected_error): - retrieve_scm_from_koji_build(nvr, build, KOJI_URL) + retrieve_scm_from_koji(nvr) -def test_retrieve_scm_from_build_with_missing_source(): - nvr = 'foo-1.2.3-1.fc29' - build = { - 'nvr': nvr - } +def test_retrieve_scm_from_build_with_missing_source(app, koji_proxy): + nvr = "foo-1.2.3-1.fc29" + koji_proxy.getBuild.return_value = {"nvr": nvr} expected_error = 'expected SCM URL in "source" attribute' with pytest.raises(NoSourceException, match=expected_error): - retrieve_scm_from_koji_build(nvr, build, KOJI_URL) + retrieve_scm_from_koji(nvr) -def test_retrieve_scm_from_build_without_namespace(): +def test_retrieve_scm_from_build_without_namespace(app, koji_proxy): nvr = 'foo-1.2.3-1.fc29' - build = { + koji_proxy.getBuild.return_value = { 'nvr': nvr, 'extra': { 'source': { @@ -91,27 +88,25 @@ def test_retrieve_scm_from_build_without_namespace(): } } } - namespace, pkg_name, rev = retrieve_scm_from_koji_build(nvr, build, KOJI_URL) + namespace, pkg_name, rev = retrieve_scm_from_koji(nvr) assert namespace == '' assert rev == 'deadbeef' assert pkg_name == 'foo' -def test_retrieve_scm_from_koji_build_not_found(koji_proxy): +def test_retrieve_scm_from_koji_build_not_found(app, koji_proxy): nvr = 'foo-1.2.3-1.fc29' - app = create_app('greenwave.config.TestingConfig') - with app.app_context(): - expected_error = '404 Not Found: Failed to find Koji build for "{}" at "{}"'.format( - nvr, app.config['KOJI_BASE_URL'] - ) - koji_proxy.getBuild.return_value = {} - with pytest.raises(NotFound, match=expected_error): - retrieve_scm_from_koji(nvr) + expected_error = '404 Not Found: Failed to find Koji build for "{}" at "{}"'.format( + nvr, app.config['KOJI_BASE_URL'] + ) + koji_proxy.getBuild.return_value = {} + with pytest.raises(NotFound, match=expected_error): + retrieve_scm_from_koji(nvr) -def test_retrieve_scm_from_build_with_missing_rev(): +def test_retrieve_scm_from_build_with_missing_rev(app, koji_proxy): nvr = 'foo-1.2.3-1.fc29' - build = { + koji_proxy.getBuild.return_value = { 'nvr': nvr, 'extra': { 'source': { @@ -121,58 +116,52 @@ def test_retrieve_scm_from_build_with_missing_rev(): } expected_error = 'missing URL fragment with SCM revision information' with pytest.raises(BadGateway, match=expected_error): - retrieve_scm_from_koji_build(nvr, build, KOJI_URL) - - -def test_retrieve_yaml_remote_rule_no_namespace(): - app = greenwave.app_factory.create_app() - with app.app_context(): - with mock.patch('greenwave.resources.requests_session') as session: - # Return 404, because we are only interested in the URL in the request - # and whether it is correct even with empty namespace. - response = mock.MagicMock() - response.status_code = 404 - session.request.return_value = response - returned_file = retrieve_yaml_remote_rule( + retrieve_scm_from_koji(nvr) + + +def test_retrieve_yaml_remote_rule_no_namespace(app): + with mock.patch('greenwave.resources.requests_session') as session: + # Return 404, because we are only interested in the URL in the request + # and whether it is correct even with empty namespace. + response = mock.MagicMock() + response.status_code = 404 + session.request.return_value = response + returned_file = retrieve_yaml_remote_rule( + app.config['REMOTE_RULE_POLICIES']['*'].format( + rev='deadbeaf', pkg_name='pkg', pkg_namespace='' + ) + ) + + assert session.request.mock_calls == [mock.call( + 'HEAD', 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml' + )] + assert returned_file is None + + +def test_retrieve_yaml_remote_rule_connection_error(app): + with mock.patch('requests.Session.request') as mocked_request: + response = mock.MagicMock() + response.status_code = 200 + mocked_request.side_effect = [ + response, ConnectionError('Something went terribly wrong...') + ] + + with pytest.raises(HTTPError) as excinfo: + retrieve_yaml_remote_rule( app.config['REMOTE_RULE_POLICIES']['*'].format( rev='deadbeaf', pkg_name='pkg', pkg_namespace='' ) ) - assert session.request.mock_calls == [mock.call( - 'HEAD', 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml' - )] - assert returned_file is None - - -def test_retrieve_yaml_remote_rule_connection_error(): - app = greenwave.app_factory.create_app() - with app.app_context(): - with mock.patch('requests.Session.request') as mocked_request: - response = mock.MagicMock() - response.status_code = 200 - mocked_request.side_effect = [ - response, ConnectionError('Something went terribly wrong...') - ] - - with pytest.raises(HTTPError) as excinfo: - retrieve_yaml_remote_rule( - app.config['REMOTE_RULE_POLICIES']['*'].format( - rev='deadbeaf', pkg_name='pkg', pkg_namespace='' - ) - ) - - assert str(excinfo.value) == ( - '502 Server Error: Something went terribly wrong... for url: ' - 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml' - ) + assert str(excinfo.value) == ( + '502 Server Error: Something went terribly wrong... for url: ' + 'https://src.fedoraproject.org/pkg/raw/deadbeaf/f/gating.yaml' + ) -def test_retrieve_scm_from_koji_build_socket_error(koji_proxy): +def test_retrieve_scm_from_koji_build_socket_error(app, koji_proxy): koji_proxy.getBuild.side_effect = socket.error('Socket is closed') - app = greenwave.app_factory.create_app() nvr = 'nethack-3.6.1-3.fc29' expected_error = 'Could not reach Koji: Socket is closed' with pytest.raises(socket.error, match=expected_error): - with app.app_context(): - retrieve_scm_from_koji(nvr) + retrieve_scm_from_koji(nvr)