Add medik8s-catalogsource Prow step with dual catalog modes#79373
Add medik8s-catalogsource Prow step with dual catalog modes#79373ugreener wants to merge 3 commits into
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a medik8s konflux-catalogsource step: ownership metadata and step definition, plus a new bash script that resolves a GitLab commit, verifies a Quay rhwa-fbc image, applies the matching ImageDigestMirrorSet, creates an OLM CatalogSource in openshift-marketplace, and waits with debug on failure. ChangesMedik8s Konflux CatalogSource Step Registry
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.sh`:
- Around line 11-12: The declaration for OCP_VERSION uses an unguarded expansion
which triggers set -u before main() can validate it; change the declaration to
use a defaulting expansion like RHWA_FBC_COMMIT_SHA (e.g., declare
OCP_VERSION="${OCP_VERSION:-}") so the script does not exit with "unbound
variable" and allow the explicit validation in main() to produce the intended
error message; update the declaration near the top (identifier OCP_VERSION) and
leave main()'s validation intact.
- Around line 14-18: The run() helper currently uses eval which enables shell
injection; change run() to accept positional args (replace eval "$cmd" with exec
of the passed arguments using "$@") and update every caller that currently
builds a single interpolated command string to call run with separate arguments
(e.g., run oc create catalogsource "$CATALOG_SOURCE_NAME" --from-latest and
similarly for commands that include "$OCP_VERSION", "$RHWA_FBC_COMMIT_SHA", and
"$node_name") so all variables are passed as safe separate parameters rather
than interpolated into one string.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 6ad2bf92-429b-425c-b398-eb81ce3f66d4
📒 Files selected for processing (4)
ci-operator/step-registry/medik8s/konflux-catalogsource/OWNERSci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.shci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.metadata.jsonci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.yaml
ab49550 to
3e31b72
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.sh (1)
61-64: 💤 Low valueFallback tag selection may not retrieve the chronologically latest image.
The
tail -1onskopeo list-tagsoutput selects the last tag in an arbitrary order (typically lexicographic), not the most recently pushed. For 40-character hex commit SHAs, this is effectively random.Since this is fallback behavior when the resolved commit lacks an image, picking any valid tag is acceptable. Consider updating the echo on line 67 from "latest" to "fallback" to avoid confusion:
- echo "Using fallback tag: $latest_tag" + echo "Using fallback tag (arbitrary valid commit): $latest_tag"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.sh` around lines 61 - 64, The fallback tag selection uses latest_tag (set by the skopeo/jq/grep/tail pipeline) but the script still echoes "latest" on failure; update the fallback wording to avoid confusion: change the echo that references "latest" to "fallback" (or similar) so it reads that a fallback tag was selected when latest_tag is used, keeping the existing latest_tag logic intact; locate references to the latest_tag variable and the echo statement near its assignment (the skopeo/jq/grep/tail pipeline and the subsequent echo) and only change the message text.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@ci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.sh`:
- Around line 61-64: The fallback tag selection uses latest_tag (set by the
skopeo/jq/grep/tail pipeline) but the script still echoes "latest" on failure;
update the fallback wording to avoid confusion: change the echo that references
"latest" to "fallback" (or similar) so it reads that a fallback tag was selected
when latest_tag is used, keeping the existing latest_tag logic intact; locate
references to the latest_tag variable and the echo statement near its assignment
(the skopeo/jq/grep/tail pipeline and the subsequent echo) and only change the
message text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 472b2409-db5d-4fb3-908b-a4e5a41738b9
📒 Files selected for processing (4)
ci-operator/step-registry/medik8s/konflux-catalogsource/OWNERSci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.shci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.metadata.jsonci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.yaml
✅ Files skipped from review due to trivial changes (2)
- ci-operator/step-registry/medik8s/konflux-catalogsource/OWNERS
- ci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.metadata.json
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.yaml
ac0d097 to
4a8b5bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.sh`:
- Around line 99-117: The ensure_marketplace() function currently returns
immediately if the openshift-marketplace namespace exists, so the pod-security
label keys (security.openshift.io/scc.podSecurityLabelSync,
pod-security.kubernetes.io/enforce, pod-security.kubernetes.io/audit,
pod-security.kubernetes.io/warn) are never applied on the pre-existing path;
change ensure_marketplace to always apply those labels: keep the existing
create-via-heredoc branch for when the namespace is missing, but after creation
(or if it already exists) run an idempotent operation to set/update the labels
(for example by applying the same Namespace manifest via oc apply -f - or by
running oc patch/label with --type=merge to add the four keys), ensuring the
labels are enforced regardless of whether the namespace was newly created or
pre-existing.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: d52338c3-7ff1-4397-b3bc-b2c4fe7766f0
📒 Files selected for processing (5)
ci-operator/step-registry/medik8s/OWNERSci-operator/step-registry/medik8s/konflux-catalogsource/OWNERSci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.shci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.metadata.jsonci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.yaml
✅ Files skipped from review due to trivial changes (1)
- ci-operator/step-registry/medik8s/OWNERS
🚧 Files skipped from review as they are similar to previous changes (2)
- ci-operator/step-registry/medik8s/konflux-catalogsource/OWNERS
- ci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.yaml
4a8b5bd to
a9010f8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.sh`:
- Around line 38-39: The network curl calls that set RHWA_FBC_COMMIT_SHA and the
later curl block (lines referenced in the diff) need explicit retry and timeout
bounds to prevent flakes/hangs: update the two curl invocations used to fetch
from ${GITLAB_API} (the assignment to RHWA_FBC_COMMIT_SHA and the subsequent
fetch at lines ~85-88) to include curl retry flags (e.g., --retry and
--retry-delay or --retry-connrefused) and a global timeout (--max-time) so
transient failures are retried and calls cannot hang indefinitely; keep existing
flags like -s -f, preserve the trailing || true behavior where present, and
choose conservative values (for example --retry 5 --retry-delay 2 and --max-time
15) or project-preferred values.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 55b15ad0-a320-43d0-b688-4a98bada128f
📒 Files selected for processing (5)
ci-operator/step-registry/medik8s/OWNERSci-operator/step-registry/medik8s/konflux-catalogsource/OWNERSci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-commands.shci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.metadata.jsonci-operator/step-registry/medik8s/konflux-catalogsource/medik8s-konflux-catalogsource-ref.yaml
✅ Files skipped from review due to trivial changes (2)
- ci-operator/step-registry/medik8s/konflux-catalogsource/OWNERS
- ci-operator/step-registry/medik8s/OWNERS
b58086f to
edfcb22
Compare
Creates a Prow step that dynamically resolves the latest medik8s/RHWA FBC catalog image and IDMS from the dragonfly/rhwa-fbc GitLab repo. Uses commit-SHA pinning to ensure IDMS and catalog image are in sync. The step: - Resolves latest dragonfly/rhwa-fbc commit SHA (or accepts override) - Fetches IDMS (images-mirror-set.yaml) from that commit - Verifies FBC catalog image exists on Quay (with fallback) - Creates CatalogSource using grpcPodConfig.extractContent (OCP 4.15+) - Waits for CatalogSource READY with full debug output on failure No credentials needed — Quay repos are public. Jira: RHWA-990
edfcb22 to
143a721
Compare
f034868 to
cbe1063
Compare
…ash-containing refs, guard oc debug with || true, add trailing newline to metadata JSON
cbe1063 to
7f79287
Compare
|
[REHEARSALNOTIFIER] Note: If this PR includes changes to step registry files ( Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@ugreener: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
There was a problem hiding this comment.
nitpick: you could use the SECONDS bash variable and decouple the counter from the sleep time
EXPIRED=$(( SECONDS + 5000 ))
while (( SECONDS < EXPIRED )); do
#.. some repeated task.
sleep 30
done
|
/lgtm from me |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: maximunited, ugreener The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Summary
medik8s-catalogsourcethat creates an OLM CatalogSource for medik8s/RHWA operator testingFBC_COMMIT_SHAfor reproducible Konflux test runsWhat this step does
Konflux mode (default,
CATALOG_MODE=konflux):dragonfly/rhwa-fbcvia GitLab API (branch configurable viaGIT_REF, defaults tomain)images-mirror-set.yamlfrom that commit, applies it as IDMS, and waits for MachineConfigPool rolloutDirect mode (
CATALOG_MODE=direct):CATALOG_IMAGE_REF(e.g., IIB, FBC, or registry.redhat.io index)Files
Test plan
Jira: RHWA-990