Skip to content

WIP Support 5.0 nightly pullspecs#2400

Open
thegreyd wants to merge 4 commits intoopenshift-eng:mainfrom
thegreyd:support_5_0_nightlies
Open

WIP Support 5.0 nightly pullspecs#2400
thegreyd wants to merge 4 commits intoopenshift-eng:mainfrom
thegreyd:support_5_0_nightlies

Conversation

@thegreyd
Copy link
Contributor

@thegreyd thegreyd commented Jan 22, 2026

OCP5 namespace differs slightly release-5 , so account for that
e.g. registry.ci.openshift.org/ocp-arm64/release-5-arm64:5.0.0-0.nightly-arm64-2026-01-21-185226

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 22, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign rayfordj for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Walkthrough

Nightly pullspec construction was centralized into a new get_nightly_pullspec(runtime, nightly) helper and callers were updated to use it. Legacy nightly-name component parsing and in-place pullspec assembly were removed; validations that enforced major.minor matching were relaxed or moved into the helper. Tests and related imports updated accordingly.

Changes

Cohort / File(s) Summary
Nightly pullspec helper
doozer/doozerlib/util.py
Added get_nightly_pullspec(runtime, nightly) to compute nightly pullspecs (parses components, validates major/minor, selects release suffix including OCP5 handling, composes registry pullspec). Export surface updated.
CLI callers - release assembly/payload
doozer/doozerlib/cli/release_gen_assembly.py, doozer/doozerlib/cli/release_gen_payload.py
Replaced manual nightly pullspec construction and component validation with calls to get_nightly_pullspec(...); added error handling for invalid nightly inputs. Removed legacy rc_suffix/reconstruction logic.
Tests
doozer/tests/cli/test_gen_assembly.py
Removed nightly-mismatch test; updated tests to mock runtime.get_major_minor_fields and added test_nightly_release_pullspecs_ocp5 to verify OCP 5 pullspec path.
Imports / removals
doozer/doozerlib/release_inspector.py, elliott/elliottlib/cli/rhcos_cli.py
Dropped unused isolate_nightly_name_components import; rhcos_cli now imports get_nightly_pullspec and removes its local implementation.
pyartcd: constants & pipeline
pyartcd/pyartcd/constants.py, pyartcd/pyartcd/pipelines/build_microshift.py
Removed NIGHTLY_PAYLOAD_REPOS constant; replaced local nightly-name parsing with get_nightly_pullspec(self.runtime, payload) and changed parse_release_payloads to instance method.
artcommon import surface
artcommon/artcommonlib/util.py
Added go_suffix_for_arch import from artcommonlib.arch_util.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

@thegreyd thegreyd force-pushed the support_5_0_nightlies branch from 8ad8d90 to bdbaa70 Compare January 22, 2026 17:27
release_suffix = f'release{rc_suffix}'
major, minor = self.runtime.get_major_minor_fields()
if (major, minor) >= (5, 0):
release_with_ver = "release-5"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have {major} here rather than 5 ? Preparing for ocp6 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It gives the impression that things are predictable .. for all we know at ocp6 it might be 6-release :D

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took a while going from OCP4 to OCP5, so I doubt there will be a OCP6 anytime soon. Let's kick the can down the road on this one.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
doozer/tests/cli/test_gen_assembly.py (1)

157-193: Update other nightly tests to mock get_major_minor_fields.

Since _get_release_pullspecs now unpacks runtime.get_major_minor_fields(), any nightly tests still only mocking get_minor_version (e.g., test_multi_nighly_arch, Line 218+) can fail before they reach the duplicate-arch assertion. Please add the same mock there to avoid test failures.

✅ Suggested fix (apply in test_multi_nighly_arch)
         runtime = MagicMock()
         runtime.get_minor_version.return_value = '4.13'
+        runtime.get_major_minor_fields.return_value = (4, 13)

@thegreyd thegreyd added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jan 22, 2026
@thegreyd thegreyd changed the title Support 5.0 nighty pullspecs Support 5.0 nightly pullspecs Jan 22, 2026
@thegreyd thegreyd changed the title Support 5.0 nightly pullspecs WIP Support 5.0 nightly pullspecs Jan 22, 2026
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 22, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 22, 2026

