network-perf-v2: support MicroShift runs#855
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds PLATFORM-aware env defaults and validation, refactors k8s-netperf to array-based execution, implements MicroShift-specific cluster discovery and early exit in utils/index.sh, and updates network-perf v2 documentation. ChangesMicroShift Platform Support
Sequence Diagram(s)sequenceDiagram
participant Runner as Network-perf Runner
participant RunSh as workloads/run.sh
participant Index as utils/index.sh
participant K8s as Kubernetes API
Runner->>RunSh: invoke workload (with PLATFORM)
RunSh->>Index: call index.sh (env includes PLATFORM=microshift)
Index->>K8s: read microshift-version ConfigMap / node info / pods
K8s-->>Index: return cluster/version/labels
Index->>Index: set MicroShift defaults and run index_tasks, exit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 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 `@workloads/network-perf-v2/run.sh`:
- Line 46: The command that runs the netperf binary uses a hardcoded path
"./k8s-netperf" which breaks when NETPERF_FILENAME is overridden; update the cmd
construction (the cmd array where TEST_TIMEOUT is used) to reference the
NETPERF_FILENAME variable instead of the hardcoded name so the script executes
the validated/downloaded file (preserving the timeout usage via TEST_TIMEOUT and
keeping the same array form).
- Around line 57-69: The print_command function currently only redacts
"--search=*" but logs other sensitive flags like "--prom=*"; update the case in
print_command (iterate over "${cmd[@]}") to also match and replace "--prom=*"
(e.g. add a pattern branch like --prom=*) so redacted_cmd receives
"--prom=<redacted>" for any prom URL/value, then continue printing redacted_cmd
as before; reference the print_command function and the cmd array and the
"--search" handling when adding the new "--prom" redaction branch.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e8cd15e4-f50a-411a-b653-1e6420f82e6e
📒 Files selected for processing (3)
utils/index.shworkloads/network-perf-v2/env.shworkloads/network-perf-v2/run.sh
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 `@utils/index.sh`:
- Around line 84-89: The export of RELEASE_STREAM currently runs the
substitution inline (export RELEASE_STREAM=$(...)), which masks failures under
set -e; change both occurrences so you first compute the value into a variable
(e.g., release_stream_TMP or RELEASE_STREAM_UNEXPORTED) using the jq/sed command
(the same expressions used now), check or let set -e catch failures, then export
the computed variable with a separate export
RELEASE_STREAM="$release_stream_TMP" assignment; update the branches that set
cluster_version and RELEASE_STREAM (the block using mscm=$(kubectl ...) and the
else branch using oc version and sed) to follow this two-step assign-then-export
pattern.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba3d3c73-02a2-4ad4-b8d6-e4acc5cfdc64
📒 Files selected for processing (3)
utils/index.shworkloads/network-perf-v2/env.shworkloads/network-perf-v2/run.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- workloads/network-perf-v2/env.sh
- workloads/network-perf-v2/run.sh
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@workloads/network-perf-v2/README.md`:
- Line 46: The "MicroShift" heading uses H3 (### MicroShift) and skips a heading
level; change it to H2 (## MicroShift) so the document hierarchy complies with
Markdown lint rule MD001. Locate the "MicroShift" heading in the README.md and
replace the leading "###" with "##" to restore correct nesting and maintain
consistency with surrounding H1/H2 headings.
- Around line 50-52: Remove the leading shell prompt from the fenced code block
so it no longer starts with "$"; locate the fenced block containing the command
"PLATFORM=microshift WORKLOAD=full-run.yaml ./run.sh" in README.md and edit it
to present the command without the "$" prefix (i.e., plain PLATFORM=...
./run.sh) to satisfy markdownlint MD014.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 63de666f-8a0a-4693-a47b-f3c15a1de7f4
📒 Files selected for processing (4)
utils/index.shworkloads/network-perf-v2/README.mdworkloads/network-perf-v2/env.shworkloads/network-perf-v2/run.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- utils/index.sh
- workloads/network-perf-v2/run.sh
- Skip OpenShift-only metadata helpers in utils/index.sh and synthesize the fields index_task() needs from Kubernetes resources, the microshift-version ConfigMap, and node labels. - Drop eval in run.sh in favor of an argv array; safer handling of values containing shell metacharacters. - env.sh: PLATFORM=microshift defaults LOCAL=true and METRICS=false so a default run does not trip the PROMETHEUS_URL guard. Signed-off-by: Sebastian Jug <seb@stianj.ug>
1 participant
levevltypo.Type of change
Description
Enables the
network-perf-v2workload to run against MicroShift clusters in addition to OpenShift, so the same continuous performance pipelines we run against OpenShift in prow CI can also be exercised against MicroShift.MicroShift has no in-cluster Prometheus and no machine-config / cluster-version / infrastructure CRs, so the workload previously failed at two points: (1) the
PROMETHEUS_URLguard inrun.shrejected the run, and (2)utils/index.sh::index_task()calledochelpers that assume OpenShift APIs are present. This PR adapts both code paths:utils/index.shadds a MicroShift branch that reads themicroshift-versionConfigMap, node labels, and basic Kubernetes resources to synthesize the sameperf_scale_cimetadata fieldsindex_task()writes for OpenShift (ocpVersion,releaseStream,platform,clusterType, node counts/types,networkType,ovnVersion,computeArch,controlPlaneArch).run.shreplacesevalwith an argv array (cmd=(...)+add_flag), which is also a hardening step regardless of platform — values containing shell metacharacters are now safely quoted.env.shkeysLOCALandMETRICSdefaults offPLATFORM. Default MicroShift runs goLOCAL=true/METRICS=false, so the existingPROMETHEUS_URLguard isn't hit when no Prometheus is configured. ExistingPLATFORM=openshiftbehavior is unchanged.README.mdis refreshed alongside: new MicroShift section, updated env-vars table that includes the variables already inenv.shbut missing from the docs, and corrected coverage tables forfull-run.yamlandossm.yaml.Related Tickets & Documents
Checklist before requesting a review
Testing
System Under Test:
masterNodesType: rhde)Steps:
cd workloads/network-perf-v2export PLATFORM=microshift WORKLOAD=full-run.yaml ES_SERVER=<url> NETPERF_VERSION=v0.1.42./run.shVerification:
Run UUID
3ca0e34b-fbe2-4d4c-96a5-688fb648ee30. Queried Elasticsearchk8s-netperfindex after the run:created=29from k8s-netperf log)TCP_STREAM×12,UDP_STREAM×12,TCP_RR×3,TCP_CRR×2 across message sizes 64/1024/4096/8192metadata.distribution=microshiftmetadata.microshift=truemetadata.microshiftVersion=4.22.0~rc.2metadata.microshiftMajorVersion=4.22metadata.k8sVersion=v1.35.3metadata.kernel=5.14.0-687.5.1.el9_8.x86_64metadata.masterNodesType=rhdemetadata.totalNodes=1throughputorlatency; all measured throughputs fall inside their reportedconfidence:[lo,hi]interval (0 violations across 29 docs).Summary by CodeRabbit
New Features
Documentation