Skip to content

Commit

Permalink
Retry buildah if incorrect arch created
Browse files Browse the repository at this point in the history
  • Loading branch information
Tulsi Chandwani committed Jun 30, 2023
1 parent 0937a41 commit 4282c27
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 7 deletions.
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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**
Expand Down
6 changes: 6 additions & 0 deletions iib/workers/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
13 changes: 12 additions & 1 deletion iib/workers/tasks/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion iib/workers/tasks/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}'
Expand Down
39 changes: 34 additions & 5 deletions tests/test_workers/test_tasks/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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(
Expand Down

0 comments on commit 4282c27

Please sign in to comment.