Dynamic image/tag resolution for deployments and automatic detection of the current managed service pod #158
Conversation
|
Warning Rate limit exceeded@andrej1991 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 3 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds guarded image/tag resolution (use ASSISTED_CHAT_IMG or fallback to quay.io IMAGE_PATH:latest), selects the latest-started assisted-service-mcp pod for diagnostics, and changes CI readiness, success, and failure logging to reference artifact paths rather than printing final pod logs. Public interfaces unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CI as scripts/ci_test.sh
participant Deploy as scripts/deploy_template.sh
participant Quay as quay.io
participant OC as oc
participant MCP as assisted-service-mcp pods
participant CHAT as assisted-chat pod
rect rgb(240,248,255)
Note over Deploy: Image/tag resolution
Deploy->>Deploy: if ASSISTED_CHAT_IMG set -> split into IMAGE/TAG
Deploy->>Quay: else -> IMAGE="quay.io/IMAGE_PATH", TAG="latest"
end
rect rgb(245,245,245)
Note over CI,OC: Pod readiness & MCP-aware diagnostics
CI->>OC: wait for assisted-chat pod to be ready
OC-->>CI: pod status updates
CI->>CI: log "Waiting for pod ... to be ready..."
alt pod ready
CI->>CI: print success message pointing to artifacts/
CI-->>CI: exit 0
else pod times out or fails
CI->>OC: get selector from assisted-service-mcp deployment
OC-->>CI: selector
CI->>OC: get pods -l <selector> (sort by startTime)
OC-->>CI: return latest MCP pod name
CI->>CI: print artifact path guidance (artifacts/eval-test/gather-extra/artifacts/pods/)
CI->>OC: oc get events -n $NAMESPACE
OC-->>CI: events
CI-->>CI: exit with main pod terminated exit code
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
andrej1991
force-pushed
the
debug_logging
branch
from
02797c2 to
170c432
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/ci_test.sh (1)
41-49: Harden numeric checks against empty values under nounset/errexit.
oc get ... -o=jsonpath=...can yield empty strings transiently;[[ "$VAR" -gt 0 ]]risks “integer expression expected”. Use arithmetic context which treats unset/empty as 0.- if [[ "$JOB_SUCCEEDED" -gt 0 ]]; then + if (( ${JOB_SUCCEEDED:-0} > 0 )); then echo "Pod ${POD_NAME} is successfully completed, exiting" oc logs -n "$NAMESPACE" "$POD_NAME" exit 0 fi - if [[ "$JOB_FAILED" -gt 0 ]]; then + if (( ${JOB_FAILED:-0} > 0 )); then echo "Pod ${POD_NAME} is Failed, exiting" - oc logs -n "$NAMESPACE" "$POD_NAME" + oc logs -n "$NAMESPACE" "$POD_NAME" || true
🧹 Nitpick comments (3)
scripts/ci_test.sh (3)
28-29: Fix misleading label in error message.We searched with
-l job-name="$JOB_NAME"but the message mentionsapp=. Align the message to avoid confusion when triaging.- echo "No pod found with label app=assisted-chat-eval-test in namespace ${NAMESPACE}" + echo "No pod found with label job-name=${JOB_NAME} in namespace ${NAMESPACE}"
18-18: Consistent quoting of variables.Minor but safer: quote
$NAMESPACElike elsewhere.- oc create secret generic -n $NAMESPACE gemini --from-file=api_key="${SECRETS_BASE_PATH}/gemini/api_key" + oc create secret generic -n "$NAMESPACE" gemini --from-file=api_key="${SECRETS_BASE_PATH}/gemini/api_key"
56-60: Don’t let describe/log failures mask other diagnostics.Add
|| trueso a failedoc describewon’t terminate the failure path before we emit the exit code.- oc events -n "$NAMESPACE" + oc events -n "$NAMESPACE" || true echo "oc describe pod -n \"$NAMESPACE\" \"$ASSISTED_CHAT_POD\"" - oc describe pod -n "$NAMESPACE" "$ASSISTED_CHAT_POD" + oc describe pod -n "$NAMESPACE" "$ASSISTED_CHAT_POD" || true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
scripts/ci_test.sh(1 hunks)scripts/deploy_template.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
| if ! oc rollout status -n $NAMESPACE deployment/assisted-chat --timeout=300s; then | ||
| echo "Deploying assisted-chat failed" | ||
| ASSISTED_CHAT_POD=$(oc get pods -n "$NAMESPACE" | tr -s ' ' | cut -d ' ' -f1 | grep assisted-chat) | ||
| echo "The logs of the pod ${ASSISTED_CHAT_POD}" | ||
| oc logs -n $NAMESPACE "$ASSISTED_CHAT_POD" | ||
| oc logs -n $NAMESPACE "$ASSISTED_CHAT_POD" --all-containers=true | ||
| echo "The events in the namespace '${NAMESPACE}'" | ||
| oc events -n $NAMESPACE | ||
| echo "oc describe pod ${ASSISTED_CHAT_POD}" | ||
| oc describe pod $ASSISTED_CHAT_POD | ||
| echo "oc describe pod -n $NAMESPACE ${ASSISTED_CHAT_POD}" | ||
| oc describe pod -n $NAMESPACE $ASSISTED_CHAT_POD | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure failure-path diagnostics don’t abort; quote namespace.
Keep collecting logs/events/describe even if a single command fails; also quote $NAMESPACE for consistency.
-if ! oc rollout status -n $NAMESPACE deployment/assisted-chat --timeout=300s; then
+if ! oc rollout status -n "$NAMESPACE" deployment/assisted-chat --timeout=300s; then
echo "Deploying assisted-chat failed"
ASSISTED_CHAT_POD=$(oc get pods -n "$NAMESPACE" | tr -s ' ' | cut -d ' ' -f1 | grep assisted-chat)
echo "The logs of the pod ${ASSISTED_CHAT_POD}"
- oc logs -n $NAMESPACE "$ASSISTED_CHAT_POD" --all-containers=true
+ oc logs -n "$NAMESPACE" "$ASSISTED_CHAT_POD" --all-containers=true || true
echo "The events in the namespace '${NAMESPACE}'"
- oc events -n $NAMESPACE
- echo "oc describe pod -n $NAMESPACE ${ASSISTED_CHAT_POD}"
- oc describe pod -n $NAMESPACE $ASSISTED_CHAT_POD
+ oc events -n "$NAMESPACE" || true
+ echo "oc describe pod -n \"$NAMESPACE\" ${ASSISTED_CHAT_POD}"
+ oc describe pod -n "$NAMESPACE" "$ASSISTED_CHAT_POD" || true
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ! oc rollout status -n $NAMESPACE deployment/assisted-chat --timeout=300s; then | |
| echo "Deploying assisted-chat failed" | |
| ASSISTED_CHAT_POD=$(oc get pods -n "$NAMESPACE" | tr -s ' ' | cut -d ' ' -f1 | grep assisted-chat) | |
| echo "The logs of the pod ${ASSISTED_CHAT_POD}" | |
| oc logs -n $NAMESPACE "$ASSISTED_CHAT_POD" | |
| oc logs -n $NAMESPACE "$ASSISTED_CHAT_POD" --all-containers=true | |
| echo "The events in the namespace '${NAMESPACE}'" | |
| oc events -n $NAMESPACE | |
| echo "oc describe pod ${ASSISTED_CHAT_POD}" | |
| oc describe pod $ASSISTED_CHAT_POD | |
| echo "oc describe pod -n $NAMESPACE ${ASSISTED_CHAT_POD}" | |
| oc describe pod -n $NAMESPACE $ASSISTED_CHAT_POD | |
| fi | |
| if ! oc rollout status -n "$NAMESPACE" deployment/assisted-chat --timeout=300s; then | |
| echo "Deploying assisted-chat failed" | |
| ASSISTED_CHAT_POD=$(oc get pods -n "$NAMESPACE" | tr -s ' ' | cut -d ' ' -f1 | grep assisted-chat) | |
| echo "The logs of the pod ${ASSISTED_CHAT_POD}" | |
| oc logs -n "$NAMESPACE" "$ASSISTED_CHAT_POD" --all-containers=true || true | |
| echo "The events in the namespace '${NAMESPACE}'" | |
| oc events -n "$NAMESPACE" || true | |
| echo "oc describe pod -n \"$NAMESPACE\" ${ASSISTED_CHAT_POD}" | |
| oc describe pod -n "$NAMESPACE" "$ASSISTED_CHAT_POD" || true | |
| fi |
🤖 Prompt for AI Agents
In scripts/deploy_template.sh around lines 84-93, the failure-path diagnostics
currently can abort early and use unquoted $NAMESPACE; update the block so
diagnostics continue even if individual commands fail (e.g., temporarily disable
errexit or append "|| true" to each diagnostic command) and quote variables
consistently (use "$NAMESPACE" and "$ASSISTED_CHAT_POD" when passed to
oc/get/describe/logs) so logs/events/describe are always attempted and
namespace/pod names are safely quoted.
|
/retest required |
|
@andrej1991: The Use In 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 kubernetes-sigs/prow repository. |
|
/test eval-test |
andrej1991
force-pushed
the
debug_logging
branch
2 times, most recently
from
ea4116d to
cd7db5a
Compare
|
/test eval-test |
andrej1991
force-pushed
the
debug_logging
branch
from
cd7db5a to
4b0bca9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
scripts/deploy_template.sh (1)
84-86: Quote NAMESPACE and keep minimal diagnostics before exit.Unquoted $NAMESPACE is brittle. Also, exiting without any inline describe/events reduces signal during failures (contrary to the PR goal of “adding even more logging”). Add lightweight, non-fatal diagnostics before exiting.
-if ! oc rollout status -n $NAMESPACE deployment/assisted-chat --timeout=300s; then - echo "Deploying assisted-chat failed, the logs of the pods are in artifacts/eval-test/gather-extra/artifacts/pods/ directory." - exit 1 +if ! oc rollout status -n "$NAMESPACE" deployment/assisted-chat --timeout=300s; then + echo "Deploying assisted-chat failed. Attempting quick diagnostics (non-fatal):" + echo "oc describe -n \"$NAMESPACE\" deployment/assisted-chat" + oc describe -n "$NAMESPACE" deployment/assisted-chat || true + echo "oc events -n \"$NAMESPACE\" --sort-by=.lastTimestamp | tail -n 200" + oc events -n "$NAMESPACE" --sort-by=.lastTimestamp | tail -n 200 || true + echo "Detailed pod logs should be in artifacts/eval-test/gather-extra/artifacts/pods/." + exit 1 fiscripts/ci_test.sh (1)
54-59: Make failure-path log collection resilient; avoid aborting diagnostics.With errexit, any oc logs failure will exit early. Also, guard empty pod vars; recomputing SELECTOR is unnecessary—reuse it if present.
- SELECTOR=$(oc get deploy -n "$NAMESPACE" assisted-service-mcp -o json | jq -r '.spec.selector.matchLabels | to_entries | map("\(.key)=\(.value)") | join(",")') - ASSISTED_SERVICE_MCP_POD=$(oc get pods -n "$NAMESPACE" -l "$SELECTOR" -o json | jq -r '.items | sort_by(.status.startTime) | last | .metadata.name') - echo "oc logs -n \"$NAMESPACE\" \"$ASSISTED_CHAT_POD\" --all-containers=true" - oc logs -n "$NAMESPACE" "$ASSISTED_CHAT_POD" --all-containers=true - echo "oc logs -n \"$NAMESPACE\" \"$ASSISTED_SERVICE_MCP_POD\" --all-containers=true" - oc logs -n "$NAMESPACE" "$ASSISTED_SERVICE_MCP_POD" --all-containers=true + # Reuse SELECTOR/ASSISTED_SERVICE_MCP_POD computed earlier; if empty, attempt best-effort recompute. + if [[ -z "${SELECTOR:-}" ]]; then + SELECTOR="$( + { oc get deploy -n "$NAMESPACE" assisted-service-mcp -o json 2>/dev/null || true; } \ + | jq -r '.spec.selector.matchLabels | to_entries | map("\(.key)=\(.value)") | join(",") // empty' 2>/dev/null || true + )" + fi + if [[ -z "${ASSISTED_SERVICE_MCP_POD:-}" && -n "${SELECTOR:-}" ]]; then + ASSISTED_SERVICE_MCP_POD="$( + oc get pods -n "$NAMESPACE" -l "$SELECTOR" -o json 2>/dev/null \ + | jq -r '.items | sort_by(.status.startTime) | last? | .metadata.name // empty' 2>/dev/null || true + )" + fi + if [[ -n "${ASSISTED_CHAT_POD:-}" ]]; then + echo "oc logs -n \"$NAMESPACE\" \"$ASSISTED_CHAT_POD\" --all-containers=true" + oc logs -n "$NAMESPACE" "$ASSISTED_CHAT_POD" --all-containers=true || true + else + echo "No assisted-chat pod name resolved; skipping assisted-chat logs." + fi + if [[ -n "${ASSISTED_SERVICE_MCP_POD:-}" ]]; then + echo "oc logs -n \"$NAMESPACE\" \"$ASSISTED_SERVICE_MCP_POD\" --all-containers=true" + oc logs -n "$NAMESPACE" "$ASSISTED_SERVICE_MCP_POD" --all-containers=true || true + else + echo "No assisted-service-mcp pod found in namespace $NAMESPACE; skipping MCP logs." + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
scripts/ci_test.sh(2 hunks)scripts/deploy_template.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Red Hat Konflux / assisted-chat-saas-main-on-pull-request
andrej1991
force-pushed
the
debug_logging
branch
from
4b0bca9 to
e284b48
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
scripts/deploy_template.sh (1)
110-113: Quote namespace and collect failure diagnostics without aborting.Keep gathering logs/events/describe even if a single command fails, and quote variables. Also remove extra spaces.
-if ! oc rollout status -n $NAMESPACE deployment/assisted-chat --timeout=300s; then - echo "Deploying assisted-chat failed, the logs of the pods are in artifacts/eval-test/gather-extra/artifacts/pods/ directory." - exit 1 +if ! oc rollout status -n "$NAMESPACE" deployment/assisted-chat --timeout=300s; then + echo "Deploying assisted-chat failed. Attempting to collect diagnostics; artifacts will be under artifacts/eval-test/gather-extra/artifacts/pods/." + ASSISTED_CHAT_POD="$(oc get pods -n "$NAMESPACE" -o jsonpath='{range .items[*]}{.metadata.name}{"\n"}{end}' \ + | grep '^assisted-chat' | head -n1 || true)" + if [[ -n "${ASSISTED_CHAT_POD:-}" ]]; then + echo "oc logs -n \"$NAMESPACE\" \"$ASSISTED_CHAT_POD\" --all-containers=true" + oc logs -n "$NAMESPACE" "$ASSISTED_CHAT_POD" --all-containers=true || true + echo "oc describe pod -n \"$NAMESPACE\" \"$ASSISTED_CHAT_POD\"" + oc describe pod -n "$NAMESPACE" "$ASSISTED_CHAT_POD" || true + else + echo "No assisted-chat pod found in namespace \"$NAMESPACE\"" + fi + echo "oc events -n \"$NAMESPACE\"" + oc events -n "$NAMESPACE" || true + exit 1 fiscripts/ci_test.sh (1)
63-66: Guard missing MCP deployment; make jq selection null-safe; don’t abort under errexit/pipefail.-SELECTOR=$(oc get deploy -n "$NAMESPACE" assisted-service-mcp -o json | jq -r '.spec.selector.matchLabels | to_entries | map("\(.key)=\(.value)") | join(",")') -ASSISTED_SERVICE_MCP_POD=$(oc get pods -n "$NAMESPACE" -l "$SELECTOR" -o json | jq -r '.items | sort_by(.status.startTime) | last | .metadata.name') -echo $ASSISTED_SERVICE_MCP_POD +SELECTOR="$( + { oc get deploy -n "$NAMESPACE" assisted-service-mcp -o json 2>/dev/null || true; } \ + | jq -r '.spec.selector.matchLabels | to_entries | map("\(.key)=\(.value)") | join(",") // empty' 2>/dev/null || true +)" +ASSISTED_SERVICE_MCP_POD="" +if [[ -n "${SELECTOR:-}" ]]; then + ASSISTED_SERVICE_MCP_POD="$( + oc get pods -n "$NAMESPACE" -l "$SELECTOR" -o json 2>/dev/null \ + | jq -r '.items | sort_by(.status.startTime) | last? | .metadata.name // empty' 2>/dev/null || true + )" +fi +echo "${ASSISTED_SERVICE_MCP_POD:-}"
🧹 Nitpick comments (4)
scripts/deploy_template.sh (1)
28-34: Optional: Prefer Docker manifest v2 for creation time.Schema v1
.history[0].v1Compatibilityis deprecated and brittle. Consider requesting v2 and reading the config blob’screated.- MANIFEST=$(curl -Lf "https://quay.io/v2/${IMAGE_PATH}/manifests/${TAG}" | jq -r '.history[0].v1Compatibility') - CREATED_DATE=$(echo "$MANIFEST" | jq -r '.created' | xargs -I {} date -d {} +%s) + # v2: get manifest, then fetch config blob to read .created + MANIFEST=$(curl -sLf -H 'Accept: application/vnd.docker.distribution.manifest.v2+json' \ + "https://quay.io/v2/${IMAGE_PATH}/manifests/${TAG}") || continue + CONFIG_DIGEST=$(echo "$MANIFEST" | jq -r '.config.digest // empty') || continue + [[ -n "$CONFIG_DIGEST" ]] || continue + CREATED_RAW=$(curl -sLf "https://quay.io/v2/${IMAGE_PATH}/blobs/${CONFIG_DIGEST}" | jq -r '.created // empty') || continue + [[ -n "$CREATED_RAW" ]] || continue + CREATED_DATE=$(date -d "$CREATED_RAW" +%s 2>/dev/null || echo 0)Also applies to: 37-42
scripts/ci_test.sh (3)
44-47: Quote namespace when creating secrets.- oc create secret generic -n $NAMESPACE gemini --from-file=api_key="${SECRETS_BASE_PATH}/gemini/api_key" + oc create secret generic -n "$NAMESPACE" gemini --from-file=api_key="${SECRETS_BASE_PATH}/gemini/api_key"
72-75: Fix user-facing grammar/typos.- echo "The evaluation test were successful. The logs of the tests are stored in the directory artifacts/eval-test/gather-extra/artifacts/pods/ in the logs of the pod ${POD_NAME}." + echo "The evaluation tests were successful. The logs are under artifacts/eval-test/gather-extra/artifacts/pods/ for pod ${POD_NAME}." @@ -echo "Timeout reached. Pod $POD_NAME did not become ready in time. PLease check the logs of the pods under the directory artifacts/eval-test/gather-extra/artifacts/pods/." +echo "Timeout reached. Pod $POD_NAME did not become ready in time. Please check the pod logs under artifacts/eval-test/gather-extra/artifacts/pods/."Also applies to: 91-92
19-30: Optional: Prefer Docker manifest v2 for creation time (align with deploy script suggestion).- MANIFEST=$(curl -Lf "https://quay.io/v2/${IMAGE_PATH}/manifests/${TAG}" | jq -r '.history[0].v1Compatibility') || continue - CREATED_RAW=$(echo "$MANIFEST" | jq -r '.created' 2>/dev/null || true) + MANIFEST=$(curl -sLf -H 'Accept: application/vnd.docker.distribution.manifest.v2+json' \ + "https://quay.io/v2/${IMAGE_PATH}/manifests/${TAG}") || continue + CONFIG_DIGEST=$(echo "$MANIFEST" | jq -r '.config.digest // empty') || continue + CREATED_RAW=$(curl -sLf "https://quay.io/v2/${IMAGE_PATH}/blobs/${CONFIG_DIGEST}" | jq -r '.created // empty' 2>/dev/null || true)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
scripts/ci_test.sh(3 hunks)scripts/deploy_template.sh(2 hunks)
scripts/ci_test.sh
Outdated
| if [[ -n $ASSISTED_CHAT_TEST ]]; then | ||
| echo "The variable ASSISTED_CHAT_TEST was proided with the value ${ASSISTED_CHAT_TEST}, using that for creating the IMAGE and TAG variables for the template" | ||
| else | ||
| IMAGE_PATH="redhat-user-workloads/assisted-installer-tenant/assisted-chat-test-image-saas-main/assisted-chat-test-image-saas-main" | ||
| IMAGE="quay.io/${IMAGE_PATH}" | ||
| echo "The variable ASSISTED_CHAT_TEST was not provieded, downloading the latest image from ${IMAGE}" | ||
| NEWEST_TAG="" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix nounset crash when ASSISTED_CHAT_TEST is unset; make tag resolution resilient; fix typos.
Use default expansion under nounset, tolerate empty grep/manifests, and correct user-facing strings.
-if [[ -n $ASSISTED_CHAT_TEST ]]; then
- echo "The variable ASSISTED_CHAT_TEST was proided with the value ${ASSISTED_CHAT_TEST}, using that for creating the IMAGE and TAG variables for the template"
+if [[ -n "${ASSISTED_CHAT_TEST:-}" ]]; then
+ echo "The variable ASSISTED_CHAT_TEST was provided with the value ${ASSISTED_CHAT_TEST}; using it to create the IMAGE and TAG variables for the template"
else
IMAGE_PATH="redhat-user-workloads/assisted-installer-tenant/assisted-chat-test-image-saas-main/assisted-chat-test-image-saas-main"
IMAGE="quay.io/${IMAGE_PATH}"
- echo "The variable ASSISTED_CHAT_TEST was not provieded, downloading the latest image from ${IMAGE}"
+ echo "The variable ASSISTED_CHAT_TEST was not provided; resolving the latest image from ${IMAGE}"
NEWEST_TAG=""
NEWEST_DATE=0
- TAGS=$(curl -Lf https://quay.io/v2/${IMAGE_PATH}/tags/list | jq -r '.tags[]'|grep -v sha256)
+ TAGS=$(curl -Lf "https://quay.io/v2/${IMAGE_PATH}/tags/list" | jq -r '.tags[]' | grep -v sha256 || true)
for TAG in $TAGS; do
if [[ "${#TAG}" == "7" ]]; then
- MANIFEST=$(curl -Lf "https://quay.io/v2/${IMAGE_PATH}/manifests/${TAG}" | jq -r '.history[0].v1Compatibility')
- CREATED_DATE=$(echo "$MANIFEST" | jq -r '.created' | xargs -I {} date -d {} +%s)
+ MANIFEST=$(curl -Lf "https://quay.io/v2/${IMAGE_PATH}/manifests/${TAG}" | jq -r '.history[0].v1Compatibility') || continue
+ CREATED_RAW=$(echo "$MANIFEST" | jq -r '.created' 2>/dev/null || true)
+ [[ -n "$CREATED_RAW" && "$CREATED_RAW" != "null" ]] || continue
+ CREATED_DATE=$(date -d "$CREATED_RAW" +%s 2>/dev/null || echo 0)Also applies to: 19-36
🤖 Prompt for AI Agents
In scripts/ci_test.sh around lines 10 to 16 (and similarly apply to 19-36), the
script currently crashes under nounset and prints typos; fix by using parameter
expansion with a safe default (e.g. ${ASSISTED_CHAT_TEST:-}) wherever
ASSISTED_CHAT_TEST is read so the script doesn't error when nounset; ensure
NEWEST_TAG is initialized to an empty string and that tag resolution commands
tolerate empty outputs (use checks for empty grep/manifests output or append ||
true) so downstream uses won't fail; also correct user-facing messages to
"provided" and rephrase to mention the derived IMAGE and TAG variables.
| oc events -n "$NAMESPACE" | ||
| echo "oc describe pod -n \"$NAMESPACE\" \"$ASSISTED_CHAT_POD\"" | ||
| oc describe pod -n "$NAMESPACE" "$ASSISTED_CHAT_POD" | ||
| exit "$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}')" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
❓ Verification inconclusive
Exit code retrieval can be empty; provide a safe fallback.
Avoid “numeric argument required” if the jsonpath yields empty.
- exit "$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}')"
+ EC="$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}' 2>/dev/null || true)"
+ [[ -n "${EC:-}" ]] || EC=1
+ exit "$EC"Use a safe fallback exit code to prevent “numeric argument required”
Shell’s exit "" fails when the JSONPath returns empty; capture the exit code with a default of 1:
- exit "$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}')"
+ EC="$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}' 2>/dev/null || true)"
+ [[ -n "${EC}" ]] || EC=1
+ exit "$EC"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| exit "$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}')" | |
| EC="$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}' 2>/dev/null || true)" | |
| [[ -n "${EC}" ]] || EC=1 | |
| exit "$EC" |
🤖 Prompt for AI Agents
In scripts/ci_test.sh around line 82, the script directly exits with the command
substitution which can produce an empty string and cause “numeric argument
required”; capture the pod container exit code into a variable by running the oc
jsonpath command and then exit using that variable with a safe default of 1
(e.g. use shell parameter expansion so if the captured value is empty the script
exits with 1).
|
@andrej1991: The following test failed, say
Full PR test history. Your PR dashboard. 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. |
andrej1991
force-pushed
the
debug_logging
branch
from
e284b48 to
2f80bc6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
scripts/deploy_template.sh (3)
14-21: Harden ASSISTED_CHAT_IMG handling; fix typos; quote and avoid nounset crashUse default expansion under nounset, quote vars, and correct user-facing text.
-if [[ -n $ASSISTED_CHAT_IMG ]]; then - echo "The variable ASSISTED_CHAT_IMG was proided with the value ${ASSISTED_CHAT_IMG}, using that for creating the IMAGE and TAG variables for the template" - IMAGE=$(echo "$ASSISTED_CHAT_IMG" | cut -d ":" -f1) - TAG=$(echo "$ASSISTED_CHAT_IMG" | cut -d ":" -f2) +if [[ -n "${ASSISTED_CHAT_IMG:-}" ]]; then + echo "The variable ASSISTED_CHAT_IMG was provided with the value ${ASSISTED_CHAT_IMG}; using it to create the IMAGE and TAG variables for the template" + IMAGE="$(echo "${ASSISTED_CHAT_IMG}" | cut -d ':' -f1)" + TAG="$(echo "${ASSISTED_CHAT_IMG}" | cut -d ':' -f2)" else IMAGE_PATH="redhat-services-prod/assisted-installer-tenant/saas/assisted-chat" IMAGE="quay.io/${IMAGE_PATH}" - echo "The variable ASSISTED_CHAT_IMG was not provieded, downloading the latest image from ${IMAGE}" + echo "The variable ASSISTED_CHAT_IMG was not provided; resolving the latest image from ${IMAGE}"
25-36: Make tag resolution resilient under errexit/pipefail; skip bad tags instead of abortingTolerate empty/failed curl|jq and malformed dates; quote URLs and vars.
- TAGS=$(curl -Lf https://quay.io/v2/${IMAGE_PATH}/tags/list | jq -r '.tags[]'|grep -v sha256) + TAGS=$(curl -Lf "https://quay.io/v2/${IMAGE_PATH}/tags/list" | jq -r '.tags[]' | grep -v sha256 || true) for TAG in $TAGS; do if [[ "${#TAG}" == "7" ]]; then - MANIFEST=$(curl -Lf "https://quay.io/v2/${IMAGE_PATH}/manifests/${TAG}" | jq -r '.history[0].v1Compatibility') - CREATED_DATE=$(echo "$MANIFEST" | jq -r '.created' | xargs -I {} date -d {} +%s) + MANIFEST=$(curl -Lf "https://quay.io/v2/${IMAGE_PATH}/manifests/${TAG}" | jq -r '.history[0].v1Compatibility') || continue + CREATED_RAW="$(echo "$MANIFEST" | jq -r '.created' 2>/dev/null || true)" + [[ -n "${CREATED_RAW:-}" && "$CREATED_RAW" != "null" ]] || continue + CREATED_DATE="$(date -d "$CREATED_RAW" +%s 2>/dev/null || echo 0)" if [[ $CREATED_DATE -gt $NEWEST_DATE ]]; then NEWEST_DATE=$CREATED_DATE NEWEST_TAG=$TAG fi fi done
109-113: Quote NAMESPACE and keep failure-path diagnostics (logs/events/describe) resilientEnsure diagnostics still run even if one command fails; also point to artifacts.
-sleep 5 -if ! oc rollout status -n $NAMESPACE deployment/assisted-chat --timeout=300s; then - echo "Deploying assisted-chat failed, the logs of the pods are in artifacts/eval-test/gather-extra/artifacts/pods/ directory." - exit 1 -fi +sleep 5 +if ! oc rollout status -n "$NAMESPACE" deployment/assisted-chat --timeout=300s; then + echo "Deploying assisted-chat failed" + ASSISTED_CHAT_POD="$(oc get pods -n "$NAMESPACE" -o jsonpath='{range .items[*]}{.metadata.name}{"\n"}{end}' | grep '^assisted-chat' | head -n1 || true)" + if [[ -n "${ASSISTED_CHAT_POD:-}" ]]; then + echo "oc logs -n \"$NAMESPACE\" \"$ASSISTED_CHAT_POD\" --all-containers=true" + oc logs -n "$NAMESPACE" "$ASSISTED_CHAT_POD" --all-containers=true || true + echo "oc describe pod -n \"$NAMESPACE\" \"$ASSISTED_CHAT_POD\"" + oc describe pod -n "$NAMESPACE" "$ASSISTED_CHAT_POD" || true + else + echo "No assisted-chat pod found in namespace $NAMESPACE" + fi + echo "oc events -n \"$NAMESPACE\"" + oc events -n "$NAMESPACE" || true + echo "Also see artifacts/eval-test/gather-extra/artifacts/pods/ for collected logs." + exit 1 +fiscripts/ci_test.sh (3)
10-17: Guard ASSISTED_CHAT_TEST under nounset; fix typos in user-facing messagesUse default expansion and improve wording.
-if [[ -n $ASSISTED_CHAT_TEST ]]; then - echo "The variable ASSISTED_CHAT_TEST was proided with the value ${ASSISTED_CHAT_TEST}, using that for creating the IMAGE and TAG variables for the template" +if [[ -n "${ASSISTED_CHAT_TEST:-}" ]]; then + echo "The variable ASSISTED_CHAT_TEST was provided with the value ${ASSISTED_CHAT_TEST}; using it to create the IMAGE and TAG variables for the template" else IMAGE_PATH="redhat-user-workloads/assisted-installer-tenant/assisted-chat-test-image-saas-main/assisted-chat-test-image-saas-main" IMAGE="quay.io/${IMAGE_PATH}" - echo "The variable ASSISTED_CHAT_TEST was not provieded, downloading the latest image from ${IMAGE}" + echo "The variable ASSISTED_CHAT_TEST was not provided; resolving the latest image from ${IMAGE}" ASSISTED_CHAT_TEST="${IMAGE}:latest" fi
44-46: Make MCP pod discovery null-safe and resilient under pipefailHandle missing deployment and empty lists; avoid bare echo of empty name.
-SELECTOR=$(oc get deploy -n "$NAMESPACE" assisted-service-mcp -o json | jq -r '.spec.selector.matchLabels | to_entries | map("\(.key)=\(.value)") | join(",")') -ASSISTED_SERVICE_MCP_POD=$(oc get pods -n "$NAMESPACE" -l "$SELECTOR" -o json | jq -r '.items | sort_by(.status.startTime) | last | .metadata.name') -echo $ASSISTED_SERVICE_MCP_POD +SELECTOR="$( + { oc get deploy -n "$NAMESPACE" assisted-service-mcp -o json 2>/dev/null || true; } \ + | jq -r '.spec.selector.matchLabels | to_entries | map("\(.key)=\(.value)") | join(",") // empty' 2>/dev/null || true +)" +ASSISTED_SERVICE_MCP_POD="" +if [[ -n "${SELECTOR:-}" ]]; then + ASSISTED_SERVICE_MCP_POD="$( + oc get pods -n "$NAMESPACE" -l "$SELECTOR" -o json 2>/dev/null \ + | jq -r '.items | sort_by(.status.startTime) | last? | .metadata.name // empty' 2>/dev/null || true + )" +fi +if [[ -n "${ASSISTED_SERVICE_MCP_POD:-}" ]]; then + echo "assisted-service-mcp pod: ${ASSISTED_SERVICE_MCP_POD}" +else + echo "No assisted-service-mcp pod found in namespace $NAMESPACE" +fi
58-64: Make failure diagnostics resilient; safe fallback for empty exit codeDon’t abort on events failure; avoid “numeric argument required” on empty JSONPath.
- echo "The evaluation tests failed, you can see the logs of the pods under the directory artifacts/eval-test/gather-extra/artifacts/pods/." - echo "oc events" - oc events -n "$NAMESPACE" - exit "$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}')" + echo "The evaluation tests failed. See logs under artifacts/eval-test/gather-extra/artifacts/pods/." + echo "oc events -n \"$NAMESPACE\"" + oc events -n "$NAMESPACE" || true + EC="$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}' 2>/dev/null || true)" + [[ -n "${EC:-}" ]] || EC=1 + exit "$EC"
🧹 Nitpick comments (2)
scripts/ci_test.sh (2)
53-55: Fix grammar in success messageMinor copy edit.
- echo "The evaluation test were successful. The logs of the tests are stored in the directory artifacts/eval-test/gather-extra/artifacts/pods/ in the logs of the pod ${POD_NAME}." + echo "The evaluation tests were successful. Logs are stored under artifacts/eval-test/gather-extra/artifacts/pods/ for pod ${POD_NAME}."
72-73: Fix typo in timeout message“PLease” → “Please”.
-echo "Timeout reached. Pod $POD_NAME did not become ready in time. PLease check the logs of the pods under the directory artifacts/eval-test/gather-extra/artifacts/pods/." +echo "Timeout reached. Pod $POD_NAME did not become ready in time. Please check the logs of the pods under artifacts/eval-test/gather-extra/artifacts/pods/."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
scripts/ci_test.sh(3 hunks)scripts/deploy_template.sh(2 hunks)
andrej1991
force-pushed
the
debug_logging
branch
from
2f80bc6 to
6ac375b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
scripts/deploy_template.sh (1)
91-93: Quote namespace; consider keeping diagnostics resilient.Unquoted $NAMESPACE can break for odd names. If you later add diagnostics here, ensure they’re suffix-guarded with || true so errexit/pipefail don’t abort early.
-if ! oc rollout status -n $NAMESPACE deployment/assisted-chat --timeout=300s; then +if ! oc rollout status -n "$NAMESPACE" deployment/assisted-chat --timeout=300s; thenscripts/ci_test.sh (3)
10-17: Guard ASSISTED_CHAT_TEST under nounset; fix typos; align message with actual usage.Use default expansion; correct strings; message shouldn’t mention IMAGE/TAG since you pass IMAGE_NAME.
-if [[ -n $ASSISTED_CHAT_TEST ]]; then - echo "The variable ASSISTED_CHAT_TEST was proided with the value ${ASSISTED_CHAT_TEST}, using that for creating the IMAGE and TAG variables for the template" +if [[ -n "${ASSISTED_CHAT_TEST:-}" ]]; then + echo "The variable ASSISTED_CHAT_TEST was provided with the value ${ASSISTED_CHAT_TEST}; using it as IMAGE_NAME for the template" else IMAGE_PATH="redhat-user-workloads/assisted-installer-tenant/assisted-chat-test-image-saas-main/assisted-chat-test-image-saas-main" IMAGE="quay.io/${IMAGE_PATH}" - echo "The variable ASSISTED_CHAT_TEST was not provieded, downloading the latest image from ${IMAGE}" + echo "The variable ASSISTED_CHAT_TEST was not provided; using ${IMAGE}:latest" ASSISTED_CHAT_TEST="${IMAGE}:latest" fi
44-46: Make MCP pod selection resilient under pipefail; handle missing deploy and empty lists.If the deployment doesn’t exist or returns empty, this pipeline can abort or print “null”. Guard both steps and default to empty.
-SELECTOR=$(oc get deploy -n "$NAMESPACE" assisted-service-mcp -o json | jq -r '.spec.selector.matchLabels | to_entries | map("\(.key)=\(.value)") | join(",")') -ASSISTED_SERVICE_MCP_POD=$(oc get pods -n "$NAMESPACE" -l "$SELECTOR" -o json | jq -r '.items | sort_by(.status.startTime) | last | .metadata.name') -echo $ASSISTED_SERVICE_MCP_POD +SELECTOR="$( + { oc get deploy -n "$NAMESPACE" assisted-service-mcp -o json 2>/dev/null || true; } \ + | jq -r '.spec.selector.matchLabels | to_entries | map("\(.key)=\(.value)") | join(",") // empty' 2>/dev/null || true +)" +ASSISTED_SERVICE_MCP_POD="" +if [[ -n "${SELECTOR:-}" ]]; then + ASSISTED_SERVICE_MCP_POD="$( + oc get pods -n "$NAMESPACE" -l "$SELECTOR" -o json 2>/dev/null \ + | jq -r '.items | sort_by(.status.startTime) | last? | .metadata.name // empty' 2>/dev/null || true + )" +fi +[[ -n "${ASSISTED_SERVICE_MCP_POD:-}" ]] && echo "$ASSISTED_SERVICE_MCP_POD" || echo "No assisted-service-mcp pod found"
60-64: Exit code may be empty; oc events should not abort diagnostics.Empty JSONPath yields exit ""; also oc events may fail under pipefail. Capture exit code with a safe default and don’t let events abort.
- echo "The evaluation tests failed, you can see the logs of the pods under the directory artifacts/eval-test/gather-extra/artifacts/pods/." - echo "oc events" - oc events -n "$NAMESPACE" - exit "$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}')" + echo "The evaluation tests failed. Logs are under artifacts/eval-test/gather-extra/artifacts/pods/." + echo "oc events" + oc events -n "$NAMESPACE" || true + EC="$(oc get pod "$POD_NAME" -n "$NAMESPACE" -o=jsonpath='{.status.containerStatuses[0].lastState.terminated.exitCode}' 2>/dev/null || true)" + [[ -n "${EC:-}" ]] || EC=1 + exit "$EC"
🧹 Nitpick comments (3)
scripts/deploy_template.sh (1)
19-23: Fix message typos and align wording with behavior.Minor grammar and clarity; also we aren’t “downloading” here.
- echo "The variable ASSISTED_CHAT_IMG was not provieded, downloading the latest image from ${IMAGE}" + echo "The variable ASSISTED_CHAT_IMG was not provided; using ${IMAGE}:latest"scripts/ci_test.sh (2)
53-55: Grammar nit: pluralize and clarify success message.- echo "The evaluation test were successful. The logs of the tests are stored in the directory artifacts/eval-test/gather-extra/artifacts/pods/ in the logs of the pod ${POD_NAME}." + echo "The evaluation tests were successful. Logs are under artifacts/eval-test/gather-extra/artifacts/pods/ for pod ${POD_NAME}."
72-73: Fix typo in timeout message.-echo "Timeout reached. Pod $POD_NAME did not become ready in time. PLease check the logs of the pods under the directory artifacts/eval-test/gather-extra/artifacts/pods/." +echo "Timeout reached. Pod $POD_NAME did not become ready in time. Please check logs under artifacts/eval-test/gather-extra/artifacts/pods/."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
scripts/ci_test.sh(3 hunks)scripts/deploy_template.sh(2 hunks)
| if [[ -n $ASSISTED_CHAT_IMG ]]; then | ||
| echo "The variable ASSISTED_CHAT_IMG was proided with the value ${ASSISTED_CHAT_IMG}, using that for creating the IMAGE and TAG variables for the template" | ||
| IMAGE=$(echo "$ASSISTED_CHAT_IMG" | cut -d ":" -f1) | ||
| TAG=$(echo "$ASSISTED_CHAT_IMG" | cut -d ":" -f2) | ||
| else |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard ASSISTED_CHAT_IMG under nounset; fix typos; handle @sha256 digest safely.
Reading an unset var with nounset aborts. Also, parsing image@sha256:digest with cut yields IMAGE containing “@sha256”. Strip digest properly; fix messages.
-if [[ -n $ASSISTED_CHAT_IMG ]]; then
- echo "The variable ASSISTED_CHAT_IMG was proided with the value ${ASSISTED_CHAT_IMG}, using that for creating the IMAGE and TAG variables for the template"
- IMAGE=$(echo "$ASSISTED_CHAT_IMG" | cut -d ":" -f1)
- TAG=$(echo "$ASSISTED_CHAT_IMG" | cut -d ":" -f2)
+if [[ -n "${ASSISTED_CHAT_IMG:-}" ]]; then
+ echo "The variable ASSISTED_CHAT_IMG was provided with the value ${ASSISTED_CHAT_IMG}; deriving IMAGE and TAG for the template"
+ # Supported forms:
+ # repo/name:tag
+ # repo/name@sha256:digest
+ IMG_NO_TAG="${ASSISTED_CHAT_IMG%%:*}"
+ TAG_OR_DIGEST="${ASSISTED_CHAT_IMG#*:}"
+ # If digest form, strip '@sha256' from IMAGE and keep digest in TAG (template must support it or you should pass a full ref param instead).
+ IMAGE="${IMG_NO_TAG%%@sha256*}"
+ TAG="${TAG_OR_DIGEST}"
+ if [[ -z "${TAG}" || "${TAG}" == "${ASSISTED_CHAT_IMG}" ]]; then
+ echo "ASSISTED_CHAT_IMG doesn't include a ':'; falling back to 'latest' tag"
+ TAG="latest"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ -n $ASSISTED_CHAT_IMG ]]; then | |
| echo "The variable ASSISTED_CHAT_IMG was proided with the value ${ASSISTED_CHAT_IMG}, using that for creating the IMAGE and TAG variables for the template" | |
| IMAGE=$(echo "$ASSISTED_CHAT_IMG" | cut -d ":" -f1) | |
| TAG=$(echo "$ASSISTED_CHAT_IMG" | cut -d ":" -f2) | |
| else | |
| if [[ -n "${ASSISTED_CHAT_IMG:-}" ]]; then | |
| echo "The variable ASSISTED_CHAT_IMG was provided with the value ${ASSISTED_CHAT_IMG}; deriving IMAGE and TAG for the template" | |
| # Supported forms: | |
| # repo/name:tag | |
| # repo/name@sha256:digest | |
| IMG_NO_TAG="${ASSISTED_CHAT_IMG%%:*}" | |
| TAG_OR_DIGEST="${ASSISTED_CHAT_IMG#*:}" | |
| # If digest form, strip '@sha256' from IMAGE and keep digest in TAG. | |
| IMAGE="${IMG_NO_TAG%%@sha256*}" | |
| TAG="${TAG_OR_DIGEST}" | |
| if [[ -z "${TAG}" || "${TAG}" == "${ASSISTED_CHAT_IMG}" ]]; then | |
| echo "ASSISTED_CHAT_IMG doesn't include a ':'; falling back to 'latest' tag" | |
| TAG="latest" | |
| fi | |
| else | |
| # ... existing else-branch ... |
🤖 Prompt for AI Agents
In scripts/deploy_template.sh around lines 14 to 18, the ASSISTED_CHAT_IMG check
must be resilient to nounset, the echo message has a typo, and image references
using a @sha256 digest must be parsed correctly; change the if test to use the
parameter expansion guard (e.g. [[ -n ${ASSISTED_CHAT_IMG-} ]]), fix the echo
text ("provided"), and parse the image safely: if the value contains '@' split
IMAGE as the substring before '@' and TAG as the substring after '@' (preserving
the sha256:digest), otherwise split IMAGE and TAG by the last ':' (IMAGE as
everything before the first ':' or using %%:* and TAG using ##*:), so you handle
both tag and digest forms without using cut.
2a8fc2d to
b2f5e2d
Compare
andrej1991
changed the title
If yes then that image is used to deploy the pods. If not then the latest image is downloaded from the repositories. Also fixing some debug logging issues. Turns out that most of the logs are already collected
andrej1991
force-pushed
the
debug_logging
branch
from
b2f5e2d to
b211914
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrej1991, carbonin The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit dda8372
into
rh-ecosystem-edge:main
2 participants
The 2 scripts in the commit can check if CI provided the needed image. If yes then that image is used to deploy the pods. If not then the latest image is downloaded from the repositories. Also fixing some debug logging issues. Turns out that most of the logs are already collected
Summary by CodeRabbit
New Features
Chores