Handling of merge-index-image for containerized IIB

iib/web/api_v1.py

@@ -56,10 +56,10 @@
     handle_recursive_related_bundles_request,
 )
 from iib.workers.tasks.build_regenerate_bundle import handle_regenerate_bundle_request
-from iib.workers.tasks.build_merge_index_image import handle_merge_request
 from iib.workers.tasks.build_containerized_create_empty_index import (
     handle_containerized_create_empty_index_request,
 )
+from iib.workers.tasks.build_containerized_merge import handle_containerized_merge_request
 from iib.workers.tasks.general import failed_request_callback
 from iib.web.iib_static_types import (
     AddDeprecationRequestPayload,
@@ -1133,7 +1133,7 @@ def merge_index_image() -> Tuple[flask.Response, int]:
 
     error_callback = failed_request_callback.s(request.id)
     try:
-        handle_merge_request.apply_async(
+        handle_containerized_merge_request.apply_async(
             args=args, link_error=error_callback, argsrepr=repr(safe_args), queue=celery_queue
         )
     except kombu.exceptions.OperationalError:

iib/workers/config.py

@@ -97,6 +97,7 @@ class Config(object):
         'iib.workers.tasks.build_containerized_fbc_operations',
         'iib.workers.tasks.build_containerized_rm',
         'iib.workers.tasks.build_containerized_create_empty_index',
+        'iib.workers.tasks.build_containerized_merge',
         'iib.workers.tasks.general',
     ]
     # Path to hidden location of SQLite database

iib/workers/tasks/build_merge_index_image.py

@@ -86,53 +86,36 @@ def _filter_out_pure_fbc_bundles(
     return res_bundles, res_pullspec
 
 
-def _add_bundles_missing_in_source(
+def get_missing_bundles_from_target_to_source(
     source_index_bundles: List[BundleImage],
     target_index_bundles: List[BundleImage],
-    base_dir: str,
-    binary_image: str,
     source_from_index: str,
-    request_id: int,
-    arch: str,
     ocp_version: str,
-    distribution_scope: str,
-    graph_update_mode: Optional[str] = None,
     target_index=None,
-    overwrite_target_index_token: Optional[str] = None,
     ignore_bundle_ocp_version: Optional[bool] = False,
 ) -> Tuple[List[BundleImage], List[BundleImage]]:
     """
-    Rebuild index image with bundles missing from source image but present in target image.
+    Generate a list of missing bundles from the source but present in the target.
 
-    If no bundles are missing in the source index image, the index image is still rebuilt
-    using the new binary image.
+    This function will not build the index image, it will only generate a list of bundles missing
+    from the source index image but present in the target index image, as well as a list of bundles
+    in the new index whose ocp_version range does not satisfy the ocp_version value of the target
+    index.
 
     :param list source_index_bundles: bundles present in the source index image.
     :param list target_index_bundles: bundles present in the target index image.
-    :param str base_dir: base directory where operation files will be located.
-    :param str binary_image: binary image to be used by the new index image.
     :param str source_from_index: index image, whose data will be contained in the new index image.
-    :param int request_id: the ID of the IIB build request.
-    :param str arch: the architecture to build this image for.
     :param str ocp_version: ocp version which will be added as a label to the image.
-    :param str graph_update_mode: Graph update mode that defines how channel graphs are updated
-        in the index.
     :param str target_index: the pull specification of the container image
-    :param str overwrite_target_index_token: the token used for overwriting the input
-        ``source_from_index`` image. This is required to use ``overwrite_target_index``.
-        The format of the token must be in the format "user:password".
     :param bool ignore_bundle_ocp_version: When set to `true` and image set as target_index is
         listed in `iib_no_ocp_label_allow_list` config then bundles without
         "com.redhat.openshift.versions" label set will be added in the result `index_image`.
-    :return: tuple where the first value is a list of bundles which were added to the index image
-        and the second value is a list of bundles in the new index whose ocp_version range does not
-        satisfy the ocp_version value of the target index.
+    :return: tuple where the first value is a list of bundles missing in the source and are present
+        in the target index image and the second value is a list of bundles whose ocp_version
+        range does not satisfy the ocp_version value of the target index.
     :rtype: tuple
     """
-    set_request_state(request_id, 'in_progress', 'Adding bundles missing in source index image')
-    log.info('Adding bundles from target index image which are missing from source index image')
     missing_bundles = []
-    missing_bundle_paths = []
     # This list stores the bundles whose ocp_version range does not satisfy the ocp_version
     # of the target index
     invalid_bundles = []
@@ -162,7 +145,6 @@ def _add_bundles_missing_in_source(
             and bundle['csvName'] not in source_bundle_csv_names
         ):
             missing_bundles.append(bundle)
-            missing_bundle_paths.append(bundle['bundlePath'])
 
     if ignore_bundle_ocp_version:
         target_index_tmp = '' if target_index is None else target_index
@@ -187,6 +169,65 @@ def _add_bundles_missing_in_source(
             '%s bundles have invalid version label and will be deprecated.', len(invalid_bundles)
         )
 
+    return missing_bundles, invalid_bundles
+
+
+def _add_bundles_missing_in_source(
+    source_index_bundles: List[BundleImage],
+    target_index_bundles: List[BundleImage],
+    base_dir: str,
+    binary_image: str,
+    source_from_index: str,
+    request_id: int,
+    arch: str,
+    ocp_version: str,
+    distribution_scope: str,
+    graph_update_mode: Optional[str] = None,
+    target_index=None,
+    overwrite_target_index_token: Optional[str] = None,
+    ignore_bundle_ocp_version: Optional[bool] = False,
+) -> Tuple[List[BundleImage], List[BundleImage]]:
+    """
+    Rebuild index image with bundles missing from source image but present in target image.
+
+    If no bundles are missing in the source index image, the index image is still rebuilt
+    using the new binary image.
+
+    :param list source_index_bundles: bundles present in the source index image.
+    :param list target_index_bundles: bundles present in the target index image.
+    :param str base_dir: base directory where operation files will be located.
+    :param str binary_image: binary image to be used by the new index image.
+    :param str source_from_index: index image, whose data will be contained in the new index image.
+    :param int request_id: the ID of the IIB build request.
+    :param str arch: the architecture to build this image for.
+    :param str ocp_version: ocp version which will be added as a label to the image.
+    :param str graph_update_mode: Graph update mode that defines how channel graphs are updated
+        in the index.
+    :param str target_index: the pull specification of the container image
+    :param str overwrite_target_index_token: the token used for overwriting the input
+        ``source_from_index`` image. This is required to use ``overwrite_target_index``.
+        The format of the token must be in the format "user:password".
+    :param bool ignore_bundle_ocp_version: When set to `true` and image set as target_index is
+        listed in `iib_no_ocp_label_allow_list` config then bundles without
+        "com.redhat.openshift.versions" label set will be added in the result `index_image`.
+    :return: tuple where the first value is a list of bundles which were added to the index image
+        and the second value is a list of bundles in the new index whose ocp_version range does not
+        satisfy the ocp_version value of the target index.
+    :rtype: tuple
+    """
+    set_request_state(request_id, 'in_progress', 'Adding bundles missing in source index image')
+    log.info('Adding bundles from target index image which are missing from source index image')
+
+    missing_bundles, invalid_bundles = get_missing_bundles_from_target_to_source(
+        source_index_bundles=source_index_bundles,
+        target_index_bundles=target_index_bundles,
+        source_from_index=source_from_index,
+        ocp_version=ocp_version,
+        target_index=target_index,
+        ignore_bundle_ocp_version=ignore_bundle_ocp_version,
+    )
+    missing_bundle_paths = [bundle['bundlePath'] for bundle in missing_bundles]
+
     with set_registry_token(overwrite_target_index_token, target_index, append=True):
         is_source_fbc = is_image_fbc(source_from_index)
         if is_source_fbc:

iib/workers/tasks/containerized_utils.py

@@ -5,7 +5,7 @@
 import os
 import queue
 import threading
-from typing import Dict, List, Optional, Tuple
+from typing import Dict, List, Optional, Tuple, Union
 
 from iib.exceptions import IIBError
 from iib.workers.api_utils import set_request_state
@@ -62,10 +62,16 @@ def run(self) -> None:
         try:
             while not self.bundles_queue.empty():
                 bundle = self.bundles_queue.get()
-                skopeo_inspect(f'docker://{bundle}', '--raw', return_json=False)
+                b_path = str(bundle["bundlePath"]) if isinstance(bundle, dict) else str(bundle)
+                skopeo_inspect(f'docker://{b_path}', '--raw', return_json=False)
         except IIBError as e:
             self.bundle = bundle
-            log.error(f"Error validating bundle {bundle}: {e}")
+            bundle_str = (
+                bundle["bundlePath"]
+                if bundle and isinstance(bundle, dict) and "bundlePath" in bundle
+                else bundle
+            )
+            log.error(f"Error validating bundle {bundle_str}: {e}")
             self.exception = e
         finally:
             while not self.bundles_queue.empty():
@@ -81,24 +87,27 @@ def wait_for_bundle_validation_threads(validation_threads: List[ValidateBundlesT
     for t in validation_threads:
         t.join()
         if t.exception:
-            bundle_str = str(t.bundle) if t.bundle else "unknown"
+            if t.bundle and isinstance(t.bundle, dict) and "bundlePath" in t.bundle:
+                bundle_str = t.bundle["bundlePath"]
+            else:
+                bundle_str = str(t.bundle) if t.bundle else "unknown"
             log.error(f"Error validating bundle {bundle_str}: {t.exception}")
             raise IIBError(f"Error validating bundle {bundle_str}: {t.exception}")
 
 
 def validate_bundles_in_parallel(
-    bundles: List[BundleImage], threads=5, wait=True
+    bundles: Union[List[BundleImage], List[str]], threads=5, wait=True
 ) -> Optional[List[ValidateBundlesThread]]:
     """
     Validate bundles in parallel.
 
-    :param list bundles: the list of bundles to validate
+    :param list bundles: the list of bundles or bundle pullspecsto validate
     :param int threads: the number of threads to use
     :param bool wait: whether to wait for all threads to complete
     :return: the list of threads if not waiting, None otherwise
     :rtype: Optional[List[ValidateBundlesThread]]
     """
-    bundles_queue: queue.Queue[BundleImage] = queue.Queue()
+    bundles_queue: queue.Queue[Union[BundleImage, str]] = queue.Queue()
 
     for bundle in bundles:
         bundles_queue.put(bundle)

iib/workers/tasks/opm_operations.py

@@ -500,24 +500,19 @@ def opm_registry_deprecatetruncate(base_dir: str, index_db: str, bundles: List[s
     run_cmd(cmd, {'cwd': base_dir}, exc_msg=f'Failed to deprecate the bundles on {index_db}')
 
 
-def deprecate_bundles_fbc(
-    bundles: List[str],
+def deprecate_bundles_db(
     base_dir: str,
-    binary_image: str,
-    from_index: str,
+    index_db_file: str,
+    bundles: List[str],
 ) -> None:
     """
-    Deprecate the specified bundles from the FBC index image.
-
-    Dockerfile is created only, no build is performed.
+    Deprecate the specified bundles from the index.db file.
 
-    :param list bundles: pull specifications of bundles to deprecate.
     :param str base_dir: base directory where operation files will be located.
-    :param str binary_image: binary image to be used by the new index image.
-    :param str from_index: index image, from which the bundles will be deprecated.
+    :param str index_db_file: path to index.db file used with opm registry deprecatetruncate.
+    :param list bundles: pull specifications of bundles to deprecate.
     """
     conf = get_worker_config()
-    index_db_file = _get_or_create_temp_index_db_file(base_dir=base_dir, from_index=from_index)
 
     # Break the bundles into chunks of at max iib_deprecate_bundles_limit bundles
     for i in range(
@@ -531,6 +526,27 @@ def deprecate_bundles_fbc(
             bundles=bundles[i : i + conf.iib_deprecate_bundles_limit],  # Pass a chunk starting at i
         )
 
+
+def deprecate_bundles_fbc(
+    bundles: List[str],
+    base_dir: str,
+    binary_image: str,
+    from_index: str,
+) -> None:
+    """
+    Deprecate the specified bundles from the FBC index image.
+
+    Dockerfile is created only, no build is performed.
+
+    :param list bundles: pull specifications of bundles to deprecate.
+    :param str base_dir: base directory where operation files will be located.
+    :param str binary_image: binary image to be used by the new index image.
+    :param str from_index: index image, from which the bundles will be deprecated.
+    """
+    index_db_file = _get_or_create_temp_index_db_file(base_dir=base_dir, from_index=from_index)
+
+    deprecate_bundles_db(base_dir=base_dir, index_db_file=index_db_file, bundles=bundles)
+
     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

tests/test_web/test_api_v1.py

@@ -1969,7 +1969,7 @@ def test_regenerate_add_rm_batch_invalid_input(payload, error_msg, app, auth_env
 
 @pytest.mark.parametrize("binary_image", ('binary:image', 'scratch'))
 @pytest.mark.parametrize('distribution_scope', (None, 'stage'))
-@mock.patch('iib.web.api_v1.handle_merge_request')
+@mock.patch('iib.web.api_v1.handle_containerized_merge_request')
 @mock.patch('iib.web.api_v1.messaging.send_message_for_state_change')
 def test_merge_index_image_success(
     mock_smfsc, mock_merge, binary_image, app, db, auth_env, client, distribution_scope
@@ -2032,7 +2032,7 @@ def test_merge_index_image_success(
     mock_smfsc.assert_called_once_with(mock.ANY, new_batch_msg=True)
 
 
-@mock.patch('iib.web.api_v1.handle_merge_request')
+@mock.patch('iib.web.api_v1.handle_containerized_merge_request')
 @mock.patch('iib.web.api_v1.messaging.send_message_for_state_change')
 def test_merge_index_image_overwrite_token_redacted(
     mock_smfsc, mock_merge, app, auth_env, client, db
@@ -2083,7 +2083,7 @@ def test_merge_index_image_overwrite_token_redacted(
         ({'[email protected]': 'Patriots'}, True, None),
     ),
 )
-@mock.patch('iib.web.api_v1.handle_merge_request')
+@mock.patch('iib.web.api_v1.handle_containerized_merge_request')
 @mock.patch('iib.web.api_v1.messaging.send_message_for_state_change')
 def test_merge_index_image_custom_user_queue(
     mock_smfsc,
@@ -2119,7 +2119,7 @@ def test_merge_index_image_custom_user_queue(
 
 
 @pytest.mark.parametrize('overwrite_from_index', (True, False))
-@mock.patch('iib.web.api_v1.handle_merge_request')
+@mock.patch('iib.web.api_v1.handle_containerized_merge_request')
 @mock.patch('iib.web.api_v1.messaging.send_message_for_state_change')
 def test_merge_index_image_fail_on_missing_overwrite_params(
     mock_smfsc, mock_merge, app, auth_env, client, overwrite_from_index
@@ -2190,7 +2190,7 @@ def test_merge_index_image_fail_on_missing_overwrite_params(
         ),
     ),
 )
-@mock.patch('iib.web.api_v1.handle_merge_request')
+@mock.patch('iib.web.api_v1.handle_containerized_merge_request')
 @mock.patch('iib.web.api_v1.messaging.send_message_for_state_change')
 def test_merge_index_image_fail_on_invalid_params(
     mock_smfsc, mock_merge, app, auth_env, client, data, error_msg

tests/test_workers/test_tasks/test_build.py

@@ -103,10 +103,15 @@ def test_cleanup(mock_rdc, mock_run_cmd):
     mock_rdc.assert_called_once_with()
 
 
+@mock.patch('iib.workers.tasks.containerized_utils.get_worker_config')
 @mock.patch('iib.workers.tasks.build.tempfile.TemporaryDirectory')
 @mock.patch('iib.workers.tasks.build.run_cmd')
 @mock.patch('iib.workers.tasks.build.open')
-def test_create_and_push_manifest_list(mock_open, mock_run_cmd, mock_td, tmp_path):
+def test_create_and_push_manifest_list(mock_open, mock_run_cmd, mock_td, mock_gwc, tmp_path):
+    mock_gwc.return_value = {
+        'iib_registry': 'registry:8443',
+        'iib_image_push_template': '{registry}/iib-build:{request_id}',
+    }
     mock_td.return_value.__enter__.return_value = tmp_path
     mock_run_cmd.side_effect = [
         IIBError('Manifest list not found locally.'),