OLS-3349: Wire agentic operand images into related_images.json and OLM bundle#1775
OLS-3349: Wire agentic operand images into related_images.json and OLM bundle#1775blublinsky wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@blublinsky: This pull request references OLS-3349 which is a valid jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAgentic console and alerts adapter image handling now uses related-image metadata across controller defaults, bundle generation, snapshot tooling, and CSV wiring. The specs and manifests were updated to match the new deployment ownership and reconciliation behavior, and the test target’s Go test parallelism was reduced. ChangesAgentic console productization and image flow
Estimated code review effort: 3 (Moderate) | ~25 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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
🧹 Nitpick comments (2)
.ai/spec/what/bundle-composition.md (1)
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: reword "Prior to this migration" for brevity.
The static analysis hint flags "Prior to" as wordy. Consider "Before this migration" or "Earlier" to tighten the sentence.
🤖 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 @.ai/spec/what/bundle-composition.md at line 35, Shorten the phrase “Prior to this migration” in the bundle-composition spec to a briefer alternative such as “Before this migration” or “Earlier.” Update the wording in the affected sentence only, keeping the meaning intact and preserving the references to agentic-operator, lightspeed-operator reconciliation, AgenticConsolePluginReady, and agenticconsole.RemoveAgenticConsole().Source: Linters/SAST tools
hack/snapshot_to_image_list.sh (1)
79-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnmanaged
snapshot.json/bundle_snapshot.jsonwritten to CWD.These files are copied into the current working directory alongside the
mktemptemp files but are never cleaned up (only the temp files are removed incleanup). If they are debug artifacts, consider dropping them; otherwise they should be cleaned up or written to a known location to avoid polluting the repo root.🤖 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/snapshot_to_image_list.sh` around lines 79 - 90, The snapshot export logic in snapshot_to_image_list.sh is writing unmanaged snapshot.json and bundle_snapshot.json into the current working directory and never cleaning them up. Update the script around the cp and cleanup flow to avoid leaving these artifacts in the repo root: either remove those copies entirely if they are only for debugging, or write them to a controlled temporary/output location and ensure they are deleted in cleanup alongside TMP_OLS_SNAPSHOT and TMP_BUNDLE_SNAPSHOT.
🤖 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 @.ai/spec/what/bundle-composition.md:
- Line 34: The bundle-composition spec still marks the agentic console plugin
ownership note as planned, which conflicts with the now-completed deployment
change. Update the relevant sentence in what/bundle-composition.md to remove the
[PLANNED: OLS-3236] marker from the Lightspeed/agentic console plugin ownership
statement, keeping the text aligned with the “Agentic Operand Deployment”
section and the OLS-3236 summary that now describes the finished CSV cleanup.
In `@bundle/manifests/lightspeed-operator.clusterserviceversion.yaml`:
- Around line 921-922: The bundle still references unpinned external agentic
images, so update the related image references used by the CSV and bundle
generation flow to mirror and digest-pin the values for
lightspeed-agentic-console-plugin and lightspeed-agentic-alerts-adapter. Make
the change where the bundle manifest values are sourced so the resulting
related_images.json no longer points to quay.io/redhat-user-workloads/...:main
or quay.io/tremes/laa, then regenerate the bundle artifacts.
---
Nitpick comments:
In @.ai/spec/what/bundle-composition.md:
- Line 35: Shorten the phrase “Prior to this migration” in the
bundle-composition spec to a briefer alternative such as “Before this migration”
or “Earlier.” Update the wording in the affected sentence only, keeping the
meaning intact and preserving the references to agentic-operator,
lightspeed-operator reconciliation, AgenticConsolePluginReady, and
agenticconsole.RemoveAgenticConsole().
In `@hack/snapshot_to_image_list.sh`:
- Around line 79-90: The snapshot export logic in snapshot_to_image_list.sh is
writing unmanaged snapshot.json and bundle_snapshot.json into the current
working directory and never cleaning them up. Update the script around the cp
and cleanup flow to avoid leaving these artifacts in the repo root: either
remove those copies entirely if they are only for debugging, or write them to a
controlled temporary/output location and ensure they are deleted in cleanup
alongside TMP_OLS_SNAPSHOT and TMP_BUNDLE_SNAPSHOT.
🪄 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: 48a97b4f-4db1-4dfa-8495-2dffbcace078
⛔ Files ignored due to path filters (2)
bundle/manifests/ols.openshift.io_olsconfigs.yamlis excluded by!bundle/manifests/ols.openshift.io_olsconfigs.yamlrelated_images.jsonis excluded by!related_images.json
📒 Files selected for processing (12)
.ai/spec/what/agentic-console-ui.md.ai/spec/what/bundle-composition.md.ai/spec/what/crd-api.md.ai/spec/what/reconciliation.mdMakefilebundle/manifests/lightspeed-operator.clusterserviceversion.yamlconfig/default/deployment-patch.yamlconfig/manifests/bases/lightspeed-operator.clusterserviceversion.yamlhack/image_placeholders.jsonhack/snapshot_to_image_list.shhack/update_bundle.shinternal/controller/utils/constants.go
| - --agentic-console-image=quay.io/redhat-user-workloads/crt-nshift-lightspeed-tenant/lightspeed-agentic-console:main | ||
| - --alerts-adapter-image=quay.io/tremes/laa |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect related_images.json agentic entries and confirm digest pinning.
fd -t f related_images.json --exec jq '.[] | select(.name|test("agentic")) | {name, image, revision}' {}Repository: openshift/lightspeed-operator
Length of output: 445
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== related_images.json agentic entries =="
jq '.[] | select(.name|test("agentic")) | {name, image, revision}' related_images.json
echo
echo "== CSV disconnected flag and related image references =="
python3 - <<'PY'
from pathlib import Path
p = Path('bundle/manifests/lightspeed-operator.clusterserviceversion.yaml')
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
if 'disconnected:' in line or 'agentic-console-image=' in line or 'alerts-adapter-image=' in line or 'relatedImages:' in line:
print(f"{i}: {line}")
PYRepository: openshift/lightspeed-operator
Length of output: 844
Pin the agentic bundle images
related_images.json still points lightspeed-agentic-console-plugin to quay.io/redhat-user-workloads/crt-nshift-lightspeed-tenant/lightspeed-agentic-console:main and lightspeed-agentic-alerts-adapter to quay.io/tremes/laa. With features.operators.openshift.io/disconnected: "true", these need mirrored, digest-pinned references before regenerating the bundle.
🤖 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 `@bundle/manifests/lightspeed-operator.clusterserviceversion.yaml` around lines
921 - 922, The bundle still references unpinned external agentic images, so
update the related image references used by the CSV and bundle generation flow
to mirror and digest-pin the values for lightspeed-agentic-console-plugin and
lightspeed-agentic-alerts-adapter. Make the change where the bundle manifest
values are sourced so the resulting related_images.json no longer points to
quay.io/redhat-user-workloads/...:main or quay.io/tremes/laa, then regenerate
the bundle artifacts.
Source: Path instructions
| "snapshot_component": "lightspeed-console", | ||
| "konflux_prefix": "quay.io/redhat-user-workloads/crt-nshift-lightspeed-tenant/ols/lightspeed-console", | ||
| "stable_prefix": "registry.redhat.io/openshift-lightspeed/lightspeed-console-plugin-rhel9" |
There was a problem hiding this comment.
what is the usage of these 3 new fields in related_images.json?
if it is for bundle generation or local deployment, is it better to put them in some scripts as they are kind of constants?
There was a problem hiding this comment.
I was trying to simplify hack/snapshot_to_image_list.sh
| - --agentic-console-image=quay.io/redhat-user-workloads/crt-nshift-lightspeed-tenant/lightspeed-agentic-console:main | ||
| - --alerts-adapter-image=quay.io/tremes/laa |
There was a problem hiding this comment.
we need add these arguments handling in the operator argument parsing, elsewise, operator will not start with unknown arguments.
|
/lgtm |
|
New changes are detected. LGTM label has been removed. |
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 `@hack/snapshot_to_image_list.sh`:
- Around line 128-178: The input expansion in snapshot_to_image_list.sh can fail
silently, leaving RESULT as an empty array and still writing OUTPUT_FILE. Fix
the flow around the ${JQ} -c '.[]' INPUT_FILE invocation by capturing its exit
status or output before the while loop, and abort immediately if parsing fails.
Use the existing RESULT, INPUT_FILE, and OUTPUT_FILE handling so malformed JSON
never produces a successful empty result.
🪄 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: da3da4d5-369a-42ed-9e37-ed5bdd43cc17
⛔ Files ignored due to path filters (2)
bundle/manifests/ols.openshift.io_olsconfigs.yamlis excluded by!bundle/manifests/ols.openshift.io_olsconfigs.yamlrelated_images.jsonis excluded by!related_images.json
📒 Files selected for processing (12)
.ai/spec/what/agentic-console-ui.md.ai/spec/what/bundle-composition.md.ai/spec/what/crd-api.md.ai/spec/what/reconciliation.mdMakefilebundle/manifests/lightspeed-operator.clusterserviceversion.yamlconfig/default/deployment-patch.yamlconfig/manifests/bases/lightspeed-operator.clusterserviceversion.yamlhack/image_placeholders.jsonhack/snapshot_to_image_list.shhack/update_bundle.shinternal/controller/utils/constants.go
✅ Files skipped from review due to trivial changes (3)
- .ai/spec/what/crd-api.md
- .ai/spec/what/agentic-console-ui.md
- .ai/spec/what/bundle-composition.md
🚧 Files skipped from review as they are similar to previous changes (8)
- config/default/deployment-patch.yaml
- Makefile
- hack/update_bundle.sh
- hack/image_placeholders.json
- internal/controller/utils/constants.go
- config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml
- bundle/manifests/lightspeed-operator.clusterserviceversion.yaml
- .ai/spec/what/reconciliation.md
| RESULT='[]' | ||
| while IFS= read -r entry; do | ||
| name=$(${JQ} -r '.name' <<<"${entry}") | ||
| image=$(${JQ} -r '.image // empty' <<<"${entry}") | ||
| revision=$(${JQ} -r '.revision // empty' <<<"${entry}") | ||
| component=$(${JQ} -r '.snapshot_component // empty' <<<"${entry}") | ||
| source=$(${JQ} -r '.snapshot_source // "ols"' <<<"${entry}") | ||
| konflux_prefix=$(${JQ} -r '.konflux_prefix // empty' <<<"${entry}") | ||
| stable_prefix=$(${JQ} -r '.stable_prefix // empty' <<<"${entry}") | ||
|
|
||
| if [ -n "${component}" ] && [ "${component}" != "null" ]; then | ||
| snapshot_file="${TMP_OLS_SNAPSHOT}" | ||
| if [ "${source}" = "bundle" ]; then | ||
| if [ -z "${BUNDLE_SNAPSHOT_REF}" ]; then | ||
| component="" | ||
| else | ||
| snapshot_file="${TMP_BUNDLE_SNAPSHOT}" | ||
| fi | ||
| fi | ||
| if [ -n "${component}" ]; then | ||
| snapshot_image=$(snapshot_component_image "${snapshot_file}" "${component}") | ||
| snapshot_revision=$(snapshot_component_revision "${snapshot_file}" "${component}") | ||
| if [ -n "${snapshot_image}" ] && [ "${snapshot_image}" != "null" ]; then | ||
| image="${snapshot_image}" | ||
| if [ -n "${snapshot_revision}" ] && [ "${snapshot_revision}" != "null" ]; then | ||
| revision="${snapshot_revision}" | ||
| fi | ||
| fi | ||
| fi | ||
| fi | ||
| fi | ||
|
|
||
| if [ -z "${POSTGRES_IMAGE}" ] || [ "${POSTGRES_IMAGE}" == "null" ]; then | ||
| if [ -f "${OUTPUT_FILE}" ]; then | ||
| POSTGRES_IMAGE=$(jq -r '.[] | select(.name == "lightspeed-postgresql") | .image' "${OUTPUT_FILE}") | ||
| if [ "${name}" = "lightspeed-postgresql" ]; then | ||
| if [ -z "${image}" ] || [ "${image}" = "null" ]; then | ||
| image=$(postgres_default_image) | ||
| fi | ||
| image=$(sed 's|quay\.io.*/lightspeed-postgresql|registry.redhat.io/rhel9/postgresql-16|g' <<<"${image}") | ||
| fi | ||
| fi | ||
|
|
||
| if [ -z "${POSTGRES_IMAGE}" ] || [ "${POSTGRES_IMAGE}" == "null" ]; then | ||
| DEFAULT_POSTGRES_IMAGE=$(grep -o 'PostgresServerImageDefault = "registry[^"]*"' "${SCRIPT_DIR}/../internal/controller/utils/constants.go" | sed 's/PostgresServerImageDefault = "\(.*\)"/\1/') | ||
| POSTGRES_IMAGE="${DEFAULT_POSTGRES_IMAGE}" | ||
| fi | ||
| image=$(map_registry_prefix "${image}" "${konflux_prefix}" "${stable_prefix}") | ||
|
|
||
| # lightspeed-to-dataverse-exporter does not comes from OLS snapshot. | ||
| # Preserve versions from the existing related images file. | ||
| DATAVERSE_EXPORTER_IMAGE="" | ||
| if [ -n "${OUTPUT_FILE}" ] && [ -f "${OUTPUT_FILE}" ]; then | ||
| DATAVERSE_EXPORTER_IMAGE=$(${JQ} -r '.[] | select(.name == "lightspeed-to-dataverse-exporter") | .image' "${OUTPUT_FILE}") | ||
| fi | ||
| if [ -z "${DATAVERSE_EXPORTER_IMAGE}" ] || [ "${DATAVERSE_EXPORTER_IMAGE}" == "null" ]; then | ||
| DEFAULT_RELATED_IMAGES="${SCRIPT_DIR}/../related_images.json" | ||
| if [ -f "${DEFAULT_RELATED_IMAGES}" ]; then | ||
| DATAVERSE_EXPORTER_IMAGE=$(${JQ} -r '.[] | select(.name == "lightspeed-to-dataverse-exporter") | .image' "${DEFAULT_RELATED_IMAGES}") | ||
| if [ -z "${image}" ] || [ "${image}" = "null" ]; then | ||
| echo "${name} image not found: ensure ${INPUT_FILE} lists a fallback image or snapshot_component metadata." >&2 | ||
| exit 1 | ||
| fi | ||
| fi | ||
| if [ -z "${DATAVERSE_EXPORTER_IMAGE}" ] || [ "${DATAVERSE_EXPORTER_IMAGE}" == "null" ]; then | ||
| echo "lightspeed-to-dataverse-exporter image not found: use -o with an existing related_images.json, or ensure ${SCRIPT_DIR}/../related_images.json lists it." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| RELATED_IMAGES=$( | ||
| cat <<-EOF | ||
| [ | ||
| { | ||
| "name": "lightspeed-service-api", | ||
| "image": "${SERVICE_IMAGE}", | ||
| "revision": "${SERVICE_REVISION}" | ||
| }, | ||
| { | ||
| "name": "lightspeed-console-plugin", | ||
| "image": "${CONSOLE_IMAGE}", | ||
| "revision": "${CONSOLE_REVISION}" | ||
| }, | ||
| { | ||
| "name": "lightspeed-operator", | ||
| "image": "${OPERATOR_IMAGE}", | ||
| "revision": "${OPERATOR_REVISION}" | ||
| }, | ||
| { | ||
| "name": "openshift-mcp-server", | ||
| "image": "${OPENSHIFT_MCP_SERVER_IMAGE}", | ||
| "revision": "${OPENSHIFT_MCP_SERVER_REVISION}" | ||
| }, | ||
| { | ||
| "name": "lightspeed-to-dataverse-exporter", | ||
| "image": "${DATAVERSE_EXPORTER_IMAGE}", | ||
| "revision": "${DATAVERSE_EXPORTER_REVISION}" | ||
| }, | ||
| { | ||
| "name": "lightspeed-ocp-rag", | ||
| "image": "${OCP_RAG_IMAGE}", | ||
| "revision": "${OCP_RAG_REVISION}" | ||
| }, | ||
| { | ||
| "name": "lightspeed-postgresql", | ||
| "image": "${POSTGRES_IMAGE}" | ||
| } | ||
| ] | ||
| EOF | ||
| ) | ||
|
|
||
| if [ -n "${BUNDLE_IMAGE}" ]; then | ||
| RELATED_IMAGES=$(echo "${RELATED_IMAGES}" | ${JQ} \ | ||
| --arg img "${BUNDLE_IMAGE}" \ | ||
| --arg rev "${BUNDLE_REVISION}" \ | ||
| '. += [{"name":"lightspeed-operator-bundle","image":$img,"revision":$rev}]') | ||
| fi | ||
| out_entry=$(${JQ} --arg image "${image}" --arg revision "${revision}" ' | ||
| .image = $image | ||
| | if ($revision | length) > 0 then .revision = $revision else . end | ||
| ' <<<"${entry}") | ||
| RESULT=$(${JQ} --argjson e "${out_entry}" '. + [$e]' <<<"${RESULT}") | ||
| done < <(${JQ} -c '.[]' "${INPUT_FILE}") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant portion of the script.
file="hack/snapshot_to_image_list.sh"
nl -ba "$file" | sed -n '110,190p'
# Demonstrate whether process substitution failures are caught under set -e.
tmpdir="$(mktemp -d)"
cat >"$tmpdir/demo.sh" <<'EOF'
#!/bin/bash
set -euo pipefail
RESULT='[]'
count=0
while IFS= read -r entry; do
count=$((count+1))
done < <(bash -c 'echo "broken json" >&2; exit 1')
echo "loop_count=$count RESULT=$RESULT"
EOF
bash "$tmpdir/demo.sh"; status=$?
echo "demo_exit=$status"
rm -rf "$tmpdir"Repository: openshift/lightspeed-operator
Length of output: 207
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="hack/snapshot_to_image_list.sh"
printf '\n=== script excerpt ===\n'
cat -n "$file" | sed -n '110,190p'
printf '\n=== process substitution demo ===\n'
tmpdir="$(mktemp -d)"
cat >"$tmpdir/demo.sh" <<'EOF'
#!/bin/bash
set -euo pipefail
count=0
while IFS= read -r entry; do
count=$((count+1))
done < <(bash -c 'echo "broken json" >&2; exit 1')
echo "loop_count=$count"
EOF
bash "$tmpdir/demo.sh"
status=$?
echo "demo_exit=$status"
rm -rf "$tmpdir"Repository: openshift/lightspeed-operator
Length of output: 3466
🌐 Web query:
Bash process substitution exit status set -e process substitution failure ignored while loop redirection
💡 Result:
In Bash, process substitution failures are ignored by set -e because process substitutions are asynchronous, backgrounded operations [1][2]. The exit status of the command containing the process substitution (e.g., cat <(false)) depends solely on the main command, not the process inside the substitution [3][4]. Consequently, set -e and set -o pipefail do not monitor or propagate the exit status of the substituted process [3][1][2]. Because process substitution is not a pipeline component, there is no built-in, idiomatic way to have the shell automatically abort when a process substitution fails [4][2]. When you redirect a process substitution into a while loop, the while loop remains unaware of any errors occurring inside that substitution [5]. To handle errors in process substitutions, you must manually track and verify the exit status [4][6]: 1. Use $!: Since Bash sets the special parameter $! to the process ID (PID) of the most recently executed asynchronous command, you can use it to capture the PID of the process substitution [4][5]. 2. Use wait: After the command finishes, you can use wait "$pid" to retrieve the exit status of the specific process [3][4][5]. Example: cat <(exit 3) pid=$! wait "$pid" status="$?" if [ "$status" -ne 0 ]; then echo "Process substitution failed with status $status" fi Alternatively, for complex flows, some users pass the exit status through the data stream itself by appending the exit code to the output of the process substitution, allowing the read loop to consume and verify it [5]. Relying on set -e for this purpose will result in silent failures [1][2].
Citations:
- 1: https://mywiki.wooledge.org/BashFAQ/105
- 2: https://unix.stackexchange.com/questions/217605/bash-how-to-propagate-errors-in-process-substitution
- 3: https://stackoverflow.com/questions/42703281/bash-pipefail-still-running-w-set-e-after-command-in-process-substitution-fail
- 4: https://unix.stackexchange.com/questions/376114/how-to-detect-an-error-using-process-substitution
- 5: https://stackoverflow.com/questions/43736021/get-exit-code-of-process-substitution-with-pipe-into-while-loop
- 6: https://lists.gnu.org/archive/html/bug-bash/2020-08/msg00050.html
Abort when the input JSON can’t be expanded
done < <(${JQ} -c '.[]' "${INPUT_FILE}") can fail without tripping set -e, so a malformed INPUT_FILE can leave RESULT='[]' and still overwrite OUTPUT_FILE with an empty array. Check the jq exit status before entering the loop, or capture the entries first and fail fast on parse errors.
🧰 Tools
🪛 Shellcheck (0.11.0)
[style] 163-163: See if you can use ${variable//search/replace} instead.
(SC2001)
[info] 173-176: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
[info] 177-177: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
🤖 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/snapshot_to_image_list.sh` around lines 128 - 178, The input expansion
in snapshot_to_image_list.sh can fail silently, leaving RESULT as an empty array
and still writing OUTPUT_FILE. Fix the flow around the ${JQ} -c '.[]' INPUT_FILE
invocation by capturing its exit status or output before the while loop, and
abort immediately if parsing fails. Use the existing RESULT, INPUT_FILE, and
OUTPUT_FILE handling so malformed JSON never produces a successful empty result.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: blublinsky 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-ci
Bot
added
the
approved
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/snapshot_to_image_list.sh`:
- Around line 133-155: The snapshot handling in snapshot_to_image_list.sh should
fail fast when a declared snapshot_component cannot be resolved instead of
silently falling back to the default image. In the entry-processing logic around
snapshot_component_image and snapshot_component_revision, detect when component
is set but lookup returns empty/null for the selected snapshot_file and exit
with an error rather than continuing; apply the same validation for the related
image path referenced in the other affected block. Ensure the script only
updates image/revision when the requested component is actually found.
- Around line 84-86: The informational preservation notice is being written to
stdout in the no-`-o` path, which breaks JSON output; update the logging in the
`bundle-snapshot-ref` missing branch to send the message to stderr instead of
stdout. Apply the same stderr fix to the corresponding preservation notice in
the later duplicated branch referenced by the review, while keeping the
JSON-emitting logic 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b09811e6-5cf6-40dd-99a0-7fb84e3bb03f
⛔ Files ignored due to path filters (2)
bundle/manifests/ols.openshift.io_olsconfigs.yamlis excluded by!bundle/manifests/ols.openshift.io_olsconfigs.yamlrelated_images.jsonis excluded by!related_images.json
📒 Files selected for processing (12)
.ai/spec/what/agentic-console-ui.md.ai/spec/what/bundle-composition.md.ai/spec/what/crd-api.md.ai/spec/what/reconciliation.mdMakefilebundle/manifests/lightspeed-operator.clusterserviceversion.yamlconfig/default/deployment-patch.yamlconfig/manifests/bases/lightspeed-operator.clusterserviceversion.yamlhack/image_placeholders.jsonhack/snapshot_to_image_list.shhack/update_bundle.shinternal/controller/utils/constants.go
✅ Files skipped from review due to trivial changes (2)
- .ai/spec/what/crd-api.md
- .ai/spec/what/agentic-console-ui.md
🚧 Files skipped from review as they are similar to previous changes (9)
- config/default/deployment-patch.yaml
- hack/update_bundle.sh
- hack/image_placeholders.json
- Makefile
- bundle/manifests/lightspeed-operator.clusterserviceversion.yaml
- config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml
- internal/controller/utils/constants.go
- .ai/spec/what/bundle-composition.md
- .ai/spec/what/reconciliation.md
| else | ||
| echo "bundle-snapshot-ref is not specified, bundle image entries are preserved" | ||
| : >"${TMP_BUNDLE_SNAPSHOT}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Send the preservation notice to stderr.
When -o is omitted, this script emits JSON to stdout; Line 85 currently prefixes it with an informational message, making stdout invalid JSON.
Proposed fix
- echo "bundle-snapshot-ref is not specified, bundle image entries are preserved"
+ echo "bundle-snapshot-ref is not specified, bundle image entries are preserved" >&2Also applies to: 180-183
🤖 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/snapshot_to_image_list.sh` around lines 84 - 86, The informational
preservation notice is being written to stdout in the no-`-o` path, which breaks
JSON output; update the logging in the `bundle-snapshot-ref` missing branch to
send the message to stderr instead of stdout. Apply the same stderr fix to the
corresponding preservation notice in the later duplicated branch referenced by
the review, while keeping the JSON-emitting logic unchanged.
blublinsky
force-pushed
the
wire-agentic
branch
2 times, most recently
from
ca90939 to
c6f84eb
Compare
|
@blublinsky: The following test failed, say
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. |
3 participants
…M bundle
Description
Completes OLM image wiring for agentic operands introduced under OLS-3348.
lightspeed-agentic-console-pluginandlightspeed-agentic-alerts-adaptertorelated_images.jsonwith snapshot metadata (snapshot_component,konflux_prefix,stable_prefix) for Konflux release automation--agentic-console-imageand--alerts-adapter-imagethroughdeployment-patch.yaml,image_placeholders.json, and bundle CSV deployment args /spec.relatedImagesrelated_images.jsonviaGetDefaultImage()inconstants.gohack/snapshot_to_image_list.shto looprelated_images.jsonentries (no per-component script edits when adding operands)config/manifests/bases.ai/specdocs to reflect implemented CSV wiring and agentic console reconciliation (OLS-3236 scope narrowed to agentic-operator cleanup + productized images)-p 6) to ease envtest contention on CINote: Agentic operand images are interim (Konflux
:main/ dev adapter image) until SHA-pinnedregistry.redhat.ioimages are available. Release automation preserves adapter image fromrelated_images.jsonwhen not yet in Konflux snapshots.Type of change
Related Tickets & Documents
OLS-3349
OLS-3349
Checklist before requesting a review
Testing
Summary by CodeRabbit