From 4282c27a85892d78d2db6ccdb78ea8686faf9a39 Mon Sep 17 00:00:00 2001 From: Tulsi Chandwani Date: Tue, 27 Jun 2023 16:03:25 -0500 Subject: [PATCH] Retry buildah if incorrect arch created --- README.md | 3 ++ iib/workers/config.py | 6 ++++ iib/workers/tasks/build.py | 13 ++++++- iib/workers/tasks/utils.py | 2 +- tests/test_workers/test_tasks/test_build.py | 39 ++++++++++++++++++--- 5 files changed, 56 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 3b7ec421..0f47d444 100644 --- a/README.md +++ b/README.md @@ -420,6 +420,9 @@ The custom configuration options for the Celery workers are listed below: * `iib_retry_jitter` - the extra seconds to be added on delay between retry attempts. It's just used for buildah when receiving HTTP 50X errors. This defaults to `5`. * `iib_retry_multiplier` - the constant in the `2^x * multiplier` formula, where x stands for attempt number. Formula is used to calculate the seconds to be added on delay between retry attempts. It's just used for buildah when receiving HTTP 50X errors. This defaults to `5`. +* `iib_supported_archs` - the architectures supported by IIB. IIB can build index images for these + architectures. The dictionary has mapping of arch aliases with formal names like + `{"arm64": "aarch64"}` If you wish to configure AWS S3 bucket for storing artifact files, the following **environment variables** diff --git a/iib/workers/config.py b/iib/workers/config.py index 703401ea..8f3d8cd2 100644 --- a/iib/workers/config.py +++ b/iib/workers/config.py @@ -61,6 +61,12 @@ class Config(object): iib_retry_delay: int = 10 iib_retry_jitter: int = 10 iib_retry_multiplier: int = 5 + iib_supported_archs: dict = { + "amd64": "x86_64", + "arm64": "aarch64", + "s390x": "s390x", + "ppc64le": "ppc64le", + } include: List[str] = [ 'iib.workers.tasks.build', 'iib.workers.tasks.build_merge_index_image', diff --git a/iib/workers/tasks/build.py b/iib/workers/tasks/build.py index 8bcd5522..2fa6f11c 100644 --- a/iib/workers/tasks/build.py +++ b/iib/workers/tasks/build.py @@ -85,7 +85,8 @@ def _build_image(dockerfile_dir: str, dockerfile_name: str, request_id: int, arc :param str arch: the architecture to build this image for :raises IIBError: if the build fails """ - destination: str = _get_local_pull_spec(request_id, arch) + local_destination: str = _get_local_pull_spec(request_id, arch, include_transport=True) + destination: str = local_destination.split('/')[1] log.info( 'Building the container image with the %s dockerfile for arch %s and tagging it as %s', dockerfile_name, @@ -117,6 +118,16 @@ def _build_image(dockerfile_dir: str, dockerfile_name: str, request_id: int, arc {'cwd': dockerfile_dir}, exc_msg=f'Failed to build the container image on the arch {arch}', ) + log.debug('Verifying that %s was built with expected arch %s', destination, arch) + archmap = get_worker_config()['iib_supported_archs'] + destination_arch = get_image_label(local_destination, 'architecture') + + if destination_arch not in archmap.values() or destination_arch != archmap.get(arch): + log.warning("Wrong arch created for %s", destination) + raise ExternalServiceError( + f'Wrong arch created, for image {destination} ' + f'expected arch {archmap.get(arch)}, found {destination_arch}' + ) def _cleanup() -> None: diff --git a/iib/workers/tasks/utils.py b/iib/workers/tasks/utils.py index fbd6bb6e..b5bed30e 100644 --- a/iib/workers/tasks/utils.py +++ b/iib/workers/tasks/utils.py @@ -548,7 +548,7 @@ def get_image_labels(pull_spec: str) -> Dict[str, str]: :return: the dictionary of the labels on the image :rtype: dict """ - if pull_spec.startswith('docker://'): + if pull_spec.startswith('docker://') or pull_spec.startswith('containers-storage'): full_pull_spec = pull_spec else: full_pull_spec = f'docker://{pull_spec}' diff --git a/tests/test_workers/test_tasks/test_build.py b/tests/test_workers/test_tasks/test_build.py index 71d0de18..6e0e86d9 100644 --- a/tests/test_workers/test_tasks/test_build.py +++ b/tests/test_workers/test_tasks/test_build.py @@ -16,12 +16,17 @@ worker_config = get_worker_config() -@pytest.mark.parametrize('arch', ('amd64', 'ppc64le', 's390x', 'spam')) +@pytest.mark.parametrize('arch', ('amd64', 'ppc64le', 's390x', 'arm64')) +@mock.patch('iib.workers.tasks.build.get_image_label') @mock.patch('iib.workers.tasks.build.run_cmd') -def test_build_image(mock_run_cmd, arch): +def test_build_image(mock_run_cmd, mock_get_label, arch): + mock_run_cmd.return_value = None + mock_get_label.return_value = worker_config['iib_supported_archs'][arch] build._build_image('/some/dir', 'some.Dockerfile', 3, arch) + destination = f'iib-build:3-{arch}' + local_destination = f'containers-storage:localhost/{destination}' - mock_run_cmd.assert_called_once_with( + mock_run_cmd.assert_called_with( [ 'buildah', 'bud', @@ -33,13 +38,37 @@ def test_build_image(mock_run_cmd, arch): '--arch', arch, '-t', - f'iib-build:3-{arch}', + destination, '-f', '/some/dir/some.Dockerfile', ], {'cwd': '/some/dir'}, - exc_msg=mock.ANY, + exc_msg=f"Failed to build the container image on the arch {arch}", ) + mock_get_label.assert_called_with(local_destination, 'architecture') + + +@mock.patch('iib.workers.tasks.build.get_image_label') +@mock.patch('iib.workers.tasks.build.run_cmd') +def test_build_image_incorrect_arch(mock_run_cmd, mock_get_label): + mock_get_label.side_effect = ['x86_64', 's390x'] + mock_run_cmd.retuen_value = None + build._build_image('/some/dir', 'some.Dockerfile', 3, 's390x') + # build_image retried once, hence buildah bud commands ran twice + assert mock_run_cmd.call_count == 2 + assert mock_get_label.call_count == 2 + + +@mock.patch('iib.workers.tasks.build.get_image_label') +@mock.patch('iib.workers.tasks.build.run_cmd') +def test_build_image_incorrect_arch_failure(mock_run_cmd, mock_get_label): + mock_get_label.side_effect = ['x86_64', 'x86_64', 'x86_64', 'x86_64', 'x86_64'] + mock_run_cmd.return_value = None + with pytest.raises(ExternalServiceError): + build._build_image('/some/dir', 'some.Dockerfile', 3, "s390x") + # build_image retried multiple times as the incorrect arch was created for image always + assert mock_run_cmd.call_count == 5 + assert mock_get_label.call_count == 5 @mock.patch(