Add --extra-image-nvrs option to release-from-fbc pipeline

[ashwindasr]

Summary

• Add --extra-image-nvrs CLI option to artcd release-from-fbc for extra image NVRs not part of the FBC
• Make --fbc-pullspecs optional (at least one of --fbc-pullspecs or --extra-image-nvrs must be provided)
• Extra NVRs are merged into the same image shipment file as FBC-related images (not a separate shipment)
• Skip FBC extraction/validation when no FBC pullspecs are provided (extra NVRs only)
• Validate extra image NVRs are not FBC builds before merging
• Add unit tests for init, categorization, and parameter handling

Context

Sometimes we need to release builds that are not part of an FBC alongside the FBC and its related builds. The extra image NVRs are appended to categorized_nvrs['image'] so they share the same image snapshot and shipment file as the FBC-extracted related images.

Depends on

• Add EXTRA_IMAGE_NVRS parameter to release-from-fbc job aos-cd-jobs#4623 for the Jenkins EXTRA_IMAGE_NVRS parameter

[openshift-ci]

[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 ximinhan 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:

• OWNERS

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

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 08a1e9ce-1c68-414e-9b6d-bfeaf11a4290

📥 Commits

Reviewing files that changed from the base of the PR and between 91e0f11 and eb5216d.

📒 Files selected for processing (1)

• pyartcd/tests/pipelines/test_release_from_fbc.py

✅ Files skipped from review due to trivial changes (1)

• pyartcd/tests/pipelines/test_release_from_fbc.py

---

Walkthrough

Adds optional extra image NVRs to ReleaseFromFbcPipeline (stored as a list), makes --fbc-pullspecs optional, merges validated extra image NVRs into the image category during run with order-preserving deduplication, and adds unit/CLI tests for these behaviors. (47 words)

Changes

Cohort / File(s)	Summary
Pipeline Core pyartcd/pyartcd/pipelines/release_from_fbc.py	Constructor accepts extra_image_nvrs: Optional[List[str]] and normalizes to a list. run() logs when extras present, skips FBC extraction when fbc_pullspecs is empty, initializes related/fbc lists lazily, only errors if both extracted NVRs and extras are empty, validates extras are not *-fbc, and merges extras into categorized_nvrs['image'] with order-preserving deduplication. CLI: --fbc-pullspecs made optional, new --extra-image-nvrs added and parsed, release_from_fbc signature updated to pass extras.
Tests pyartcd/tests/pipelines/test_release_from_fbc.py	Adds tests covering extra_image_nvrs init semantics (default to empty list, preserving provided list, None→[]), NVR categorization (image vs fbc), run-time validation rejecting FBC-like extras, and CLI validation requiring at least one of --fbc-pullspecs or --extra-image-nvrs (using patched pipeline to avoid execution).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

pyartcd/tests/pipelines/test_release_from_fbc.py (1)

75-106: Please exercise the new run() and CLI branches too.

The feature addition lives in run() and release_from_fbc(), but this file currently stops at categorize_nvrs(). One extra-NVR-only run() test plus one “both inputs empty” CLI test would cover the new control flow that can regress here.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pyartcd/tests/pipelines/test_release_from_fbc.py` around lines 75 - 106,
Tests only cover categorize_nvrs(); add unit tests to exercise the new run() and
CLI control flow so changes in run() and release_from_fbc() aren’t missed. Add
one test in TestReleaseFromFbcCategorize that constructs a pipeline with
extra_image_nvrs (use _make_pipeline) and calls run() to assert the extra NVRs
are merged into the final output (verify behavior of ReleaseFromFbcPipeline.run
and release_from_fbc path), and add a second test that simulates CLI invocation
(e.g., call the module's CLI entry or main function) with both inputs empty to
assert it follows the expected empty-input branch. Use existing helpers
(_make_pipeline, pipeline.run, and the CLI entry point/release_from_fbc function
names) to locate where to add these tests.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pyartcd/pyartcd/pipelines/release_from_fbc.py`:
- Around line 593-596: The extra_image_nvrs list is not validated before being
merged into categorized_nvrs['image'], which allows malformed NVRs or *-fbc
builds to slip through; before calling
categorized_nvrs['image'].extend(self.extra_image_nvrs) validate each entry in
self.extra_image_nvrs by running the same parsing/validation used by
categorize_nvrs()/parse_nvr() (or canonicalize them), filter out or raise on
invalid NVRs (and reject *-fbc builds), and only extend with the
normalized/validated NVRs so downstream elliott snapshot/new and shipment
generation receive correct values.

---

Nitpick comments:
In `@pyartcd/tests/pipelines/test_release_from_fbc.py`:
- Around line 75-106: Tests only cover categorize_nvrs(); add unit tests to
exercise the new run() and CLI control flow so changes in run() and
release_from_fbc() aren’t missed. Add one test in TestReleaseFromFbcCategorize
that constructs a pipeline with extra_image_nvrs (use _make_pipeline) and calls
run() to assert the extra NVRs are merged into the final output (verify behavior
of ReleaseFromFbcPipeline.run and release_from_fbc path), and add a second test
that simulates CLI invocation (e.g., call the module's CLI entry or main
function) with both inputs empty to assert it follows the expected empty-input
branch. Use existing helpers (_make_pipeline, pipeline.run, and the CLI entry
point/release_from_fbc function names) to locate where to add these tests.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5b8e803e-d4be-416f-85b5-cd234a9cc280

📥 Commits

Reviewing files that changed from the base of the PR and between 5450c7a and 01be065.

📒 Files selected for processing (2)

• pyartcd/pyartcd/pipelines/release_from_fbc.py
• pyartcd/tests/pipelines/test_release_from_fbc.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pyartcd/pyartcd/pipelines/release_from_fbc.py`:
- Around line 778-781: The merged image NVR list can contain duplicates because
categorized_nvrs['image'] may already include entries also passed via
self.extra_image_nvrs and create_snapshot() writes the merged list verbatim; fix
by deduplicating before snapshot creation: after extending
categorized_nvrs['image'] with self.extra_image_nvrs (or instead of extending),
produce a de-duplicated list that preserves order (e.g., use an order-preserving
dedupe like list(dict.fromkeys(...)) or similar) and assign that back to
categorized_nvrs['image'] so create_snapshot() receives a unique set of image
NVRs.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f4b6c4e8-4941-4812-b41f-1ea407527da8

📥 Commits

Reviewing files that changed from the base of the PR and between 01be065 and a293229.

📒 Files selected for processing (1)

• pyartcd/pyartcd/pipelines/release_from_fbc.py

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pyartcd/pyartcd/pipelines/release_from_fbc.py`:
- Around line 871-874: The code merges self.extra_image_nvrs into
categorized_nvrs['image'] without validating they are not FBC builds; update the
logic before the merge in the same block to check each NVR in
self.extra_image_nvrs for an FBC component (e.g., component name ending with
"-fbc" or using the same detection helper used elsewhere), and either raise a
validation error or separate them into the FBC category instead of adding them
to categorized_nvrs['image']; then perform the deduplicated merge into
categorized_nvrs['image'] only with the validated non-FBC NVRs.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3b2f88aa-8b33-4a58-becf-42d8e32b418e

📥 Commits

Reviewing files that changed from the base of the PR and between a293229 and f6db455.

📒 Files selected for processing (2)

• pyartcd/pyartcd/pipelines/release_from_fbc.py
• pyartcd/tests/pipelines/test_release_from_fbc.py

🚧 Files skipped from review as they are similar to previous changes (1)

• pyartcd/tests/pipelines/test_release_from_fbc.py

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

pyartcd/pyartcd/pipelines/release_from_fbc.py (1)

848-879: Consider fail-fast validation of --extra-image-nvrs before FBC extraction.

extra_image_nvrs validation currently happens after FBC-related extraction/validation (Line 848-Line 858). Moving Line 873-Line 877 earlier would fail faster and avoid unnecessary network/CLI work when invalid extras are provided.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pyartcd/pyartcd/pipelines/release_from_fbc.py` around lines 848 - 879, Move
the validation of self.extra_image_nvrs to run before any FBC
extraction/validation: check for FBC-like extras with parse_nvr(...
)['name'].endswith('-fbc') and raise the same RuntimeError if any are found, and
log the "Adding X extra image NVRs..." message only after the check passes;
perform this check before calling
validate_fbc_related_images(self.fbc_pullspecs) and before building fbc_nvrs via
extract_fbc_nvr, so invalid --extra-image-nvrs fail fast and avoid unnecessary
calls to validate_fbc_related_images and extract_fbc_nvr (keep categorize_nvrs
usage unchanged).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pyartcd/tests/pipelines/test_release_from_fbc.py`:
- Around line 583-614: The tests in TestCliValidation duplicate parsing logic
instead of exercising the actual Click command; replace the inline parsing in
test_both_empty_raises_error, test_fbc_only_does_not_raise, and
test_extra_nvrs_only_does_not_raise with real CLI invocations using
click.testing.CliRunner to invoke the release-from-fbc command (the Click
command function or entrypoint used in the application), passing the
corresponding --fbc-pullspecs and --extra-image-nvrs arguments; assert that
invoking with both empty yields a non-zero exit code and that the output
contains "At least one of", and assert that invoking with only --fbc-pullspecs
or only --extra-image-nvrs exits zero (or does not raise) and returns expected
output or success exit code.
- Around line 560-580: The tests duplicate the validation logic instead of
invoking the pipeline behavior; update test_fbc_nvr_in_extra_image_nvrs_raises
and test_non_fbc_nvrs_pass_validation to call the pipeline's runtime validation
(e.g., invoke pipeline.run() or the pipeline's public validation method) rather
than reimplementing parse_nvr filtering, assert that pipeline.run() raises
RuntimeError when extra_image_nvrs contains an -fbc NVR and that it completes
(no exception) for a non-fbc NVR; remove the inline parse_nvr/filter logic and
rely on the pipeline's existing validation code paths to avoid false positives
on regressions.

---

Nitpick comments:
In `@pyartcd/pyartcd/pipelines/release_from_fbc.py`:
- Around line 848-879: Move the validation of self.extra_image_nvrs to run
before any FBC extraction/validation: check for FBC-like extras with
parse_nvr(... )['name'].endswith('-fbc') and raise the same RuntimeError if any
are found, and log the "Adding X extra image NVRs..." message only after the
check passes; perform this check before calling
validate_fbc_related_images(self.fbc_pullspecs) and before building fbc_nvrs via
extract_fbc_nvr, so invalid --extra-image-nvrs fail fast and avoid unnecessary
calls to validate_fbc_related_images and extract_fbc_nvr (keep categorize_nvrs
usage unchanged).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8f9396d4-6835-4fdd-840b-87aff91d3c57

📥 Commits

Reviewing files that changed from the base of the PR and between f6db455 and 91e0f11.

📒 Files selected for processing (2)

• pyartcd/pyartcd/pipelines/release_from_fbc.py
• pyartcd/tests/pipelines/test_release_from_fbc.py

[openshift-ci]

@ashwindasr: 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	eb5216d	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.