feature/CSTACKEX-237: Pilot changes for scalebenmark framework by piyush5netapp · Pull Request #85 · NetApp/cloudstack

piyush5netapp · 2026-08-07T07:48:27Z

Description

This PR...

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
Build/CI
Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

sandeeplocharla

Overall, the pattern looks good but try to optimize the loggers and add more guardrails to the agent.

sandeeplocharla · 2026-08-08T07:28:49Z

+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


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.

Protocols, sequential checkpoints, and concurrency levels are now read exclusively from config.yaml.

sandeeplocharla · 2026-08-08T07:32:39Z

+# 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


See if this run id can be randomized or create a pattern using timestamp or something?

Run IDs are now generated automatically using a UTC timestamp and a random four-character suffix, for example RUN_20260930_160000_a1b2

sandeeplocharla · 2026-08-08T07:36:53Z

+  6.1.1 Parallel pool creation
+  6.1.2 Parallel pool deletion
+
+See benchmark_storage_pool_sequential.py for the sequential scale matrix


Maybe add a rule in agents.md/custom-instruction file to only refer confluence but not add references to it in the code?

private-cicd/benchmark/ontap/AGENTS.md now says the scale matrix is referenced from README.md and USAGE.md only.

sandeeplocharla · 2026-08-08T07:46:03Z

+        else:
+            print(f"    !! {result.error}")
+        if i in checkpoints:
+            total = sum(create_durations)


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.

Sequential and concurrency scripts now both use mean_seconds() from benchmark_support.py.

sandeeplocharla · 2026-08-08T07:48:07Z

+                "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 "


Add a rule to keep the loggers also consistent across the scripts for better readability.

All benchmark scripts now use configure_logging() and get_logger() from benchmark_support.py

sandeeplocharla · 2026-08-08T07:51:31Z

+
+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)")


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.

run.py is now the entry point. --test selects the benchmark, and the script loads config.yaml and calls that test.

Copilot

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/deleteStoragePool and deployVirtualMachine/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.

+        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()


rajiv-jain-netapp · 2026-09-25T06:31:52Z

+
+    if not args.dry_run and not args.skip_cleanup:
+        print(f"\nVerifying no orphaned VMs remain for run {run_id}...")
+        cleanup_by_filter(client, run_id)


in this file, you can not converting the VM name pattern from "-" to "-", would not it create VMs not matching and you may not be actually deleting any created VM?

VM creation and cleanup now use the same underscore-to-hyphen conversion through format_vm_name() and vm_name_token().

rajiv-jain-netapp · 2026-09-25T06:38:08Z

+        print(f"  {name} ({vm_id})")
+
+
+def cleanup_by_filter(client, name_filter):


do we need any sort of guard rails to not delete unintended VMs, in case of shaed setup. We do not even have provider check I am assuming, no impact of that?
Similar clean up you do for pool as well, same questions for that as well in storage_pool_common.py

Pool cleanup now requires the benchmark name pattern plus the configured zone, cluster, and storage provider. VM cleanup requires the benchmark name pattern and zone. VMs have no provider field, so the provider check is applied to the pool that holds any leftover unattached data disk.

github-actions · 2026-09-30T18:23:39Z

🔴 Test Coverage Grade: `D` — Marginal

Metric	Value
Line coverage	24.74%
Branch coverage	18.87%

Grade Scale

Grade	Line Coverage	Meaning
🟢 A	≥ 80%	Excellent - this code sleeps well at night 😴
🟡 B	60-79%	Good - almost there, don't stop now 😉
🟠 C	40-59%	Acceptable - your code is wearing a seatbelt, but no airbags 😬
🔴 D	20-39%	Marginal - boldly shipping where no test has gone before 🖖
⛔ F	< 20%	Failing - tests? what tests? 🔥

Branch coverage is shown as a secondary signal. Grade is determined by line coverage.
View full Actions run

feature/CSTACKEX-237: Pilot changes for scalebenmark framework

2b4070e

sandeeplocharla reviewed Aug 8, 2026

View reviewed changes

piyush5netapp marked this pull request as ready for review August 11, 2026 06:45

Copilot AI lite review requested due to automatic review settings August 11, 2026 06:45

Copilot started reviewing on behalf of piyush5netapp August 11, 2026 06:46 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

rajiv-jain-netapp reviewed Sep 25, 2026

View reviewed changes

Address ONTAP benchmark review comments

8395e72

		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

		python3 render_report.py --run-id RUN-20260722-120000 \
		--cloudstack-build 4.23.0.0-SNAPSHOT --ontap-version 9.15.1

		print(f" {name} ({vm_id})")


		def cleanup_by_filter(client, name_filter):

Conversation

piyush5netapp commented Aug 7, 2026

Description

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?

Uh oh!

sandeeplocharla left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 30, 2026

🔴 Test Coverage Grade: D — Marginal

Grade Scale

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

🔴 Test Coverage Grade: `D` — Marginal