OLS-0000 Add otel-collector image mirror to quickstart install - #390
OLS-0000 Add otel-collector image mirror to quickstart install#390xrajesh wants to merge 1 commit into
Conversation
The OLS bundle CSV references the otel-collector image at registry.redhat.io which may not be published yet. Add a new step (3/10) that creates an ImageDigestMirrorSet to pull the same digest from the Konflux quay.io repo as a fallback. The step is idempotent (skips if IDMS already exists) and can be disabled with SKIP_OTEL_MIRROR=1. After creating the IDMS, it waits for MachineConfigPool to finish updating before proceeding with the OLS install. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe quickstart installer now includes an optional OpenTelemetry image mirror configuration step, waits for the MachineConfigPool update, and renumbers the complete installation flow from 9 stages to 10. ChangesQuickstart installer
Sequence Diagram(s)sequenceDiagram
participant Installer
participant OpenShiftAPI
participant MachineConfigPool
Installer->>OpenShiftAPI: Apply ImageDigestMirrorSet
OpenShiftAPI->>MachineConfigPool: Start configuration update
Installer->>MachineConfigPool: Poll update status
MachineConfigPool-->>Installer: Report completed update
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@hack/quickstart/install.sh`:
- Around line 519-523: Update the IDMS handling around IDMS_NAME so an existing
ImageDigestMirrorSet is not skipped solely because its name matches. Compare its
configured source and target with the values derived from the current
environment, and reconcile or apply the manifest when they differ; retain the
skip behavior only when the existing configuration already matches.
- Around line 539-543: The MCP wait loop around mcp_updating must fail closed:
capture and validate the oc command result instead of masking failures with “||
true”, and only break after every MachineConfigPool reports the required
post-apply updated/settled state. Keep retrying on API errors, empty output, or
stale Updating=False status until MCP_WAIT_TIMEOUT, then follow the existing
timeout failure path.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f9a05c8a-3635-431b-95ab-7189b376020a
📒 Files selected for processing (1)
hack/quickstart/install.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
| if [ "${SKIP_OTEL_MIRROR:-}" = "1" ]; then | ||
| info "Skipped (SKIP_OTEL_MIRROR=1)" | ||
| elif oc get imagedigestmirrorset "${IDMS_NAME}" >/dev/null 2>&1; then | ||
| info "ImageDigestMirrorSet ${IDMS_NAME} already exists — skipping" | ||
| else |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reconcile existing IDMS configuration instead of trusting its name.
This branch skips any existing ols-otel-collector-mirror, even when its source or target differs from the current environment variables. Compare the existing spec with the desired values, or apply the manifest on every run and wait only when it changes.
🤖 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 `@hack/quickstart/install.sh` around lines 519 - 523, Update the IDMS handling
around IDMS_NAME so an existing ImageDigestMirrorSet is not skipped solely
because its name matches. Compare its configured source and target with the
values derived from the current environment, and reconcile or apply the manifest
when they differ; retain the skip behavior only when the existing configuration
already matches.
| while (( local_elapsed < MCP_WAIT_TIMEOUT )); do | ||
| mcp_updating="$(oc get mcp -o jsonpath='{range .items[*]}{.status.conditions[?(@.type=="Updating")].status}{" "}{end}' 2>/dev/null || true)" | ||
| if [[ "${mcp_updating}" != *"True"* ]]; then | ||
| info "MachineConfigPool updated" | ||
| break |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the MCP wait fail closed.
|| true converts oc failures into empty output, and the predicate treats empty output or Updating=False as settled without requiring every pool to report the post-apply configuration as updated. A transient API error or stale status can let installation continue before nodes receive the mirror configuration.
🤖 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 `@hack/quickstart/install.sh` around lines 539 - 543, The MCP wait loop around
mcp_updating must fail closed: capture and validate the oc command result
instead of masking failures with “|| true”, and only break after every
MachineConfigPool reports the required post-apply updated/settled state. Keep
retrying on API errors, empty output, or stale Updating=False status until
MCP_WAIT_TIMEOUT, then follow the existing timeout failure path.
|
@xrajesh: 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. |
|
I can confirm this fixes the issue at hand. |
|
/lgtm |
|
@xrajesh this does not work on HyperShift-hosted clusters |
openshift-ci
Bot
added
the
needs-rebase
|
PR needs rebase. 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. |
3 participants
Summary
install.shthat creates anImageDigestMirrorSetfor the otel-collector imageregistry.redhat.io/openshift-lightspeed/otelcol-lightspeed-rhel9→quay.io/redhat-user-workloads/crt-nshift-lightspeed-tenant/lightspeed-otel-collectorso the cluster can pull the image when registry.redhat.io hasn't published it yetSKIP_OTEL_MIRROR=1MCP_WAIT_TIMEOUT, default 600s)Test plan
install.shon a cluster without prior IDMS — verify IDMS is created and MCP wait completesinstall.shagain — verify IDMS step is skipped ("already exists")SKIP_OTEL_MIRROR=1— verify step is skippeduninstall.sh+ reinstall — full cycle works🤖 Generated with Claude Code