RHDH: fix IMAGE_NAME using empty QUAY_REPO instead of IMAGE_REPO#79469
Conversation
IMAGE_NAME was constructed as "${QUAY_REPO}:${TAG_NAME}" in the PR
Branch Handling block, but QUAY_REPO is initialized empty and only gets
set from IMAGE_REPO after Image Tag Resolution. This produced broken
image names like ":pr-123-abcdef12" (missing repo prefix).
Fix by using IMAGE_REPO (the primary variable since PR openshift#76964) with a
fallback default: "${IMAGE_REPO:-rhdh-community/rhdh}:${TAG_NAME}".
This respects Gangway API overrides (MULTISTAGE_PARAM_OVERRIDE_IMAGE_REPO)
when set, and falls back to the default for standard presubmit runs.
Also set IMAGE_NAME in the else branch of Image Tag Resolution right
after IMAGE_REPO is assigned, so the wait loop echo statements display
the correct image name instead of the stale empty value.
Default IMAGE_REGISTRY to "quay.io" at initialization instead of empty
string, since that is always the intended default. Remove the now
redundant ${IMAGE_REGISTRY:-quay.io} fallbacks and duplicate export
statements that followed Image Tag Resolution.
Align osd-gcp/helm/nightly with the other scripts: add Gangway API
override logic, TAG_NAME override guard in PR Branch Handling,
override-first check in Image Tag Resolution, IMAGE_REGISTRY
conditional for the wait loop, and declare all MULTISTAGE_PARAM_OVERRIDE
env vars in the ref YAML.
Affected scripts:
- ocp/helm (presubmit)
- ocp/helm/nightly
- ocp/helm/upgrade/nightly
- eks/helm/nightly
- gke/helm/nightly
- aks/helm/nightly
- osd-gcp/helm/nightly
Assisted-by: OpenCode
WalkthroughSix CI operator command scripts for Red Hat Developer Hub Helm deployments across AKS, EKS, GKE, and OCP platforms are refactored to standardize image registry defaults, unify PR image naming logic, and simplify variable exports. The OSD-GCP script receives more extensive enhancements including parameter override support and registry-aware image availability polling. ChangesCloud Platform Helm CI Scripts Refactor
OSD-GCP Configuration Enhancements
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[REHEARSALNOTIFIER]
A total of 44 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ci-operator/step-registry/redhat-developer/rhdh/osd-gcp/helm/nightly/redhat-developer-rhdh-osd-gcp-helm-nightly-commands.sh (2)
2-2: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse
set -euo pipefailfor more robust error handling.The script currently uses only
set -e, but step registry command scripts should useset -euo pipefailto catch unset variable references (-u) and pipe failures (-o pipefail), improving reliability in CI environments.🛡️ Recommended change
-set -e +set -euo pipefailAs per coding guidelines: "Step registry command scripts should use set -euo pipefail by default"
🤖 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/redhat-developer/rhdh/osd-gcp/helm/nightly/redhat-developer-rhdh-osd-gcp-helm-nightly-commands.sh` at line 2, The script uses only "set -e" which misses unset-variable and pipe-failure checks; update the script's shell flags by replacing the existing set invocation (the "set -e" line) with "set -euo pipefail" so the script exits on errors, treats unset variables as errors, and fails on pipeline errors; ensure any later code that relies on unset variables is adjusted or variables are explicitly initialized to avoid -u failures.
123-147:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd validation before constructing
IMAGE_NAMEin the else branch to prevent emptyTAG_NAME.The else branch at line 147 can be reached by job types that don't match the periodic (line 125) or specific presubmit conditions (lines 90, 134). While all current jobs using this step are periodic and have
TAG_NAMEset, a postsubmit, batch, or other job type would reach this branch with an unsetTAG_NAME, resulting in an invalid image name likerhdh-community/rhdh:.Suggested fix
else IMAGE_REPO="rhdh-community/rhdh" + if [[ -z "${TAG_NAME}" ]]; then + echo "ERROR: TAG_NAME is not set. Cannot construct IMAGE_NAME." + exit 1 + fi IMAGE_NAME="${IMAGE_REPO}:${TAG_NAME}"🤖 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/redhat-developer/rhdh/osd-gcp/helm/nightly/redhat-developer-rhdh-osd-gcp-helm-nightly-commands.sh` around lines 123 - 147, The else branch currently sets IMAGE_REPO and constructs IMAGE_NAME="${IMAGE_REPO}:${TAG_NAME}" without validating TAG_NAME; add a check in that branch to ensure TAG_NAME is non-empty — if TAG_NAME is empty either set a safe default (e.g., "next") or log an error and exit; update the block that assigns IMAGE_REPO and IMAGE_NAME (referencing IMAGE_REPO, TAG_NAME, and IMAGE_NAME) to perform this validation and logging before constructing IMAGE_NAME so you never produce an image name ending with a trailing colon.
🧹 Nitpick comments (1)
ci-operator/step-registry/redhat-developer/rhdh/osd-gcp/helm/nightly/redhat-developer-rhdh-osd-gcp-helm-nightly-commands.sh (1)
161-161: 💤 Low valueQuote variable to prevent word splitting.
The
$responsevariable should be quoted to prevent potential issues with whitespace or glob characters in the JSON response.🔧 Shellcheck fix
- tag_count=$(echo $response | jq '.tags | length') + tag_count=$(echo "$response" | jq '.tags | length')🤖 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/redhat-developer/rhdh/osd-gcp/helm/nightly/redhat-developer-rhdh-osd-gcp-helm-nightly-commands.sh` at line 161, The assignment to tag_count uses echo $response unquoted which can cause word splitting or globbing; change the command that computes tag_count (the line setting tag_count from echo $response | jq '.tags | length') to echo the variable quoted (use echo "$response") so the JSON is passed intact into jq and prevents splitting or glob 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.
Outside diff comments:
In
`@ci-operator/step-registry/redhat-developer/rhdh/osd-gcp/helm/nightly/redhat-developer-rhdh-osd-gcp-helm-nightly-commands.sh`:
- Line 2: The script uses only "set -e" which misses unset-variable and
pipe-failure checks; update the script's shell flags by replacing the existing
set invocation (the "set -e" line) with "set -euo pipefail" so the script exits
on errors, treats unset variables as errors, and fails on pipeline errors;
ensure any later code that relies on unset variables is adjusted or variables
are explicitly initialized to avoid -u failures.
- Around line 123-147: The else branch currently sets IMAGE_REPO and constructs
IMAGE_NAME="${IMAGE_REPO}:${TAG_NAME}" without validating TAG_NAME; add a check
in that branch to ensure TAG_NAME is non-empty — if TAG_NAME is empty either set
a safe default (e.g., "next") or log an error and exit; update the block that
assigns IMAGE_REPO and IMAGE_NAME (referencing IMAGE_REPO, TAG_NAME, and
IMAGE_NAME) to perform this validation and logging before constructing
IMAGE_NAME so you never produce an image name ending with a trailing colon.
---
Nitpick comments:
In
`@ci-operator/step-registry/redhat-developer/rhdh/osd-gcp/helm/nightly/redhat-developer-rhdh-osd-gcp-helm-nightly-commands.sh`:
- Line 161: The assignment to tag_count uses echo $response unquoted which can
cause word splitting or globbing; change the command that computes tag_count
(the line setting tag_count from echo $response | jq '.tags | length') to echo
the variable quoted (use echo "$response") so the JSON is passed intact into jq
and prevents splitting or glob expansion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 3f62569b-9ee2-46c4-ad45-401d59ab195f
📒 Files selected for processing (8)
ci-operator/step-registry/redhat-developer/rhdh/aks/helm/nightly/redhat-developer-rhdh-aks-helm-nightly-commands.shci-operator/step-registry/redhat-developer/rhdh/eks/helm/nightly/redhat-developer-rhdh-eks-helm-nightly-commands.shci-operator/step-registry/redhat-developer/rhdh/gke/helm/nightly/redhat-developer-rhdh-gke-helm-nightly-commands.shci-operator/step-registry/redhat-developer/rhdh/ocp/helm/nightly/redhat-developer-rhdh-ocp-helm-nightly-commands.shci-operator/step-registry/redhat-developer/rhdh/ocp/helm/redhat-developer-rhdh-ocp-helm-commands.shci-operator/step-registry/redhat-developer/rhdh/ocp/helm/upgrade/nightly/redhat-developer-rhdh-ocp-helm-upgrade-nightly-commands.shci-operator/step-registry/redhat-developer/rhdh/osd-gcp/helm/nightly/redhat-developer-rhdh-osd-gcp-helm-nightly-commands.shci-operator/step-registry/redhat-developer/rhdh/osd-gcp/helm/nightly/redhat-developer-rhdh-osd-gcp-helm-nightly-ref.yaml
|
/pj-rehearse periodic-ci-redhat-developer-rhdh-main-e2e-ocp-helm-nightly |
|
@zdrapela: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: albarbaro, zdrapela 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 |
|
/pj-rehearse ack |
|
@zdrapela: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/retest |
|
@zdrapela: 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. |
fa6330e
into
openshift:main
…nshift#79469) IMAGE_NAME was constructed as "${QUAY_REPO}:${TAG_NAME}" in the PR Branch Handling block, but QUAY_REPO is initialized empty and only gets set from IMAGE_REPO after Image Tag Resolution. This produced broken image names like ":pr-123-abcdef12" (missing repo prefix). Fix by using IMAGE_REPO (the primary variable since PR openshift#76964) with a fallback default: "${IMAGE_REPO:-rhdh-community/rhdh}:${TAG_NAME}". This respects Gangway API overrides (MULTISTAGE_PARAM_OVERRIDE_IMAGE_REPO) when set, and falls back to the default for standard presubmit runs. Also set IMAGE_NAME in the else branch of Image Tag Resolution right after IMAGE_REPO is assigned, so the wait loop echo statements display the correct image name instead of the stale empty value. Default IMAGE_REGISTRY to "quay.io" at initialization instead of empty string, since that is always the intended default. Remove the now redundant ${IMAGE_REGISTRY:-quay.io} fallbacks and duplicate export statements that followed Image Tag Resolution. Align osd-gcp/helm/nightly with the other scripts: add Gangway API override logic, TAG_NAME override guard in PR Branch Handling, override-first check in Image Tag Resolution, IMAGE_REGISTRY conditional for the wait loop, and declare all MULTISTAGE_PARAM_OVERRIDE env vars in the ref YAML. Affected scripts: - ocp/helm (presubmit) - ocp/helm/nightly - ocp/helm/upgrade/nightly - eks/helm/nightly - gke/helm/nightly - aks/helm/nightly - osd-gcp/helm/nightly Assisted-by: OpenCode
Fix
IMAGE_NAMEconstruction using emptyQUAY_REPOinstead ofIMAGE_REPOacross all RHDH CI scripts.IMAGE_NAMEwas built as"${QUAY_REPO}:${TAG_NAME}"in PR Branch Handling, butQUAY_REPOis initialized empty and only set later fromIMAGE_REPOafter Image Tag Resolution — producing broken names like":pr-123-abcdef12"in https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/redhat-developer_rhdh/4821/pull-ci-redhat-developer-rhdh-release-1.8-e2e-ocp-helm/2056662986294038528#1:build-log.txt%3A109.Changes:
"${IMAGE_REPO:-rhdh-community/rhdh}:${TAG_NAME}"in PR Branch HandlingIMAGE_NAMEin theelsebranch of Image Tag ResolutionIMAGE_REGISTRY="quay.io"at init; remove redundant fallbacks and duplicate exportsMAX_WAIT_TIME_SECONDSand wait loop echo across all scriptsAffected scripts: ocp/helm, ocp/helm/nightly, ocp/helm/upgrade/nightly, eks/helm/nightly, gke/helm/nightly, aks/helm/nightly, osd-gcp/helm/nightly
RHDH CI image name construction fix
This PR fixes a bug in RHDH CI scripts where image names were being constructed incorrectly, resulting in malformed Docker image references (e.g.,
:pr-123-abcdef12with a missing repository prefix).Root cause and fix
The scripts were building
IMAGE_NAMEusingQUAY_REPObefore it was properly initialized. SinceQUAY_REPOstarted empty and was only later synced fromIMAGE_REPO, the resulting image names lacked the repository component. The fix involves:IMAGE_REGISTRYupfront — Set toquay.ioduring variable initialization instead of leaving it empty or defaulting it inline laterIMAGE_REPOfor PR builds — Switch from${QUAY_REPO}:${TAG_NAME}to${IMAGE_REPO:-rhdh-community/rhdh}:${TAG_NAME}when handling PR branches, ensuring a valid fallbackIMAGE_NAMEimmediately afterIMAGE_REPO— In non-overridden image tag resolution paths, assignIMAGE_NAMEright afterIMAGE_REPOis determined, avoiding stale valuesAffected scripts
Consistent updates across 6 nightly test scripts (for EKS, GKE, AKS, OCP, and OSD-GCP deployments) plus the main OCP Helm script, with a parallel update to the OSD-GCP Helm ref.yaml configuration to support runtime parameter overrides via Gangway API.
Additional alignment
The osd-gcp/helm/nightly script received additional changes to align its logic with other test scripts: added Gangway API override support for image and Git parameters, restructured image tag resolution precedence, and extended the Docker image availability check to handle both quay.io and non-quay.io registries appropriately.