Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 60 additions & 20 deletions doozer/doozerlib/cli/images_streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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 = ''
Expand Down Expand Up @@ -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(
Expand All @@ -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'
Expand All @@ -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}'
)
Expand Down
39 changes: 39 additions & 0 deletions doozer/tests/cli/test_images_streams.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
11 changes: 10 additions & 1 deletion pyartcd/pyartcd/pipelines/open_reconciliation_prs_layered.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return rc, stdout, stderr

Expand Down Expand Up @@ -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:
Expand Down
56 changes: 56 additions & 0 deletions pyartcd/tests/pipelines/test_open_reconciliation_prs_layered.py
Original file line number Diff line number Diff line change
@@ -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
Loading