From 1ed9adf6824c52f9a4a852c34b1067ea050c32a3 Mon Sep 17 00:00:00 2001 From: Siddharth Sharma Date: Wed, 10 Jun 2026 13:05:59 -0400 Subject: [PATCH 1/2] Add --enabled-override for manual base image release runs Allow operators to bypass base_image_release.enabled: false when invoking images:release-to-base-repo, with clear qualification errors when the image still does not qualify. Co-authored-by: Cursor rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED --- .../doozerlib/backend/base_image_handler.py | 10 +- doozer/doozerlib/cli/images.py | 22 ++- doozer/doozerlib/image.py | 28 +++- .../tests/backend/test_base_image_handler.py | 43 ++++++ doozer/tests/cli/test_images.py | 130 +++++++++++++++++- doozer/tests/test_image.py | 27 ++++ 6 files changed, 249 insertions(+), 11 deletions(-) diff --git a/doozer/doozerlib/backend/base_image_handler.py b/doozer/doozerlib/backend/base_image_handler.py index b0dd995ad1..780bdb5e7f 100644 --- a/doozer/doozerlib/backend/base_image_handler.py +++ b/doozer/doozerlib/backend/base_image_handler.py @@ -97,10 +97,16 @@ def __init__(self, runtime, dry_run: bool = False): def _scoped_logger(self, entity: str): return logutil.EntityLoggingAdapter(self.runtime.logger, extra={'entity': entity}) - async def snapshot_release(self, snapshot_input: BaseImageSnapshotInput) -> Optional[BaseImageReleaseResult]: + async def snapshot_release( + self, snapshot_input: BaseImageSnapshotInput, enabled_override: bool = False + ) -> Optional[BaseImageReleaseResult]: """ Run snapshot→release for exactly one base-image component. + Args: + enabled_override: When True, bypass image/group ``base_image_release.enabled: false`` + (see :meth:`ImageMetadata.should_trigger_base_image_release`). + Returns: :class:`BaseImageReleaseResult` on success, else ``None``. """ @@ -117,7 +123,7 @@ async def snapshot_release(self, snapshot_input: BaseImageSnapshotInput) -> Opti self.logger = self._scoped_logger(metadata.qualified_key) - if not metadata.should_trigger_base_image_release(): + if not metadata.should_trigger_base_image_release(enabled_override=enabled_override): self.logger.warning("Does not qualify for base image release workflow") return None diff --git a/doozer/doozerlib/cli/images.py b/doozer/doozerlib/cli/images.py index f8231b56d8..94407b94fa 100644 --- a/doozer/doozerlib/cli/images.py +++ b/doozer/doozerlib/cli/images.py @@ -1489,9 +1489,18 @@ def query_rpm_version(runtime, repo_type): required=True, help='Image build NVR', ) +@click.option( + '--enabled-override', + is_flag=True, + default=False, + help=( + 'Bypass base_image_release.enabled: false for base_only/golang images on non-test assemblies. ' + 'Does not bypass test assembly, OKD, or non-qualifying images (use base_image_release.force in metadata for those).' + ), +) @click_coroutine @pass_runtime -async def release_to_base_repo(runtime, nvr): +async def release_to_base_repo(runtime, nvr, enabled_override): """Latest SUCCESS Konflux image row → snapshot/release, then INSERT follow-up with ``release_pipeline`` / ``released_pullspec``. Golang builder is inferred from image metadata when present; otherwise from the openshift-golang-builder name in the NVR. @@ -1518,6 +1527,13 @@ async def release_to_base_repo(runtime, nvr): metadata = runtime.image_map.get(source_row.name) is_golang_builder = metadata.is_golang_builder() if metadata else GOLANG_BUILDER_IMAGE_NAME in image_nvr + if metadata is None: + raise DoozerFatalError(f"Image metadata not found for distgit key {source_row.name}") + + if not metadata.should_trigger_base_image_release(enabled_override=enabled_override): + reason = metadata.base_image_release_qualification_failure_reason(enabled_override=enabled_override) + raise DoozerFatalError(f"Image does not qualify for base image release workflow: {reason}") + snapshot_input = BaseImageSnapshotInput( nvr=image_nvr, distgit_key=source_row.name, @@ -1528,7 +1544,7 @@ async def release_to_base_repo(runtime, nvr): ) handler = BaseImageHandler(runtime, dry_run=False) - result = await handler.snapshot_release(snapshot_input) + result = await handler.snapshot_release(snapshot_input, enabled_override=enabled_override) if result: logger.info( @@ -1544,4 +1560,4 @@ async def release_to_base_repo(runtime, nvr): await runtime.konflux_db.add_builds([followup]) return - raise DoozerFatalError("snapshot_release returned no result") + raise DoozerFatalError(f"snapshot-release did not complete for {image_nvr}; check logs for Konflux/snapshot errors") diff --git a/doozer/doozerlib/image.py b/doozer/doozerlib/image.py index 5fd3e75f0b..f715b21197 100644 --- a/doozer/doozerlib/image.py +++ b/doozer/doozerlib/image.py @@ -1392,7 +1392,7 @@ def is_golang_builder(self) -> bool: image_name = self.config.name return image_name == GOLANG_BUILDER_IMAGE_NAME or image_name == 'openshift/golang-builder' - def should_trigger_base_image_release(self) -> bool: + def should_trigger_base_image_release(self, enabled_override: bool = False) -> bool: """ Determines whether this image should trigger the base image release workflow. @@ -1401,7 +1401,8 @@ def should_trigger_base_image_release(self) -> bool: 1. Image metadata ``base_image_release.force`` set to true enables workflow (OCP only) 2. Assembly must not be ``test`` 3. Image must be ``base_only`` or a golang builder - 4. Base image release enabled override: + 4. Base image release enabled override (qualifying base/golang images only): + - CLI ``--enabled-override`` bypasses image/group ``enabled: false`` (not ``force``) - Image metadata configuration (``self.config.base_image_release.enabled``) - Group configuration (``self.runtime.group_config.base_image_release.enabled``) If no override is set for step 4, the enabled flag defaults to True. @@ -1423,6 +1424,10 @@ def should_trigger_base_image_release(self) -> bool: if not (self.is_base_image() or self.is_golang_builder()): return False + if enabled_override: + self.logger.info("Base image release enabled from --enabled-override") + return True + source = None base_image_release_enabled = True base_image_release_config_override = self.config.base_image_release.enabled @@ -1439,6 +1444,25 @@ def should_trigger_base_image_release(self) -> bool: self.logger.info(f"Base image release enabled set from {source} {base_image_release_enabled}") return base_image_release_enabled + def base_image_release_qualification_failure_reason(self, enabled_override: bool = False) -> str: + """ + Human-readable reason when :meth:`should_trigger_base_image_release` is False. + + Call only when qualification has already failed. + """ + if self.runtime.variant is BuildVariant.OKD: + return "OKD variant does not use RH base image snapshot→release" + if self.runtime.assembly == "test": + return "test assembly is excluded (--enabled-override does not bypass)" + if not (self.is_base_image() or self.is_golang_builder()): + return ( + "image must be base_only or a golang builder " + "(--enabled-override does not bypass; use base_image_release.force in image metadata instead)" + ) + if not enabled_override: + return "base_image_release.enabled is false in image or group config; pass --enabled-override to run anyway" + return "see image/group base_image_release settings" + def is_base_image_release_quay_fallback_enabled(self) -> bool: """ When False and ``should_trigger_base_image_release()`` applies, late parent resolve diff --git a/doozer/tests/backend/test_base_image_handler.py b/doozer/tests/backend/test_base_image_handler.py index 8d9f56e74d..7638edf9c4 100644 --- a/doozer/tests/backend/test_base_image_handler.py +++ b/doozer/tests/backend/test_base_image_handler.py @@ -184,6 +184,49 @@ async def test_snapshot_release_golang_builder_success( self.assertEqual(result.release_pipeline, "https://konflux.example/releases/golang-release") self.assertEqual(result.released_pullspec, rh_art_images_base_pullspec(golang_nvr)) + @patch("doozerlib.backend.base_image_handler.KonfluxClient.from_kubeconfig") + @patch("doozerlib.backend.base_image_handler.resolve_konflux_namespace_by_product") + @patch("doozerlib.backend.base_image_handler.resolve_konflux_kubeconfig_by_product") + async def test_snapshot_release_enabled_override_overrides_enabled_false( + self, mock_kubeconfig, mock_namespace, mock_konflux_client_init + ): + mock_namespace.return_value = "ocp-art-tenant" + mock_kubeconfig.return_value = "/path/to/kubeconfig" + mock_konflux_client_init.return_value = AsyncMock() + + disabled_base_model = Model( + { + "name": "test-base", + "base_only": True, + "base_image_release": {"enabled": False}, + "distgit": {"component": "ose-test-base-container"}, + } + ) + disabled_data = Model({"key": "test-base", "data": disabled_base_model, "filename": "test-base.yaml"}) + disabled_metadata = ImageMetadata(self.runtime, disabled_data) + disabled_metadata.distgit_key = "test-base" + + handler = BaseImageHandler(self.runtime, dry_run=True) + self.runtime.image_map = {"test-base": disabled_metadata} + + result_without_enabled_override = await handler.snapshot_release(self.default_input) + self.assertIsNone(result_without_enabled_override) + + with patch.object(handler, "_snapshot_from_component", new=AsyncMock(return_value="test-snapshot")): + with patch.object( + handler, + "_create_release_from_snapshot", + new=AsyncMock(return_value=("test-release", "https://konflux.example/releases/test-release")), + ): + with patch.object(handler, "_wait_for_release_completion", return_value=True): + result_with_enabled_override = await handler.snapshot_release( + self.default_input, enabled_override=True + ) + + self.assertIsNotNone(result_with_enabled_override) + self.assertIsInstance(result_with_enabled_override, BaseImageReleaseResult) + self.assertEqual(result_with_enabled_override.release_name, "test-release") + @patch("doozerlib.backend.base_image_handler.KonfluxClient.from_kubeconfig") @patch("doozerlib.backend.base_image_handler.resolve_konflux_namespace_by_product") @patch("doozerlib.backend.base_image_handler.resolve_konflux_kubeconfig_by_product") diff --git a/doozer/tests/cli/test_images.py b/doozer/tests/cli/test_images.py index 8b6533d153..69927befd2 100644 --- a/doozer/tests/cli/test_images.py +++ b/doozer/tests/cli/test_images.py @@ -14,12 +14,12 @@ from doozerlib.image import ImageMetadata -def _invoke_release_to_base_repo_inner(runtime, nvr: str): +def _invoke_release_to_base_repo_inner(runtime, nvr: str, enabled_override: bool = False): """Call the bare async handler (avoid Click/pass_runtime context in unit tests).""" fn = release_to_base_repo.callback while hasattr(fn, '__wrapped__'): fn = fn.__wrapped__ - return asyncio.run(fn(runtime, nvr)) + return asyncio.run(fn(runtime, nvr, enabled_override)) class TestImagesCli(unittest.TestCase): @@ -179,7 +179,7 @@ async def capture_builds(builds): with patch("doozerlib.cli.images.BaseImageHandler") as bh_cls: - async def snap_release(inp): + async def snap_release(inp, enabled_override=False): self.assertFalse(inp.is_golang_builder) self.assertEqual(inp.distgit_key, source.name) self.assertEqual(inp.container_image, source.image_pullspec) @@ -246,7 +246,7 @@ def test_golang_builder_snapshot_input_openshift_slash_builder_name(self): with patch("doozerlib.cli.images.BaseImageHandler") as bh_cls: - async def snap_release(inp): + async def snap_release(inp, enabled_override=False): self.assertTrue(inp.is_golang_builder) return release_out @@ -256,6 +256,128 @@ async def snap_release(inp): kb.add_builds.assert_awaited_once() + def test_enabled_override_flag_passed_to_snapshot_release(self): + runtime = self._runtime_with_base_image() + nvr = "ose-base-container-v4.22-1.el9" + source = KonfluxBuildRecord( + name="openshift-enterprise-base-rhel9", + group=runtime.group, + nvr=nvr, + outcome=KonfluxBuildOutcome.SUCCESS, + engine=Engine.KONFLUX, + image_pullspec="quay.io/base@sha256:abc", + rebase_repo_url="https://git.example/r.git", + rebase_commitish="deadbeef", + ) + + kb = MagicMock() + kb.get_build_record_by_nvr = AsyncMock(return_value=source) + kb.bind = MagicMock() + kb.add_builds = AsyncMock() + runtime.konflux_db = kb + runtime.initialize = MagicMock() + + release_out = BaseImageReleaseResult( + release_name="r1", + snapshot_name="s1", + nvr=nvr, + release_pipeline="https://pipeline", + released_pullspec="registry.example/img:tag", + ) + + with patch("doozerlib.cli.images.BaseImageHandler") as bh_cls: + received_enabled_override = [] + + async def snap_release(inp, enabled_override=False): + received_enabled_override.append(enabled_override) + return release_out + + bh_cls.return_value.snapshot_release = snap_release + + _invoke_release_to_base_repo_inner(runtime, nvr, enabled_override=True) + + self.assertEqual(received_enabled_override, [True]) + + def _konflux_db_for_source(self, runtime, source): + kb = MagicMock() + kb.get_build_record_by_nvr = AsyncMock(return_value=source) + kb.bind = MagicMock() + kb.add_builds = AsyncMock() + runtime.konflux_db = kb + runtime.initialize = MagicMock() + return kb + + def test_raises_when_base_image_release_disabled_without_override(self): + runtime = self._runtime_with_base_image() + base_md = runtime.image_map["openshift-enterprise-base-rhel9"] + base_md.config.base_image_release.enabled = False + + nvr = "ose-base-container-v4.22-1.el9" + source = KonfluxBuildRecord( + name="openshift-enterprise-base-rhel9", + group=runtime.group, + nvr=nvr, + outcome=KonfluxBuildOutcome.SUCCESS, + engine=Engine.KONFLUX, + image_pullspec="quay.io/base@sha256:abc", + rebase_repo_url="https://git.example/r.git", + rebase_commitish="deadbeef", + ) + self._konflux_db_for_source(runtime, source) + + with patch("doozerlib.cli.images.BaseImageHandler") as bh_cls: + with self.assertRaises(DoozerFatalError) as ctx: + _invoke_release_to_base_repo_inner(runtime, nvr) + self.assertIn("base_image_release.enabled is false", str(ctx.exception)) + self.assertIn("--enabled-override", str(ctx.exception)) + bh_cls.assert_not_called() + + def test_raises_on_test_assembly_even_with_enabled_override(self): + runtime = self._runtime_with_base_image() + runtime.assembly = "test" + nvr = "ose-base-container-v4.22-1.el9" + source = KonfluxBuildRecord( + name="openshift-enterprise-base-rhel9", + group=runtime.group, + nvr=nvr, + outcome=KonfluxBuildOutcome.SUCCESS, + engine=Engine.KONFLUX, + image_pullspec="quay.io/base@sha256:abc", + rebase_repo_url="https://git.example/r.git", + rebase_commitish="deadbeef", + ) + self._konflux_db_for_source(runtime, source) + + with patch("doozerlib.cli.images.BaseImageHandler") as bh_cls: + with self.assertRaises(DoozerFatalError) as ctx: + _invoke_release_to_base_repo_inner(runtime, nvr, enabled_override=True) + self.assertIn("test assembly is excluded", str(ctx.exception)) + bh_cls.assert_not_called() + + def test_raises_when_snapshot_release_returns_none(self): + runtime = self._runtime_with_base_image() + nvr = "ose-base-container-v4.22-1.el9" + source = KonfluxBuildRecord( + name="openshift-enterprise-base-rhel9", + group=runtime.group, + nvr=nvr, + outcome=KonfluxBuildOutcome.SUCCESS, + engine=Engine.KONFLUX, + image_pullspec="quay.io/base@sha256:abc", + rebase_repo_url="https://git.example/r.git", + rebase_commitish="deadbeef", + ) + self._konflux_db_for_source(runtime, source) + + with patch("doozerlib.cli.images.BaseImageHandler") as bh_cls: + bh_cls.return_value.snapshot_release = AsyncMock(return_value=None) + + with self.assertRaises(DoozerFatalError) as ctx: + _invoke_release_to_base_repo_inner(runtime, nvr) + + self.assertIn("snapshot-release did not complete", str(ctx.exception)) + self.assertIn(nvr, str(ctx.exception)) + if __name__ == '__main__': unittest.main() diff --git a/doozer/tests/test_image.py b/doozer/tests/test_image.py index 832ce2a6b7..0f145a5dc9 100644 --- a/doozer/tests/test_image.py +++ b/doozer/tests/test_image.py @@ -1855,6 +1855,33 @@ def test_should_trigger_base_image_release(self): golang_snap_off_metadata = ImageMetadata(runtime, golang_snap_off_data) self.assertFalse(golang_snap_off_metadata.should_trigger_base_image_release()) + # enabled_override=True overrides enabled=false for base image + self.assertTrue(base_snap_off_metadata.should_trigger_base_image_release(enabled_override=True)) + + # enabled_override=True overrides enabled=false for golang builder + self.assertTrue(golang_snap_off_metadata.should_trigger_base_image_release(enabled_override=True)) + + # enabled_override=True does NOT override non-base/non-golang (step 3 still blocks) + self.assertFalse(regular_metadata.should_trigger_base_image_release(enabled_override=True)) + + # enabled_override=True does NOT bypass test assembly + self.assertFalse( + ImageMetadata(test_assembly_runtime, base_snap_off_data).should_trigger_base_image_release( + enabled_override=True + ) + ) + + # enabled_override=True does NOT bypass OKD + okd_runtime = MagicMock() + okd_runtime.logger = logging.getLogger('test_runtime') + okd_runtime.variant = BuildVariant.OKD + okd_runtime.assembly = 'stream' + okd_runtime.product = 'ocp' + okd_runtime.group_config = Model({}) + self.assertFalse( + ImageMetadata(okd_runtime, base_snap_off_data).should_trigger_base_image_release(enabled_override=True) + ) + # Force=true overrides enabled=false force_enabled_off = Model( {'name': 'test-regular', 'base_image_release': Model({'enabled': False, 'force': True})} From 45fbb38f8b6eaf59d2b33db8c554b34973bf2b0f Mon Sep 17 00:00:00 2001 From: Siddharth Sharma Date: Wed, 10 Jun 2026 13:10:19 -0400 Subject: [PATCH 2/2] Return qualification reason from should_trigger_base_image_release Replace the separate failure-reason helper with BaseImageReleaseQualification so the decision and message come from a single code path. Co-authored-by: Cursor rh-pre-commit.version: 2.4.0 rh-pre-commit.check-secrets: ENABLED --- doozer/doozerlib/cli/images.py | 8 +++-- doozer/doozerlib/image.py | 61 ++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/doozer/doozerlib/cli/images.py b/doozer/doozerlib/cli/images.py index 94407b94fa..f21c5c9eaa 100644 --- a/doozer/doozerlib/cli/images.py +++ b/doozer/doozerlib/cli/images.py @@ -1530,9 +1530,11 @@ async def release_to_base_repo(runtime, nvr, enabled_override): if metadata is None: raise DoozerFatalError(f"Image metadata not found for distgit key {source_row.name}") - if not metadata.should_trigger_base_image_release(enabled_override=enabled_override): - reason = metadata.base_image_release_qualification_failure_reason(enabled_override=enabled_override) - raise DoozerFatalError(f"Image does not qualify for base image release workflow: {reason}") + qualification = metadata.should_trigger_base_image_release(enabled_override=enabled_override) + if not qualification: + raise DoozerFatalError( + f"Image does not qualify for base image release workflow: {qualification.reason}" + ) snapshot_input = BaseImageSnapshotInput( nvr=image_nvr, diff --git a/doozer/doozerlib/image.py b/doozer/doozerlib/image.py index f715b21197..a36bd7455e 100644 --- a/doozer/doozerlib/image.py +++ b/doozer/doozerlib/image.py @@ -7,7 +7,7 @@ from copy import copy from functools import lru_cache from multiprocessing import Event -from typing import Any, Dict, List, Optional, Set, Tuple, cast +from typing import Any, Dict, List, NamedTuple, Optional, Set, Tuple, cast from artcommonlib import util as artlib_util from artcommonlib.constants import GOLANG_BUILDER_IMAGE_NAME @@ -28,6 +28,16 @@ from doozerlib.source_resolver import SourceResolver +class BaseImageReleaseQualification(NamedTuple): + """Whether base image snapshot→release should run, with an optional failure reason.""" + + trigger: bool + reason: Optional[str] = None + + def __bool__(self) -> bool: + return self.trigger + + @lru_cache(maxsize=500) def extract_builder_info_from_pullspec( pullspec: str, @@ -1392,7 +1402,7 @@ def is_golang_builder(self) -> bool: image_name = self.config.name return image_name == GOLANG_BUILDER_IMAGE_NAME or image_name == 'openshift/golang-builder' - def should_trigger_base_image_release(self, enabled_override: bool = False) -> bool: + def should_trigger_base_image_release(self, enabled_override: bool = False) -> BaseImageReleaseQualification: """ Determines whether this image should trigger the base image release workflow. @@ -1408,25 +1418,34 @@ def should_trigger_base_image_release(self, enabled_override: bool = False) -> b If no override is set for step 4, the enabled flag defaults to True. Returns: - bool: True if base image release workflow should be triggered, False otherwise. + :class:`BaseImageReleaseQualification`: ``trigger`` is True when the workflow should run; + ``reason`` is set when ``trigger`` is False. The result is truthy when ``trigger`` is True. """ if self.runtime.variant is BuildVariant.OKD: - return False + return BaseImageReleaseQualification( + False, "OKD variant does not use RH base image snapshot→release" + ) base_image_release_force_override = self.config.base_image_release.force if base_image_release_force_override not in [Missing, None] and bool(base_image_release_force_override): self.logger.info("Base image release force enabled from metadata config True") - return True + return BaseImageReleaseQualification(True) if self.runtime.assembly == "test": - return False + return BaseImageReleaseQualification( + False, "test assembly is excluded (--enabled-override does not bypass)" + ) if not (self.is_base_image() or self.is_golang_builder()): - return False + return BaseImageReleaseQualification( + False, + "image must be base_only or a golang builder " + "(--enabled-override does not bypass; use base_image_release.force in image metadata instead)", + ) if enabled_override: self.logger.info("Base image release enabled from --enabled-override") - return True + return BaseImageReleaseQualification(True) source = None base_image_release_enabled = True @@ -1442,26 +1461,12 @@ def should_trigger_base_image_release(self, enabled_override: bool = False) -> b source = "group config" self.logger.info(f"Base image release enabled set from {source} {base_image_release_enabled}") - return base_image_release_enabled - - def base_image_release_qualification_failure_reason(self, enabled_override: bool = False) -> str: - """ - Human-readable reason when :meth:`should_trigger_base_image_release` is False. - - Call only when qualification has already failed. - """ - if self.runtime.variant is BuildVariant.OKD: - return "OKD variant does not use RH base image snapshot→release" - if self.runtime.assembly == "test": - return "test assembly is excluded (--enabled-override does not bypass)" - if not (self.is_base_image() or self.is_golang_builder()): - return ( - "image must be base_only or a golang builder " - "(--enabled-override does not bypass; use base_image_release.force in image metadata instead)" - ) - if not enabled_override: - return "base_image_release.enabled is false in image or group config; pass --enabled-override to run anyway" - return "see image/group base_image_release settings" + if base_image_release_enabled: + return BaseImageReleaseQualification(True) + return BaseImageReleaseQualification( + False, + "base_image_release.enabled is false in image or group config; pass --enabled-override to run anyway", + ) def is_base_image_release_quay_fallback_enabled(self) -> bool: """