diff --git a/iib/workers/tasks/build.py b/iib/workers/tasks/build.py index 4fb56e8c..a433083e 100644 --- a/iib/workers/tasks/build.py +++ b/iib/workers/tasks/build.py @@ -37,7 +37,7 @@ deprecate_bundles, Opm, verify_operators_exists, - opm_generate_dockerfile, + create_dockerfile, opm_validate, ) from iib.workers.tasks.utils import ( @@ -1119,7 +1119,7 @@ def handle_rm_request( # validate fbc config opm_validate(fbc_dir_path) - opm_generate_dockerfile( + create_dockerfile( fbc_dir=fbc_dir_path, base_dir=temp_dir, index_db=index_db_path, diff --git a/iib/workers/tasks/build_merge_index_image.py b/iib/workers/tasks/build_merge_index_image.py index 87c0aa52..6e0c2cf3 100644 --- a/iib/workers/tasks/build_merge_index_image.py +++ b/iib/workers/tasks/build_merge_index_image.py @@ -12,7 +12,7 @@ from iib.workers.tasks.opm_operations import ( opm_registry_add_fbc, opm_migrate, - opm_generate_dockerfile, + create_dockerfile, deprecate_bundles_fbc, opm_index_add, deprecate_bundles, @@ -368,7 +368,7 @@ def handle_merge_request( if os.path.isfile(dockerfile_path): log.info('Removing previously generated dockerfile.') os.remove(dockerfile_path) - opm_generate_dockerfile( + create_dockerfile( fbc_dir=fbc_dir, base_dir=temp_dir, index_db=index_db_file, diff --git a/iib/workers/tasks/opm_operations.py b/iib/workers/tasks/opm_operations.py index 688e9f88..76a63606 100644 --- a/iib/workers/tasks/opm_operations.py +++ b/iib/workers/tasks/opm_operations.py @@ -8,6 +8,7 @@ import socket import subprocess import tempfile +import textwrap import time from typing import Callable, List, Optional, Set, Tuple, Union from packaging.version import Version @@ -589,7 +590,7 @@ def deprecate_bundles_fbc( fbc_dir, _ = opm_migrate(index_db_file, base_dir) # we should keep generating Dockerfile here # to have the same behavior as we run `opm index deprecatetruncate` with '--generate' option - opm_generate_dockerfile( + create_dockerfile( fbc_dir=fbc_dir, base_dir=base_dir, index_db=index_db_file, @@ -637,7 +638,7 @@ def opm_migrate( return fbc_dir_path, None -def opm_generate_dockerfile( +def create_dockerfile( fbc_dir: str, base_dir: str, index_db: str, @@ -645,7 +646,7 @@ def opm_generate_dockerfile( dockerfile_name: Optional[str] = None, ) -> str: """ - Generate Dockerfile using opm command and adding index.db to hidden location. + Create Dockerfile and adding index.db to hidden location. :param str fbc_dir: directory containing file-based catalog (JSON or YAML files). :param str base_dir: base directory where Dockerfile should be created. @@ -656,8 +657,6 @@ def opm_generate_dockerfile( :raises: IIBError when Dockerfile was not generated :rtype: str """ - from iib.workers.tasks.utils import run_cmd - # we do not want to continue if Dockerfile already exists dockerfile_name_opm_default = f"{os.path.basename(fbc_dir)}.Dockerfile" tmp_dockerfile_name = dockerfile_name or dockerfile_name_opm_default @@ -671,24 +670,28 @@ def opm_generate_dockerfile( ) return dockerfile_path - cmd = [ - Opm.opm_version, - 'generate', - 'dockerfile', - os.path.abspath(fbc_dir), - '--binary-image', - binary_image, - ] - - log.info('Generating Dockerfile with binary image %s' % binary_image) - run_cmd(cmd, {'cwd': base_dir}, exc_msg='Failed to generate Dockerfile for file-based catalog') - - # check if opm command generated Dockerfile successfully + log.info('Creating Dockerfile with binary image %s' % binary_image) dockerfile_path_opm_default = os.path.join(base_dir, dockerfile_name_opm_default) - if not os.path.isfile(dockerfile_path_opm_default): - error_msg = f"Cannot find generated Dockerfile at {dockerfile_path_opm_default}" - log.error(error_msg) - raise IIBError(error_msg) + with open(dockerfile_path_opm_default, 'w') as dockerfile: + dockerfile.write( + textwrap.dedent( + f"""\ + FROM {binary_image} + + # Configure the entrypoint and command + ENTRYPOINT ["/bin/opm"] + CMD ["serve", "/configs", "--cache-dir=/tmp/cache"] + + # Copy declarative config root and cache into image + ADD catalog /configs + COPY --chown=1001:0 cache /tmp/cache + + # Set DC-specific label for the location of the DC root directory + # in the image + LABEL operators.operatorframework.io.index.configs.v1=/configs + """ + ) + ) # we should rename Dockerfile generated by opm if `dockerfile_name` parameter is set if dockerfile_name: @@ -696,8 +699,6 @@ def opm_generate_dockerfile( log.info('Rewriting Dockerfile %s with newly generated by opm.', dockerfile_path) os.rename(dockerfile_path_opm_default, dockerfile_path) - insert_cache_into_dockerfile(dockerfile_path) - db_path = get_worker_config()['hidden_index_db_path'] rel_path_index_db = os.path.relpath(index_db, base_dir) with open(dockerfile_path, 'a') as f: @@ -707,44 +708,6 @@ def opm_generate_dockerfile( return dockerfile_path -def verify_cache_insertion_edit_dockerfile(file_list: list) -> None: - """ - Verify Dockerfile edit to insert local cache was successful. - - :param list file_list: Generated dockerfile as a list. - :raises: IIBError when the Dockerfile edit is unsuccessful. - """ - copied_cache_found = False - match_str = 'COPY --chown=1001:0 cache /tmp/cache' - for line in file_list: - if match_str in line: - copied_cache_found = True - break - if not copied_cache_found: - raise IIBError('Dockerfile edit to insert locally built cache failed.') - - -def insert_cache_into_dockerfile(dockerfile_path: str) -> None: - """ - Insert built cache into the Dockerfile. - - :param str dockerfile_path: path to the generated Dockerfile. - """ - with open(dockerfile_path, 'r') as f: - file_data = f.read() - - file_data = file_data.replace( - 'RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]', - 'COPY --chown=1001:0 cache /tmp/cache', - ) - - with open(dockerfile_path, 'w') as f: - f.write(file_data) - - with open(dockerfile_path, 'r') as f: - verify_cache_insertion_edit_dockerfile(f.readlines()) - - @create_port_filelocks(port_purposes=["opm_pprof_port"]) def generate_cache_locally( base_dir: str, @@ -923,7 +886,7 @@ def opm_registry_add_fbc( fbc_dir, _ = opm_migrate(index_db=index_db_file, base_dir=base_dir) # we should keep generating Dockerfile here # to have the same behavior as we run `opm index add` with '--generate' option - opm_generate_dockerfile( + create_dockerfile( fbc_dir=fbc_dir, base_dir=base_dir, index_db=index_db_file, @@ -1055,7 +1018,7 @@ def opm_create_empty_fbc( # Migrate the index to FBC fbc_dir, _ = opm_migrate(index_db=index_db_path, base_dir=temp_dir) - opm_generate_dockerfile( + create_dockerfile( fbc_dir=fbc_dir, base_dir=temp_dir, index_db=index_db_path, @@ -1145,7 +1108,7 @@ def opm_registry_add_fbc_fragment( ) log.info("Dockerfile generated from %s", from_index_configs_dir) - opm_generate_dockerfile( + create_dockerfile( fbc_dir=from_index_configs_dir, base_dir=temp_dir, index_db=index_db_path, diff --git a/tests/test_workers/test_tasks/test_build.py b/tests/test_workers/test_tasks/test_build.py index 40c8b8f2..b0b62648 100644 --- a/tests/test_workers/test_tasks/test_build.py +++ b/tests/test_workers/test_tasks/test_build.py @@ -1064,7 +1064,7 @@ def test_handle_rm_request( @mock.patch('iib.workers.tasks.build.get_catalog_dir') @mock.patch('iib.workers.tasks.build.merge_catalogs_dirs') @mock.patch('iib.workers.tasks.build.generate_cache_locally') -@mock.patch('iib.workers.tasks.build.opm_generate_dockerfile') +@mock.patch('iib.workers.tasks.build.create_dockerfile') @mock.patch('os.rename') @mock.patch('iib.workers.tasks.opm_operations.Opm.set_opm_version') @mock.patch('iib.workers.tasks.opm_operations.Opm.get_opm_version_number') 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 321f34d4..426cbc57 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 @@ -22,7 +22,7 @@ @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.opm_generate_dockerfile') +@mock.patch('iib.workers.tasks.build_merge_index_image.create_dockerfile') @mock.patch('iib.workers.tasks.build._get_index_database') @mock.patch('iib.workers.tasks.build_merge_index_image.opm_migrate') @mock.patch('iib.workers.tasks.build_merge_index_image.opm_registry_add_fbc') @@ -190,7 +190,7 @@ def side_effect(*args, base_dir, **kwargs): @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.opm_generate_dockerfile') +@mock.patch('iib.workers.tasks.build_merge_index_image.create_dockerfile') @mock.patch('iib.workers.tasks.build._get_index_database') @mock.patch('iib.workers.tasks.build_merge_index_image.opm_migrate') @mock.patch('iib.workers.tasks.build_merge_index_image.opm_registry_add_fbc') diff --git a/tests/test_workers/test_tasks/test_opm_operations.py b/tests/test_workers/test_tasks/test_opm_operations.py index 0d3f6b44..e35dcbe8 100644 --- a/tests/test_workers/test_tasks/test_opm_operations.py +++ b/tests/test_workers/test_tasks/test_opm_operations.py @@ -416,64 +416,45 @@ def test_opm_migrate( @pytest.mark.parametrize("dockerfile", (None, 'index.Dockerfile')) -@mock.patch('iib.workers.tasks.utils.run_cmd') -@mock.patch('iib.workers.tasks.opm_operations.insert_cache_into_dockerfile') -def test_opm_generate_dockerfile(mock_icid, mock_run_cmd, tmpdir, dockerfile): +def test_create_dockerfile(tmpdir, dockerfile): index_db_file = os.path.join(tmpdir, 'database/index.db') fbc_dir = os.path.join(tmpdir, 'catalogs') - def create_dockerfile(*args, **kwargs): - with open(os.path.join(tmpdir, 'catalogs.Dockerfile'), 'a'): - pass - - mock_run_cmd.side_effect = create_dockerfile - - opm_operations.opm_generate_dockerfile( + opm_operations.create_dockerfile( fbc_dir, tmpdir, index_db_file, "some:image", dockerfile_name=dockerfile ) df_name = dockerfile if dockerfile else f"{os.path.basename(fbc_dir)}.Dockerfile" - - mock_run_cmd.assert_called_once_with( - ['opm', 'generate', 'dockerfile', fbc_dir, '--binary-image', 'some:image'], - {'cwd': tmpdir}, - exc_msg='Failed to generate Dockerfile for file-based catalog', - ) - df_path = os.path.join(tmpdir, df_name) with open(df_path, 'r') as f: - assert any(line.find('/var/lib/iib/_hidden/do.not.edit.db') != -1 for line in f.readlines()) + dockerfile = f.read() - mock_icid.assert_called_once_with(df_path) + expected_dockerfile = textwrap.dedent( + '''\ + FROM some:image + # Configure the entrypoint and command + ENTRYPOINT ["/bin/opm"] + CMD ["serve", "/configs", "--cache-dir=/tmp/cache"] -@pytest.mark.parametrize("set_index_db_file", (False, True)) -@mock.patch.object(opm_operations.Opm, 'opm_version', 'opm-v1.26.8') -@mock.patch('iib.workers.tasks.utils.run_cmd') -def test_opm_generate_dockerfile_no_dockerfile(mock_run_cmd, tmpdir, set_index_db_file): - index_db_file = os.path.join(tmpdir, 'database/index.db') if set_index_db_file else None - fbc_dir = os.path.join(tmpdir, 'catalogs') - df_path = os.path.join(tmpdir, f"{os.path.basename(fbc_dir)}.Dockerfile") - - with pytest.raises(IIBError, match=f"Cannot find generated Dockerfile at {df_path}"): - opm_operations.opm_generate_dockerfile( - fbc_dir, - tmpdir, - index_db_file, - "some:image", - ) + # Copy declarative config root and cache into image + ADD catalog /configs + COPY --chown=1001:0 cache /tmp/cache - mock_run_cmd.assert_called_once_with( - ['opm-v1.26.8', 'generate', 'dockerfile', fbc_dir, '--binary-image', 'some:image'], - {'cwd': tmpdir}, - exc_msg='Failed to generate Dockerfile for file-based catalog', + # Set DC-specific label for the location of the DC root directory + # in the image + LABEL operators.operatorframework.io.index.configs.v1=/configs + + ADD database/index.db /var/lib/iib/_hidden/do.not.edit.db + ''' ) + assert dockerfile == expected_dockerfile @pytest.mark.parametrize("set_index_db_file", (False, True)) @pytest.mark.parametrize("dockerfile", (None, 'index.Dockerfile')) @mock.patch('iib.workers.tasks.utils.run_cmd') -def test_opm_generate_dockerfile_exist(mock_run_cmd, tmpdir, dockerfile, set_index_db_file): +def test_create_dockerfile_exist(mock_run_cmd, tmpdir, dockerfile, set_index_db_file): index_db_file = os.path.join(tmpdir, 'database/index.db') if set_index_db_file else None fbc_dir = os.path.join(tmpdir, 'catalogs') df_name = f"{os.path.basename(fbc_dir)}.Dockerfile" if not dockerfile else dockerfile @@ -483,7 +464,7 @@ def test_opm_generate_dockerfile_exist(mock_run_cmd, tmpdir, dockerfile, set_ind with open(df_path, 'a'): pass - opm_operations.opm_generate_dockerfile( + opm_operations.create_dockerfile( fbc_dir, tmpdir, index_db_file, "some:image", dockerfile_name=dockerfile ) @@ -541,7 +522,7 @@ def test_opm_registry_add( @pytest.mark.parametrize('overwrite_csv', (True, False)) @pytest.mark.parametrize('container_tool', (None, 'podwoman')) @pytest.mark.parametrize('graph_update_mode', (None, 'semver-skippatch')) -@mock.patch('iib.workers.tasks.opm_operations.opm_generate_dockerfile') +@mock.patch('iib.workers.tasks.opm_operations.create_dockerfile') @mock.patch('iib.workers.tasks.opm_operations.opm_migrate') @mock.patch('iib.workers.tasks.opm_operations._opm_registry_add') @mock.patch('iib.workers.tasks.build._get_index_database') @@ -600,7 +581,7 @@ def test_opm_registry_add_fbc( @pytest.mark.parametrize('operators', (['abc-operator', 'xyz-operator'], [])) -@mock.patch('iib.workers.tasks.opm_operations.opm_generate_dockerfile') +@mock.patch('iib.workers.tasks.opm_operations.create_dockerfile') @mock.patch('iib.workers.tasks.opm_operations.opm_migrate') @mock.patch('iib.workers.tasks.opm_operations._opm_registry_rm') @mock.patch('iib.workers.tasks.opm_operations.get_hidden_index_database') @@ -647,7 +628,7 @@ def test_opm_registry_rm(mock_run_cmd): @pytest.mark.parametrize( 'from_index, is_fbc', [('some-fbc-index:latest', True), ('some-sqlite-index:latest', False)] ) -@mock.patch('iib.workers.tasks.opm_operations.opm_generate_dockerfile') +@mock.patch('iib.workers.tasks.opm_operations.create_dockerfile') @mock.patch('iib.workers.tasks.opm_operations.opm_migrate') @mock.patch('iib.workers.tasks.opm_operations._opm_registry_rm') @mock.patch('iib.workers.tasks.opm_operations.get_hidden_index_database') @@ -735,7 +716,7 @@ def test_opm_registry_deprecatetruncate(mock_run_cmd, bundles): @pytest.mark.parametrize('from_index', (None, 'some_index:latest')) @pytest.mark.parametrize('bundles', (['bundle:1.2', 'bundle:1.3'], [])) -@mock.patch('iib.workers.tasks.opm_operations.opm_generate_dockerfile') +@mock.patch('iib.workers.tasks.opm_operations.create_dockerfile') @mock.patch('iib.workers.tasks.opm_operations.opm_migrate') @mock.patch('iib.workers.tasks.opm_operations.opm_registry_deprecatetruncate') @mock.patch('iib.workers.tasks.opm_operations._get_or_create_temp_index_db_file') @@ -848,67 +829,11 @@ def test_generate_cache_locally_failed( ) -def test_insert_cache_into_dockerfile(tmpdir): - local_cache_dir = tmpdir.mkdir('cache') - generated_dockerfile = local_cache_dir.join('catalog.Dockerfile') - - dockerfile_template = textwrap.dedent( - """\ - ADD /configs - RUN something-else - {run_command} - COPY . . - """ - ) - - generated_dockerfile.write( - dockerfile_template.format( - run_command=( - 'RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]' - ) - ) - ) - - opm_operations.insert_cache_into_dockerfile(generated_dockerfile) - - assert generated_dockerfile.read_text('utf-8') == dockerfile_template.format( - run_command='COPY --chown=1001:0 cache /tmp/cache' - ) - - -def test_insert_cache_into_dockerfile_no_matching_line(tmpdir): - local_cache_dir = tmpdir.mkdir('cache') - generated_dockerfile = local_cache_dir.join('catalog.Dockerfile') - - dockerfile_template = textwrap.dedent( - """\ - ADD /configs - RUN something-else - COPY . . - """ - ) - - generated_dockerfile.write(dockerfile_template) - with pytest.raises(IIBError, match='Dockerfile edit to insert locally built cache failed.'): - opm_operations.insert_cache_into_dockerfile(generated_dockerfile) - - -def test_verify_cache_insertion_edit_dockerfile(): - input_list = ['ADD /configs', 'COPY . .' 'COPY --chown=1001:0 cache /tmp/cache'] - opm_operations.verify_cache_insertion_edit_dockerfile(input_list) - - -def test_verify_cache_insertion_edit_dockerfile_failed(): - input_list = ['ADD /configs', 'COPY . .' 'RUN something'] - with pytest.raises(IIBError, match='Dockerfile edit to insert locally built cache failed.'): - opm_operations.verify_cache_insertion_edit_dockerfile(input_list) - - @pytest.mark.parametrize( 'operators_exists, index_db_path', [(['test-operator'], "index_path"), ([], "index_path")], ) -@mock.patch('iib.workers.tasks.opm_operations.opm_generate_dockerfile') +@mock.patch('iib.workers.tasks.opm_operations.create_dockerfile') @mock.patch('iib.workers.tasks.opm_operations.generate_cache_locally') @mock.patch('iib.workers.tasks.opm_operations.shutil.rmtree') @mock.patch('iib.workers.tasks.opm_operations.shutil.copytree')