From da7dab0874b765702f266f820fbc64e0da81e852 Mon Sep 17 00:00:00 2001 From: Luiz Carvalho Date: Wed, 15 Jul 2020 16:36:08 -0400 Subject: [PATCH] Restrict backporting just to v4.5 * CLOUDBLD-1880 Signed-off-by: Luiz Carvalho --- iib/workers/tasks/build.py | 48 +++++++++++++++----- iib/workers/tasks/legacy.py | 8 +++- tests/test_workers/test_tasks/test_build.py | 47 +++++++++++++++---- tests/test_workers/test_tasks/test_legacy.py | 25 +++++++--- 4 files changed, 100 insertions(+), 28 deletions(-) diff --git a/iib/workers/tasks/build.py b/iib/workers/tasks/build.py index f0b107a3c..fb23bd1fb 100644 --- a/iib/workers/tasks/build.py +++ b/iib/workers/tasks/build.py @@ -563,25 +563,46 @@ def _prepare_request_for_build( if operator: bundle_mapping.setdefault(operator, []).append(bundle) - payload = { + return { + 'arches': arches, 'binary_image_resolved': binary_image_resolved, + 'bundle_mapping': bundle_mapping, + 'from_index_resolved': from_index_resolved, + 'ocp_version': ocp_version, + } + + +def _update_index_image_build_state(request_id, prebuild_info): + """ + Update the build request state with pre-determined build information. + + :param int request_id: the ID of the IIB build request + :param dict prebuild_info: the information relevant to the build operation. The key ``arches`` + is required and must be set to the list of arches to build for. The key + ``binary_image_resolved`` is required and must be set to the image digest pull spec of the + binary image. The key ``bundle_mapping`` is optional. When provided, its value must be a + dict mapping an operator to a list of bundle images. The key ``from_index_resolved`` is + optional. When provided it must be set to the image digest pull spec of the from index + image. + """ + arches_str = ', '.join(sorted(prebuild_info['arches'])) + payload = { + 'binary_image_resolved': prebuild_info['binary_image_resolved'], 'state': 'in_progress', 'state_reason': f'Building the index image for the following arches: {arches_str}', } + + bundle_mapping = prebuild_info.get('bundle_mapping') if bundle_mapping: payload['bundle_mapping'] = bundle_mapping + + from_index_resolved = prebuild_info.get('from_index_resolved') if from_index_resolved: payload['from_index_resolved'] = from_index_resolved + exc_msg = 'Failed setting the resolved images on the request' update_request(request_id, payload, exc_msg) - return { - 'arches': arches, - 'binary_image_resolved': binary_image_resolved, - 'from_index_resolved': from_index_resolved, - 'ocp_version': ocp_version, - } - @retry(wait_on=IIBError, logger=log) def _push_image(request_id, arch): @@ -747,18 +768,20 @@ def handle_add_request( if greenwave_config: gate_bundles(resolved_bundles, greenwave_config) + prebuild_info = _prepare_request_for_build( + binary_image, request_id, from_index, overwrite_from_index_token, add_arches, bundles + ) + log.info('Checking if interacting with the legacy app registry is required') legacy_support_packages = get_legacy_support_packages( - resolved_bundles, request_id, force_backport=force_backport + resolved_bundles, request_id, prebuild_info['ocp_version'], force_backport=force_backport ) if legacy_support_packages: validate_legacy_params_and_config( legacy_support_packages, resolved_bundles, cnr_token, organization ) - prebuild_info = _prepare_request_for_build( - binary_image, request_id, from_index, overwrite_from_index_token, add_arches, bundles - ) + _update_index_image_build_state(request_id, prebuild_info) with tempfile.TemporaryDirectory(prefix='iib-') as temp_dir: _opm_index_add( @@ -837,6 +860,7 @@ def handle_rm_request( prebuild_info = _prepare_request_for_build( binary_image, request_id, from_index, overwrite_from_index_token, add_arches ) + _update_index_image_build_state(request_id, prebuild_info) with tempfile.TemporaryDirectory(prefix='iib-') as temp_dir: _opm_index_rm(temp_dir, operators, binary_image, from_index, overwrite_from_index_token) diff --git a/iib/workers/tasks/legacy.py b/iib/workers/tasks/legacy.py index afb77e348..488797880 100644 --- a/iib/workers/tasks/legacy.py +++ b/iib/workers/tasks/legacy.py @@ -51,20 +51,24 @@ def _get_base_dir_and_pkg_name(package_dir): return os.path.dirname(package_dir), os.path.basename(package_dir) -def get_legacy_support_packages(bundles, request_id, force_backport=False): +def get_legacy_support_packages(bundles, request_id, ocp_version, force_backport=False): """ Get the packages that must be pushed to the legacy application registry. :param list bundles: a list of strings representing the pull specifications of the bundles to add to the index image being built. :param int request_id: the ID of the IIB build request. + :param str ocp_version: the OCP version that the index is intended for. :param bool force_backport: if True, backport legacy support is forced for every package :return: a set of packages that require legacy support :rtype: set """ + packages = set() + if ocp_version != 'v4.5': + log.info('Backport legacy support is disabled for %s', ocp_version) + return packages if force_backport: set_request_state(request_id, 'in_progress', 'Backport legacy support will be forced') - packages = set() for bundle in bundles: labels = get_image_labels(bundle) if force_backport or labels.get('com.redhat.delivery.backport', False): diff --git a/tests/test_workers/test_tasks/test_build.py b/tests/test_workers/test_tasks/test_build.py index 4da2ea578..9c22962f4 100644 --- a/tests/test_workers/test_tasks/test_build.py +++ b/tests/test_workers/test_tasks/test_build.py @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-3.0-or-later +import copy import os import re import textwrap @@ -469,18 +470,42 @@ def test_prepare_request_for_build( assert rv == { 'arches': expected_arches, 'binary_image_resolved': binary_image_resolved, + 'bundle_mapping': expected_bundle_mapping, 'from_index_resolved': from_index_resolved, 'ocp_version': ocp_version, } - mock_ur.assert_called_once() - update_request_payload = mock_ur.call_args[0][1] - if expected_bundle_mapping: - assert update_request_payload['bundle_mapping'] == expected_bundle_mapping - else: - assert 'bundle_mapping' not in update_request_payload - assert update_request_payload.keys() == expected_payload_keys +@pytest.mark.parametrize('bundle_mapping', (True, False)) +@pytest.mark.parametrize('from_index_resolved', (True, False)) +@mock.patch('iib.workers.tasks.build.update_request') +def test_update_index_image_build_state( + mock_ur, bundle_mapping, from_index_resolved, +): + prebuild_info = { + 'arches': ['amd64', 's390x'], + 'binary_image_resolved': 'binary-image@sha256:12345', + 'extra': 'ignored', + } + + if bundle_mapping: + prebuild_info['bundle_mapping'] = { + 'some-bundle': ['quay.io/some-bundle:v1'], + 'some-bundle2': ['quay.io/some-bundle2:v1'], + } + + if from_index_resolved: + prebuild_info['from_index_resolved'] = 'from-index-image@sha256:abcde' + + expected_payload = copy.deepcopy(prebuild_info) + del expected_payload['arches'] + del expected_payload['extra'] + expected_payload['state'] = 'in_progress' + expected_payload['state_reason'] = mock.ANY + + request_id = 1 + build._update_index_image_build_state(request_id, prebuild_info) + mock_ur.assert_called_once_with(request_id, expected_payload, mock.ANY) @mock.patch('iib.workers.tasks.build.set_request_state') @@ -574,6 +599,7 @@ def test_skopeo_copy_fail_max_retries(mock_run_cmd): @mock.patch('iib.workers.tasks.build._cleanup') @mock.patch('iib.workers.tasks.build._verify_labels') @mock.patch('iib.workers.tasks.build._prepare_request_for_build') +@mock.patch('iib.workers.tasks.build._update_index_image_build_state') @mock.patch('iib.workers.tasks.build._opm_index_add') @mock.patch('iib.workers.tasks.build._build_image') @mock.patch('iib.workers.tasks.build._push_image') @@ -601,6 +627,7 @@ def test_handle_add_request( mock_pi, mock_bi, mock_oia, + mock_uiibs, mock_prfb, mock_vl, mock_cleanup, @@ -642,7 +669,7 @@ def test_handle_add_request( mock_prfb.assert_called_once() mock_gb.assert_called_once() mock_aolti.assert_called_once() - mock_glsp.assert_called_once_with(['some-bundle@sha'], 3, force_backport=force_backport) + mock_glsp.assert_called_once_with(['some-bundle@sha'], 3, 'v4.5', force_backport=force_backport) add_args = mock_oia.call_args[0] assert ['some-bundle@sha'] in add_args @@ -727,6 +754,7 @@ def test_handle_add_request_bundle_resolution_failure(mock_grb, mock_srs, mock_c @mock.patch('iib.workers.tasks.build._cleanup') @mock.patch('iib.workers.tasks.build._verify_labels') @mock.patch('iib.workers.tasks.build._prepare_request_for_build') +@mock.patch('iib.workers.tasks.build._update_index_image_build_state') @mock.patch('iib.workers.tasks.build._opm_index_add') @mock.patch('iib.workers.tasks.build._build_image') @mock.patch('iib.workers.tasks.build._push_image') @@ -754,6 +782,7 @@ def test_handle_add_request_backport_failure_no_overwrite( mock_pi, mock_bi, mock_oia, + mock_uiibs, mock_prfb, mock_vl, mock_cleanup, @@ -796,6 +825,7 @@ def test_handle_add_request_backport_failure_no_overwrite( @mock.patch('iib.workers.tasks.build._cleanup') @mock.patch('iib.workers.tasks.build._prepare_request_for_build') +@mock.patch('iib.workers.tasks.build._update_index_image_build_state') @mock.patch('iib.workers.tasks.build._opm_index_rm') @mock.patch('iib.workers.tasks.build._build_image') @mock.patch('iib.workers.tasks.build._push_image') @@ -812,6 +842,7 @@ def test_handle_rm_request( mock_vii, mock_pi, mock_bi, + mock_uiibs, mock_oir, mock_prfb, mock_cleanup, diff --git a/tests/test_workers/test_tasks/test_legacy.py b/tests/test_workers/test_tasks/test_legacy.py index c6e6fa667..55d5cd92c 100644 --- a/tests/test_workers/test_tasks/test_legacy.py +++ b/tests/test_workers/test_tasks/test_legacy.py @@ -9,13 +9,21 @@ @pytest.mark.parametrize( - 'backport_label, force_backport, expect_backport', - ((True, True, True), (True, False, True), (False, True, True), (False, False, False),), + 'backport_label, ocp_version, force_backport, expect_backport', + ( + (True, 'v4.5', True, True), + (True, 'v4.5', False, True), + (False, 'v4.5', True, True), + (False, 'v4.5', False, False), + (True, 'v4.6', True, False), + (True, 'v4.6', False, False), + (False, 'v4.6', True, False), + ), ) @mock.patch('iib.workers.tasks.legacy.set_request_state') @mock.patch('iib.workers.tasks.utils.skopeo_inspect') def test_get_legacy_support_packages( - mock_skopeo_inspect, mock_srs, backport_label, force_backport, expect_backport + mock_skopeo_inspect, mock_srs, backport_label, ocp_version, force_backport, expect_backport ): mock_skopeo_inspect.return_value = { 'config': { @@ -26,14 +34,19 @@ def test_get_legacy_support_packages( } } - packages = legacy.get_legacy_support_packages(['some_bundle'], 1, force_backport=force_backport) - mock_skopeo_inspect.assert_called_once() + packages = legacy.get_legacy_support_packages( + ['some_bundle'], 1, ocp_version, force_backport=force_backport + ) + if ocp_version == 'v4.5': + mock_skopeo_inspect.assert_called_once() + else: + mock_skopeo_inspect.assert_not_called() if expect_backport: assert packages == {'prometheus'} else: assert packages == set() - if force_backport: + if force_backport and expect_backport: mock_srs.assert_called_once_with(1, 'in_progress', 'Backport legacy support will be forced') else: mock_srs.assert_not_called()