diff --git a/doozer/doozerlib/backend/konflux_fbc.py b/doozer/doozerlib/backend/konflux_fbc.py index e5c67a2ee9..5a86a762d3 100644 --- a/doozer/doozerlib/backend/konflux_fbc.py +++ b/doozer/doozerlib/backend/konflux_fbc.py @@ -1202,10 +1202,15 @@ def _update_channel(channel: Dict): # FIXME: We try to mimic how `skips` field is updated by the old ET centric process. # We should verify if this is correct. skips = None - bundle_with_skips = next( - (it for it in channel['entries'] if it.get('skips')), None - ) # Find which bundle has the skips field - if bundle_with_skips is not None: + + # Detect whether existing entries form a replaces-based update graph + # (e.g. ACM). These graphs are deliberately managed — preserve them + # as-is and do NOT migrate skips onto the new head entry. + has_replaces_graph = any(e.get('replaces') for e in channel['entries']) + + if has_replaces_graph: + logger.info("Channel has replaces-based update graph; preserving existing entries") + elif (bundle_with_skips := next((it for it in channel['entries'] if it.get('skips')), None)) is not None: # the channel already has skips like # ------------ # entries: @@ -1223,6 +1228,13 @@ def _update_channel(channel: Dict): # and add the bundle name of bundle_with_skips to the skips field skips = set(bundle_with_skips.pop('skips')) skips = (skips | {bundle_with_skips['name']}) - {olm_bundle_name} + # Also collect skips from any remaining entries (handles branching + # upgrade graphs where multiple entries independently define skips) + for other in channel['entries']: + if other is not bundle_with_skips and other.get('skips'): + skips.update(other.pop('skips')) + skips.add(other['name']) + skips.discard(olm_bundle_name) elif len(channel['entries']) == 1: # The channel only has one entry like # ------------ @@ -1278,12 +1290,13 @@ def _update_channel(channel: Dict): # Calculate replaces based on current head before appending replaces = None if not self.group.startswith('openshift-'): - # Find the current head - the entry that is not replaced by any other entry - bundle_with_replaces = [it for it in channel['entries']] - replaced_names = {it.get('replaces') for it in bundle_with_replaces if it.get('replaces')} - current_head = next( - (it for it in bundle_with_replaces if it['name'] not in replaced_names), None - ) + # Find the current head - the entry not superseded + # (not replaced or skipped) by any other entry + all_entries = list(channel['entries']) + superseded_names = {it.get('replaces') for it in all_entries if it.get('replaces')} + for it in all_entries: + superseded_names.update(it.get('skips', [])) + current_head = next((it for it in all_entries if it['name'] not in superseded_names), None) if current_head: # The new bundle should replace the current head replaces = current_head['name'] diff --git a/doozer/tests/backend/test_konflux_fbc.py b/doozer/tests/backend/test_konflux_fbc.py index cce948e7d6..b6531d041b 100644 --- a/doozer/tests/backend/test_konflux_fbc.py +++ b/doozer/tests/backend/test_konflux_fbc.py @@ -1932,6 +1932,383 @@ async def test_rebase_dir_writes_skips_for_openshift_when_disabled( self.assertIn("skips", new_entry, "OpenShift groups must still write skips even when flag is True") self.assertEqual(new_entry["skipRange"], ">=6.3.0-0 <6.5.1") + @patch("doozerlib.opm.generate_dockerfile") + @patch("pathlib.Path.unlink") + @patch("doozerlib.backend.konflux_fbc.KonfluxFbcRebaser._load_csv_from_bundle") + @patch("doozerlib.backend.konflux_fbc.KonfluxFbcRebaser._get_referenced_images") + @patch("doozerlib.backend.konflux_fbc.DockerfileParser") + @patch("pathlib.Path.mkdir") + @patch("pathlib.Path.is_file", return_value=True) + @patch("pathlib.Path.open") + @patch("doozerlib.backend.konflux_fbc.KonfluxFbcRebaser._fetch_olm_bundle_image_info", new_callable=AsyncMock) + @patch("doozerlib.backend.konflux_fbc.KonfluxFbcRebaser._fetch_olm_bundle_blob", new_callable=AsyncMock) + async def test_rebase_dir_replaces_graph_preserves_entries( + self, + mock_fetch_olm_bundle_blob, + mock_fetch_olm_bundle_image_info, + mock_open, + mock_is_file, + mock_mkdir, + MockDockerfileParser, + mock_get_referenced_images, + mock_load_csv_from_bundle: AsyncMock, + mock_path_unlink: Mock, + mock_generate_dockerfile: AsyncMock, + ): + """Replaces-based graphs (like ACM) are preserved — no skip migration, correct head.""" + rebaser = KonfluxFbcRebaser( + base_dir=self.base_dir, + group="acm-2.16", + assembly=self.assembly, + version="2.16.3", + release="1", + commit_message="test", + push=False, + fbc_repo="https://example.com/fbc.git", + upcycle=False, + ) + + metadata = MagicMock(spec=ImageMetadata) + metadata.distgit_key = "multiclusterhub-operator" + metadata.runtime = MagicMock() + metadata.runtime.group_config = MagicMock() + metadata.runtime.group_config.vars = MagicMock() + metadata.runtime.group_config.vars.MAJOR = "2" + metadata.runtime.group_config.vars.MINOR = "16" + metadata.runtime.group_config.vars.FBC_DISABLE_CHANNEL_SKIPS = False + metadata.runtime.konflux_db = MagicMock() + metadata.config = MagicMock() + metadata.config.delivery = MagicMock() + metadata.config.delivery.delivery_repo_names = ["acm-hub-repo"] + metadata.get_olm_bundle_delivery_repo_name.return_value = "acm-hub-repo" + + build_repo = MagicMock() + build_repo.local_dir = self.base_dir + bundle_build = MagicMock( + spec=KonfluxBundleBuildRecord, + nvr="multiclusterhub-operator-bundle-2.16.3-1", + operator_nvr="multiclusterhub-operator-2.16.3-1", + operand_nvrs=[], + image_pullspec="example.com/acm-bundle@sha256:abc", + image_tag="abc123", + source_repo="https://example.com/multiclusterhub-operator.git", + commitish="abc123def", + ) + logger = MagicMock() + + mock_fetch_olm_bundle_image_info.return_value = { + "config": { + "config": { + "Labels": { + "name": "multiclusterhub-operator", + "operators.operatorframework.io.bundle.channels.v1": "release-2.16", + "operators.operatorframework.io.bundle.channel.default.v1": "release-2.16", + "operators.operatorframework.io.bundle.package.v1": "advanced-cluster-management", + "operators.operatorframework.io.bundle.manifests.v1": "manifests/", + }, + }, + }, + } + + mock_fetch_olm_bundle_blob.return_value = ( + "advanced-cluster-management.v2.16.3", + "advanced-cluster-management", + { + "schema": "olm.bundle", + "name": "advanced-cluster-management.v2.16.3", + "package": "advanced-cluster-management", + "properties": [], + "relatedImages": [{"name": "", "image": "example.com/acm-bundle@sha256:abc"}], + }, + ) + + # ACM-style replaces-based graph matching the real production index. + # Existing entries have deliberate replaces/skips that must be preserved. + org_catalog_blobs = [ + {"schema": "olm.package", "name": "advanced-cluster-management", "defaultChannel": "release-2.16"}, + { + "schema": "olm.channel", + "name": "release-2.16", + "package": "advanced-cluster-management", + "entries": [ + { + "name": "advanced-cluster-management.v2.16.0", + "skipRange": ">=2.15.0 <2.16.0", + }, + { + "name": "advanced-cluster-management.v2.16.1", + "replaces": "advanced-cluster-management.v2.16.0", + "skipRange": ">=2.15.0 <2.16.1", + }, + { + "name": "advanced-cluster-management.v2.16.2", + "replaces": "advanced-cluster-management.v2.16.0", + "skips": ["advanced-cluster-management.v2.16.1"], + "skipRange": ">=2.15.0 <2.16.2", + }, + ], + }, + { + "schema": "olm.bundle", + "name": "advanced-cluster-management.v2.16.0", + "package": "advanced-cluster-management", + "properties": [], + "relatedImages": [], + }, + { + "schema": "olm.bundle", + "name": "advanced-cluster-management.v2.16.1", + "package": "advanced-cluster-management", + "properties": [], + "relatedImages": [], + }, + { + "schema": "olm.bundle", + "name": "advanced-cluster-management.v2.16.2", + "package": "advanced-cluster-management", + "properties": [], + "relatedImages": [], + }, + ] + + org_catalog_file = StringIO() + yaml.dump_all(org_catalog_blobs, org_catalog_file) + org_catalog_file.seek(0) + result_catalog_file = StringIO() + images_mirror_set_file = StringIO() + mock_open.return_value.__enter__.side_effect = [org_catalog_file, result_catalog_file, images_mirror_set_file] + + mock_get_referenced_images.return_value = [] + mock_load_csv_from_bundle.return_value = { + "metadata": { + "name": "multiclusterhub-operator", + "annotations": {"olm.skipRange": ">=2.15.0 <2.16.3"}, + }, + "spec": {"icon": []}, + } + + mock_dfp = MockDockerfileParser.return_value + mock_dfp.envs = {} + mock_dfp.labels = {} + + mock_generate_dockerfile.return_value = None + + await rebaser._rebase_dir(metadata, build_repo, bundle_build, "2.16.3", "1", logger) + + result_catalog_file.seek(0) + result_catalog_blobs = list(yaml.load_all(result_catalog_file)) + result_catalog_blobs = rebaser._catagorize_catalog_blobs(result_catalog_blobs) + + entries = result_catalog_blobs["advanced-cluster-management"]["olm.channel"]["release-2.16"]["entries"] + + # Existing entries must be preserved exactly as they were + v2_16_0 = next(e for e in entries if e["name"] == "advanced-cluster-management.v2.16.0") + self.assertNotIn("replaces", v2_16_0) + self.assertNotIn("skips", v2_16_0) + + v2_16_1 = next(e for e in entries if e["name"] == "advanced-cluster-management.v2.16.1") + self.assertEqual(v2_16_1["replaces"], "advanced-cluster-management.v2.16.0") + self.assertNotIn("skips", v2_16_1) + + v2_16_2 = next(e for e in entries if e["name"] == "advanced-cluster-management.v2.16.2") + self.assertEqual(v2_16_2["replaces"], "advanced-cluster-management.v2.16.0") + self.assertEqual(v2_16_2["skips"], ["advanced-cluster-management.v2.16.1"]) + + # New entry replaces the true head (v2.16.2) and has no accumulated skips + new_entry = next(e for e in entries if e["name"] == "advanced-cluster-management.v2.16.3") + self.assertEqual(new_entry["replaces"], "advanced-cluster-management.v2.16.2") + self.assertEqual(new_entry["skipRange"], ">=2.15.0 <2.16.3") + self.assertNotIn("skips", new_entry) + + @patch("doozerlib.opm.generate_dockerfile") + @patch("pathlib.Path.unlink") + @patch("doozerlib.backend.konflux_fbc.KonfluxFbcRebaser._load_csv_from_bundle") + @patch("doozerlib.backend.konflux_fbc.KonfluxFbcRebaser._get_referenced_images") + @patch("doozerlib.backend.konflux_fbc.DockerfileParser") + @patch("pathlib.Path.mkdir") + @patch("pathlib.Path.is_file", return_value=True) + @patch("pathlib.Path.open") + @patch("doozerlib.backend.konflux_fbc.KonfluxFbcRebaser._fetch_olm_bundle_image_info", new_callable=AsyncMock) + @patch("doozerlib.backend.konflux_fbc.KonfluxFbcRebaser._fetch_olm_bundle_blob", new_callable=AsyncMock) + async def test_rebase_dir_branching_skips_no_replaces( + self, + mock_fetch_olm_bundle_blob, + mock_fetch_olm_bundle_image_info, + mock_open, + mock_is_file, + mock_mkdir, + MockDockerfileParser, + mock_get_referenced_images, + mock_load_csv_from_bundle: AsyncMock, + mock_path_unlink: Mock, + mock_generate_dockerfile: AsyncMock, + ): + """Branching skips without replaces still accumulate skips onto new head (backward compat).""" + rebaser = KonfluxFbcRebaser( + base_dir=self.base_dir, + group="acm-2.16", + assembly=self.assembly, + version="2.16.3", + release="1", + commit_message="test", + push=False, + fbc_repo="https://example.com/fbc.git", + upcycle=False, + ) + + metadata = MagicMock(spec=ImageMetadata) + metadata.distgit_key = "multiclusterhub-operator" + metadata.runtime = MagicMock() + metadata.runtime.group_config = MagicMock() + metadata.runtime.group_config.vars = MagicMock() + metadata.runtime.group_config.vars.MAJOR = "2" + metadata.runtime.group_config.vars.MINOR = "16" + metadata.runtime.group_config.vars.FBC_DISABLE_CHANNEL_SKIPS = False + metadata.runtime.konflux_db = MagicMock() + metadata.config = MagicMock() + metadata.config.delivery = MagicMock() + metadata.config.delivery.delivery_repo_names = ["acm-hub-repo"] + metadata.get_olm_bundle_delivery_repo_name.return_value = "acm-hub-repo" + + build_repo = MagicMock() + build_repo.local_dir = self.base_dir + bundle_build = MagicMock( + spec=KonfluxBundleBuildRecord, + nvr="multiclusterhub-operator-bundle-2.16.3-1", + operator_nvr="multiclusterhub-operator-2.16.3-1", + operand_nvrs=[], + image_pullspec="example.com/acm-bundle@sha256:abc", + image_tag="abc123", + source_repo="https://example.com/multiclusterhub-operator.git", + commitish="abc123def", + ) + logger = MagicMock() + + mock_fetch_olm_bundle_image_info.return_value = { + "config": { + "config": { + "Labels": { + "name": "multiclusterhub-operator", + "operators.operatorframework.io.bundle.channels.v1": "release-2.16", + "operators.operatorframework.io.bundle.channel.default.v1": "release-2.16", + "operators.operatorframework.io.bundle.package.v1": "advanced-cluster-management", + "operators.operatorframework.io.bundle.manifests.v1": "manifests/", + }, + }, + }, + } + + mock_fetch_olm_bundle_blob.return_value = ( + "advanced-cluster-management.v2.16.3", + "advanced-cluster-management", + { + "schema": "olm.bundle", + "name": "advanced-cluster-management.v2.16.3", + "package": "advanced-cluster-management", + "properties": [], + "relatedImages": [{"name": "", "image": "example.com/acm-bundle@sha256:abc"}], + }, + ) + + # Branching graph WITHOUT replaces — skip migration should still collect + # skips from all entries and accumulate them onto the new head. + org_catalog_blobs = [ + {"schema": "olm.package", "name": "advanced-cluster-management", "defaultChannel": "release-2.16"}, + { + "schema": "olm.channel", + "name": "release-2.16", + "package": "advanced-cluster-management", + "entries": [ + {"name": "advanced-cluster-management.v2.14.5", "skipRange": ">=2.12.0 <2.14.5"}, + { + "name": "advanced-cluster-management.v2.15.3", + "skipRange": ">=2.12.0 <2.15.3", + "skips": ["advanced-cluster-management.v2.14.5"], + }, + { + "name": "advanced-cluster-management.v2.16.0", + "skipRange": ">=2.12.0 <2.16.0", + "skips": ["advanced-cluster-management.v2.15.3"], + }, + ], + }, + { + "schema": "olm.bundle", + "name": "advanced-cluster-management.v2.14.5", + "package": "advanced-cluster-management", + "properties": [], + "relatedImages": [], + }, + { + "schema": "olm.bundle", + "name": "advanced-cluster-management.v2.15.3", + "package": "advanced-cluster-management", + "properties": [], + "relatedImages": [], + }, + { + "schema": "olm.bundle", + "name": "advanced-cluster-management.v2.16.0", + "package": "advanced-cluster-management", + "properties": [], + "relatedImages": [], + }, + ] + + org_catalog_file = StringIO() + yaml.dump_all(org_catalog_blobs, org_catalog_file) + org_catalog_file.seek(0) + result_catalog_file = StringIO() + images_mirror_set_file = StringIO() + mock_open.return_value.__enter__.side_effect = [org_catalog_file, result_catalog_file, images_mirror_set_file] + + mock_get_referenced_images.return_value = [] + mock_load_csv_from_bundle.return_value = { + "metadata": { + "name": "multiclusterhub-operator", + "annotations": {"olm.skipRange": ">=2.12.0 <2.16.3"}, + }, + "spec": {"icon": []}, + } + + mock_dfp = MockDockerfileParser.return_value + mock_dfp.envs = {} + mock_dfp.labels = {} + + mock_generate_dockerfile.return_value = None + + await rebaser._rebase_dir(metadata, build_repo, bundle_build, "2.16.3", "1", logger) + + result_catalog_file.seek(0) + result_catalog_blobs = list(yaml.load_all(result_catalog_file)) + result_catalog_blobs = rebaser._catagorize_catalog_blobs(result_catalog_blobs) + + entries = result_catalog_blobs["advanced-cluster-management"]["olm.channel"]["release-2.16"]["entries"] + + new_entry = next(e for e in entries if e["name"] == "advanced-cluster-management.v2.16.3") + self.assertIn("skips", new_entry) + self.assertEqual( + sorted(new_entry["skips"]), + sorted( + [ + "advanced-cluster-management.v2.14.5", + "advanced-cluster-management.v2.15.3", + "advanced-cluster-management.v2.16.0", + ] + ), + ) + + # Previous entries must have their skips removed (migrated to the new head) + for entry in entries: + if entry["name"] != "advanced-cluster-management.v2.16.3": + self.assertNotIn( + "skips", + entry, + f"Entry {entry['name']} should not retain skips after migration", + ) + + self.assertIn("replaces", new_entry) + class TestFetchCsvFromGit(unittest.IsolatedAsyncioTestCase): """Test the _fetch_csv_from_git helper function."""