[PERFSCALE-5187] Enable Control plane baseline tests on existing BM CPT Lab for Telco Core RDS - OCP 5.0 Nightly builds - #83169
Conversation
Signed-off-by: Raviteja Sahukari <rsahukar@redhat.com>
…support in prega-operators step Signed-off-by: Raviteja Sahukari <rsahukar@redhat.com> Assisted-by: Cursor
WalkthroughAdds the ChangesTelco performance and PREGA updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant TelcoCoreJob
participant PREGAInstaller
participant BastionArtifact
Scheduler->>TelcoCoreJob: start daily-telco-core-6nodes
TelcoCoreJob->>PREGAInstaller: run bare-metal PREGA installation
PREGAInstaller->>PREGAInstaller: derive OCP and PREGA versions
PREGAInstaller->>BastionArtifact: retrieve fallback IDMS manifest when local validation fails
BastionArtifact-->>PREGAInstaller: return version-specific manifest
PREGAInstaller->>PREGAInstaller: apply IDMS and stabilize cluster
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[REHEARSALNOTIFIER]
The following jobs are not rehearsable without the
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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-perfscale/openshift-eng-ocp-perfscale-main__metal-5.0-nightly-x86.yaml`:
- Around line 110-150: Run make update to regenerate artifacts for the new
daily-telco-core-6nodes scheduled job, then include all resulting generated Prow
job configuration, determinized CI configuration, and registry metadata changes
in the commit.
In
`@ci-operator/step-registry/openshift-qe/installer/bm/day2/prega-operators/openshift-qe-installer-bm-day2-prega-operators-commands.sh`:
- Around line 31-45: Update validate_yaml to require the exact
ImageDigestMirrorSet schema: apiVersion must be config.openshift.io/v1 and kind
must be ImageDigestMirrorSet, using yq when available and equivalent field
checks in the grep fallback. Ensure this validation is applied to both the
downloaded and bastion artifact files before proceeding to oc apply.
- Around line 17-18: Disable shell tracing before the authenticated curl
requests that expand QUAY_ACCESS_TOKEN in the DIGEST and OPERATOR_PREGA_VERSION
assignments, then re-enable tracing immediately afterward with set +x and set
-x. Preserve the existing command behavior and ordering.
🪄 Autofix
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: Pro Plus
Run ID: 70a85f7c-e0e4-4f44-8a61-16f2fdd47023
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift-eng/ocp-perfscale/openshift-eng-ocp-perfscale-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (2)
ci-operator/config/openshift-eng/ocp-perfscale/openshift-eng-ocp-perfscale-main__metal-5.0-nightly-x86.yamlci-operator/step-registry/openshift-qe/installer/bm/day2/prega-operators/openshift-qe-installer-bm-day2-prega-operators-commands.sh
| - as: daily-telco-core-6nodes | ||
| capabilities: | ||
| - intranet | ||
| cron: 0 19 * * * | ||
| reporter_config: | ||
| channel: '#ocp-qe-scale-ci-results' | ||
| job_states_to_report: | ||
| - success | ||
| - failure | ||
| - error | ||
| report_template: '{{if eq .Status.State "success"}} :white_check_mark: Job *{{.Spec.Job}}* | ||
| ended with *{{.Status.State}}*. <{{.Status.URL}}|View logs> :white_check_mark: | ||
| {{else}} :warning: Job *{{.Spec.Job}}* ended with *{{.Status.State}}*. <{{.Status.URL}}|View | ||
| logs> :warning: {{end}}' | ||
| restrict_network_access: false | ||
| steps: | ||
| allow_skip_on_success: true | ||
| cluster_profile: metal-perfscale-cpt | ||
| env: | ||
| CHURN_CYCLES: "2" | ||
| CHURN_PERCENT: "50" | ||
| EXTRA_FLAGS: --perf-profile=cpt-pao --gc-metrics=false --gc=true --phased=true | ||
| --idle-duration=15m | ||
| ITERATIONS: "15" | ||
| LABEL: node-role.kubernetes.io/worker-dpdk,node-role.kubernetes.io/worker-metallb | ||
| OCP_BUILD: dev | ||
| OPERATOR_SOURCE_INDEX: prega-operator-index | ||
| RUN_ORION: "true" | ||
| TELCO: "true" | ||
| post: | ||
| - ref: openshift-qe-installer-bm-gather-extra | ||
| - ref: openshift-qe-installer-bm-must-gather | ||
| test: | ||
| - ref: openshift-qe-installer-bm-day2-label | ||
| - ref: openshift-qe-installer-bm-day2-prega-operators | ||
| - ref: openshift-qe-installer-bm-day2-sriov | ||
| - ref: openshift-qe-installer-bm-day2-metallb | ||
| - ref: openshift-qe-installer-bm-day2-perf-profile-rds-core | ||
| - ref: openshift-qe-rds-core | ||
| - ref: openshift-qe-orion-rds-core | ||
| workflow: openshift-qe-installer-bm-deploy |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
make update
git status --short
git diff --checkRepository: openshift/release
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- git status ---\n'
git status --short
printf '\n--- Makefile metadata generation references ---\n'
if [ -f Makefile ]; then
rg -n 'update|zz_generated_metadata|ci-operator|Prow|prow|metadata' Makefile || true
printf '\n--- Makefile update target context ---\n'
sed -n '/^update:/,/^$/p' Makefile | head -80
else
printf 'No Makefile found\n'
fi
printf '\n--- Relevant diff summary ---\n'
git diff --stat
printf '\n--- Diff for modified CI config ---\n'
git diff -- ci-operator/config/openshift-eng/ocp-perfscale/openshift-eng-ocp-perfscale-main__metal-5.0-nightly-x86.yaml | sed -n '1,220p'Repository: openshift/release
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- git status ---'
git status --short
printf '%s\n' ''
printf '%s\n' '--- Makefile generation references ---'
if [ -f Makefile ]; then
rg -n 'update|zz_generated_metadata|ci-operator|Prow|prow|metadata' Makefile || true
printf '%s\n' ''
printf '%s\n' '--- Makefile update target context ---'
sed -n '/^update:/,/^$/p' Makefile | head -80
else
printf '%s\n' 'No Makefile found'
fi
printf '%s\n' ''
printf '%s\n' '--- Relevant diff summary ---'
git diff --stat
printf '%s\n' ''
printf '%s\n' '--- Diff for modified CI config ---'
git diff -- ci-operator/config/openshift-eng/ocp-perfscale/openshift-eng-ocp-perfscale-main__metal-5.0-nightly-x86.yaml | sed -n '1,220p'Repository: openshift/release
Length of output: 11712
Regenerate CI configuration artifacts before merge.
This change adds a scheduled CI job under ci-operator/config/openshift-eng/ocp-perfscale. Run make update and include any generated Prow job config, determinized CI config, and registry metadata changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@ci-operator/config/openshift-eng/ocp-perfscale/openshift-eng-ocp-perfscale-main__metal-5.0-nightly-x86.yaml`
around lines 110 - 150, Run make update to regenerate artifacts for the new
daily-telco-core-6nodes scheduled job, then include all resulting generated Prow
job configuration, determinized CI configuration, and registry metadata changes
in the commit.
Sources: Coding guidelines, Learnings
| DIGEST=$(curl -s -H "Authorization: Bearer ${QUAY_ACCESS_TOKEN}" ${QUAY_URL} | jq -r --arg tag "$OCP_VERSION" '.tags[] | select(.name == $tag) | .manifest_digest' | head -1) | ||
| OPERATOR_PREGA_VERSION=$(curl -s -H "Authorization: Bearer ${QUAY_ACCESS_TOKEN}" ${QUAY_URL} | jq -r --arg digest "$DIGEST" --arg tag "$OCP_VERSION" '.tags[] | select(.manifest_digest == $digest and .name != $tag) | .name' | sort -u) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='ci-operator/step-registry/openshift-qe/installer/bm/day2/prega-operators/openshift-qe-installer-bm-day2-prega-operators-commands.sh'
rg -n -C 3 'set .*x|set -o xtrace|set \+x|QUAY_ACCESS_TOKEN' "$file"Repository: openshift/release
Length of output: 1496
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='cioperator/step-registry/openshift-qe/installer/bm/day2/prega-operators/openshift-qe-installer-bm-day2-prega-operators-commands.sh'
printf 'Tracing/token occurrences in %s:\n' "$file"
rg -n -C 2 'set .*x|set -o xtrace|set \+x|QUAY_ACCESS_TOKEN|curl -s -H "Authorization: Bearer \${QUAY_ACCESS_TOKEN}"|^ *set -x$' "$file" || true
printf '\nFirst 90 lines:\n'
sed -n '1,90p' "$file"Repository: openshift/release
Length of output: 801
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='ci-operator/step-registry/openshift-qe/installer/bm/day2/prega-operators/openshift-qe-installer-bm-day2-prega-operators-commands.sh'
printf 'Tracing/token occurrences in %s:\n' "$file"
rg -n -C 2 'set .*x|set -o xtrace|set \+x|QUAY_ACCESS_TOKEN|Bearer \${QUAY_ACCESS_TOKEN}|^ *set -x$' "$file" || true
printf '\nFirst 90 lines:\n'
sed -n '1,90p' "$file"
python3 - <<'PY'
from pathlib import Path
import re
p = Path('ci-operator/step-registry/openshift-qe/installer/bm/day2/prega-operators/openshift-qe-installer-bm-day2-prega-operators-commands.sh')
lines = p.read_text().splitlines()
sensitive_pat = re.compile(r'Authorization:\s*Bearer\s+\${QUAY_ACCESS_TOKEN}')
for i, line in enumerate(lines, 1):
if sensitive_pat.search(line):
# Walk back to find nearest preceding set -x / set -o xtrace
enabled_before = False
disabled_since = None
for j in range(i, 0, -1):
stripped = lines[j-1].strip()
if stripped in ('set -x', 'set -o xtrace') or stripped.startswith('set -o xtrace '):
enabled_before = True
break
if stripped in ('set +x', 'set +o xtrace') or stripped.startswith('set +o xtrace '):
disabled_since = j
break
print(f"line {i}: enabled_before={enabled_before} since_disabled_line={disabled_since}")
print(f" {line}")
PYRepository: openshift/release
Length of output: 5445
Disable xtrace around the Quay token requests.
The script enables set -x in its prologue, but lines 17-18 expand QUAY_ACCESS_TOKEN in curl commands. Surround these authenticated requests with set +x / set -x to keep the bearer token out of CI logs.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 17-17: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 18-18: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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/installer/bm/day2/prega-operators/openshift-qe-installer-bm-day2-prega-operators-commands.sh`
around lines 17 - 18, Disable shell tracing before the authenticated curl
requests that expand QUAY_ACCESS_TOKEN in the DIGEST and OPERATOR_PREGA_VERSION
assignments, then re-enable tracing immediately afterward with set +x and set
-x. Preserve the existing command behavior and ordering.
Source: Coding guidelines
| validate_yaml() { | ||
| local f="$1" | ||
| [[ -s "$f" ]] || return 1 | ||
| if command -v yq &>/dev/null; then | ||
| yq eval 'has("kind") and has("apiVersion")' "$f" 2>/dev/null | grep -q '^true$' | ||
| else | ||
| grep -qE '^(apiVersion|kind):' "$f" | ||
| fi | ||
| } | ||
|
|
||
| if ! validate_yaml /tmp/idms_${OCP_VERSION}.yaml; then | ||
| echo "Downloaded /tmp/idms_${OCP_VERSION}.yaml is not valid YAML; falling back to bastion artifact for ${OCP_VERSION}" | ||
| scp -q ${SSH_ARGS} root@${bastion}:/root/prega_artifacts/idms_${OCP_VERSION}.yaml /tmp/idms_${OCP_VERSION}.yaml | ||
| validate_yaml /tmp/idms_${OCP_VERSION}.yaml \ | ||
| || { echo "Fallback IDMS /root/prega_artifacts/idms_${OCP_VERSION}.yaml is also invalid or missing"; exit 1; } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the expected ImageDigestMirrorSet schema.
validate_yaml accepts any nonempty YAML object with apiVersion and kind. A Kubernetes Status response or an unrelated resource then bypasses the fallback on Lines 41-46. The later oc apply can fail or apply an unintended resource.
Require apiVersion: config.openshift.io/v1 and kind: ImageDigestMirrorSet for both sources.
Proposed fix
[[ -s "$f" ]] || return 1
if command -v yq &>/dev/null; then
- yq eval 'has("kind") and has("apiVersion")' "$f" 2>/dev/null | grep -q '^true$'
+ yq eval '.apiVersion == "config.openshift.io/v1" and .kind == "ImageDigestMirrorSet"' "$f" 2>/dev/null | grep -q '^true$'
else
- grep -qE '^(apiVersion|kind):' "$f"
+ grep -qx 'apiVersion: config.openshift.io/v1' "$f" &&
+ grep -qx 'kind: ImageDigestMirrorSet' "$f"
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.
| validate_yaml() { | |
| local f="$1" | |
| [[ -s "$f" ]] || return 1 | |
| if command -v yq &>/dev/null; then | |
| yq eval 'has("kind") and has("apiVersion")' "$f" 2>/dev/null | grep -q '^true$' | |
| else | |
| grep -qE '^(apiVersion|kind):' "$f" | |
| fi | |
| } | |
| if ! validate_yaml /tmp/idms_${OCP_VERSION}.yaml; then | |
| echo "Downloaded /tmp/idms_${OCP_VERSION}.yaml is not valid YAML; falling back to bastion artifact for ${OCP_VERSION}" | |
| scp -q ${SSH_ARGS} root@${bastion}:/root/prega_artifacts/idms_${OCP_VERSION}.yaml /tmp/idms_${OCP_VERSION}.yaml | |
| validate_yaml /tmp/idms_${OCP_VERSION}.yaml \ | |
| || { echo "Fallback IDMS /root/prega_artifacts/idms_${OCP_VERSION}.yaml is also invalid or missing"; exit 1; } | |
| validate_yaml() { | |
| local f="$1" | |
| [[ -s "$f" ]] || return 1 | |
| if command -v yq &>/dev/null; then | |
| yq eval '.apiVersion == "config.openshift.io/v1" and .kind == "ImageDigestMirrorSet"' "$f" 2>/dev/null | grep -q '^true$' | |
| else | |
| grep -qx 'apiVersion: config.openshift.io/v1' "$f" && | |
| grep -qx 'kind: ImageDigestMirrorSet' "$f" | |
| fi | |
| } | |
| if ! validate_yaml /tmp/idms_${OCP_VERSION}.yaml; then | |
| echo "Downloaded /tmp/idms_${OCP_VERSION}.yaml is not valid YAML; falling back to bastion artifact for ${OCP_VERSION}" | |
| scp -q ${SSH_ARGS} root@${bastion}:/root/prega_artifacts/idms_${OCP_VERSION}.yaml /tmp/idms_${OCP_VERSION}.yaml | |
| validate_yaml /tmp/idms_${OCP_VERSION}.yaml \ | |
| || { echo "Fallback IDMS /root/prega_artifacts/idms_${OCP_VERSION}.yaml is also invalid or missing"; exit 1; } |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 41-41: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 43-43: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 43-43: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 43-43: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 43-43: Double quote to prevent globbing and word splitting.
(SC2086)
[info] 44-44: Double quote to prevent globbing and word splitting.
(SC2086)
🤖 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/installer/bm/day2/prega-operators/openshift-qe-installer-bm-day2-prega-operators-commands.sh`
around lines 31 - 45, Update validate_yaml to require the exact
ImageDigestMirrorSet schema: apiVersion must be config.openshift.io/v1 and kind
must be ImageDigestMirrorSet, using yq when available and equivalent field
checks in the grep fallback. Ensure this validation is applied to both the
downloaded and bastion artifact files before proceeding to oc apply.
|
/pj-rehearse network-access-allowed |
|
@mcornea: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mcornea, sraviteja-maker The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
@sraviteja-maker: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary by CodeRabbit
daily-telco-core-6nodesnightly control-plane baseline test on the existing metal performance-scale cluster.