Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry buildah if incorrect arch created #529

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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