From afdda962bf88c0cf490d6e59f648d5f6dc7c2359 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lipovsk=C3=BD?= Date: Tue, 14 Jan 2025 10:58:03 +0100 Subject: [PATCH 1/3] Add functions that use opm render to extract information [CLOUDDST-25207] --- iib/workers/tasks/opm_operations.py | 142 ++++++++- .../test_tasks/test_opm_operations.py | 284 +++++++++++++++++- 2 files changed, 403 insertions(+), 23 deletions(-) diff --git a/iib/workers/tasks/opm_operations.py b/iib/workers/tasks/opm_operations.py index 8df97d73..d1814360 100644 --- a/iib/workers/tasks/opm_operations.py +++ b/iib/workers/tasks/opm_operations.py @@ -1,3 +1,4 @@ +import json from functools import wraps from copy import deepcopy import logging @@ -29,6 +30,7 @@ get_hidden_index_database, extract_fbc_fragment, ) +from iib.workers.tasks.iib_static_types import BundleImage log = logging.getLogger(__name__) @@ -487,6 +489,134 @@ def _serve_cmd_at_port( raise IIBError(f'Index registry has not been initialized after {max_tries} tries') +def get_operator_package_list( + input_image_or_path: str, + base_dir: str, +) -> List[str]: + """ + Get list of olm.package names from input data. + + :param str input_image_or_path: input data for opm render + Example: catalog-image | catalog-directory | bundle-image | bundle-directory | sqlite-file + :param str base_dir: temp directory where opm will be executed. + :return: list of package names present in input data. + :rtype: [str] + """ + olm_packages = opm_render(input_image_or_path, base_dir) + + package_names = [ + olm_package['name'] + for olm_package in olm_packages + if olm_package['schema'] == 'olm.package' + ] + + return package_names + + +def _get_olm_bundle_version(olm_bundle: dict) -> str: + """ + Find and return version of OLM bundle. + + :param BundleImage olm_bundle: olm bundle dictionary + :return: OLM bundle version + :rtype: str + """ + for property in olm_bundle['properties']: + if property['type'] == "olm.package": + return property['value']['version'] + + error_msg = "No olm package version found for OLM bundle." + log.warning(error_msg) + raise IIBError(error_msg) + + +def _get_input_data_path(input_image_or_path: str, base_dir: str) -> str: + """ + Retrieve correct path to data which will be used when calling opm render. + + If input_data is FBC index image we extract configs folder and return path to it. + If input_data is not FBC we extract hidden database and return path to it. + + :param str input_image_or_path: input data - index image, directory path + :param str base_dir: temp directory where data are located. + + :return: path to input for opm render + :rtype: str + """ + if os.path.exists(input_image_or_path): + return input_image_or_path + + if not is_image_fbc(input_image_or_path): + from iib.workers.tasks.build import _get_index_database + + log.info('Extracting SQLite DB from image %s', input_image_or_path) + return _get_index_database(input_image_or_path, base_dir) + else: + log.info('Extracting FBC from image %s', input_image_or_path) + return get_catalog_dir(input_image_or_path, base_dir) + + +def get_list_bundles( + input_data: str, + base_dir: str, +) -> List[BundleImage]: + """ + Run OPM render to get list of bundles present in input data. + + :param str input_data: input data for opm render + Example: catalog-image | catalog-directory | bundle-image | bundle-directory | sqlite-file + :param str base_dir: temp directory where opm will be executed. + :return: list of bundle images parsed from input data + :rtype: list(dict) + """ + log.info("Get list of bundles from %s", input_data) + + opm_data = opm_render(input_data, base_dir) + + # convert opm data to list of BundleImage + olm_bundles: List[BundleImage] = [ + BundleImage( + bundlePath=olm_bundle['image'], + csvName=olm_bundle['name'], + packageName=olm_bundle['package'], + version=_get_olm_bundle_version(olm_bundle), + ) + for olm_bundle in opm_data + if olm_bundle['schema'] == 'olm.bundle' + ] + + return olm_bundles + + +def opm_render( + input_data: str, + base_dir: str, +): + """ + Run OPM render and extract data as valid JSON. + + :param str input_data: input data for opm render + Example: catalog-image | catalog-directory | bundle-image | bundle-directory | sqlite-file + :param str base_dir: temp directory where opm will be executed. + :return: list of parsed data from input + :rtype: list(dict) + """ + from iib.workers.tasks.utils import run_cmd + + input_data_path = _get_input_data_path(input_data, base_dir) + cmd = [Opm.opm_version, 'render', input_data_path] + opm_render_output = run_cmd( + cmd, {'cwd': base_dir}, exc_msg=f'Failed to run opm render with input: {input_data}' + ) + + if not opm_render_output: + log.info("There are no data in %s", input_data) + return [] + + log.debug("Parsing data from opm render") + return [json.loads(package) for package in re.split(r'(?<=})\n(?={)', opm_render_output)] + + def _get_or_create_temp_index_db_file( base_dir: str, from_index: Optional[str] = None, @@ -1171,9 +1301,7 @@ def verify_operators_exists( :return: packages_in_index, index_db_path :rtype: (set, str) """ - from iib.workers.tasks.build import terminate_process, get_bundle_json from iib.workers.tasks.iib_static_types import BundleImage - from iib.workers.tasks.utils import run_cmd from iib.workers.tasks.utils import set_registry_token packages_in_index: Set[str] = set() @@ -1181,16 +1309,14 @@ def verify_operators_exists( log.info("Verifying if operator packages %s exists in index %s", operator_packages, from_index) # check if operator packages exists in hidden index.db + # we are not checking /config dir since it contains FBC opted-in operators and to remove those + # fbc-operations endpoint should be used with set_registry_token(overwrite_from_index_token, from_index, append=True): index_db_path = get_hidden_index_database(from_index=from_index, base_dir=base_dir) - port, rpc_proc = opm_registry_serve(db_path=index_db_path) - bundles = run_cmd( - ['grpcurl', '-plaintext', f'localhost:{port}', 'api.Registry/ListBundles'], - exc_msg='Failed to get bundle data from index image', + present_bundles: List[BundleImage] = get_list_bundles( + input_data=index_db_path, base_dir=base_dir ) - terminate_process(rpc_proc) - present_bundles: List[BundleImage] = get_bundle_json(bundles) for bundle in present_bundles: if bundle['packageName'] in operator_packages: diff --git a/tests/test_workers/test_tasks/test_opm_operations.py b/tests/test_workers/test_tasks/test_opm_operations.py index 5658c0a0..2b3d16c7 100644 --- a/tests/test_workers/test_tasks/test_opm_operations.py +++ b/tests/test_workers/test_tasks/test_opm_operations.py @@ -9,12 +9,17 @@ from iib.exceptions import IIBError, AddressAlreadyInUse from iib.workers.config import get_worker_config from iib.workers.tasks import opm_operations +from iib.workers.tasks.iib_static_types import BundleImage from iib.workers.tasks.opm_operations import ( Opm, PortFileLock, create_port_filelocks, get_opm_port_stacks, PortFileLockGenerator, + _get_input_data_path, + _get_olm_bundle_version, + get_list_bundles, + get_operator_package_list, ) @@ -998,39 +1003,42 @@ def test_opm_registry_add_fbc_fragment( 'bundles_in_db, opr_exists', [ ( - '{"packageName": "test-operator", "version": "v1.0", "bundlePath":"bundle1"\n}' - '\n{"packageName": "test-operator", "version": "v1.2", "bundlePath":"bundle1"\n}' - '\n{\n"packageName": "package2", "version": "v2.0", "bundlePath":"bundle2"}', + [ + {"packageName": "test-operator", "version": "v1.0", "bundlePath": "bundle1"}, + {"packageName": "test-operator", "version": "v1.2", "bundlePath": "bundle1"}, + {"packageName": "package2", "version": "v2.0", "bundlePath": "bundle2"}, + ], {"test-operator"}, ), ( - '{"packageName": "test-operator", "version": "v1.0", "bundlePath":"bundle1"\n}' - '\n{\n"packageName": "package2", "version": "v2.0", "bundlePath":"bundle2"}', + [ + {"packageName": "test-operator", "version": "v1.0", "bundlePath": "bundle1"}, + {"packageName": "package2", "version": "v2.0", "bundlePath": "bundle2"}, + ], {"test-operator"}, ), ( - '{"packageName": "package1", "version": "v1.0", "bundlePath":"bundle1"\n}' - '\n{\n"packageName": "package2", "version": "v2.0", "bundlePath":"bundle2"}', + [ + {"packageName": "package1", "version": "v1.0", "bundlePath": "bundle1"}, + {"packageName": "package2", "version": "v2.0", "bundlePath": "bundle2"}, + ], set(), ), ], ) -@mock.patch('iib.workers.tasks.utils.terminate_process') @mock.patch('iib.workers.tasks.utils.run_cmd') -@mock.patch('iib.workers.tasks.opm_operations.opm_registry_serve') +@mock.patch('iib.workers.tasks.opm_operations.get_list_bundles') @mock.patch('iib.workers.tasks.build._copy_files_from_image') @mock.patch('iib.workers.tasks.utils.set_registry_token') def test_verify_operator_exists( - mock_srt, mock_cffi, mock_ors, mock_rc, mock_tp, bundles_in_db, opr_exists, tmpdir + mock_srt, mock_cffi, mock_glb, mock_rc, bundles_in_db, opr_exists, tmpdir ): from_index = "example.com/test/index" - mock_ors.return_value = 500, mock.MagicMock() - mock_rc.return_value = bundles_in_db - index_db_path = os.path.join(tmpdir, get_worker_config()['temp_index_db_path']) + mock_glb.return_value = bundles_in_db package_exists, index_db_path = opm_operations.verify_operators_exists( - from_index, tmpdir, 'test-operator', None + from_index, tmpdir, ['test-operator'], None ) - mock_ors.assert_has_calls([mock.call(db_path=index_db_path)]) + mock_glb.assert_has_calls([mock.call(input_data=index_db_path, base_dir=tmpdir)]) assert package_exists == opr_exists @@ -1163,3 +1171,249 @@ def test_get_opm_version_number_fail(mock_run_cmd): match='Opm version not found in the output of \"OPM version\" command', ): Opm.get_opm_version_number() + + +@pytest.mark.parametrize( + 'input, is_input_dir, is_fbc', + [ + ('/tmp/somedir', True, False), + ( + 'registry.example.com/example-rhel9:v4.17', + False, + False, + ), + ( + 'registry.example.com/example-rhel9:v4.17', + False, + True, + ), + ], +) +@mock.patch('iib.workers.tasks.opm_operations.get_catalog_dir') +@mock.patch('iib.workers.tasks.build._get_index_database') +@mock.patch('iib.workers.tasks.opm_operations.is_image_fbc') +def test__get_input_data_path(mock_iif, mock_gid, mock_gcd, input, is_input_dir, is_fbc, tmpdir): + mock_iif.return_value = is_fbc + + _get_input_data_path(input_image_or_path=input, base_dir=tmpdir) + + if is_input_dir: + mock_iif.assert_called_once_with(input) + else: + mock_iif.assert_called_once_with(input) + if is_fbc: + mock_gcd.assert_called_once_with(input, tmpdir) + else: + mock_gid.assert_called_once_with(input, tmpdir) + + +@pytest.mark.parametrize( + "bundle, version", + [ + ( + { + "csvName": "example.v2.0.8", + "packageName": "example_operator", + "channelName": "1.0", + "bundlePath": "registry.example.com/example_operator@sha256:SHA-LONG_H3X_NUMBER", + "providedApis": [ + { + "group": "example_operator.io", + "version": "v1", + "kind": "ServiceMeshControlPlane", + }, + { + "group": "example_operator.io", + "version": "v2", + "kind": "ServiceMeshControlPlane", + }, + {"group": "example_operator.io", "version": "v1", "kind": "ServiceMeshMember"}, + { + "group": "example_operator.io", + "version": "v1", + "kind": "ServiceMeshMemberRoll", + }, + ], + "version": "2.0.8-0", + "skipRange": ">=1.0.2 <2.0.8-0", + "properties": [ + { + "type": "olm.package", + "value": {'packageName': 'example_operator', 'version': '2.0.8-0'}, + }, + ], + "replaces": "example_operator.v2.0.7.1", + }, + "2.0.8-0", + ) + ], +) +def test__get_olm_package_version(bundle, version): + ver = _get_olm_bundle_version(olm_bundle=bundle) + assert ver == version + + +def test__get_olm_package_version_raise(): + bundle = { + "csvName": "incorrect_budnle_data", + "properties": [ + { + "type": "olm.not_package", + }, + ], + } + with pytest.raises(IIBError, match="No olm package version found for OLM bundle."): + _get_olm_bundle_version(olm_bundle=bundle) + + +@mock.patch('iib.workers.tasks.opm_operations._get_input_data_path') +@mock.patch('iib.workers.tasks.utils.run_cmd') +@mock.patch.object(opm_operations.Opm, 'opm_version', 'opm-v1.26.8') +def test_get_list_bundles(mock_run_cmd, mock_gidp, tmpdir): + input_image = 'registry.example.com/example_operator:tag' + input_data_path = '/tmp/path' + mock_gidp.return_value = input + mock_gidp.return_value = input_data_path + + opm_render_output = """ + { + "schema": "olm.package", + "name": "example-operator", + "defaultChannel": "stable" +} +{ + "schema": "olm.channel", + "name": "stable", + "package": "example-operator", + "entries": [ + { + "name": "example-operator.v0.1.0" + } + ] +} +{ + "schema": "olm.bundle", + "name": "example-operator.v0.1.0", + "package": "example-operator", + "image": "quay.io/joelanford/example-operator-bundle:0.1.0", + "properties": [ + { + "type": "olm.package", + "value": { + "packageName": "example-operator", + "version": "0.1.0" + } + } + ], + "relatedImages": [ + { + "name": "", + "image": "quay.io/joelanford/example-operator:0.1.0" + } + ] +} +{ + "schema": "olm.bundle", + "name": "example-operator.v0.2.0", + "package": "example-operator", + "image": "quay.io/joelanford/example-operator-bundle:0.2.0", + "properties": [ + { + "type": "olm.package", + "value": { + "packageName": "example-operator", + "version": "0.2.0" + } + } + ], + "relatedImages": [ + { + "name": "", + "image": "quay.io/joelanford/example-operator:0.2.0" + } + ] +} + """ + + mock_run_cmd.return_value = opm_render_output + + bundles = get_list_bundles(input_data=input_image, base_dir=tmpdir) + + assert bundles == [ + BundleImage( + bundlePath='quay.io/joelanford/example-operator-bundle:0.1.0', + csvName='example-operator.v0.1.0', + packageName='example-operator', + version='0.1.0', + ), + BundleImage( + bundlePath='quay.io/joelanford/example-operator-bundle:0.2.0', + csvName='example-operator.v0.2.0', + packageName='example-operator', + version='0.2.0', + ), + ] + mock_run_cmd.assert_called_once_with( + ['opm-v1.26.8', 'render', input_data_path], + {'cwd': tmpdir}, + exc_msg=f'Failed to run opm render with input: {input_image}', + ) + + +@mock.patch('iib.workers.tasks.opm_operations._get_input_data_path') +@mock.patch('iib.workers.tasks.utils.run_cmd') +@mock.patch.object(opm_operations.Opm, 'opm_version', 'opm-v1.26.8') +def test_get_operator_package_list(mock_run_cmd, mock_gidp, tmpdir): + input_image = 'registry.example.com/example_operator:tag' + input_data_path = '/tmp/path' + mock_gidp.return_value = input + mock_gidp.return_value = input_data_path + + opm_render_output = """ + { + "schema": "olm.package", + "name": "example-operator", + "defaultChannel": "stable" +} +{ + "schema": "olm.channel", + "name": "stable", + "package": "example-operator", + "entries": [ + { + "name": "example-operator.v0.1.0" + } + ] +} +{ + "schema": "olm.bundle", + "name": "example-operator.v0.1.0", + "package": "example-operator", + "image": "quay.io/joelanford/example-operator-bundle:0.1.0", + "properties": [ + { + "type": "olm.package", + "value": { + "packageName": "example-operator", + "version": "0.1.0" + } + } + ], + "relatedImages": [ + { + "name": "", + "image": "quay.io/joelanford/example-operator:0.1.0" + } + ] +} + """ + + mock_run_cmd.return_value = opm_render_output + packages = get_operator_package_list(input_image_or_path=input_image, base_dir=tmpdir) + + assert packages == ['example-operator'] + mock_run_cmd.assert_called_once_with( + ['opm-v1.26.8', 'render', input_data_path], + {'cwd': tmpdir}, + exc_msg=f'Failed to run opm render with input: {input_image}', + ) From 6cde106c2aad66b52eab6a5cc163db886f90c5ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lipovsk=C3=BD?= Date: Tue, 14 Jan 2025 13:17:48 +0100 Subject: [PATCH 2/3] Use opm render - get_operator_package_list, get_list_bundles [CLOUDDST-25207] --- iib/workers/tasks/build.py | 24 +--- iib/workers/tasks/build_create_empty_index.py | 39 ++---- iib/workers/tasks/utils.py | 36 +---- tests/test_workers/test_tasks/test_build.py | 130 ++++++++++-------- .../test_build_create_empty_index.py | 44 ++---- .../test_build_merge_index_image.py | 52 +++---- .../test_tasks/test_opm_operations.py | 110 --------------- tests/test_workers/test_tasks/test_utils.py | 51 +------ 8 files changed, 132 insertions(+), 354 deletions(-) diff --git a/iib/workers/tasks/build.py b/iib/workers/tasks/build.py index 58ffae54..1a9efaa5 100644 --- a/iib/workers/tasks/build.py +++ b/iib/workers/tasks/build.py @@ -26,7 +26,6 @@ from iib.workers.greenwave import gate_bundles from iib.workers.tasks.fbc_utils import is_image_fbc, get_catalog_dir, merge_catalogs_dirs from iib.workers.tasks.opm_operations import ( - opm_serve_from_index, opm_registry_add_fbc, opm_migrate, opm_registry_rm_fbc, @@ -40,12 +39,12 @@ verify_operators_exists, create_dockerfile, opm_validate, + get_list_bundles, ) from iib.workers.tasks.utils import ( add_max_ocp_version_property, chmod_recursively, get_bundles_from_deprecation_list, - get_bundle_json, get_resolved_bundles, get_resolved_image, podman_pull, @@ -58,7 +57,6 @@ get_image_label, verify_labels, prepare_request_for_build, - terminate_process, get_bundle_metadata, ) from iib.workers.tasks.iib_static_types import ( @@ -392,22 +390,14 @@ def _get_present_bundles(from_index: str, base_dir: str) -> Tuple[List[BundleIma :rtype: list, list :raises IIBError: if any of the commands fail. """ - port, rpc_proc = opm_serve_from_index(base_dir, from_index=from_index) - - bundles = run_cmd( - ['grpcurl', '-plaintext', f'localhost:{port}', 'api.Registry/ListBundles'], - exc_msg='Failed to get bundle data from index image', - ) - terminate_process(rpc_proc) - - # If no data is returned there are not bundles present - if not bundles: - return [], [] - - # Transform returned data to parsable json + # Get list of bundles unique_present_bundles: List[BundleImage] = [] unique_present_bundles_pull_spec: List[str] = [] - present_bundles: List[BundleImage] = get_bundle_json(bundles) + present_bundles: List[BundleImage] = get_list_bundles(from_index, base_dir) + + # If no data is returned there are no bundles present + if not present_bundles: + return [], [] for bundle in present_bundles: bundle_path = bundle['bundlePath'] diff --git a/iib/workers/tasks/build_create_empty_index.py b/iib/workers/tasks/build_create_empty_index.py index 21f0601d..c054201f 100644 --- a/iib/workers/tasks/build_create_empty_index.py +++ b/iib/workers/tasks/build_create_empty_index.py @@ -1,9 +1,7 @@ # SPDX-License-Identifier: GPL-3.0-or-later import logging import tempfile -import json -import re -from typing import Dict, List, Optional +from typing import Dict, Optional from iib.common.tracing import instrument_tracing from iib.exceptions import IIBError @@ -20,12 +18,16 @@ ) from iib.workers.tasks.celery import app from iib.workers.tasks.fbc_utils import is_image_fbc -from iib.workers.tasks.opm_operations import opm_create_empty_fbc, opm_index_rm, Opm +from iib.workers.tasks.opm_operations import ( + opm_create_empty_fbc, + opm_index_rm, + Opm, + get_operator_package_list, +) from iib.workers.tasks.utils import ( request_logger, prepare_request_for_build, RequestConfigCreateIndexImage, - grpcurl_get_db_data, ) from iib.workers.tasks.iib_static_types import PrebuildInfo @@ -34,31 +36,6 @@ log = logging.getLogger(__name__) -def _get_present_operators(from_index: str, base_dir: str) -> List[str]: - """Get a list of operators already present in the index image. - - :param str from_index: index image to inspect. - :param str base_dir: base directory to create temporary files in. - :return: list of unique present operators as provided by the grpc query - :rtype: list - :raises IIBError: if any of the commands fail. - """ - operators = grpcurl_get_db_data(from_index, base_dir, "api.Registry/ListPackages") - - # If no data is returned there are not operators present - if not operators: - return [] - - # Transform returned data to parsable json - present_operators = [] - new_operators = [json.loads(operator) for operator in re.split(r'(?<=})\n(?={)', operators)] - - for operator in new_operators: - present_operators.append(operator['name']) - - return present_operators - - @app.task @request_logger @instrument_tracing( @@ -108,7 +85,7 @@ def handle_create_empty_index_request( with tempfile.TemporaryDirectory(prefix=f'iib-{request_id}-') as temp_dir: set_request_state(request_id, 'in_progress', 'Checking operators present in index image') - operators = _get_present_operators(from_index_resolved, temp_dir) + operators = get_operator_package_list(from_index_resolved, temp_dir) # if output_fbc parameter is true, create an empty FBC index image # else create empty SQLite index image diff --git a/iib/workers/tasks/utils.py b/iib/workers/tasks/utils.py index dddde80e..05499d6b 100644 --- a/iib/workers/tasks/utils.py +++ b/iib/workers/tasks/utils.py @@ -37,7 +37,7 @@ from iib.workers.config import get_worker_config from iib.workers.s3_utils import upload_file_to_s3_bucket from iib.workers.api_utils import set_request_state -from iib.workers.tasks.opm_operations import opm_registry_serve, opm_serve_from_index +from iib.workers.tasks.opm_operations import get_list_bundles from iib.workers.tasks.iib_static_types import ( IndexImageInfo, AllIndexImagesInfo, @@ -96,26 +96,20 @@ def add_max_ocp_version_property(resolved_bundles: List[str], temp_dir: str) -> # Get the CSV name and version (not just the bundle path) temp_index_db_path = get_worker_config()['temp_index_db_path'] db_path = os.path.join(temp_dir, temp_index_db_path) - port, rpc_proc = opm_registry_serve(db_path=db_path) - - raw_bundles = run_cmd( - ['grpcurl', '-plaintext', f'localhost:{port}', 'api.Registry/ListBundles'], - exc_msg='Failed to get bundle data from index image', - ) - terminate_process(rpc_proc) + bundles = get_list_bundles(input_data=db_path, base_dir=temp_dir) # This branch is hit when `bundles` attribute is empty and the index image is empty. # Ideally the code should not reach here if the bundles attribute is empty but adding # this here as a failsafe if it's called from some other place. Also, if the bundles # attribute is not empty, the index image cannot be empty here because we add the # bundle to the index before adding the maxOpenShiftVersion property - if not raw_bundles: + if not bundles: log.info('No bundles found in the index image') return # Filter index image bundles to get pull spec for bundles in the request updated_bundles: List[BundleImage] = list( - filter(lambda b: b['bundlePath'] in resolved_bundles, get_bundle_json(raw_bundles)) + filter(lambda b: b['bundlePath'] in resolved_bundles, bundles) ) for bundle in updated_bundles: @@ -1236,28 +1230,6 @@ def prepare_request_for_build( } -def grpcurl_get_db_data(from_index: str, base_dir: str, endpoint: str) -> str: - """Get a str with operators already present in the index image. - - :param str from_index: index image to inspect. - :param str base_dir: base directory to create temporary files in. - :return: str result of the grpc query - :rtype: str - :raises IIBError: if any of the commands fail. - """ - port, rpc_proc = opm_serve_from_index(base_dir, from_index=from_index) - - if endpoint not in ["api.Registry/ListPackages", "api.Registry/ListBundles"]: - raise IIBError(f"The endpoint '{endpoint}' is not allowed to be used") - - result = run_cmd( - ['grpcurl', '-plaintext', f'localhost:{port}', endpoint], - exc_msg=f'Failed to get {endpoint} data from index image', - ) - terminate_process(rpc_proc) - return result - - def get_bundle_metadata( operator_manifest: OperatorManifest, pinned_by_iib: bool, diff --git a/tests/test_workers/test_tasks/test_build.py b/tests/test_workers/test_tasks/test_build.py index f39c0b9d..c0af0adc 100644 --- a/tests/test_workers/test_tasks/test_build.py +++ b/tests/test_workers/test_tasks/test_build.py @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-3.0-or-later import copy +import json import os import re import stat @@ -10,6 +11,7 @@ from iib.exceptions import ExternalServiceError, IIBError from iib.workers.tasks import build +from iib.workers.tasks.iib_static_types import BundleImage from iib.workers.tasks.utils import RequestConfigAddRm from iib.workers.config import get_worker_config from operator_manifest.operator import ImageName @@ -521,8 +523,7 @@ def test_buildah_fail_max_retries(mock_run_cmd: mock.MagicMock) -> None: ) @pytest.mark.parametrize('distribution_scope', ('dev', 'stage', 'prod')) @pytest.mark.parametrize('deprecate_bundles', (True, False)) -@mock.patch('iib.workers.tasks.utils.run_cmd') -@mock.patch('iib.workers.tasks.utils.opm_registry_serve') +@mock.patch('iib.workers.tasks.utils.get_list_bundles') @mock.patch('iib.workers.tasks.build.deprecate_bundles') @mock.patch('iib.workers.tasks.utils.get_resolved_bundles') @mock.patch('iib.workers.tasks.build._cleanup') @@ -566,8 +567,7 @@ def test_handle_add_request( mock_cleanup, mock_ugrb, mock_dep_b, - mock_ors, - mock_run_cmd, + mock_glb, force_backport, binary_image, distribution_scope, @@ -611,11 +611,11 @@ def side_effect(*args, base_dir, **kwargs): mock_dep_b.side_effect = side_effect - port = 0 - my_mock = mock.MagicMock() - mock_ors.return_value = (port, my_mock) - mock_run_cmd.return_value = '{"packageName": "package1", "version": "v1.0", \ - "bundlePath": "bundle1"\n}' + mock_glb.return_value = [ + {"packageName": "test-operator", "version": "v1.0", "bundlePath": "bundle1"}, + {"packageName": "test-operator", "version": "v1.2", "bundlePath": "bundle1"}, + {"packageName": "package2", "version": "v2.0", "bundlePath": "bundle2"}, + ] build.handle_add_request( bundles, @@ -635,16 +635,7 @@ def side_effect(*args, base_dir, **kwargs): build_tags=["extra_tag1", "extra_tag2"], ) - mock_ors.assert_called_once() - mock_run_cmd.assert_called_once() - mock_run_cmd.assert_has_calls( - [ - mock.call( - ['grpcurl', '-plaintext', f'localhost:{port}', 'api.Registry/ListBundles'], - exc_msg=mock.ANY, - ), - ] - ) + mock_glb.assert_called_once() assert mock_cleanup.call_count == 2 mock_vl.assert_called_once() @@ -697,10 +688,11 @@ def side_effect(*args, base_dir, **kwargs): mock_dep_b.assert_not_called() +@mock.patch('iib.workers.tasks.build.update_request') @mock.patch('iib.workers.tasks.build._cleanup') @mock.patch('iib.workers.tasks.build.run_cmd') @mock.patch('iib.workers.tasks.build.is_image_fbc') -def test_handle_add_request_raises(mock_iifbc, mock_runcmd, mock_c): +def test_handle_add_request_raises(mock_iifbc, mock_runcmd, mock_c, mock_ur): mock_iifbc.return_value = True with pytest.raises(IIBError): build.handle_add_request( @@ -724,7 +716,7 @@ def test_handle_add_request_raises(mock_iifbc, mock_runcmd, mock_c): @mock.patch('iib.workers.tasks.build.get_worker_config') @mock.patch('iib.workers.tasks.utils.sqlite3.connect') @mock.patch('iib.workers.tasks.utils.run_cmd') -@mock.patch('iib.workers.tasks.utils.opm_registry_serve') +@mock.patch('iib.workers.tasks.utils.get_list_bundles') @mock.patch('iib.workers.tasks.build.deprecate_bundles') @mock.patch('iib.workers.tasks.utils.get_resolved_bundles') @mock.patch('iib.workers.tasks.build._cleanup') @@ -766,7 +758,7 @@ def test_handle_add_request_check_index_label_behavior( mock_cleanup, mock_ugrb, mock_dep_b, - mock_ors, + mock_glb, mock_run_cmd, mock_sqlite, mock_gwc, @@ -815,14 +807,18 @@ def deprecate_bundles_mock(*args, **kwargs): mock_dep_b.side_effect = deprecate_bundles_mock - port = 0 - my_mock = mock.MagicMock() - mock_ors.return_value = (port, my_mock) mock_run_cmd.side_effect = [ - '{"packageName": "package1", "version": "v1.0", "csvName": "random-csv", \ - "bundlePath": "some-bundle@sha256:123"\n}', '{"passed":false, "outputs": [{"message": "olm.maxOpenShiftVersion not present"}]}', ] + + mock_glb.return_value = [ + { + "packageName": "package1", + "version": "v1.0", + "csvName": "random-csv", + "bundlePath": "some-bundle@sha256:123", + } + ] mock_sqlite.execute.return_value = 200 build.handle_add_request( @@ -842,15 +838,11 @@ def deprecate_bundles_mock(*args, **kwargs): deprecation_list=deprecation_list, ) - mock_ors.assert_called_once() - assert mock_run_cmd.call_count == 2 + mock_glb.assert_called_once() + mock_run_cmd.assert_called_once() mock_run_cmd.assert_has_calls( [ - mock.call( - ['grpcurl', '-plaintext', f'localhost:{port}', 'api.Registry/ListBundles'], - exc_msg=mock.ANY, - ), mock.call( [ 'operator-sdk', @@ -1042,9 +1034,7 @@ def test_handle_rm_request( @mock.patch('iib.workers.tasks.build.opm_validate') -@mock.patch('iib.workers.tasks.build.get_bundle_json') @mock.patch('iib.workers.tasks.build.verify_operators_exists') -@mock.patch('iib.workers.tasks.opm_operations._serve_cmd_at_port') @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') @@ -1092,9 +1082,7 @@ def test_handle_rm_request_fbc( mock_uiibs, mock_prfb, mock_c, - mock_scap, mock_voe, - mock_gbj, mock_opmvalidate, tmpdir, ): @@ -1303,39 +1291,67 @@ def test_get_missing_bundles_match_hash(): ) -@mock.patch('iib.workers.tasks.build.run_cmd') -@mock.patch('iib.workers.tasks.build.opm_serve_from_index') -def test_get_present_bundles(moc_osfi, mock_run_cmd, tmpdir): - rpc_mock = mock.MagicMock() - moc_osfi.return_value = (50051, rpc_mock) - - mock_run_cmd.return_value = ( - '{"packageName": "package1", "version": "v1.0", "bundlePath":"bundle1"\n}' - '\n{\n"packageName": "package2", "version": "v2.0", "bundlePath":"bundle2"}' - '\n{\n"packageName": "package2", "version": "v2.0", "bundlePath":"bundle2"}' +@mock.patch('iib.workers.tasks.opm_operations._get_input_data_path') +@mock.patch('iib.workers.tasks.utils.run_cmd') +def test_get_present_bundles(mock_run_cmd, mock_gidp, tmpdir): + mock_gidp.return_value = '/tmp' + mock_run_cmd.return_value = '\n'.join( + json.dumps(a) + for a in [ + { + "schema": "olm.bundle", + "image": "bundle1", + "name": "name1", + "package": "package1", + "version": "v1.0", + "properties": [{"type": "olm.package", "value": {"version": "0.1.0"}}], + }, + { + "schema": "olm.bundle", + "image": "bundle2", + "name": "name2", + "package": "package2", + "version": "v2.0", + "properties": [{"type": "olm.package", "value": {"version": "0.2.2"}}], + }, + { + "schema": "olm.bundle", + "image": "bundle2", + "name": "name2", + "package": "package2", + "version": "v2.0", + "properties": [{"type": "olm.package", "value": {"version": "0.2.2"}}], + }, + ] ) - bundles, bundles_pull_spec = build._get_present_bundles('quay.io/index-image:4.5', str(tmpdir)) assert bundles == [ - {'packageName': 'package1', 'version': 'v1.0', 'bundlePath': 'bundle1'}, - {'packageName': 'package2', 'version': 'v2.0', 'bundlePath': 'bundle2'}, + BundleImage( + bundlePath='bundle1', + csvName='name1', + packageName='package1', + version='0.1.0', + ), + BundleImage( + bundlePath='bundle2', + csvName='name2', + packageName='package2', + version='0.2.2', + ), ] assert bundles_pull_spec == ['bundle1', 'bundle2'] mock_run_cmd.assert_called_once() -@mock.patch('iib.workers.tasks.build.run_cmd') -@mock.patch('iib.workers.tasks.build.opm_serve_from_index') +@mock.patch('iib.workers.tasks.opm_operations._get_input_data_path') +@mock.patch('iib.workers.tasks.utils.run_cmd') def test_get_no_present_bundles( - moc_osfi, mock_run_cmd, + mock_gidp, tmpdir, ): - - rpc_mock = mock.MagicMock() - moc_osfi.return_value = (50051, rpc_mock) - mock_run_cmd.return_value = '' + mock_gidp.return_value = '/tmp' bundle, bundle_pull_spec = build._get_present_bundles('quay.io/index-image:4.5', str(tmpdir)) assert bundle == [] diff --git a/tests/test_workers/test_tasks/test_build_create_empty_index.py b/tests/test_workers/test_tasks/test_build_create_empty_index.py index 916d6a7d..132201cb 100644 --- a/tests/test_workers/test_tasks/test_build_create_empty_index.py +++ b/tests/test_workers/test_tasks/test_build_create_empty_index.py @@ -9,37 +9,13 @@ from tests.test_workers.test_tasks.test_opm_operations import ensure_opm_default # noqa: F401 -@mock.patch('iib.workers.tasks.build_create_empty_index.grpcurl_get_db_data') -def test_get_present_operators(mock_grpcurl, tmpdir): - - mock_grpcurl.side_effect = ['{\n"name": "package1"\n}\n{\n"name": "package2"\n}\n'] - operators = build_create_empty_index._get_present_operators( - 'quay.io/index-image:4.5', tmpdir.join("index.db") - ) - - mock_grpcurl.assert_called_once() - assert operators == ['package1', 'package2'] - - -@mock.patch('iib.workers.tasks.build_create_empty_index.grpcurl_get_db_data') -def test_get_no_present_operators(mock_grpcurl, tmpdir): - - mock_grpcurl.return_value = None - operators = build_create_empty_index._get_present_operators( - 'quay.io/index-image:4.5', tmpdir.join("index.db") - ) - - assert mock_grpcurl.call_count == 1 - assert operators == [] - - @pytest.mark.parametrize('binary_image', ('binary-image:latest', None)) @pytest.mark.parametrize('from_index', ('index-image:latest', 'index-image:latest')) @mock.patch('iib.workers.tasks.build_create_empty_index._cleanup') @mock.patch('iib.workers.tasks.build_create_empty_index.prepare_request_for_build') @mock.patch('iib.workers.tasks.build_create_empty_index._update_index_image_build_state') @mock.patch('iib.workers.tasks.build_create_empty_index.set_request_state') -@mock.patch('iib.workers.tasks.build_create_empty_index._get_present_operators') +@mock.patch('iib.workers.tasks.build_create_empty_index.get_operator_package_list') @mock.patch('iib.workers.tasks.build_create_empty_index.opm_index_rm') @mock.patch('iib.workers.tasks.build_create_empty_index._add_label_to_index') @mock.patch('iib.workers.tasks.build_create_empty_index._build_image') @@ -57,7 +33,7 @@ def test_handle_create_empty_index_request( mock_bi, mock_alti, mock_oir, - mock_gpo, + mock_glp, mock_srs, mock_uiibs, mock_prfb, @@ -79,7 +55,7 @@ def test_handle_create_empty_index_request( 'distribution_scope': 'prod', } - mock_gpo.return_value = ["operator1", "operator2"] + mock_glp.return_value = ["operator1", "operator2"] output_pull_spec = 'quay.io/namespace/some-image:3' mock_capml.return_value = output_pull_spec @@ -160,7 +136,7 @@ def test_handle_create_empty_index_request_raises(mock_fiv, mock_prfb, mock_iifb @mock.patch('iib.workers.tasks.build_create_empty_index.prepare_request_for_build') @mock.patch('iib.workers.tasks.build_create_empty_index._update_index_image_build_state') @mock.patch('iib.workers.tasks.build_create_empty_index.set_request_state') -@mock.patch('iib.workers.tasks.build_create_empty_index._get_present_operators') +@mock.patch('iib.workers.tasks.build_create_empty_index.get_operator_package_list') @mock.patch('iib.workers.tasks.build_create_empty_index._add_label_to_index') @mock.patch('iib.workers.tasks.build_create_empty_index._build_image') @mock.patch('iib.workers.tasks.build_create_empty_index._push_image') @@ -176,7 +152,7 @@ def test_handle_create_empty_index_request_fbc( mock_pi, mock_bi, mock_alti, - mock_gpo, + mock_glp, mock_srs, mock_uiibs, mock_prfb, @@ -196,7 +172,7 @@ def test_handle_create_empty_index_request_fbc( 'distribution_scope': 'prod', } - mock_gpo.return_value = ["operator1", "operator2"] + mock_glp.return_value = ["operator1", "operator2"] output_pull_spec = 'quay.io/namespace/some-image:3' mock_capml.return_value = output_pull_spec @@ -224,7 +200,7 @@ def test_handle_create_empty_index_request_fbc( mock_ocef.call_args_list[0][1]['from_index'] = from_index mock_ocef.call_args_list[0][1]['from_index_resolved'] = from_index_resolved mock_ocef.call_args_list[0][1]['binary_image'] = binary_image - mock_ocef.call_args_list[0][1]['operators'] = mock_gpo.return_value + mock_ocef.call_args_list[0][1]['operators'] = mock_glp.return_value mock_uiibs.asser_called_once() assert mock_srs.call_count == 4 @@ -254,7 +230,7 @@ def test_handle_create_empty_index_request_fbc( @mock.patch('iib.workers.tasks.build_create_empty_index.prepare_request_for_build') @mock.patch('iib.workers.tasks.build_create_empty_index._update_index_image_build_state') @mock.patch('iib.workers.tasks.build_create_empty_index.set_request_state') -@mock.patch('iib.workers.tasks.build_create_empty_index._get_present_operators') +@mock.patch('iib.workers.tasks.build_create_empty_index.get_operator_package_list') @mock.patch('iib.workers.tasks.build_create_empty_index._add_label_to_index') @mock.patch('iib.workers.tasks.build_create_empty_index._build_image') @mock.patch('iib.workers.tasks.build_create_empty_index._push_image') @@ -274,7 +250,7 @@ def test_handle_create_empty_index_request_opm_ver( mock_pi, mock_bi, mock_alti, - mock_gpo, + mock_glp, mock_srs, mock_uiibs, mock_prfb, @@ -297,7 +273,7 @@ def test_handle_create_empty_index_request_opm_ver( 'distribution_scope': 'prod', } - mock_gpo.return_value = ["operator1", "operator2"] + mock_glp.return_value = ["operator1", "operator2"] output_pull_spec = 'quay.io/namespace/some-image:3' mock_capml.return_value = output_pull_spec diff --git a/tests/test_workers/test_tasks/test_build_merge_index_image.py b/tests/test_workers/test_tasks/test_build_merge_index_image.py index f3b71588..a63e287c 100644 --- a/tests/test_workers/test_tasks/test_build_merge_index_image.py +++ b/tests/test_workers/test_tasks/test_build_merge_index_image.py @@ -1,4 +1,5 @@ # SPDX-License-Identifier: GPL-3.0-or-later +import json import os import stat from unittest import mock @@ -18,8 +19,8 @@ (None, None, None), ), ) +@mock.patch('iib.workers.tasks.opm_operations._get_input_data_path') @mock.patch('iib.workers.tasks.utils.run_cmd') -@mock.patch('iib.workers.tasks.utils.opm_registry_serve') @mock.patch('iib.workers.tasks.build_merge_index_image._update_index_image_pull_spec') @mock.patch('iib.workers.tasks.build._verify_index_image') @mock.patch('iib.workers.tasks.build_merge_index_image.create_dockerfile') @@ -70,8 +71,8 @@ def test_handle_merge_request( mock_ogd, mock_vii, mock_uiips, - mock_ors, mock_run_cmd, + mock_gidp, target_index, target_index_resolved, binary_image, @@ -118,11 +119,18 @@ def side_effect(*args, base_dir, **kwargs): mock_dep_b.side_effect = side_effect mock_dep_b_fbc.side_effect = side_effect - port = 0 - my_mock = mock.MagicMock() - mock_ors.return_value = (port, my_mock) - mock_run_cmd.return_value = '{"packageName": "package1", "version": "v1.0", \ - "bundlePath": "bundle1"\n}' + mock_gidp.return_value = '/tmp' + mock_run_cmd.return_value = json.dumps( + { + "schema": "olm.bundle", + "image": "bundle1", + "name": "name1", + "package": "package1", + "version": "v1.0", + "properties": [{"type": "olm.package", "value": {"version": "0.1.0"}}], + } + ) + build_merge_index_image.handle_merge_request( 'source-from-index:1.0', ['some-bundle:1.0'], @@ -170,22 +178,13 @@ def side_effect(*args, base_dir, **kwargs): mock_uiips.assert_called_once() mock_sov.assert_called_once_with(target_index_resolved) - mock_ors.assert_called_once() mock_run_cmd.assert_called_once() - mock_run_cmd.assert_has_calls( - [ - mock.call( - ['grpcurl', '-plaintext', f'localhost:{port}', 'api.Registry/ListBundles'], - exc_msg=mock.ANY, - ), - ] - ) @pytest.mark.parametrize('source_fbc, target_fbc', [(False, False), (False, True), (True, True)]) @pytest.mark.parametrize('invalid_bundles', ([], [{'bundlePath': 'invalid_bundle:1.0'}])) +@mock.patch('iib.workers.tasks.opm_operations._get_input_data_path') @mock.patch('iib.workers.tasks.utils.run_cmd') -@mock.patch('iib.workers.tasks.utils.opm_serve_from_index') @mock.patch('iib.workers.tasks.build_merge_index_image._update_index_image_pull_spec') @mock.patch('iib.workers.tasks.build._verify_index_image') @mock.patch('iib.workers.tasks.build_merge_index_image.create_dockerfile') @@ -234,8 +233,8 @@ def test_handle_merge_request_no_deprecate( mock_ogd, mock_vii, mock_uiips, - mock_osfi, mock_run_cmd, + mock_gidp, invalid_bundles, source_fbc, target_fbc, @@ -263,11 +262,16 @@ def test_handle_merge_request_no_deprecate( mock_gid.return_value = 'database/index.db' mock_om.return_value = 'catalog', 'cache' - port = 0 - my_mock = mock.MagicMock() - mock_osfi.return_value = (port, my_mock) - mock_run_cmd.return_value = '{"packageName": "package1", "version": "v1.0", \ - "bundlePath": "bundle1"\n}' + mock_run_cmd.return_value = json.dumps( + { + "schema": "olm.bundle", + "image": "bundle1", + "name": "name1", + "package": "package1", + "version": "v1.0", + "properties": [{"type": "olm.package", "value": {"version": "0.1.0"}}], + } + ) build_merge_index_image.handle_merge_request( 'source-from-index:1.0', @@ -329,8 +333,6 @@ def test_handle_merge_request_no_deprecate( mock_capml.assert_called_once_with(1, {'amd64', 'other_arch'}, None) mock_sov.assert_called_once_with(target_index_resolved) mock_uiips.assert_called_once() - - mock_osfi.assert_not_called() mock_run_cmd.assert_not_called() diff --git a/tests/test_workers/test_tasks/test_opm_operations.py b/tests/test_workers/test_tasks/test_opm_operations.py index 2b3d16c7..08648e72 100644 --- a/tests/test_workers/test_tasks/test_opm_operations.py +++ b/tests/test_workers/test_tasks/test_opm_operations.py @@ -281,116 +281,6 @@ def test_func(argument, opm_port, opm_pprof_port=None): assert mock_pfl_u.call_count == 0 -@mock.patch('iib.workers.tasks.opm_operations._serve_cmd_at_port') -@mock.patch( - 'iib.workers.tasks.opm_operations.get_opm_port_stacks', - return_value=( - [[5001], [5002]], - ['opm_port'], - ), -) -@mock.patch('iib.workers.tasks.opm_operations.PortFileLock.lock_acquire') -@mock.patch('iib.workers.tasks.opm_operations.PortFileLock.unlock') -def test_opm_registry_serve(mock_pfl_u, mock_pfl_la, mock_gops, mock_scap): - """Test opm_registry_serve and create_port_file_lock working correctly together.""" - port, _ = opm_operations.opm_registry_serve(db_path='some_path.db') - assert port == 5001 - assert mock_scap.call_count == 1 - assert mock_pfl_la.call_count == 1 - assert mock_pfl_u.call_count == 1 - - -@pytest.mark.parametrize('is_fbc', (True, False)) -@mock.patch('iib.workers.tasks.opm_operations.get_catalog_dir') -@mock.patch('iib.workers.tasks.build._get_index_database') -@mock.patch('iib.workers.tasks.opm_operations.is_image_fbc') -@mock.patch( - 'iib.workers.tasks.opm_operations._serve_cmd_at_port', - side_effect=[AddressAlreadyInUse(), AddressAlreadyInUse(), (5003, 456)], -) -@mock.patch('iib.workers.tasks.opm_operations.PortFileLock.lock_acquire') -@mock.patch('iib.workers.tasks.opm_operations.PortFileLock.unlock') -@mock.patch( - 'iib.workers.tasks.opm_operations.get_opm_port_stacks', -) -def test_opm_serve_from_index( - mock_gops, - mock_pfl_u, - mock_pfl_la, - mock_scap, - mock_ifbc, - mock_gid, - mock_cd, - tmpdir, - is_fbc, -): - """ - Test exception during opm command, together with opm_serve and opm_registry_serve. - - Also test create_port_filelocks working correctly with opm_serve_from_index. - """ - mock_gops.return_value = ( - [[5001], [5002], [5003]], - ['opm_port'], - ) - my_mock = mock.MagicMock() - mock_ifbc.return_value = is_fbc - mock_gid.return_value = "some.db" - mock_cd.return_value = "/some/path" - my_mock.poll.return_value = None - port, _ = opm_operations.opm_serve_from_index( - base_dir=tmpdir, from_index='docker://test_pull_spec:latest' - ) - assert port == 5003 - assert mock_scap.call_count == 3 - - -@mock.patch('time.time') -@mock.patch('time.sleep') -@mock.patch('subprocess.Popen') -@mock.patch('iib.workers.tasks.utils.run_cmd') -def test_serve_cmd_at_port_not_initialize( - mock_run_cmd, mock_popen, mock_sleep, mock_time, tmpdir, mock_config -): - mock_run_cmd.side_effect = ['', '', '', '', ''] * 4 - mock_time.side_effect = list(range(1, 80)) - my_mock = mock.MagicMock() - mock_popen.return_value = my_mock - my_mock.poll.return_value = None - - cmd = ['opm', 'registry', 'serve', '-p', '50051', '-d', '/tmp/dummy.db', '-t', '/dev/null'] - with pytest.raises(IIBError, match='Index registry has not been initialized after 5 tries'): - opm_operations._serve_cmd_at_port(" ".join(cmd), '/tmp', 50051, 5, 3) - assert mock_run_cmd.call_count == 20 - - -@mock.patch('time.time') -@mock.patch('time.sleep') -@mock.patch('subprocess.Popen') -@mock.patch('iib.workers.tasks.utils.run_cmd') -def test_serve_cmd_at_port_delayed_initialize( - mock_run_cmd, mock_popen, mock_sleep, mock_time, tmpdir, mock_config -): - mock_time.side_effect = [i * 0.5 for i in range(1, 80)] - mock_run_cmd.side_effect = [ - '', - '', - '', - '', - '', - '', - 'api.Registry.ListBundles', - ] - - my_mock = mock.MagicMock() - mock_popen.return_value = my_mock - my_mock.poll.return_value = None - - cmd = ['opm', 'registry', 'serve', '-p', '50051', '-d', '/tmp/dummy.db', '-t', '/dev/null'] - opm_operations._serve_cmd_at_port(" ".join(cmd), '/tmp', 50051, 5, 3) - assert mock_run_cmd.call_count == 7 - - @pytest.mark.parametrize( "opm_version,migrate_args", [ diff --git a/tests/test_workers/test_tasks/test_utils.py b/tests/test_workers/test_tasks/test_utils.py index 4c1f0210..a07e9756 100644 --- a/tests/test_workers/test_tasks/test_utils.py +++ b/tests/test_workers/test_tasks/test_utils.py @@ -1254,56 +1254,11 @@ def test_get_binary_image_config_no_config_val(): rc.binary_image(index_info, "prod") -@pytest.mark.parametrize( - 'endpoint', - ( - "api.Registry/ListPackages", - "api.Registry/ListBundles", - ), -) -@mock.patch('iib.workers.tasks.utils.run_cmd') -@mock.patch('iib.workers.tasks.utils.opm_serve_from_index') -@mock.patch('iib.workers.tasks.build._get_index_database') -def test_grpcurl_get_db_data_success(mock_gid, mock_osfi, mock_run_cmd, tmpdir, endpoint): - mock_gid.return_value = tmpdir.join('index.db') - mock_popen = mock.MagicMock() - mock_osfi.return_value = 50051, mock_popen - mock_run_cmd.side_effect = ['{\n"name": "package1"\n}\n{\n"name": "package2"\n}\n'] - utils.grpcurl_get_db_data('quay.io/index-image:4.5', str(tmpdir), endpoint) - - -@pytest.mark.parametrize( - 'endpoint, err_msg', - ( - ( - "api.Registry/GetPackages", - "The endpoint 'api.Registry/GetPackages' is not allowed to be used", - ), - ("something", "The endpoint 'something' is not allowed to be used"), - ), -) -@mock.patch('iib.workers.tasks.utils.run_cmd') -@mock.patch('iib.workers.tasks.utils.opm_serve_from_index') -def test_grpcurl_get_db_data_wrong_endpoint(mock_osfi, mock_run_cmd, tmpdir, endpoint, err_msg): - mock_popen = mock.MagicMock() - mock_osfi.return_value = 50051, mock_popen - - with pytest.raises(IIBError, match=err_msg): - utils.grpcurl_get_db_data('quay.io/index-image:4.5', str(tmpdir), endpoint) - - mock_osfi.assert_called_once() - mock_run_cmd.assert_not_called() - - -@mock.patch('iib.workers.tasks.utils.opm_registry_serve') @mock.patch('iib.workers.tasks.utils.get_bundle_json') -@mock.patch('iib.workers.tasks.utils.run_cmd') +@mock.patch('iib.workers.tasks.utils.get_list_bundles') @mock.patch('iib.workers.tasks.utils._add_property_to_index') -def test_add_max_ocp_version_property_empty_index(mock_apti, mock_cmd, mock_gbj, mock_ors, tmpdir): - port = 0 - my_mock = mock.MagicMock() - mock_ors.return_value = (port, my_mock) - mock_cmd.return_value = None +def test_add_max_ocp_version_property_empty_index(mock_apti, mock_glb, mock_gbj, tmpdir): + mock_glb.return_value = [] utils.add_max_ocp_version_property([], tmpdir) From 8562ac169a2dc362125d617b5cc6064dde87a306 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lipovsk=C3=BD?= Date: Wed, 15 Jan 2025 14:12:39 +0100 Subject: [PATCH 3/3] Remove unused functions [CLOUDDST-25207] --- iib/workers/tasks/opm_operations.py | 194 ---------------------------- 1 file changed, 194 deletions(-) diff --git a/iib/workers/tasks/opm_operations.py b/iib/workers/tasks/opm_operations.py index d1814360..afbc182f 100644 --- a/iib/workers/tasks/opm_operations.py +++ b/iib/workers/tasks/opm_operations.py @@ -7,10 +7,8 @@ import re import shutil import socket -import subprocess import tempfile import textwrap -import time from typing import Callable, List, Optional, Set, Tuple, Union from packaging.version import Version @@ -297,198 +295,6 @@ def inner(*args, **kwargs): return decorator -def opm_serve_from_index(base_dir: str, from_index: str) -> Tuple[int, subprocess.Popen]: - """ - Locally start OPM registry service, which can be communicated with using gRPC queries. - - Due to IIB's paralellism, the service can run multiple times, which could lead to port - binding conflicts. Resolution of port conflicts is handled in this function as well. - - :param str base_dir: base directory to create temporary files in. - :param str from_index: index image to inspect. - :return: tuple containing port number of the running service and the running Popen object. - :rtype: (int, Popen) - """ - from iib.workers.tasks.build import _get_index_database - - log.info('Serving data from image %s', from_index) - if not is_image_fbc(from_index): - db_path = _get_index_database(from_index, base_dir) - return opm_registry_serve(db_path=db_path) - - catalog_dir = get_catalog_dir(from_index, base_dir) - return opm_serve(catalog_dir=catalog_dir) - - -@create_port_filelocks(port_purposes=["opm_port", "opm_pprof_port"]) -def opm_serve( - opm_port: int, - catalog_dir: str, - opm_pprof_port: Optional[int] = None, -) -> Tuple[int, subprocess.Popen]: - """ - Locally start OPM service, which can be communicated with using gRPC queries. - - Due to IIB's paralellism, the service can run multiple times, which could lead to port - binding conflicts. Resolution of port conflicts is handled in this function as well. - - :param int opm_port: OPM port number obtained from create_port_filelock decorator - :param int opm_pprof_port: Pprof opm port number obtained from create_port_filelock decorator - :param str catalog_dir: path to file-based catalog directory that should be served. - :return: tuple containing port number of the running service and the running Popen object. - :rtype: (int, Popen) - """ - log.info('Serving data from file-based catalog %s', catalog_dir) - - cmd = [ - Opm.opm_version, - 'serve', - catalog_dir, - '-p', - str(opm_port), - '-t', - '/dev/null', - ] - - if opm_pprof_port: - # by default opm uses the 127.0.0.1:6060 - cmd.extend(["--pprof-addr", f"127.0.0.1:{str(opm_pprof_port)}"]) - - cwd = os.path.abspath(os.path.join(catalog_dir, os.path.pardir)) - result = ( - opm_port, - _serve_cmd_at_port_defaults(cmd, cwd, opm_port), - ) - return result - - -@create_port_filelocks(port_purposes=["opm_port"]) -def opm_registry_serve( - opm_port: int, - db_path: str, -) -> Tuple[int, subprocess.Popen]: - """ - Locally start OPM registry service, which can be communicated with using gRPC queries. - - Due to IIB's paralellism, the service can run multiple times, which could lead to port - binding conflicts. Resolution of port conflicts is handled in this function as well. - - :param int opm_port: OPM port number obtained from create_port_filelock decorator - :param int opm_pprof_port: Pprof opm port number obtained from create_port_filelock decorator - :param str db_path: path to index database containing the registry data. - :return: tuple containing port number of the running service and the running Popen object. - :rtype: (int, Popen) - """ - log.info('Serving data from index.db %s', db_path) - - cmd = [ - Opm.opm_version, - 'registry', - 'serve', - '-p', - str(opm_port), - '-d', - db_path, - '-t', - '/dev/null', - ] - - cwd = os.path.dirname(db_path) - result = ( - opm_port, - _serve_cmd_at_port_defaults(cmd, cwd, opm_port), - ) - return result - - -def _serve_cmd_at_port_defaults(serve_cmd: List[str], cwd: str, port: int) -> subprocess.Popen: - """ - Call `_serve_cmd_at_port()` with default values from IIB config. - - :param list serve_cmd: opm command to be run (serve FBC or index.db). - :param str cwd: path to folder which should be set as current working directory. - :param str int port: port to start the service on. - """ - log.debug('Run _serve_cmd_at_port with default loaded from IIB config.') - conf = get_worker_config() - return _serve_cmd_at_port( - serve_cmd, cwd, port, conf['iib_grpc_max_tries'], conf['iib_grpc_init_wait_time'] - ) - - -@retry( - before_sleep=before_sleep_log(log, logging.WARNING), - reraise=True, - retry=retry_if_exception_type(IIBError), - stop=stop_after_attempt(2), -) -def _serve_cmd_at_port( - serve_cmd: List[str], - cwd: str, - port: int, - max_tries: int, - wait_time: int, -) -> subprocess.Popen: - """ - Start an opm service at a specified port. - - :param list serve_cmd: opm command to be run (serve FBC or index.db). - :param str cwd: path to folder which should be set as current working directory. - :param str int port: port to start the service on. - :param max_tries: how many times to try to start the service before giving up. - :param wait_time: time to wait before checking if the service is initialized. - :return: object of the running Popen process. - :rtype: subprocess.Popen - :raises IIBError: if the process has failed to initialize too many times, or an unexpected - error occurred. - :raises AddressAlreadyInUse: if the specified port is already being used by another service. - """ - from iib.workers.tasks.utils import run_cmd, terminate_process - - log.debug('Run command %s with up to %d retries', ' '.join(serve_cmd), max_tries) - for _ in range(max_tries): - rpc_proc = subprocess.Popen( - serve_cmd, - cwd=cwd, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - universal_newlines=True, - ) - start_time = time.time() - while time.time() - start_time < wait_time: - time.sleep(5) - ret = rpc_proc.poll() - # process has terminated - if ret is not None: - if not rpc_proc.stderr: - raise IIBError( - f'Command "{" ".join(serve_cmd)}" has failed, stderr was not captured' - ) - stderr_message = rpc_proc.stderr.read() - if 'address already in use' in stderr_message: - raise AddressAlreadyInUse(f'Port {port} is already used by a different service') - raise IIBError( - f'Command "{" ".join(serve_cmd)}" has failed with error "{stderr_message}"' - ) - - # query the service to see if it has started - try: - output = run_cmd( - ['grpcurl', '-plaintext', f'localhost:{port}', 'list', 'api.Registry'] - ) - except IIBError: - output = '' - - if 'api.Registry.ListBundles' in output or 'api.Registry.ListPackages' in output: - log.debug('Started the command "%s"; pid: %d', ' '.join(serve_cmd), rpc_proc.pid) - log.info('Index registry service has been initialized.') - return rpc_proc - - terminate_process(rpc_proc) - - raise IIBError(f'Index registry has not been initialized after {max_tries} tries') - - def get_operator_package_list( input_image_or_path: str, base_dir: str,