Add medik8s disconnected CatalogSource step for air-gapped operator testing#79687
Merged
openshift-merge-bot[bot] merged 12 commits intoJun 1, 2026
Merged
Conversation
Adds a reusable Prow step that creates a CatalogSource for medik8s operator testing in both connected and disconnected environments. In connected mode (default): resolves FBC from Quay (no mirroring). In disconnected mode (MIRROR_OPERATORS=true): mirrors FBC catalog and operator images to the CI mirror registry using oc-mirror v2, creates IDMS pointing to the mirror, and creates CatalogSource from mirrored image. Based on the proven file-integrity-konflux-catalogsource pattern. Uses the IDMS from dragonfly/rhwa-fbc as the source of truth for image mirror mappings (parsed with awk, no Python/PyYAML dependency). Jira: RHWA-840 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
/pj-rehearse auto-ack |
Contributor
|
@ugreener: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Contributor
WalkthroughAdds a CI step-ref, metadata/OWNERS, and a Bash orchestration that resolves an FBC commit, optionally runs oc-mirror to mirror catalogs/operators, applies an ImageDigestMirrorSet, creates an openshift-marketplace CatalogSource, and waits for readiness while emitting diagnostics on failure. Changesmedik8s disconnected CatalogSource
Sequence DiagramsequenceDiagram
participant Script as medik8s-disconnected-catalogsource script
participant GitLab as GitLab Raw/API
participant ocMirror as oc-mirror
participant MirrorReg as Mirror Registry
participant Cluster as OpenShift Cluster
Script->>GitLab: fetch IDMS YAML / resolve FBC commit SHA
Script->>ocMirror: generate ImageSetConfiguration + run oc-mirror
ocMirror->>MirrorReg: push mirrored catalog & operator images
Script->>Cluster: apply ImageDigestMirrorSet and namespace/CatalogSource manifests
Cluster->>Cluster: MachineConfigPool rollout (Rendered config -> Updated)
Script->>Cluster: poll CatalogSource for READY and collect diagnostics on timeout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ 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 |
openshift-ci
Bot
added
the
approved
openshift-merge-bot
Bot
added
the
rehearsals-ack
Contributor
|
@ugreener: no rehearsable tests are affected by this change |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh`:
- Around line 169-173: The tar commands that create
"${ARTIFACT_DIR}/mirror-debug.tar.gz" and "${ARTIFACT_DIR}/mirror-output.tar.gz"
currently archive the entire "${TMP_DIR}", which can include sensitive files
like run/containers/auth.json; modify the archive step(s) in the script so they
explicitly exclude credentials (for example exclude patterns for
run/containers/auth.json and any kubeconfig or token files) or instead package
only known-safe subpaths under "${TMP_DIR}"; update the two tar invocations that
reference TMP_DIR and ARTIFACT_DIR to use exclusion patterns or a whitelist of
safe files to ensure no registry auth, kubeconfigs, tokens, or vault-derived
secrets are included.
🪄 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: a9974eaf-bbb7-4192-a937-9765caf27d90
📒 Files selected for processing (4)
ci-operator/step-registry/medik8s/disconnected-catalogsource/OWNERSci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.shci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-ref.metadata.jsonci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-ref.yaml
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh (1)
236-238: 💤 Low valueUse parameter expansion instead of sed for prefix removal.
This avoids spawning a subshell and is more idiomatic Bash.
✏️ Suggested fix
local original_image="${FBC_IMAGE_REPO}/${FBC_IMAGE_PREFIX}-${OCP_VERSION}:${FBC_COMMIT_SHA}" - local image_path - image_path=$(echo "$original_image" | sed 's|^quay.io/||') + local image_path="${original_image#quay.io/}" local catalog_image="${MIRROR_REGISTRY_HOST}/${image_path}"🤖 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/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh` around lines 236 - 238, The code uses sed to strip the quay.io prefix from original_image (image_path=$(echo "$original_image" | sed 's|^quay.io/||')), which spawns a subshell; replace that with Bash parameter expansion to remove the prefix in-place (assign image_path="${original_image#quay.io/}") and keep the subsequent catalog_image="${MIRROR_REGISTRY_HOST}/${image_path}" unchanged so no external process is invoked.
🤖 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/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh`:
- Around line 236-238: The code uses sed to strip the quay.io prefix from
original_image (image_path=$(echo "$original_image" | sed 's|^quay.io/||')),
which spawns a subshell; replace that with Bash parameter expansion to remove
the prefix in-place (assign image_path="${original_image#quay.io/}") and keep
the subsequent catalog_image="${MIRROR_REGISTRY_HOST}/${image_path}" unchanged
so no external process is invoked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5deb82da-cdc2-42d3-9195-5ea957706b5a
📒 Files selected for processing (2)
ci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.shci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-ref.yaml
ugreener
force-pushed
the
rhwa-disconnected-catalogsource
branch
from
43979aa to
46cb286
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh (1)
237-239: 💤 Low valueUse parameter expansion instead of sed.
Replace the
sedcommand with bash parameter expansion for stripping the prefix.♻️ Proposed fix
local original_image="${FBC_IMAGE_REPO}/${FBC_IMAGE_PREFIX}-${OCP_VERSION}:${FBC_COMMIT_SHA}" - local image_path - image_path=$(echo "$original_image" | sed 's|^quay.io/||') + local image_path="${original_image#quay.io/}" local catalog_image="${MIRROR_REGISTRY_HOST}/${image_path}"🤖 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/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh` around lines 237 - 239, Replace the sed invocation used to strip the quay.io prefix with bash parameter expansion: set image_path using image_path="${original_image#quay.io/}" (preserving the existing local image_path declaration) and leave catalog_image="${MIRROR_REGISTRY_HOST}/${image_path}" unchanged; this removes the external sed call and achieves the same prefix removal using shell expansion.
🤖 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/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh`:
- Around line 237-239: Replace the sed invocation used to strip the quay.io
prefix with bash parameter expansion: set image_path using
image_path="${original_image#quay.io/}" (preserving the existing local
image_path declaration) and leave
catalog_image="${MIRROR_REGISTRY_HOST}/${image_path}" unchanged; this removes
the external sed call and achieves the same prefix removal using shell
expansion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2abb77b0-4329-4f0e-aa8d-1c45f252f8f9
📒 Files selected for processing (2)
ci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.shci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-ref.yaml
The connected mode was redundant with the existing medik8s-catalogsource step (PR openshift#79373). This step is now disconnected-only: - Removed MIRROR_OPERATORS env var (always mirrors) - Mirror registry is required (fails if not found) - Documentation points to medik8s-catalogsource for connected use Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ugreener
force-pushed
the
rhwa-disconnected-catalogsource
branch
from
46cb286 to
cf18458
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh (1)
239-241: 💤 Low valueUse bash parameter expansion instead of sed.
Per shellcheck SC2001, bash parameter expansion is more efficient and idiomatic here.
♻️ Suggested fix
local original_image="${FBC_IMAGE_REPO}/${FBC_IMAGE_PREFIX}-${OCP_VERSION}:${FBC_COMMIT_SHA}" - local image_path - image_path=$(echo "$original_image" | sed 's|^quay.io/||') + local image_path="${original_image#quay.io/}" local catalog_image="${MIRROR_REGISTRY_HOST}/${image_path}"🤖 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/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh` around lines 239 - 241, Replace the sed call that strips the "quay.io/" prefix with bash parameter expansion: instead of using image_path=$(echo "$original_image" | sed 's|^quay.io/||'), assign image_path using parameter expansion to remove the leading "quay.io/" (e.g., use the ${original_image#quay.io/} form) and then construct catalog_image with "${MIRROR_REGISTRY_HOST}/${image_path}"; keep the local declarations (local image_path) or combine declaration+assignment but ensure you reference original_image and MIRROR_REGISTRY_HOST exactly as in the diff.
🤖 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/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh`:
- Around line 239-241: Replace the sed call that strips the "quay.io/" prefix
with bash parameter expansion: instead of using image_path=$(echo
"$original_image" | sed 's|^quay.io/||'), assign image_path using parameter
expansion to remove the leading "quay.io/" (e.g., use the
${original_image#quay.io/} form) and then construct catalog_image with
"${MIRROR_REGISTRY_HOST}/${image_path}"; keep the local declarations (local
image_path) or combine declaration+assignment but ensure you reference
original_image and MIRROR_REGISTRY_HOST exactly as in the diff.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8915c108-394a-4c93-8f7b-e01884d42e10
📒 Files selected for processing (2)
ci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.shci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-ref.yaml
Contributor
Author
|
Fixed in the latest push — both |
Infrastructure references (dragonfly/rhwa-fbc, rhwa-tenant) are unchanged as they are real external resource names.
Use generic naming for the SHARED_DIR output file.
ugreener
force-pushed
the
rhwa-disconnected-catalogsource
branch
from
39b1111 to
d495361
Compare
razo7
suggested changes
May 31, 2026
Port verify_fbc_image() from the connected medik8s-catalogsource step to catch missing FBC images on Quay before the expensive oc-mirror operation. Adds QUAY_REPO_PATH and FBC_SHA_PINNED variables for parity with the connected step. Addresses review comment from @razo7.
Rename fbc_commit_sha to rhwa_fbc_commit_sha for consistency with the connected medik8s-catalogsource step, so downstream consumers can use the same filename regardless of connected/disconnected mode. Addresses review comment from @razo7.
Relax regex from ^[0-9]{3,4}$ to ^[0-9]{2,4}$ to match the connected
medik8s-catalogsource step and support 2-digit OCP versions.
Addresses review comment from @razo7.
Replace hardcoded quay.io/redhat-user-workloads/rhwa-tenant with values derived from FBC_IMAGE_REPO using parameter expansion, so the IDMS source entry stays in sync if the repo path changes. Addresses review comment from @razo7.
Addresses review comment from @razo7.
razo7
reviewed
May 31, 2026
Member
razo7
left a comment
razo7
left a comment
There was a problem hiding this comment.
Looks good, and after it gets merged, we can do some refactoring to avoid creating similar functions in multiple places #79896.
Keeping it for QE review as they are much more familiar with the disconnected setup than me for the final approval. You may unhold if you think the other way...
/hold
openshift-ci
Bot
added
the
do-not-merge/hold
razo7
approved these changes
May 31, 2026
ugreener
changed the title
The step-registry-metadata CI check runs `make registry-metadata` which generates JSON without trailing newlines and performs a byte-for-byte comparison. The trailing newline added in af3c94a breaks this check. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai resume |
Contributor
✅ Actions performedReviews resumed. |
mhabashrh
reviewed
May 31, 2026
There was a problem hiding this comment.
did you add the nexus? , i am not sure i find this step
in the disconnected env , it differs than the connected
have you tried it manually?
Contributor
Author
There was a problem hiding this comment.
This step does not use Nexus. Nexus is a registry proxy used in Jenkins-based CI pipelines (e.g., ocp-edge lab); this is a Prow step-registry step, which uses a different infrastructure. In Prow, disconnected steps mirror images to the CI mirror registry using vault-mounted openshift-custom-mirror-registry credentials and oc-mirror v2.
This follows the same pattern as file-integrity-konflux-catalogsource (same credential, same oc-mirror workflow, same CI mirror registry).
The flow: resolve FBC commit from GitLab, verify image on Quay, mirror catalog + operator images via oc-mirror v2 to the CI mirror registry, create IDMS pointing to the mirror, create CatalogSource from the mirrored image.
Not yet tested in a live disconnected cluster (waiting for the periodic job configs in RHWA-1038/RHWA-1039 to be created). The step logic is adapted from the file-integrity step which runs successfully in CI.
- Add node-side podman pull diagnostic on CatalogSource timeout (matches connected step's debug block for diagnosing image pull failures at the node level) - Add elapsed seconds to wait loop log for triage - Remove unnecessary credential copy to SHARED_DIR (no downstream consumer; reduces credential exposure surface) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
[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: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh (1)
356-362:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLog CatalogSource wait elapsed time, not shell uptime.
Line 362 uses the shell-global
SECONDS, so after the earlier mirroring/setup work this log no longer reflects how longwait_for_catalogsource()has actually been polling. Capture a start timestamp for the wait loop and logSECONDS - start_secondsinstead.Suggested fix
wait_for_catalogsource() { log "Waiting for CatalogSource ${CATALOG_SOURCE_NAME} to be READY..." - local -i deadline=$(( SECONDS + 600 )) + local -i start_seconds=$SECONDS + local -i deadline=$(( start_seconds + 600 )) local status="" while (( SECONDS < deadline )); do status=$(oc -n openshift-marketplace get catalogsource "$CATALOG_SOURCE_NAME" \ -o=jsonpath="{.status.connectionState.lastObservedState}" 2>/dev/null || true) - log " $(( SECONDS ))s - status: ${status:-pending}" + log " $(( SECONDS - start_seconds ))s - status: ${status:-pending}"🤖 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/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh` around lines 356 - 362, The log in wait_for_catalogsource() currently uses the global SECONDS, which yields shell uptime rather than the function's polling duration; capture a start timestamp at the top of wait_for_catalogsource() (e.g., start_seconds=$(date +%s) or start_seconds=$SECONDS) and change the log line to print elapsed time using SECONDS - start_seconds (or $(($(date +%s) - start_seconds)) when using date) so the message reflects how long the function has been polling the CatalogSource.
🤖 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/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh`:
- Around line 382-384: The current call uses run() which echoes the full oc
debug/podman pull command (including the MIRROR_REGISTRY host) into CI logs;
replace the run invocation for the node-side probe by invoking the oc debug
command directly (not via run) so it is not printed, redirect stdout/stderr to
/dev/null (e.g. >/dev/null 2>&1), capture its exit code, and then use log to
emit only a success or failure message referencing node_name (but do NOT include
catalog_image or the registry host). Specifically, remove run(...) around the oc
debug invocation that uses catalog_image, perform the oc debug "node/$node_name"
-- chroot /host podman pull --authfile /var/lib/kubelet/config.json
"${catalog_image}" quietly, check the command's exit status, and call log
"node-side pull succeeded on ${node_name}" or log "node-side pull failed on
${node_name}" accordingly.
---
Outside diff comments:
In
`@ci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh`:
- Around line 356-362: The log in wait_for_catalogsource() currently uses the
global SECONDS, which yields shell uptime rather than the function's polling
duration; capture a start timestamp at the top of wait_for_catalogsource()
(e.g., start_seconds=$(date +%s) or start_seconds=$SECONDS) and change the log
line to print elapsed time using SECONDS - start_seconds (or $(($(date +%s) -
start_seconds)) when using date) so the message reflects how long the function
has been polling the CatalogSource.
🪄 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: c0518644-2ac2-48bf-b0ff-fe33fac22263
📒 Files selected for processing (1)
ci-operator/step-registry/medik8s/disconnected-catalogsource/medik8s-disconnected-catalogsource-commands.sh
|
/lgtm from my side |
Contributor
Author
|
/unhold |
openshift-ci
Bot
removed
the
do-not-merge/hold
|
/lgtm |
|
LGTM 👍 |
Contributor
Author
|
/retest |
1 similar comment
Contributor
Author
|
/retest |
razo7
approved these changes
Jun 1, 2026
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: maximunited, razo7, 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 |
openshift-merge-bot
Bot
merged commit 2c97543
into
openshift:main
10 of 11 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a reusable Prow step
medik8s-disconnected-catalogsourcethat mirrors medik8s FBC catalog and operator images to the CI mirror registry via oc-mirror v2, creates IDMS pointing to the mirror, and provisions a CatalogSource for air-gapped operator testing. Complements the existingmedik8s-catalogsourcestep which handles connected environments.What this step does
dragonfly/rhwa-fbcvia GitLab APIregistries.conffor oc-mirror (mapsregistry.redhat.ioto Quay mirrors)oc-mirror v2to the CI mirror registryProven pattern
Based on the file-integrity-konflux-catalogsource step. Uses
openshift-custom-mirror-registrycredential (same as kueue, file-integrity disconnected steps). IDMS source mappings are parsed from the rhwa-fbc repo using awk.Files
Dependencies
This step will be used by future disconnected periodic job configs (RHWA-1038, RHWA-1039).
Requires
cucushift-installer-rehearse-aws-ipi-disconnected-privateworkflow for cluster provisioning.Jira: RHWA-840