@thegreyd: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/security bc85012 link false /test security

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@doozer/doozerlib/util.py`:
- Around line 301-316: The validation in get_nightly_pullspec incorrectly
interpolates major_minor into the ValueError message; update the exception to
show the expected group version from runtime.get_minor_version() and the
provided nightly component (major_minor) for clarity. Locate
get_nightly_pullspec and modify the raise ValueError line so the message
references runtime.get_minor_version() as the expected value and includes
major_minor (the extracted value) and nightly to make the mismatch clear.

In `@elliott/elliottlib/cli/rhcos_cli.py`:
- Line 9: Elliott's runtime is missing get_minor_version(), which causes
doozerlib.util.get_nightly_pullspec to raise AttributeError; add a
get_minor_version(self) method to Elliott's runtime class that reads the group
config and returns the "major.minor" string (e.g., "4.12") so
get_nightly_pullspec(runtime, ...) works, or alternatively stop calling
get_nightly_pullspec and replace that call with code that computes the same
nightly pullspec without relying on runtime.get_minor_version(); locate the
usage of get_nightly_pullspec and the Elliott runtime class to implement the new
method or swap in the equivalent computation.

In `@pyartcd/pyartcd/pipelines/build_microshift.py`:
- Around line 443-449: parse_release_payloads calls
get_nightly_pullspec(self.runtime, payload) but pyartcd.runtime.Runtime lacks
the methods/attributes doozerlib expects (get_minor_version,
get_major_minor_fields, build_system), causing AttributeError; fix it by
creating a small wrapper or extracting and passing the required values directly:
inside parse_release_payloads derive major/minor using whatever
pyartcd.runtime.Runtime exposes (e.g., use runtime.version or equivalent
getters), obtain build_system from the runtime config, and then call a helper
that maps those values into the doozerlib-compatible signature (or construct an
object with get_minor_version(), get_major_minor_fields(), build_system) before
calling get_nightly_pullspec; update references to parse_release_payloads,
get_nightly_pullspec and Runtime to use the new wrapper/helper so the call no
longer relies on missing Runtime methods.
- Line 19: The call to doozerlib.get_nightly_pullspec is being passed
pyartcd.runtime.Runtime which lacks the required methods/attributes
(get_minor_version, get_major_minor_fields, build_system), causing
AttributeError when parse_release_payloads processes a nightly payload; fix by
either constructing or obtaining a doozerlib.runtime.Runtime (or equivalent
object) and passing that into get_nightly_pullspec, or refactor
get_nightly_pullspec usage in build_microshift.parse_release_payloads to call a
new helper that accepts the explicit values (minor/major_minor fields and
build_system) extracted from pyartcd.runtime.Runtime; update the call site in
parse_release_payloads and ensure the symbol get_nightly_pullspec and the
pyartcd Runtime class usage are adjusted consistently.
🧹 Nitpick comments (1)
doozer/tests/cli/test_gen_assembly.py (1)

176-191: Consider adding build_system mock for completeness.

The test validates OCP 5 nightly pullspec generation correctly. However, the get_nightly_pullspec function checks runtime.build_system == 'brew' on line 306 of util.py. The MagicMock will return another mock for build_system, which is truthy but not equal to 'brew', so it will fall through to checking uses_konflux_imagestream_override. This happens to work because version 5.0 satisfies that check, but explicitly mocking build_system would make the test more robust and clear about which code path is being tested.

Suggested enhancement
     def test_nightly_release_pullspecs_ocp5(self):
         runtime = MagicMock()
         runtime.get_minor_version.return_value = '5.0'
         runtime.get_major_minor_fields.return_value = (5, 0)
+        runtime.build_system = 'brew'  # Explicitly set build system

         gacli = GenAssemblyCli(

Comment on lines +301 to +316
def get_nightly_pullspec(runtime, nightly: str) -> str:
major_minor, brew_cpu_arch, priv = isolate_nightly_name_components(nightly)
if major_minor != runtime.get_minor_version():
raise ValueError(f"Specified nightly {nightly} does not match group major.minor {major_minor}")

if runtime.build_system == 'brew' or uses_konflux_imagestream_override(major_minor):
major, _ = runtime.get_major_minor_fields()
if major == 5:
release_suffix = "release-5"
else:
release_suffix = "release"
else:
release_suffix = 'konflux-release'

rc_suffix = go_suffix_for_arch(brew_cpu_arch, priv)
return f"registry.ci.openshift.org/ocp{rc_suffix}/{release_suffix}{rc_suffix}:{nightly}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix the error message in the validation.

The error message on line 304 references major_minor (extracted from the nightly name) instead of runtime.get_minor_version() (the expected version), making the message confusing. The message says the nightly doesn't match group major.minor {major_minor}, but major_minor is what was extracted from the nightly, not the group version.

Proposed fix
 def get_nightly_pullspec(runtime, nightly: str) -> str:
     major_minor, brew_cpu_arch, priv = isolate_nightly_name_components(nightly)
     if major_minor != runtime.get_minor_version():
-        raise ValueError(f"Specified nightly {nightly} does not match group major.minor {major_minor}")
+        raise ValueError(f"Specified nightly {nightly} does not match group major.minor {runtime.get_minor_version()}")
🧰 Tools
🪛 Ruff (0.14.13)

304-304: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents
In `@doozer/doozerlib/util.py` around lines 301 - 316, The validation in
get_nightly_pullspec incorrectly interpolates major_minor into the ValueError
message; update the exception to show the expected group version from
runtime.get_minor_version() and the provided nightly component (major_minor) for
clarity. Locate get_nightly_pullspec and modify the raise ValueError line so the
message references runtime.get_minor_version() as the expected value and
includes major_minor (the extracted value) and nightly to make the mismatch
clear.

from artcommonlib.arch_util import BREW_ARCHES, GO_ARCHES, brew_arch_for_go_arch, go_suffix_for_arch
from artcommonlib.format_util import green_print
from artcommonlib.rhcos import get_build_id_from_rhcos_pullspec, get_primary_container_name
from doozerlib.util import get_nightly_pullspec
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the rhcos_cli.py file around line 99 to see the usage context
echo "=== Checking usage at line 99 in rhcos_cli.py ==="
sed -n '90,110p' elliott/elliottlib/cli/rhcos_cli.py

# Look at the runtime class definition in elliott
echo "=== Looking for runtime class in elliott ==="
find elliott -name "runtime.py" -type f

# Search for get_minor_version and get_major_minor_fields in elliott
echo "=== Searching for required methods in elliott ==="
rg -n "def get_minor_version|def get_major_minor_fields" elliott/

# Check for build_system attribute
echo "=== Searching for build_system in elliott ==="
rg -n "build_system" elliott/ | head -20

Repository: openshift-eng/art-tools

Length of output: 3090


🏁 Script executed:

# Let's examine the runtime.py file to see all available methods
echo "=== Checking elliott runtime.py for all methods ==="
grep -n "def " elliott/elliottlib/runtime.py | head -30

# Search more broadly for get_minor_version
echo "=== Searching for get_minor_version anywhere ==="
rg -n "get_minor_version" elliott/

# Let's check the doozerlib function to understand what it actually requires
echo "=== Checking doozerlib.util for get_nightly_pullspec ==="
find . -path "*/doozerlib/util.py" -type f

Repository: openshift-eng/art-tools

Length of output: 1613


🏁 Script executed:

# Let's look at the get_nightly_pullspec function in doozerlib to see what it actually requires
echo "=== Searching for get_nightly_pullspec in doozerlib/util.py ==="
rg -A 20 "def get_nightly_pullspec" ./doozer/doozerlib/util.py

# Also check if there's a get_minor_version in doozer
echo "=== Checking for get_minor_version in doozer ==="
rg -n "def get_minor_version" ./doozer/

Repository: openshift-eng/art-tools

Length of output: 1188


🏁 Script executed:

# Final confirmation: comprehensive search for get_minor_version in entire elliott directory
echo "=== Final comprehensive search for get_minor_version in elliott ==="
find elliott -type f -name "*.py" | xargs grep -l "get_minor_version"

# Also check what get_minor_version does in doozer for understanding
echo "=== Checking get_minor_version implementation in doozer ==="
rg -A 5 "def get_minor_version" ./doozer/doozerlib/runtime.py

Repository: openshift-eng/art-tools

Length of output: 423


Elliott runtime lacks get_minor_version() method required by get_nightly_pullspec.

The function get_nightly_pullspec from doozerlib.util calls runtime.get_minor_version() immediately upon invocation. Elliott's runtime class does not have this method, so the usage at line 99 will raise an AttributeError at runtime. Either implement get_minor_version() in elliott's runtime class (returning major.minor as a string from group config) or use a different approach that doesn't rely on doozer's function.

🤖 Prompt for AI Agents
In `@elliott/elliottlib/cli/rhcos_cli.py` at line 9, Elliott's runtime is missing
get_minor_version(), which causes doozerlib.util.get_nightly_pullspec to raise
AttributeError; add a get_minor_version(self) method to Elliott's runtime class
that reads the group config and returns the "major.minor" string (e.g., "4.12")
so get_nightly_pullspec(runtime, ...) works, or alternatively stop calling
get_nightly_pullspec and replace that call with code that computes the same
nightly pullspec without relying on runtime.get_minor_version(); locate the
usage of get_nightly_pullspec and the Elliott runtime class to implement the new
method or swap in the equivalent computation.

from artcommonlib.model import Model
from artcommonlib.util import get_ocp_version_from_group, new_roundtrip_yaml_handler
from doozerlib.util import isolate_nightly_name_components
from doozerlib.util import get_nightly_pullspec
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if pyartcd Runtime has the required interface for get_nightly_pullspec

echo "=== Checking pyartcd Runtime class ==="
find pyartcd -name "runtime.py" -type f

echo "=== Checking for get_minor_version in pyartcd ==="
rg -n "def get_minor_version" pyartcd/

echo "=== Checking for get_major_minor_fields in pyartcd ==="
rg -n "def get_major_minor_fields" pyartcd/

echo "=== Checking for build_system in pyartcd runtime ==="
rg -n "build_system" pyartcd/pyartcd/runtime.py

echo "=== Examining Runtime class definition ==="
cat pyartcd/pyartcd/runtime.py

Repository: openshift-eng/art-tools

Length of output: 4432


🏁 Script executed:

#!/bin/bash
# Check what self.runtime is in build_microshift.py context

echo "=== Checking build_microshift.py for runtime usage ==="
rg -n "self.runtime" pyartcd/pyartcd/pipelines/build_microshift.py | head -20

echo "=== Checking parse_release_payloads method and context ==="
rg -B5 -A10 "def parse_release_payloads" pyartcd/pyartcd/pipelines/build_microshift.py

echo "=== Checking class definition that contains parse_release_payloads ==="
rg -B20 "def parse_release_payloads" pyartcd/pyartcd/pipelines/build_microshift.py | head -30

echo "=== Checking what get_nightly_pullspec expects ==="
find . -name "*.py" -type f -exec grep -l "def get_nightly_pullspec" {} \;

Repository: openshift-eng/art-tools

Length of output: 3012


🏁 Script executed:

#!/bin/bash
# Check get_nightly_pullspec implementation
echo "=== Getting get_nightly_pullspec implementation ==="
rg -A20 "def get_nightly_pullspec" doozer/doozerlib/util.py

echo "=== Checking MicroShiftBuildPipeline class init ==="
rg -B5 -A15 "class MicroShiftBuildPipeline" pyartcd/pyartcd/pipelines/build_microshift.py | head -40

Repository: openshift-eng/art-tools

Length of output: 1104


🏁 Script executed:

#!/bin/bash
# Get the full MicroShiftBuildPipeline class definition
echo "=== Getting MicroShiftBuildPipeline class definition ==="
rg -B2 "^class MicroShiftBuildPipeline" pyartcd/pyartcd/pipelines/build_microshift.py

echo "=== Getting MicroShiftBuildPipeline __init__ ==="
rg -A30 "class MicroShiftBuildPipeline" pyartcd/pyartcd/pipelines/build_microshift.py | head -50

echo "=== Checking type hints and documentation ==="
sed -n '1,100p' pyartcd/pyartcd/pipelines/build_microshift.py

Repository: openshift-eng/art-tools

Length of output: 3596


🏁 Script executed:

#!/bin/bash
# Check how MicroShiftBuildPipeline is instantiated
echo "=== Checking MicroShiftBuildPipeline instantiation ==="
rg -n "MicroShiftBuildPipeline\(" pyartcd/

echo "=== Checking if pyartcd Runtime has these methods anywhere ==="
rg -n "get_minor_version\|get_major_minor_fields\|build_system" pyartcd/

echo "=== Checking if artcommonlib runtime has these methods ==="
rg -n "class GroupRuntime" pyartcd/pyartcd/runtime.py -A30

echo "=== Check artcommonlib runtime ==="
find . -name "runtime.py" -path "*/artcommonlib/*" -type f

Repository: openshift-eng/art-tools

Length of output: 1333


🏁 Script executed:

#!/bin/bash
# Check artcommonlib runtime for required methods
echo "=== Checking artcommonlib runtime.GroupRuntime ==="
grep -n "def get_minor_version\|def get_major_minor_fields\|build_system" artcommon/artcommonlib/runtime.py

echo "=== Check if BuildMicroShiftPipeline is used elsewhere in codebase ==="
find pyartcd -name "*.py" -type f -exec grep -l "BuildMicroShiftPipeline" {} \;

echo "=== Check __main__ or entry points ==="
rg -n "BuildMicroShiftPipeline" pyartcd/

Repository: openshift-eng/art-tools

Length of output: 639


🏁 Script executed:

#!/bin/bash
# Get more details on artcommonlib runtime methods
echo "=== Checking artcommonlib runtime GroupRuntime class ==="
sed -n '1,200p' artcommon/artcommonlib/runtime.py

echo "=== Searching for get_minor_version in entire artcommon ==="
rg -n "def get_minor_version" artcommon/

echo "=== Searching for get_major_minor_fields in entire artcommon ==="
rg -n "def get_major_minor_fields" artcommon/

Repository: openshift-eng/art-tools

Length of output: 2792


🏁 Script executed:

#!/bin/bash
# Check where get_minor_version and get_major_minor_fields are defined
echo "=== Searching for get_minor_version in doozerlib ==="
rg -n "def get_minor_version" doozer/

echo "=== Searching for get_major_minor_fields in doozerlib ==="
rg -n "def get_major_minor_fields" doozer/

echo "=== Checking doozerlib runtime classes ==="
rg -n "class.*Runtime" doozer/doozerlib/runtime.py | head -20

Repository: openshift-eng/art-tools

Length of output: 415


🏁 Script executed:

#!/bin/bash
# Check the instantiation and context of MicroShiftBuildPipeline
echo "=== Checking MicroShiftBuildPipeline instantiation context ==="
rg -B10 -A5 "pipeline = BuildMicroShiftPipeline" pyartcd/pyartcd/pipelines/build_microshift.py

echo "=== Checking the CLI decorator and runtime parameter ==="
sed -n '650,690p' pyartcd/pyartcd/pipelines/build_microshift.py

Repository: openshift-eng/art-tools

Length of output: 1796


🏁 Script executed:

#!/bin/bash
# Check the CLI decorator and pass_runtime
echo "=== Checking pass_runtime decorator ==="
rg -B5 -A20 "def pass_runtime" pyartcd/pyartcd/cli.py

echo "=== Checking click_coroutine ==="
rg -B5 -A10 "def click_coroutine" pyartcd/pyartcd/cli.py

echo "=== Checking where build_microshift is decorated ==="
sed -n '630,670p' pyartcd/pyartcd/pipelines/build_microshift.py

Repository: openshift-eng/art-tools

Length of output: 1833


🏁 Script executed:

#!/bin/bash
# Check if there's a doozerlib Runtime being created anywhere in the context
echo "=== Checking for doozerlib.runtime.Runtime in pyartcd ==="
rg -n "from doozerlib.runtime import\|from doozerlib import.*runtime\|doozerlib.runtime.Runtime" pyartcd/

echo "=== Checking imports in build_microshift.py ==="
head -50 pyartcd/pyartcd/pipelines/build_microshift.py | grep -n "import\|from"

echo "=== Verifying doozerlib Runtime has the required methods ==="
sed -n '1306,1320p' doozer/doozerlib/runtime.py

Repository: openshift-eng/art-tools

Length of output: 2079


Runtime type mismatch: get_nightly_pullspec expects doozerlib.runtime.Runtime, not pyartcd.runtime.Runtime.

The get_nightly_pullspec function (from doozerlib) requires a runtime object with get_minor_version(), get_major_minor_fields(), and build_system attribute. The pyartcd Runtime class passed at line 448 lacks these methods and will raise an AttributeError when parse_release_payloads is invoked with a nightly name (payload without "/" character).

Either inject a doozerlib runtime instance, or refactor get_nightly_pullspec to accept the required parameters directly instead of a full runtime object.

🤖 Prompt for AI Agents
In `@pyartcd/pyartcd/pipelines/build_microshift.py` at line 19, The call to
doozerlib.get_nightly_pullspec is being passed pyartcd.runtime.Runtime which
lacks the required methods/attributes (get_minor_version,
get_major_minor_fields, build_system), causing AttributeError when
parse_release_payloads processes a nightly payload; fix by either constructing
or obtaining a doozerlib.runtime.Runtime (or equivalent object) and passing that
into get_nightly_pullspec, or refactor get_nightly_pullspec usage in
build_microshift.parse_release_payloads to call a new helper that accepts the
explicit values (minor/major_minor fields and build_system) extracted from
pyartcd.runtime.Runtime; update the call site in parse_release_payloads and
ensure the symbol get_nightly_pullspec and the pyartcd Runtime class usage are
adjusted consistently.

Comment on lines +443 to +449
async def parse_release_payloads(self, payloads: Iterable[str]):
result = {}
pullspecs = []
for payload in payloads:
if "/" not in payload:
# Convert nightly name to pullspec
# 4.12.0-0.nightly-2022-10-25-210451 ==> registry.ci.openshift.org/ocp/release:4.12.0-0.nightly-2022-10-25-210451
_, brew_cpu_arch, _ = isolate_nightly_name_components(payload)
pullspecs.append(constants.NIGHTLY_PAYLOAD_REPOS[brew_cpu_arch] + ":" + payload)
pullspec = get_nightly_pullspec(self.runtime, payload)
pullspecs.append(pullspec)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's find the runtime.py file and examine it
fd -t f "runtime\.py" pyartcd/

Repository: openshift-eng/art-tools

Length of output: 190


🏁 Script executed:

# Let's also find where get_nightly_pullspec is defined
rg -n "def get_nightly_pullspec" --type py

Repository: openshift-eng/art-tools

Length of output: 151


🏁 Script executed:

# Check the build_microshift.py file to understand the context better
cat -n pyartcd/pyartcd/pipelines/build_microshift.py | sed -n '1,50p'

Repository: openshift-eng/art-tools

Length of output: 1970


🏁 Script executed:

# Look for imports and see where get_nightly_pullspec comes from
rg -n "get_nightly_pullspec|from.*import|import " pyartcd/pyartcd/pipelines/build_microshift.py | head -30

Repository: openshift-eng/art-tools

Length of output: 1279


🏁 Script executed:

# Get the implementation of get_nightly_pullspec from doozerlib
rg -A 20 "def get_nightly_pullspec" doozer/doozerlib/util.py

Repository: openshift-eng/art-tools

Length of output: 1000


🏁 Script executed:

# Find and examine pyartcd's runtime.py
fd -path "*/pyartcd/runtime.py" -type f | xargs cat | head -300

Repository: openshift-eng/art-tools

Length of output: 235


🏁 Script executed:

# Check if pyartcd.runtime.Runtime has the suspected methods
rg -n "get_minor_version|get_major_minor_fields|build_system" pyartcd/

Repository: openshift-eng/art-tools

Length of output: 9539


🏁 Script executed:

# Find runtime.py file in pyartcd
find pyartcd -name "runtime.py" -type f

Repository: openshift-eng/art-tools

Length of output: 93


🏁 Script executed:

# Examine the Runtime class in pyartcd
cat pyartcd/pyartcd/runtime.py | head -500

Repository: openshift-eng/art-tools

Length of output: 4165


🏁 Script executed:

# Get the rest of the GroupRuntime class
cat pyartcd/pyartcd/runtime.py | tail -200

Repository: openshift-eng/art-tools

Length of output: 4165


🏁 Script executed:

# Check what artcommonlib.runtime.GroupRuntime provides
find . -path "*/artcommonlib/runtime.py" -type f | head -1 | xargs cat | head -400

Repository: openshift-eng/art-tools

Length of output: 2610


🏁 Script executed:

# Let's check the context around line 221 in build_microshift.py to see what self.runtime is
sed -n '210,230p' pyartcd/pyartcd/pipelines/build_microshift.py

Repository: openshift-eng/art-tools

Length of output: 1190


🏁 Script executed:

# Check the full parse_release_payloads method context and how self.runtime is set up
sed -n '40,100p' pyartcd/pyartcd/pipelines/build_microshift.py

Repository: openshift-eng/art-tools

Length of output: 2099


🏁 Script executed:

# Let's check the full BuildMicroShiftPipeline.__init__ to understand what runtime object is actually used
sed -n '47,85p' pyartcd/pyartcd/pipelines/build_microshift.py

Repository: openshift-eng/art-tools

Length of output: 1287


🏁 Script executed:

# Check if there are tests for parse_release_payloads to see if this actually works
rg -n "parse_release_payloads" pyartcd/ --type py

Repository: openshift-eng/art-tools

Length of output: 308


🏁 Script executed:

# Let's see the full implementation of parse_release_payloads around line 443-449
sed -n '440,460p' pyartcd/pyartcd/pipelines/build_microshift.py

Repository: openshift-eng/art-tools

Length of output: 961


🏁 Script executed:

# Final verification: check if Runtime might inherit or mixin these methods somehow
grep -n "class Runtime" pyartcd/pyartcd/runtime.py

Repository: openshift-eng/art-tools

Length of output: 84


🏁 Script executed:

# Check if there's any monkey-patching or dynamic attribute assignment
rg -n "get_minor_version|get_major_minor_fields|build_system" pyartcd/pyartcd/runtime.py

Repository: openshift-eng/art-tools

Length of output: 49


Runtime compatibility issue: get_nightly_pullspec will fail with AttributeError on pyartcd.runtime.Runtime

The code calls get_nightly_pullspec(self.runtime, payload) at line 449, but pyartcd.runtime.Runtime does not provide the required interface:

  • get_nightly_pullspec from doozerlib expects: runtime.get_minor_version(), runtime.get_major_minor_fields(), and runtime.build_system
  • pyartcd.runtime.Runtime has none of these methods or attributes

This will cause an AttributeError at runtime when the method is executed. Either:

  1. Modify parse_release_payloads to extract the necessary values and construct a doozerlib-compatible runtime wrapper, or
  2. Pass the required parameters directly to a wrapper function instead of relying on the Runtime object to provide them
🤖 Prompt for AI Agents
In `@pyartcd/pyartcd/pipelines/build_microshift.py` around lines 443 - 449,
parse_release_payloads calls get_nightly_pullspec(self.runtime, payload) but
pyartcd.runtime.Runtime lacks the methods/attributes doozerlib expects
(get_minor_version, get_major_minor_fields, build_system), causing
AttributeError; fix it by creating a small wrapper or extracting and passing the
required values directly: inside parse_release_payloads derive major/minor using
whatever pyartcd.runtime.Runtime exposes (e.g., use runtime.version or
equivalent getters), obtain build_system from the runtime config, and then call
a helper that maps those values into the doozerlib-compatible signature (or
construct an object with get_minor_version(), get_major_minor_fields(),
build_system) before calling get_nightly_pullspec; update references to
parse_release_payloads, get_nightly_pullspec and Runtime to use the new
wrapper/helper so the call no longer relies on missing Runtime methods.

Copy link
Contributor

@vfreex vfreex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2026
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2026
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants