feat: allow in-cluster perf benchmarks with a kubectl one-liner

[hhzhang16]

Overview:

Details:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• closes GitHub issue: #xxx

Summary by CodeRabbit

• New Features
  • Added support for running benchmarks directly inside Kubernetes clusters using internal service URLs and cluster DNS.
  • Improved input handling to accept in-cluster endpoints and auto-scheme prefixing for seamless execution.
  • Provided a ready-to-run Kubernetes Job template for configurable, isolated benchmark runs with resource controls and automatic cleanup.
• Documentation
  • Added an in-cluster benchmarking guide with prerequisites, quick start, configuration via environment variables, monitoring/debugging steps, result retrieval, and troubleshooting.
  • Documented result storage on a persistent volume with a structure consistent with local benchmarking.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

Walkthrough

Adds in-cluster benchmarking support: new Kubernetes utility is_running_in_cluster, updates to input validation and workflow to accept internal service URLs and normalize schemes in cluster, a Kubernetes Job manifest to run benchmarks in-cluster with PVC-backed storage, and a README documenting the end-to-end in-cluster benchmarking process.

Changes

Cohort / File(s)	Summary
In-cluster docs benchmarks/incluster/README.md	New documentation detailing prerequisites, deployment, execution, result retrieval, monitoring, and troubleshooting for running Dynamo benchmarks inside Kubernetes using internal service URLs and PVC storage.
K8s Job manifest benchmarks/incluster/benchmark_job.yaml	Adds a batch/v1 Job (dynamo-benchmark) that runs the benchmark runner with configurable env vars, mounts PVC dynamo-pvc at /data, uses dynamo-sa, references secrets, and executes python3 -m benchmarks.utils.benchmark with in-cluster inputs.
Benchmark validation + workflow benchmarks/utils/benchmark.py, benchmarks/utils/workflow.py	Enables cluster-aware input handling: accepts internal host:port endpoints in-cluster, prefixes http:// when missing, updates logging and parameter wiring to use service_url, and passes stddev and output_dir/label to concurrency sweep.
Kubernetes utilities deploy/utils/kubernetes.py	Introduces is_running_in_cluster() to detect Kubernetes environment by presence of the service account token file at /var/run/secrets/kubernetes.io/serviceaccount/token.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant K8s as Kubernetes Job
  participant C as Benchmark Runner (container)
  participant WF as Workflow
  participant V as Input Validation
  participant S as Target Service
  participant PVC as PVC Storage

  User->>K8s: Apply benchmark_job.yaml
  K8s->>C: Start pod with env and PVC mounted
  C->>WF: run_endpoint_benchmark(model, endpoint, ...)
  WF->>WF: Detect is_running_in_cluster()
  WF->>WF: Build service_url (prefix http:// if missing)
  WF->>V: validate_inputs(service_url)
  V-->>WF: OK (accept internal host:port in-cluster)
  WF->>S: Send benchmark requests
  WF->>PVC: Write results to /data/results
  WF-->>C: Complete sweep
  C-->>K8s: Job completes (TTL for cleanup)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat: decouple dynamo k8s setup from additional benchmarking requirements #2973 — Also introduces/uses deploy/utils/kubernetes.py:is_running_in_cluster and aligns with in-cluster benchmarking resources.
• feat: remove kubectl dependencies from benchmarking #3098 — Touches benchmarks/utils/benchmark.py and workflow paths but restricts inputs to HTTP endpoints; potentially conflicts with this PR’s in-cluster URL acceptance.

Poem

In pods I hop, a cluster-bound sprite,
Nibbling on endpoints, day and night.
PVC burrow where results now dwell,
I chart the sweeps—oh, data to tell!
With kube-side trails and service smell,
Benchmarks thump—all is well. 🐇📊

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The PR description contains only the template headings with no substantive content and a placeholder Related Issues entry ("#xxx"), so it does not describe the changes, rationale, or reviewer guidance required by the repository's description_template and is therefore largely incomplete.	Please populate the template: add an Overview that summarizes the goal (enable in-cluster perf benchmarks and the kubectl one-liner), a Details section that lists and explains key file changes (e.g., benchmarks/incluster/README.md, benchmarks/incluster/benchmark_job.yaml, benchmarks/utils/{benchmark.py,workflow.py}, deploy/utils/kubernetes.py) and the rationale, and a "Where should the reviewer start?" section pointing to those files and any tests or commands to run; replace the placeholder Related Issues entry with the actual issue number and include brief testing instructions and expected outcomes so reviewers can validate the change.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "feat: allow in-cluster perf benchmarks with a kubectl one-liner" directly summarizes the primary change: enabling in-cluster performance benchmarking and providing a kubectl one-liner deployment workflow; it is concise, specific, and related to the README, Job manifest, workflow, and cluster-detection helper introduced in the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

• Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
• Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (10)

deploy/utils/kubernetes.py (1)

25-28: Make in-cluster detection more robust (env var fallback) and avoid repeated fs stats.

Also handle cases where SA token isn’t mounted (automount disabled) by checking KUBERNETES_SERVICE_HOST. Cache the token path for clarity.

-import os
+import os
 from pathlib import Path
 from typing import List
@@
-def is_running_in_cluster() -> bool:
-    """Check if we're running inside a Kubernetes cluster"""
-    # Check for Kubernetes service account token (most reliable indicator)
-    return os.path.exists("/var/run/secrets/kubernetes.io/serviceaccount/token")
+K8S_SA_TOKEN = Path("/var/run/secrets/kubernetes.io/serviceaccount/token")
+
+def is_running_in_cluster() -> bool:
+    """Return True if running inside a Kubernetes cluster."""
+    # Prefer well-known env var; fall back to SA token presence
+    return bool(os.environ.get("KUBERNETES_SERVICE_HOST")) or K8S_SA_TOKEN.exists()

Also applies to: 16-16

benchmarks/incluster/benchmark_job.yaml (1)

57-58: Add trailing newline.

Satisfies yamllint new-line-at-end-of-file.

benchmarks/utils/workflow.py (2)

34-41: Factor URL normalization into a helper to avoid drift and handle scheme-less inputs cleanly.

Keeps logic in one place and aligns with in-cluster behavior.

-    # Handle internal service URLs by adding http:// prefix if needed
-    service_url = endpoint
-    if is_running_in_cluster() and not endpoint.lower().startswith(
-        ("http://", "https://")
-    ):
-        service_url = f"http://{endpoint}"
+    # Normalize endpoint to a usable URL (handles in-cluster scheme-less inputs)
+    service_url = normalize_service_url(endpoint)

Add this helper near the top of the module:

from urllib.parse import urlsplit

def normalize_service_url(endpoint: str) -> str:
    e = endpoint.strip()
    if e.lower().startswith(("http://", "https://")):
        return e
    if is_running_in_cluster():
        return f"http://{e}"
    return e  # Outside cluster, validation will have ensured scheme is present

---

84-85: Update docstring to reflect in-cluster support.

-    """Main benchmark workflow orchestrator for HTTP endpoints only"""
+    """Main benchmark workflow orchestrator for HTTP endpoints (and in-cluster internal service URLs)"""

benchmarks/utils/benchmark.py (2)

130-132: Stale comment: now validates HTTP or in-cluster URLs.

-        # Validate that all inputs are HTTP endpoints
+        # Validate that inputs are HTTP endpoints or in-cluster service URLs

---

25-27: Action: shorten the long exception text or disable Ruff TRY003 for this file.

ruff.toml enables tryceratops (select includes "TRY") and no TRY003 ignore exists, so Ruff will report TRY003 for these long exception messages.

File: benchmarks/utils/benchmark.py — lines 25–27 and 31–33.

Options:

• Shorten the exception message(s).
• Add an inline ignore on the raise line: # noqa: TRY003
• Add a per-file ignore in config: [tool.ruff.lint.extend-per-file-ignores] "benchmarks/utils/benchmark.py" = ["TRY003"].

benchmarks/incluster/README.md (4)

52-53: Path correctness: ensure envsubst reads the right file.

Command assumes cwd at benchmarks/incluster. Use explicit path or add a cd.

-# Deploy the benchmark job
-envsubst < benchmark_job.yaml | kubectl apply -f -
+# Deploy the benchmark job
+envsubst < benchmarks/incluster/benchmark_job.yaml | kubectl apply -f -

---

61-64: Prefer heading over emphasis for section titles (markdownlint MD036).

-**Option B: One-liner deployment**
+### Option B: One-liner deployment

---

42-45: Prefer heading over emphasis for section titles (markdownlint MD036).

-**Option A: Set environment variables (recommended for multiple commands)**
+### Option A: Set environment variables (recommended for multiple commands)

---

173-175: Fix service verification command: SERVICE_URL includes port; kubectl expects a Service name.

Extract the name before the colon.

-# Verify your service URL is accessible
-kubectl get svc $SERVICE_URL -n $NAMESPACE
+# Verify your service exists and has endpoints
+SVC_NAME="${SERVICE_URL%%:*}"
+kubectl get svc "$SVC_NAME" -n "$NAMESPACE"
+kubectl get endpoints "$SVC_NAME" -n "$NAMESPACE"

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2d39f1b and e7ed272.

📒 Files selected for processing (5)

• benchmarks/incluster/README.md (1 hunks)
• benchmarks/incluster/benchmark_job.yaml (1 hunks)
• benchmarks/utils/benchmark.py (1 hunks)
• benchmarks/utils/workflow.py (2 hunks)
• deploy/utils/kubernetes.py (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

benchmarks/utils/workflow.py (2)

deploy/utils/kubernetes.py (1)

• is_running_in_cluster (25-28)

benchmarks/utils/genai.py (1)

• run_concurrency_sweep (104-118)

benchmarks/utils/benchmark.py (1)

deploy/utils/kubernetes.py (1)

• is_running_in_cluster (25-28)

🪛 Ruff (0.12.2)

benchmarks/utils/benchmark.py

25-27: Avoid specifying long messages outside the exception class

(TRY003)

---

31-33: Avoid specifying long messages outside the exception class

(TRY003)

🪛 markdownlint-cli2 (0.17.2)

benchmarks/incluster/README.md

42-42: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

61-61: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🪛 Checkov (3.2.334)

benchmarks/incluster/benchmark_job.yaml

[medium] 4-58: Containers should not run with allowPrivilegeEscalation

(CKV_K8S_20)

---

[medium] 4-58: Minimize the admission of root containers

(CKV_K8S_23)

🪛 YAMLlint (1.37.1)

benchmarks/incluster/benchmark_job.yaml

[error] 58-58: no new line character at the end of file

(new-line-at-end-of-file)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Build and Test - dynamo

🔇 Additional comments (2)

benchmarks/utils/workflow.py (1)

48-55: Signature verified — no action required.
run_concurrency_sweep is defined in benchmarks/utils/genai.py:104 as (service_url, model_name, isl, osl, stddev, output_dir) and the call at benchmarks/utils/workflow.py:48 uses matching keyword args.

benchmarks/incluster/benchmark_job.yaml (1)

57-57: Verify TTL controller availability before using ttlSecondsAfterFinished

kubectl was unavailable in the verification environment (kubectl: command not found). Run one of these on a cluster with kubectl access to confirm TTL support:

• kubectl api-resources | grep -i 'ttlSecondsAfterFinished'
• kubectl api-resources | grep -i ttl

If the TTL resource/controller is not present, remove or guard the ttlSecondsAfterFinished field in benchmarks/incluster/benchmark_job.yaml (line 57) or enable the TTL controller on the cluster.

[biswapanda]

left few comments

[biswapanda]

Commit history seems to be messed up

[biswapanda]

lgtm.
please address Itay's concerns docs around server/client side distinctions may be summary/images might help.

[biswapanda]

lgtm

[hhzhang16]

/ok to test de853cf