Add SHARED_DIR kubeconfig support for benchmark-runner#80496
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThe benchmark runner shell script adds a ChangesSHARED_DIR kubeconfig mode and AWS benchmark tests
Sequence Diagram(s)sequenceDiagram
participant Caller
participant BenchmarkRunner as benchmark-runner script
participant SharedDir as SHARED_DIR
participant SecretMounts as /secret mounts
participant Vault
Caller->>BenchmarkRunner: Execute with USE_SHARED_DIR flag
alt USE_SHARED_DIR == true
BenchmarkRunner->>SharedDir: Read KUBECONFIG
SharedDir-->>BenchmarkRunner: kubeconfig
BenchmarkRunner->>SharedDir: Read KUBEADMIN_PASSWORD (optional)
SharedDir-->>BenchmarkRunner: password
BenchmarkRunner-->>Caller: Ready (SHARED_DIR mode)
else USE_SHARED_DIR == false
BenchmarkRunner->>SecretMounts: Fetch SSH credentials
SecretMounts-->>BenchmarkRunner: credentials
BenchmarkRunner->>SecretMounts: Read /secret/kubeconfig
SecretMounts-->>BenchmarkRunner: kubeconfig
BenchmarkRunner->>Vault: Export Vault-backed secrets
Vault-->>BenchmarkRunner: secrets
BenchmarkRunner-->>Caller: Ready (/secret mode)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ci-operator/step-registry/redhat-performance/benchmark-runner/redhat-performance-benchmark-runner-commands.sh (1)
8-18: 💤 Low valueConsider explicitly checking SHARED_DIR is set before file test.
The current condition
${SHARED_DIR:-}/kubeconfigwill check/kubeconfigif SHARED_DIR is unset. While this is extremely unlikely to cause issues in CI, a more explicit check is clearer:-if [[ -s "${SHARED_DIR:-}/kubeconfig" ]]; then +if [[ -n "${SHARED_DIR:-}" ]] && [[ -s "${SHARED_DIR}/kubeconfig" ]]; thenThis makes the intent explicit and avoids the edge case entirely.
🤖 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-performance/benchmark-runner/redhat-performance-benchmark-runner-commands.sh` around lines 8 - 18, The condition checking for the kubeconfig file does not explicitly verify that SHARED_DIR is set before using it. Currently, if SHARED_DIR is unset, the condition evaluates to checking `/kubeconfig` due to the empty default expansion. Replace the current file test condition with an explicit check that SHARED_DIR is non-empty (using -n or by checking that the variable is not null) combined with the file existence test for the kubeconfig file, making the intent clear and avoiding the edge case entirely.
🤖 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.
Nitpick comments:
In
`@ci-operator/step-registry/redhat-performance/benchmark-runner/redhat-performance-benchmark-runner-commands.sh`:
- Around line 8-18: The condition checking for the kubeconfig file does not
explicitly verify that SHARED_DIR is set before using it. Currently, if
SHARED_DIR is unset, the condition evaluates to checking `/kubeconfig` due to
the empty default expansion. Replace the current file test condition with an
explicit check that SHARED_DIR is non-empty (using -n or by checking that the
variable is not null) combined with the file existence test for the kubeconfig
file, making the intent clear and avoiding the edge case entirely.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 0fce211a-ab6e-4fe0-941a-844d53074b58
📒 Files selected for processing (1)
ci-operator/step-registry/redhat-performance/benchmark-runner/redhat-performance-benchmark-runner-commands.sh
|
/ok-to-test |
openshift-ci
Bot
added
the
ok-to-test
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: arpsharm 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 pull-ci-redhat-performance-benchmark-runner-main-test-step-aws-sysbench-pod |
|
@arpsharm: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
arpsharm
force-pushed
the
benchmark-runner-shared-dir
branch
from
526604b to
ea7c23d
Compare
|
/pj-rehearse pull-ci-redhat-performance-benchmark-runner-main-test-step-aws-sysbench-pod |
|
@arpsharm: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 33 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: |
|
@arpsharm: 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. |
1 participant
Summary
Summary by CodeRabbit
This PR updates the OpenShift CI redhat-performance benchmark-runner to support a high-priority, external-workload kubeconfig input via the
SHARED_DIRenvironment variable, and extends CI test coverage for new AWS pod workloads.Key changes (benchmark-runner step script):
SHARED_DIR/kubeconfigexists, the script copies it to/tmp/kubeconfigand setsKUBECONFIGaccordingly.SHARED_DIR/kubeadmin-passwordis present, it is read and exported asKUBEADMIN_PASSWORD.SHARED_DIRis used, the script bypasses the SSH/Vault-based cluster access and the/secret/kubeconfigfallback, avoiding redundant secret retrieval./secretlogic remains the fallback whenSHARED_DIR/kubeconfigis not available, preserving backward compatibility.Key changes (CI test-step configuration):
redhat-performance-benchmark-runnerusing theipi-awsworkflow:aws-sysbench-pod(WORKLOAD:sysbench_pod)aws-uperf-pod(WORKLOAD:uperf_pod)cluster_profile: aws-rhdh-performanceand the AWS IPI-related environment variables appropriate for the benchmark pods.