Retry buildah if incorrect arch created (#529)

Co-authored-by: Tulsi Chandwani <tchandwa@tchandwa.users.ipa.redhat.com>

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**

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',

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:

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}'

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(