feature/CSTACKEX-237: Pilot changes for scalebenmark framework - #85
feature/CSTACKEX-237: Pilot changes for scalebenmark framework#85piyush5netapp wants to merge 1 commit into
Conversation
sandeeplocharla
left a comment
sandeeplocharla
left a comment
There was a problem hiding this comment.
Overall, the pattern looks good but try to optimize the loggers and add more guardrails to the agent.
| python3 benchmark_storage_pool_concurrency.py --config config.yaml | ||
|
|
||
| # Concurrency matrix, iSCSI only, overriding the levels to run: | ||
| python3 benchmark_storage_pool_concurrency.py --config config.yaml --protocol iscsi --levels 2,5,10,20,30 |
There was a problem hiding this comment.
It should be enough to set config.yaml with all necessary combinations and use the fields appropriately in the code. Additional params might not be needed at this point.
| # Use the SAME --run-id across both scripts to combine 5.1.x + 6.1.x into one | ||
| # summary_<run_id>.csv / report (run sequential first, review it, then decide | ||
| # whether to proceed with concurrency under the same run id): | ||
| python3 benchmark_storage_pool_sequential.py --config config.yaml --run-id RUN-0001 |
There was a problem hiding this comment.
See if this run id can be randomized or create a pattern using timestamp or something?
| 6.1.1 Parallel pool creation | ||
| 6.1.2 Parallel pool deletion | ||
|
|
||
| See benchmark_storage_pool_sequential.py for the sequential scale matrix |
There was a problem hiding this comment.
Maybe add a rule in agents.md/custom-instruction file to only refer confluence but not add references to it in the code?
| else: | ||
| print(f" !! {result.error}") | ||
| if i in checkpoints: | ||
| total = sum(create_durations) |
There was a problem hiding this comment.
I guess the same statistics mean used in concurrent run script could be used here. Maybe include a rule to keep the code and libraries used consistent across the scripts.
| "total_time_sec": round(total, 3), "avg_time_sec": round(avg, 3), | ||
| "success_count": deleted_ok, "failure_count": idx - deleted_ok, "notes": "", | ||
| }) | ||
| print(f" >> checkpoint remaining={remaining}: total={total:.3f}s avg={avg:.3f}s/op " |
There was a problem hiding this comment.
Add a rule to keep the loggers also consistent across the scripts for better readability.
|
|
||
| def main(): | ||
| parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) | ||
| parser.add_argument("--config", default="config.yaml", help="Path to config YAML (default: config.yaml)") |
There was a problem hiding this comment.
As all the scripts seem to have same/similar args, it would be better to have a run.py file to which you would pass both the args and the name of the test you want to run. The respective script could be invoked from there while the additional args passed could be used to edit config file.
There was a problem hiding this comment.
Pull request overview
Adds a downstream-only benchmarking toolkit under private-cicd/benchmark/ontap to measure CloudStack ONTAP-plugin operations (storage pools and VM instances), log raw/summary CSVs, and render Confluence-ready markdown reports.
Changes:
- Introduces a minimal CloudStack session-key API client and shared helper modules for storage-pool and VM-instance benchmarks.
- Adds sequential/concurrency benchmark runners for
createStoragePool/deleteStoragePoolanddeployVirtualMachine/destroyVirtualMachine, including cleanup helpers. - Adds documentation, example configuration, requirements, and a report renderer to generate Confluence-aligned tables.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| private-cicd/benchmark/ontap/cloudstack_client.py | Minimal CloudStack HTTP client with session-key auth and async job polling. |
| private-cicd/benchmark/ontap/storage_pool_common.py | Shared storage-pool benchmark helpers (params, logging, cleanup, config). |
| private-cicd/benchmark/ontap/vm_instance_common.py | Shared VM-instance benchmark helpers (deploy/destroy, logging, cleanup, config). |
| private-cicd/benchmark/ontap/benchmark_storage_pool_sequential.py | Sequential scale benchmark driver for storage pools. |
| private-cicd/benchmark/ontap/benchmark_storage_pool_concurrency.py | Concurrency benchmark driver for storage pools. |
| private-cicd/benchmark/ontap/benchmark_vm_instance_sequential.py | Sequential scale benchmark driver for VM instances. |
| private-cicd/benchmark/ontap/benchmark_vm_instance_concurrency.py | Concurrency benchmark driver for VM instances. |
| private-cicd/benchmark/ontap/benchmark_vm_instance_combined.py | Convenience wrapper to run VM sequential + concurrency in one run. |
| private-cicd/benchmark/ontap/render_report.py | Converts run CSVs into Confluence-ready markdown tables + results-log rows. |
| private-cicd/benchmark/ontap/config.example.yaml | Example config template for lab environment and benchmark parameters. |
| private-cicd/benchmark/ontap/requirements.txt | Python dependencies for the benchmark scripts. |
| private-cicd/benchmark/ontap/README.md | Design/usage documentation for storage-pool + VM-instance benchmarks. |
| private-cicd/benchmark/ontap/USAGE.md | Quick “what to run” cheat sheet for scripts and flags. |
| private-cicd/benchmark/ontap/.gitignore | Ignores local config, venv, caches, and generated results artifacts. |
| private-cicd/benchmark/ontap/results/.gitkeep | Keeps the gitignored results directory present in the tree. |
Suppressed comments (1)
private-cicd/benchmark/ontap/README.md:148
- This example uses a hyphenated run id (RUN-20260722-101500), but elsewhere the tooling generates underscore-separated run ids (RUN_...). Keeping examples consistent helps avoid copying an ONTAP-invalid run id into the storage-pool scripts.
python3 render_report.py --run-id RUN-20260722-101500 \
--cloudstack-build 4.23.0.0-SNAPSHOT --ontap-version 9.15.1
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| rows = [r for r in summary_rows if r["test_id"] == test_id] | ||
| rows.sort(key=lambda r: (int(r["checkpoint"]), PROTOCOL_ORDER.index(r["protocol"]) | ||
| if r["protocol"] in PROTOCOL_ORDER else 99)) | ||
| lines = ["| Concurrency (C) | Protocol | Total Wall-clock (s) | Success | Failure | Avg Time/pool (s) | Notes |", |
| cleanup_by_filter(client, name_filter) | ||
| return | ||
|
|
||
| run_id = args.run_id or new_run_id() |
| python3 benchmark_storage_pool_sequential.py --config config.yaml --run-id RUN-0001 | ||
| python3 benchmark_storage_pool_concurrency.py --config config.yaml --run-id RUN-0001 |
| |---|---| | ||
| | `--config PATH` | Config YAML to use (default `config.yaml`). | | ||
| | `--protocol {nfs3,iscsi,both}` | Restrict the run to one protocol (default `both`). | | ||
| | `--run-id ID` | Reuse a specific run id (auto-generated otherwise) - use the **same id** across the sequential and concurrency scripts to merge both into one `summary_<run_id>.csv`/report. | |
| python3 render_report.py --run-id RUN-20260722-120000 \ | ||
| --cloudstack-build 4.23.0.0-SNAPSHOT --ontap-version 9.15.1 |
| cleanup_by_filter(client, name_filter) | ||
| return | ||
|
|
||
| run_id = args.run_id or new_run_id() |
Description
This PR...
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?