Dnm hcp runs for 422#79362
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughCI job configs update ROSA HCP perfsector tests to use ChangesROSA HCP Performance Testing Configuration
Step-registry: e2e-benchmarking checkout simplification
🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/uncc @mohit-sheth @rsevilla87 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/openshift-qe/hypershift-infra/openshift-qe-hypershift-infra-commands.sh`:
- Around line 107-121: The current check can set VERIFY_SUCCESS=true when no
prometheus pods were evaluated because ALL_ON_INFRA defaults to true; modify the
for node in $(oc get pods -n openshift-monitoring -o wide | grep -i
"prometheus-k8s-" | grep -i running | awk '{print$7}') loop to track whether any
pods were found (e.g., increment a POD_COUNT or set a FOUND_PROM_POD flag inside
the loop) and after the loop only set VERIFY_SUCCESS=true and break if BOTH
ALL_ON_INFRA is true and the pod count/FOUND_PROM_POD indicates at least one pod
was evaluated; ensure you update the conditional that currently checks [
"$ALL_ON_INFRA" = true ] to require the additional non-zero pod condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2ff3fde4-3dc1-47ab-8104-03514ededc9e
⛔ Files ignored due to path filters (2)
ci-operator/jobs/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (3)
ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.21-nightly-x86.yamlci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.22-nightly-x86.yamlci-operator/step-registry/openshift-qe/hypershift-infra/openshift-qe-hypershift-infra-commands.sh
| while [ $RETRY -lt $MAX_RETRIES ]; do | ||
| ALL_ON_INFRA=true | ||
| for node in $(oc get pods -n openshift-monitoring -o wide | grep -i "prometheus-k8s-" | grep -i running | awk '{print$7}'); do | ||
| if [[ $(oc get nodes --no-headers -l node-role.kubernetes.io/infra | awk '{print$1}' | grep -w "$node") != "" ]]; then | ||
| log "$(date) - prometheus pod on $node (infra node) ✓" | ||
| else | ||
| log "$(date) - WARNING: prometheus pod on $node is NOT an infra node" | ||
| ALL_ON_INFRA=false | ||
| fi | ||
| done | ||
|
|
||
| if [ "$ALL_ON_INFRA" = true ]; then | ||
| log "$(date) - All prometheus-k8s pods are on infra nodes ✓" | ||
| VERIFY_SUCCESS=true | ||
| break |
There was a problem hiding this comment.
Avoid false success when no Prometheus pods are matched.
Line 118 can mark verification successful even when zero pods were evaluated, because ALL_ON_INFRA starts as true and the loop may not run. This can incorrectly pass migration validation.
Proposed fix
while [ $RETRY -lt $MAX_RETRIES ]; do
ALL_ON_INFRA=true
- for node in $(oc get pods -n openshift-monitoring -o wide | grep -i "prometheus-k8s-" | grep -i running | awk '{print$7}'); do
+ mapfile -t PROM_NODES < <(oc get pods -n openshift-monitoring -o wide | awk '/prometheus-k8s-/ && tolower($3)=="running" {print $7}')
+
+ if [ "${`#PROM_NODES`[@]}" -eq 0 ]; then
+ log "$(date) - WARNING: no running prometheus-k8s pods found yet"
+ ALL_ON_INFRA=false
+ fi
+
+ for node in "${PROM_NODES[@]}"; do
if [[ $(oc get nodes --no-headers -l node-role.kubernetes.io/infra | awk '{print$1}' | grep -w "$node") != "" ]]; then
log "$(date) - prometheus pod on $node (infra node) ✓"
else
log "$(date) - WARNING: prometheus pod on $node is NOT an infra node"
ALL_ON_INFRA=false
fi
done📝 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.
| while [ $RETRY -lt $MAX_RETRIES ]; do | |
| ALL_ON_INFRA=true | |
| for node in $(oc get pods -n openshift-monitoring -o wide | grep -i "prometheus-k8s-" | grep -i running | awk '{print$7}'); do | |
| if [[ $(oc get nodes --no-headers -l node-role.kubernetes.io/infra | awk '{print$1}' | grep -w "$node") != "" ]]; then | |
| log "$(date) - prometheus pod on $node (infra node) ✓" | |
| else | |
| log "$(date) - WARNING: prometheus pod on $node is NOT an infra node" | |
| ALL_ON_INFRA=false | |
| fi | |
| done | |
| if [ "$ALL_ON_INFRA" = true ]; then | |
| log "$(date) - All prometheus-k8s pods are on infra nodes ✓" | |
| VERIFY_SUCCESS=true | |
| break | |
| while [ $RETRY -lt $MAX_RETRIES ]; do | |
| ALL_ON_INFRA=true | |
| mapfile -t PROM_NODES < <(oc get pods -n openshift-monitoring -o wide | awk '/prometheus-k8s-/ && tolower($3)=="running" {print $7}') | |
| if [ "${`#PROM_NODES`[@]}" -eq 0 ]; then | |
| log "$(date) - WARNING: no running prometheus-k8s pods found yet" | |
| ALL_ON_INFRA=false | |
| fi | |
| for node in "${PROM_NODES[@]}"; do | |
| if [[ $(oc get nodes --no-headers -l node-role.kubernetes.io/infra | awk '{print$1}' | grep -w "$node") != "" ]]; then | |
| log "$(date) - prometheus pod on $node (infra node) ✓" | |
| else | |
| log "$(date) - WARNING: prometheus pod on $node is NOT an infra node" | |
| ALL_ON_INFRA=false | |
| fi | |
| done | |
| if [ "$ALL_ON_INFRA" = true ]; then | |
| log "$(date) - All prometheus-k8s pods are on infra nodes ✓" | |
| VERIFY_SUCCESS=true | |
| break |
🤖 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/openshift-qe/hypershift-infra/openshift-qe-hypershift-infra-commands.sh`
around lines 107 - 121, The current check can set VERIFY_SUCCESS=true when no
prometheus pods were evaluated because ALL_ON_INFRA defaults to true; modify the
for node in $(oc get pods -n openshift-monitoring -o wide | grep -i
"prometheus-k8s-" | grep -i running | awk '{print$7}') loop to track whether any
pods were found (e.g., increment a POD_COUNT or set a FOUND_PROM_POD flag inside
the loop) and after the loop only set VERIFY_SUCCESS=true and break if BOTH
ALL_ON_INFRA is true and the pod count/FOUND_PROM_POD indicates at least one pod
was evaluated; ensure you update the conditional that currently checks [
"$ALL_ON_INFRA" = true ] to require the additional non-zero pod condition.
Sandeepyadav93
force-pushed
the
dnm_hcp_runs_for_422
branch
from
2316b75 to
085ae44
Compare
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Sandeepyadav93
force-pushed
the
dnm_hcp_runs_for_422
branch
from
085ae44 to
077b269
Compare
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.21-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Sandeepyadav93
force-pushed
the
dnm_hcp_runs_for_422
branch
2 times, most recently
from
fd9292c to
c86e396
Compare
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-node-density-heavy-24nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-node-density-heavy-24nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@Sandeepyadav93: job(s): either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Sandeepyadav93
force-pushed
the
dnm_hcp_runs_for_422
branch
from
c86e396 to
44a7867
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Sandeepyadav93 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 |
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.22-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.22-nightly-x86.yaml`:
- Line 211: The CLUSTER_SECTOR variable was changed to perf4 for this job
(CLUSTER_SECTOR) while most onperfsector jobs still reference perf2; confirm
whether this is an intentional staged migration or a mistake, then either (A)
make the change consistent by updating all onperfsector job definitions (the
other job entries that still use perf2) to use CLUSTER_SECTOR: perf4, or (B)
revert this single job back to perf2; also add a short comment near
CLUSTER_SECTOR and/or a top-of-file note describing the migration plan and scope
so future reviewers know whether perf2→perf4 is complete or in-progress.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 144741ff-f38f-49f7-a45b-90d9aba40466
📒 Files selected for processing (2)
ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.21-nightly-x86.yamlci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.22-nightly-x86.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.21-nightly-x86.yaml
| env: | ||
| CHANNEL_GROUP: nightly | ||
| CLUSTER_SECTOR: perf2 | ||
| CLUSTER_SECTOR: perf4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify sector usage across all onperfsector jobs in 4.21 and 4.22 configs
# Check CLUSTER_SECTOR values in related config files
echo "=== 4.22 config ==="
rg -n 'CLUSTER_SECTOR:' ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.22-nightly-x86.yaml
echo -e "\n=== 4.21 config ==="
rg -n 'CLUSTER_SECTOR:' ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.21-nightly-x86.yamlRepository: openshift/release
Length of output: 774
Clarify whether the sector migration to perf4 is intentional and complete.
CLUSTER_SECTOR was changed to perf4 for this job at line 211, but the majority of onperfsector jobs in this file still use perf2 (lines 180, 244, 290, 336, 382, 427, 473, 519, 566). Only two jobs in the 4.22 config use perf4 (lines 150 and 211). If this is part of a deliberate sector migration strategy, clarify the plan; if unintended, migrate remaining jobs consistently.
🤖 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/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.22-nightly-x86.yaml`
at line 211, The CLUSTER_SECTOR variable was changed to perf4 for this job
(CLUSTER_SECTOR) while most onperfsector jobs still reference perf2; confirm
whether this is an intentional staged migration or a mistake, then either (A)
make the change consistent by updating all onperfsector job definitions (the
other job entries that still use perf2) to use CLUSTER_SECTOR: perf4, or (B)
revert this single job back to perf2; also add a short comment near
CLUSTER_SECTOR and/or a top-of-file note describing the migration plan and scope
so future reviewers know whether perf2→perf4 is complete or in-progress.
Sandeepyadav93
force-pushed
the
dnm_hcp_runs_for_422
branch
from
44a7867 to
dd9f520
Compare
|
/pj-rehearse periodic-ci-openshift-eng-ocp-qe-perfscale-ci-main-rosa_hcp-4.21-nightly-x86-control-plane-120nodes-onperfsector |
|
@Sandeepyadav93: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 640 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.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@ci-operator/step-registry/openshift-qe/cluster-density-v2/openshift-qe-cluster-density-v2-commands.sh`:
- Line 52: The git clone step currently clones full history and is unpinned (git
clone $REPO_URL); change it to perform a shallow, ref-pinned clone (e.g., use
--depth 1 and --branch <ref_var> / --single-branch) and introduce an env var
(e.g., REPO_REF or GIT_REF) to pin the commit/branch used for benchmarking. Also
prevent credential leakage by wrapping any reads/exports of Elasticsearch
credentials (ES_PASSWORD, ES_USERNAME) and any other sensitive vars with tracing
disabled (use set +x before reading/exporting and set -x after); ensure both the
locations referenced in the review (the blocks that read/export
ES_USERNAME/ES_PASSWORD and any later usages around line 66) are updated to
disable tracing only for the minimal sensitive operations.
In
`@ci-operator/step-registry/openshift-qe/crd-scale/openshift-qe-crd-scale-commands.sh`:
- Around line 52-54: Remove the unused external GitHub API call and stale
variables by deleting the LATEST_TAG and TAG_OPTION computation (they are never
used) and ensure git clone $REPO_URL is used without calling the GitHub releases
API; additionally, prevent credential leakage by disabling shell tracing around
reading and exporting secrets: wrap the reads of ES_PASSWORD and ES_USERNAME and
the export of ES_SERVER with tracing off (temporarily run "set +x" before
reading/exporting and "set -x" after) so the sensitive values are not printed to
CI logs; references to update: LATEST_TAG, TAG_OPTION, REPO_URL, ES_PASSWORD,
ES_USERNAME, ES_SERVER, and any existing set -x usage in this script.
In
`@ci-operator/step-registry/openshift-qe/node-density-cni/openshift-qe-node-density-cni-commands.sh`:
- Line 54: The git clone line uses an unpinned, full-depth clone (git clone
$REPO_URL); update it to match the node-density-heavy pattern by performing a
shallow, pinned checkout: use the existing REPO_URL but clone with --depth 1 and
--single-branch and pin to a fixed ref/branch variable (e.g., REPO_REF or
NODE_DENSITY_REF) or fetch that ref explicitly and checkout it, ensuring
deterministic, shallow clones instead of full history downloads.
In
`@ci-operator/step-registry/openshift-qe/node-density/openshift-qe-node-density-commands.sh`:
- Line 57: Replace the full clone command (git clone $REPO_URL) with a shallow,
branch-pinned clone to speed and stabilize checkouts (use the repo branch/ref
variable and depth=1) and wrap sensitive operations where tracing is active:
temporarily disable shell tracing (set +x) before the credential-related blocks
(the clone/checkout area and the Elasticsearch URL construction that
interpolates $ES_USERNAME and $ES_PASSWORD) and re-enable tracing (set -x)
immediately after; ensure the exact blocks to change are the git clone
invocation and the block that builds/uses the ES server URL so no credentials
are emitted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: ba3d56ef-f829-499e-a822-aec114a12c82
⛔ Files ignored due to path filters (2)
ci-operator/jobs/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main-periodics.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (6)
ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.21-nightly-x86.yamlci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.22-nightly-x86.yamlci-operator/step-registry/openshift-qe/cluster-density-v2/openshift-qe-cluster-density-v2-commands.shci-operator/step-registry/openshift-qe/crd-scale/openshift-qe-crd-scale-commands.shci-operator/step-registry/openshift-qe/node-density-cni/openshift-qe-node-density-cni-commands.shci-operator/step-registry/openshift-qe/node-density/openshift-qe-node-density-commands.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/config/openshift-eng/ocp-qe-perfscale-ci/openshift-eng-ocp-qe-perfscale-ci-main__rosa_hcp-4.22-nightly-x86.yaml
| LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/e2e-benchmarking/releases/latest" | jq -r '.tag_name'); | ||
| TAG_OPTION="--branch $(if [ "$E2E_VERSION" == "default" ]; then echo "$LATEST_TAG"; else echo "$E2E_VERSION"; fi)"; | ||
| git clone $REPO_URL $TAG_OPTION --depth 1 | ||
| git clone $REPO_URL |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the file
fd -t f "openshift-qe-cluster-density-v2-commands.sh"Repository: openshift/release
Length of output: 163
🏁 Script executed:
# Check the content of the file
head -60 ci-operator/step-registry/openshift-qe/cluster-density-v2/openshift-qe-cluster-density-v2-commands.shRepository: openshift/release
Length of output: 2671
🏁 Script executed:
# Look for context around line 52 and check tracing/set flags
sed -n '1,80p' ci-operator/step-registry/openshift-qe/cluster-density-v2/openshift-qe-cluster-density-v2-commands.sh | cat -nRepository: openshift/release
Length of output: 3730
Pin checkout to explicit ref with shallow clone AND fix credential leakage in tracing.
Line 52 clones without pinning to a specific ref and pulls full history, which increases clone overhead and can skew benchmark comparability across runs. Additionally, lines 15–16 and 66 read and export Elasticsearch credentials (password and username) while set -x tracing is enabled, causing credential leakage in CI logs. This violates the step registry security guideline requiring set +x around sensitive data.
Suggested changes
-ES_PASSWORD=$(cat "${ES_SECRETS_PATH}/password")
-ES_USERNAME=$(cat "${ES_SECRETS_PATH}/username")
+set +x
+ES_PASSWORD=$(cat "${ES_SECRETS_PATH}/password")
+ES_USERNAME=$(cat "${ES_SECRETS_PATH}/username")
+set -x-export ES_SERVER="https://$ES_USERNAME:$ES_PASSWORD@$ES_HOST"
+set +x
+export ES_SERVER="https://$ES_USERNAME:$ES_PASSWORD@$ES_HOST"
+set -x-git clone $REPO_URL
+E2E_REF="${E2E_VERSION:-main}"
+git clone --depth 1 --branch "${E2E_REF}" "${REPO_URL}"🤖 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/openshift-qe/cluster-density-v2/openshift-qe-cluster-density-v2-commands.sh`
at line 52, The git clone step currently clones full history and is unpinned
(git clone $REPO_URL); change it to perform a shallow, ref-pinned clone (e.g.,
use --depth 1 and --branch <ref_var> / --single-branch) and introduce an env var
(e.g., REPO_REF or GIT_REF) to pin the commit/branch used for benchmarking. Also
prevent credential leakage by wrapping any reads/exports of Elasticsearch
credentials (ES_PASSWORD, ES_USERNAME) and any other sensitive vars with tracing
disabled (use set +x before reading/exporting and set -x after); ensure both the
locations referenced in the review (the blocks that read/export
ES_USERNAME/ES_PASSWORD and any later usages around line 66) are updated to
disable tracing only for the minimal sensitive operations.
| LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/e2e-benchmarking/releases/latest" | jq -r '.tag_name'); | ||
| TAG_OPTION="--branch $(if [ "$E2E_VERSION" == "default" ]; then echo "$LATEST_TAG"; else echo "$E2E_VERSION"; fi)"; | ||
| git clone $REPO_URL $TAG_OPTION --depth 1 | ||
| git clone $REPO_URL |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n ci-operator/step-registry/openshift-qe/crd-scale/openshift-qe-crd-scale-commands.shRepository: openshift/release
Length of output: 3118
Remove stale tag-resolution logic that creates unnecessary external API calls.
Lines 52-53 fetch and compute TAG_OPTION, but Line 54 ignores it entirely. This keeps an external GitHub API dependency on the hot path with no functional effect.
Additionally, this script violates the step registry security guidelines: Lines 17-18 read sensitive credentials (ES_PASSWORD, ES_USERNAME) and Line 58 exports ES_SERVER with embedded credentials while set -x tracing is active. Disable tracing around these sensitive operations to prevent credential leakage in CI logs.
Suggested changes
REPO_URL="https://github.com/cloud-bulldozer/e2e-benchmarking";
-LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/e2e-benchmarking/releases/latest" | jq -r '.tag_name');
-TAG_OPTION="--branch $(if [ "$E2E_VERSION" == "default" ]; then echo "$LATEST_TAG"; else echo "$E2E_VERSION"; fi)";
git clone $REPO_URL
ES_SECRETS_PATH=${ES_SECRETS_PATH:-/secret}
ES_HOST=${ES_HOST:-"search-ocp-qe-perf-scale-test-elk-hcm7wtsqpxy7xogbu72bor4uve.us-east-1.es.amazonaws.com"}
+set +x
ES_PASSWORD=$(cat "${ES_SECRETS_PATH}/password")
ES_USERNAME=$(cat "${ES_SECRETS_PATH}/username")
+set -x
if [ -e "${ES_SECRETS_PATH}/host" ]; then
ES_HOST=$(cat "${ES_SECRETS_PATH}/host")
fiAnd around line 58:
+set +x
export ES_SERVER="https://$ES_USERNAME:$ES_PASSWORD@$ES_HOST"
+set -x
export EXTRA_FLAGS="${KB_FLAGS} ${CRD_SCALE_EXTRA_FLAGS} --gc-metrics=false --profile-type=${PROFILE_TYPE} --iterations=${ITERATIONS}"🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 53-53: TAG_OPTION appears unused. Verify use (or export if used externally).
(SC2034)
🤖 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/openshift-qe/crd-scale/openshift-qe-crd-scale-commands.sh`
around lines 52 - 54, Remove the unused external GitHub API call and stale
variables by deleting the LATEST_TAG and TAG_OPTION computation (they are never
used) and ensure git clone $REPO_URL is used without calling the GitHub releases
API; additionally, prevent credential leakage by disabling shell tracing around
reading and exporting secrets: wrap the reads of ES_PASSWORD and ES_USERNAME and
the export of ES_SERVER with tracing off (temporarily run "set +x" before
reading/exporting and "set -x" after) so the sensitive values are not printed to
CI logs; references to update: LATEST_TAG, TAG_OPTION, REPO_URL, ES_PASSWORD,
ES_USERNAME, ES_SERVER, and any existing set -x usage in this script.
| LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/e2e-benchmarking/releases/latest" | jq -r '.tag_name'); | ||
| TAG_OPTION="--branch $(if [ "$E2E_VERSION" == "default" ]; then echo "$LATEST_TAG"; else echo "$E2E_VERSION"; fi)"; | ||
| git clone $REPO_URL $TAG_OPTION --depth 1 | ||
| git clone $REPO_URL |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the file and read its contents around line 54
fd -t f "openshift-qe-node-density-cni-commands.sh" ci-operator/step-registry/Repository: openshift/release
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Read the file to see the context around line 54
wc -l ci-operator/step-registry/openshift-qe/node-density-cni/openshift-qe-node-density-cni-commands.shRepository: openshift/release
Length of output: 162
🏁 Script executed:
#!/bin/bash
# Read the full file content with line numbers
cat -n ci-operator/step-registry/openshift-qe/node-density-cni/openshift-qe-node-density-cni-commands.shRepository: openshift/release
Length of output: 4132
🏁 Script executed:
#!/bin/bash
# Search for E2E_VERSION usage in step registry
rg "E2E_VERSION" ci-operator/step-registry/ --type shRepository: openshift/release
Length of output: 5833
🏁 Script executed:
#!/bin/bash
# Search for git clone patterns in step registry scripts
rg "git clone" ci-operator/step-registry/ --type sh -A 2 -B 1 | head -100Repository: openshift/release
Length of output: 12380
🏁 Script executed:
#!/bin/bash
# Look for similar benchmark/e2e workload scripts in the step registry
fd -t f "*commands.sh" ci-operator/step-registry/ | xargs grep -l "e2e-benchmarking\|kube-burner" | head -10Repository: openshift/release
Length of output: 353
🏁 Script executed:
#!/bin/bash
# Search for other node-density related scripts to compare patterns
find ci-operator/step-registry -name "*node-density*-commands.sh" -type fRepository: openshift/release
Length of output: 761
🏁 Script executed:
#!/bin/bash
# Check one of the similar workload scripts to see how they handle git clone
cat -n ci-operator/step-registry/openshift-qe/node-density-heavy/openshift-qe-node-density-heavy-commands.sh | head -100Repository: openshift/release
Length of output: 3174
🏁 Script executed:
#!/bin/bash
# Look for cluster-density-v2 to compare
find ci-operator/step-registry -name "*cluster-density-v2*-commands.sh" -type f -exec cat -n {} \; | head -150Repository: openshift/release
Length of output: 6301
🏁 Script executed:
#!/bin/bash
# Extract the git clone lines from node-density-heavy to compare patterns
sed -n '32,36p' ci-operator/step-registry/openshift-qe/node-density-heavy/openshift-qe-node-density-heavy-commands.shRepository: openshift/release
Length of output: 463
Align repository checkout with node-density-heavy using fixed ref and shallow clone.
Line 54 clones without version pinning or shallow depth, causing full history downloads and non-deterministic performance results. The node-density-heavy script demonstrates the established pattern in this codebase for handling this correctly.
Suggested change
+LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/e2e-benchmarking/releases/latest" | jq -r '.tag_name');
+TAG_OPTION="--branch $(if [ "$E2E_VERSION" == "default" ]; then echo "$LATEST_TAG"; else echo "$E2E_VERSION"; fi)";
-git clone $REPO_URL
+git clone $REPO_URL $TAG_OPTION --depth 1🤖 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/openshift-qe/node-density-cni/openshift-qe-node-density-cni-commands.sh`
at line 54, The git clone line uses an unpinned, full-depth clone (git clone
$REPO_URL); update it to match the node-density-heavy pattern by performing a
shallow, pinned checkout: use the existing REPO_URL but clone with --depth 1 and
--single-branch and pin to a fixed ref/branch variable (e.g., REPO_REF or
NODE_DENSITY_REF) or fetch that ref explicitly and checkout it, ensuring
deterministic, shallow clones instead of full history downloads.
| LATEST_TAG=$(curl -s "https://api.github.com/repos/cloud-bulldozer/e2e-benchmarking/releases/latest" | jq -r '.tag_name'); | ||
| TAG_OPTION="--branch $(if [ "$E2E_VERSION" == "default" ]; then echo "$LATEST_TAG"; else echo "$E2E_VERSION"; fi)"; | ||
| git clone $REPO_URL $TAG_OPTION --depth 1 | ||
| git clone $REPO_URL |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n ci-operator/step-registry/openshift-qe/node-density/openshift-qe-node-density-commands.shRepository: openshift/release
Length of output: 4306
Disable tracing around credential operations to prevent leakage in CI logs.
Line 62 exposes $ES_PASSWORD and $ES_USERNAME in the Elasticsearch server URL while set -x is active, which will leak credentials to CI logs. Per coding guidelines, tracing must be temporarily disabled around sensitive URLs and credentials.
Additionally, line 57 should use a shallow clone with a pinned branch for faster, more consistent checkouts.
Suggested changes
REPO_URL="https://github.com/cloud-bulldozer/e2e-benchmarking";
-git clone $REPO_URL
+E2E_REF="${E2E_VERSION:-main}"
+git clone --depth 1 --branch "${E2E_REF}" "${REPO_URL}"
export EXTRA_FLAGS UUID
set +o errexit
./run.sh
RUN_EXIT_CODE=$?
set -o errexit
METRICS_FOLDER="collected-metrics-${UUID}"
if [[ -f ${METRICS_FOLDER}/jobSummary.json ]]; then
cp -r ${METRICS_FOLDER} "${ARTIFACT_DIR}/"
if [[ ${JOB_NAME} == *openshift-eng-ocp-qe-perfscale-ci* ]] && [[ ${JOB_TYPE} == "periodic" ]]; then
set +e
+ set +x
OCP_PERF_DASH_HOST=$(cat ${ES_SECRETS_PATH}/ocp-perf-dash-address)
OCP_PERF_DASH_DIR="/usr/share/ocp-perf-dash/${JOB_NAME}/${WORKLOAD}/${UUID}"
METRICS="${METRICS_FOLDER}/*QuantilesMeasurement*.json ${METRICS_FOLDER}/jobSummary.json"
SSH_ARGS="-i ${ES_SECRETS_PATH}/ocp-perf-dash-id_rsa -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null"
ssh ${SSH_ARGS} ${OCP_PERF_DASH_HOST} "mkdir -p ${OCP_PERF_DASH_DIR}"
scp ${SSH_ARGS} ${METRICS} ${OCP_PERF_DASH_HOST}:${OCP_PERF_DASH_DIR}"
+ set -x
set -eAdditionally, lines 25-30 and 62 need tracing disabled around sensitive data operations.
🤖 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/openshift-qe/node-density/openshift-qe-node-density-commands.sh`
at line 57, Replace the full clone command (git clone $REPO_URL) with a shallow,
branch-pinned clone to speed and stabilize checkouts (use the repo branch/ref
variable and depth=1) and wrap sensitive operations where tracing is active:
temporarily disable shell tracing (set +x) before the credential-related blocks
(the clone/checkout area and the Elasticsearch URL construction that
interpolates $ES_USERNAME and $ES_PASSWORD) and re-enable tracing (set -x)
immediately after; ensure the exact blocks to change are the git clone
invocation and the block that builds/uses the ES server URL so no credentials
are emitted.
|
@Sandeepyadav93: The following tests 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. |
1 participant
Summary
This PR updates OpenShift CI configuration and step-registry scripts used by the openshift-eng ocp-qe-perfscale CI jobs that run ROSA HCP (Hosted Control Plane) performance and scalability tests.
What changed (practical terms)
CI job definitions (ci-operator configs) for the perfscale test jobs were modified so scheduled ROSA HCP tests target the perf4 cluster sector and include explicit cron schedules.
0 2,14 * * *), and the always_run:false restriction was removed to allow scheduled execution.0 2,14 * * *rosa-aws-sts-hcp-mpperf4TicketId:731, REGION:us-east-2, REPLICAS:"24", OPENSHIFT_VERSION:"4.22"Several step-registry shell scripts used by openshift-qe tests were changed to alter repository checkout behavior for the e2e-benchmarking repository:
--depth 1) has been replaced with a plaingit clone $REPO_URL(no tag/branch selection and no shallow-depth). This changes which revision is fetched and will retrieve full git history by default.Impact
Files of interest