diff --git a/doozer/doozerlib/cli/images_streams.py b/doozer/doozerlib/cli/images_streams.py index 39a4fbab66..de559086bd 100644 --- a/doozer/doozerlib/cli/images_streams.py +++ b/doozer/doozerlib/cli/images_streams.py @@ -1444,6 +1444,22 @@ def compute_dockerfile_digest(dockerfile_path): return m.hexdigest() +def _should_preserve_member(image_entry, preserve_non_base_members): + member = image_entry.member + return preserve_non_base_members and member is not Missing and member and member != 'base-rhel9' + + +def _materialize_preserved_parents(desired_parents, preserved_parent_indexes, source_parents): + cardinality_mismatch = len(desired_parents) != len(source_parents) + if cardinality_mismatch: + return desired_parents, cardinality_mismatch + + desired_parents = desired_parents.copy() + for index in preserved_parent_indexes: + desired_parents[index] = source_parents[index] + return desired_parents, cardinality_mismatch + + def resolve_upstream_from(runtime, image_entry): """ :param runtime: The runtime object @@ -1849,6 +1865,12 @@ def search_issues(query): @click.option( '--ignore-missing-images', default=False, is_flag=True, help='Do not exit if an image is missing upstream.' ) +@click.option( + '--preserve-non-base-members', + default=False, + is_flag=True, + help='Preserve Dockerfile FROMs for member images other than base-rhel9.', +) @click.option('--draft-prs', default=False, is_flag=True, help='Open PRs as draft PRs') @click.option('--dry-run', default=False, is_flag=True, help='Do everything except any remote writes/pushes') @click.option('--moist-run', default=False, is_flag=True, help='Do everything except opening the final PRs') @@ -1873,6 +1895,7 @@ def images_streams_prs( ignore_ci_master, force_merge, ignore_missing_images, + preserve_non_base_members, draft_prs, dry_run, moist_run, @@ -1951,6 +1974,7 @@ def check_if_upstream_image_exists(upstream_image): ) # Don't check this image again since it is a little slow to do so. desired_parents = [] + preserved_parent_indexes = set() # There are two methods to find the desired parents for upstream Dockerfiles. # 1. We can analyze the image_metadata "from:" stanza and determine the upstream @@ -1966,6 +1990,10 @@ def check_if_upstream_image_exists(upstream_image): else: builders = from_config.builder or [] for builder in builders: + if _should_preserve_member(builder, preserve_non_base_members): + preserved_parent_indexes.add(len(desired_parents)) + desired_parents.append(None) + continue try: upstream_image = resolve_upstream_from(runtime, builder) except Exception as e: @@ -1978,20 +2006,26 @@ def check_if_upstream_image_exists(upstream_image): check_if_upstream_image_exists(upstream_image) desired_parents.append(upstream_image) - try: - parent_upstream_image = resolve_upstream_from(runtime, from_config) - except Exception as e: - message = f'Error while resolving upstream image for {from_config} in {dgk} for {major}.{minor}: {e}' - logger.error(message) - raise IOError(message) - if len(desired_parents) != len(builders) or not parent_upstream_image: + if len(desired_parents) != len(builders): logger.warning('Unable to find all ART equivalent upstream images for this image') continue - desired_parents.append(parent_upstream_image) - - desired_parent_digest = calc_parent_digest(desired_parents) - logger.info(f'Found desired FROM state of: {desired_parents} with digest: {desired_parent_digest}') + if _should_preserve_member(from_config, preserve_non_base_members): + preserved_parent_indexes.add(len(desired_parents)) + desired_parents.append(None) + else: + try: + parent_upstream_image = resolve_upstream_from(runtime, from_config) + except Exception as e: + message = ( + f'Error while resolving upstream image for {from_config} in {dgk} for {major}.{minor}: {e}' + ) + logger.error(message) + raise IOError(message) + if not parent_upstream_image: + logger.warning('Unable to find all ART equivalent upstream images for this image') + continue + desired_parents.append(parent_upstream_image) desired_ci_build_root_coordinate = None desired_ci_build_root_image = '' @@ -2176,6 +2210,14 @@ def check_if_upstream_image_exists(upstream_image): errors_raised = True continue + desired_parents, cardinality_mismatch = _materialize_preserved_parents( + desired_parents, preserved_parent_indexes, source_branch_parents + ) + desired_parent_digest = ( + 'n/a (cardinality mismatch)' if cardinality_mismatch else calc_parent_digest(desired_parents) + ) + logger.info(f'Found desired FROM state of: {desired_parents} with digest: {desired_parent_digest}') + source_branch_ci_build_root_coordinate = None if ci_operator_config_path.exists(): source_branch_ci_operator_config = yaml.safe_load( @@ -2194,9 +2236,13 @@ def check_if_upstream_image_exists(upstream_image): Fork build_root (in .ci-operator.yaml): {fork_ci_build_root_coordinate} ''') - if desired_parent_digest == source_branch_parent_digest and ( - desired_ci_build_root_coordinate is None - or desired_ci_build_root_coordinate == source_branch_ci_build_root_coordinate + if ( + not cardinality_mismatch + and desired_parent_digest == source_branch_parent_digest + and ( + desired_ci_build_root_coordinate is None + or desired_ci_build_root_coordinate == source_branch_ci_build_root_coordinate + ) ): green_print( 'Desired digest and source digest match; desired build_root unset OR coordinates match; Upstream is in a good state' @@ -2210,12 +2256,6 @@ def check_if_upstream_image_exists(upstream_image): pr.edit(state='closed') continue - cardinality_mismatch = False - if len(desired_parents) != len(source_branch_parents): - # The number of FROM statements in the ART metadata does not match the number - # of FROM statements in the upstream Dockerfile. - cardinality_mismatch = True - yellow_print( f'Upstream dockerfile does not match desired state in {public_repo_url}/blob/{public_branch}/{dockerfile_name}' ) diff --git a/doozer/tests/cli/test_images_streams.py b/doozer/tests/cli/test_images_streams.py index 378cc5c7f9..fcde9a2f38 100644 --- a/doozer/tests/cli/test_images_streams.py +++ b/doozer/tests/cli/test_images_streams.py @@ -163,6 +163,45 @@ def test_resolve_upstream_from_with_stream_entry(mocker, mock_runtime): mock_runtime.resolve_stream.assert_called_once_with('golang') +@pytest.mark.parametrize( + 'entry,preserve_non_base_members,expected', + [ + ({'member': 'mta-static-report'}, True, True), + ({'member': 'base-rhel9'}, True, False), + ({'stream': 'rhel-9-golang'}, True, False), + ({'member': 'mta-static-report'}, False, False), + ], +) +def test_should_preserve_member(entry, preserve_non_base_members, expected): + assert images_streams._should_preserve_member(Model(entry), preserve_non_base_members) is expected + + +def test_materialize_preserved_parents(): + desired_parents, cardinality_mismatch = images_streams._materialize_preserved_parents( + ['registry.redhat.io/openshift/golang-builder:1.25', None, 'registry.redhat.io/ubi9/ubi:9.8'], + {1}, + ['registry.redhat.io/ubi9/go-toolset:1.23', 'internal/member:current', 'registry.redhat.io/ubi9/ubi:latest'], + ) + + assert cardinality_mismatch is False + assert desired_parents == [ + 'registry.redhat.io/openshift/golang-builder:1.25', + 'internal/member:current', + 'registry.redhat.io/ubi9/ubi:9.8', + ] + + +def test_materialize_preserved_parents_with_cardinality_mismatch(): + desired_parents = ['registry.redhat.io/openshift/golang-builder:1.25', None] + + result, cardinality_mismatch = images_streams._materialize_preserved_parents( + desired_parents, {1}, ['registry.redhat.io/ubi9/go-toolset:1.23'] + ) + + assert cardinality_mismatch is True + assert result == desired_parents + + # Tests for _get_upstreaming_entries diff --git a/pyartcd/pyartcd/pipelines/open_reconciliation_prs_layered.py b/pyartcd/pyartcd/pipelines/open_reconciliation_prs_layered.py index 67f31ab7c7..4576dac0a1 100644 --- a/pyartcd/pyartcd/pipelines/open_reconciliation_prs_layered.py +++ b/pyartcd/pyartcd/pipelines/open_reconciliation_prs_layered.py @@ -205,8 +205,16 @@ async def _run_doozer_command( self._logger.info(f"Running doozer command: {cmd}") + env = os.environ.copy() + if quay_auth_file := env.get("QUAY_AUTH_FILE"): + # `oc image info` honors REGISTRY_AUTH_FILE when checking whether + # canonical parent images are accessible. + env["REGISTRY_AUTH_FILE"] = quay_auth_file + else: + env.pop("REGISTRY_AUTH_FILE", None) + # Stream output to Jenkins console in real-time - rc, stdout, stderr = await exectools.cmd_gather_async(cmd, check=check, stdout=None) + rc, stdout, stderr = await exectools.cmd_gather_async(cmd, check=check, stdout=None, env=env) return rc, stdout, stderr @@ -270,6 +278,7 @@ async def _open_reconciliation_prs(self, doozer_opts: str) -> int: ) pr_args = f"--interstitial {self.PR_INTERSTITIAL_SECONDS}" + pr_args += " --preserve-non-base-members" pr_args += ' --add-auto-labels' pr_args += ' --add-label "jira/valid-bug" --add-label "verified"' if self.add_labels: diff --git a/pyartcd/tests/pipelines/test_open_reconciliation_prs_layered.py b/pyartcd/tests/pipelines/test_open_reconciliation_prs_layered.py new file mode 100644 index 0000000000..142abb51a3 --- /dev/null +++ b/pyartcd/tests/pipelines/test_open_reconciliation_prs_layered.py @@ -0,0 +1,56 @@ +import asyncio +import os +from unittest import mock +from unittest.mock import AsyncMock + +import pytest +from pyartcd.pipelines.open_reconciliation_prs_layered import ReconcileCIUpstreamLayeredPipeline +from pyartcd.runtime import Runtime + + +@pytest.fixture +def pipeline(): + runtime = mock.MagicMock(spec=Runtime) + runtime.logger = mock.MagicMock() + runtime.doozer_working = "/workspace/doozer_working" + return ReconcileCIUpstreamLayeredPipeline(runtime, group="mta-8.1") + + +@mock.patch("pyartcd.pipelines.open_reconciliation_prs_layered.exectools.cmd_gather_async", new_callable=AsyncMock) +def test_run_doozer_command_uses_quay_auth_file_for_registry_auth(cmd_gather_async, pipeline): + cmd_gather_async.return_value = (0, "stdout", "stderr") + + with mock.patch.dict(os.environ, {"QUAY_AUTH_FILE": "/path/to/auth.json", "EXISTING_VAR": "value"}, clear=True): + result = asyncio.run(pipeline._run_doozer_command("--group mta-8.1", "images:streams prs open")) + + assert result == (0, "stdout", "stderr") + env = cmd_gather_async.await_args.kwargs["env"] + assert env["QUAY_AUTH_FILE"] == "/path/to/auth.json" + assert env["REGISTRY_AUTH_FILE"] == "/path/to/auth.json" + assert env["EXISTING_VAR"] == "value" + + +@mock.patch("pyartcd.pipelines.open_reconciliation_prs_layered.exectools.cmd_gather_async", new_callable=AsyncMock) +def test_run_doozer_command_without_quay_auth_file_preserves_anonymous_behavior(cmd_gather_async, pipeline): + cmd_gather_async.return_value = (0, "stdout", "stderr") + + with mock.patch.dict( + os.environ, {"REGISTRY_AUTH_FILE": "/path/to/inherited-auth.json", "EXISTING_VAR": "value"}, clear=True + ): + asyncio.run(pipeline._run_doozer_command("--group mta-8.1", "images:streams prs open")) + + env = cmd_gather_async.await_args.kwargs["env"] + assert "QUAY_AUTH_FILE" not in env + assert "REGISTRY_AUTH_FILE" not in env + assert env["EXISTING_VAR"] == "value" + + +def test_open_reconciliation_prs_preserves_non_base_members(pipeline): + pipeline._run_doozer_command = AsyncMock(return_value=(0, "", "")) + + with mock.patch.dict(os.environ, {"GITHUB_TOKEN": "token"}, clear=True): + result = asyncio.run(pipeline._open_reconciliation_prs("--group mta-8.1")) + + assert result == 0 + pr_args = pipeline._run_doozer_command.await_args.args[2] + assert "--preserve-non-base-members" in pr_args