feat(cube-bench): add host-mount flag for create requests by zyl1121 · Pull Request #651 · TencentCloud/CubeSandbox

zyl1121 · 2026-06-25T13:13:48Z

Motivation

examples/cube-bench currently benchmarks the default raw sandbox create path. That makes it hard to measure create-time overhead for host-mount, which is the concrete create-time variation we want to compare.

CubeAPI already accepts host-mount through metadata["host-mount"] as a JSON string. cube-bench should be able to send that same create-time payload so host-mount sandbox setups can be benchmarked directly.

What Changed

This PR adds a --host-mount '<json-array>' flag to cube-bench and includes the provided host-mount payload in sandbox create requests.

Implementation highlights:

examples/cube-bench/main.go adds the new flag, validates a non-empty host-mount JSON array once during config parsing, preserves the original JSON for config display and exported reports, and caches the compacted host-mount string plus request body for request-time reuse
examples/cube-bench/runner.go sends the cached request body and headers on sandbox create requests, so benchmark throughput is not distorted by re-building host-mount metadata on every request
examples/cube-bench/main_test.go covers valid host-mount arrays, invalid JSON, non-array input, empty input, and empty-array rejection
examples/cube-bench/README.md documents the new flag and a host-mount example

This keeps the benchmark helper focused on the current benchmark need without changing the backend metadata contract.

Validation

go test ./... passed in examples/cube-bench
focused tests cover:
- valid host-mount arrays being compacted once before request-time use
- invalid host-mount JSON being rejected
- non-array host-mount input being rejected
- empty host-mount arrays being rejected
- final create requests keeping metadata["host-mount"] as a string value
real-machine validation passed with the cube-bench binary using a --host-mount '[...]' payload

cubesandboxbot · 2026-06-25T13:20:23Z

Review: PR #651

Well-structured change. The defer fix closes a real connection leak.

Findings

1. runner.go:49 -- Silently discarded io.ReadAll error

Fix: check the error from io.ReadAll.

2. runner.go:51-55 -- API response body in error messages

Error includes up to 200 chars of API body, which could leak sensitive data into exported reports.

3. runner.go:82 -- dresp.Body.Close()

Correct fix. Consider also draining: io.Copy(io.Discard, dresp.Body).

Test coverage gaps

No benchOne error path tests: non-2xx create, empty sandboxID
buildCreateRequestBody tested only indirectly
RunBenchmark should be unexported (runner.go:119)

Positive

prepareHostMount validates structure before compacting
No API key leakage
Values via json.Marshal -- no injection risk
README accurately describes CLI usage and backend contract

cubesandboxbot · 2026-06-25T13:21:09Z

+	if cfg.Metadata != "" {
+		metaMap := cfg.metadataMap
+		if metaMap == nil {
+			var err error
+			metaMap, err = normalizeMetadata(cfg.Metadata)
+			if err != nil {
+				r.Err = fmt.Sprintf("metadata: %v", err)
+				return r
+			}


Dead fallback code on benchmark hot path. parseConfig() (main.go:151-162) already guarantees that when cfg.Metadata != "", cfg.metadataMap is populated — or the program exits with os.Exit(1). The nil-check and re-normalization on lines 32–39 are unreachable in production, yet they run on every warmup + measured iteration.

Recommendation: remove lines 32–39 and simplify to:

if cfg.Metadata != "" { reqBody["metadata"] = cfg.metadataMap }

cubesandboxbot · 2026-06-25T13:21:11Z

+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost || r.URL.Path != "/sandboxes" {
+			t.Fatalf("request = %s %s", r.Method, r.URL.Path)
+		}
+		if auth := r.Header.Get("Authorization"); auth != "Bearer test-key" {
+			t.Fatalf("Authorization=%q, want Bearer test-key", auth)
+		}
+		if err := json.NewDecoder(r.Body).Decode(&got); err != nil {
+			t.Fatalf("decode body: %v", err)
+		}


t.Fatalf inside httptest handler can hang the test. The handler runs on the server goroutine, not the test goroutine. If t.Fatal/t.Fatalf fires here (lines 21, 24, 27), runtime.Goexit() terminates only the handler goroutine — no HTTP response is written, and the test goroutine blocks forever in client.Do(req). The test times out silently (10 min) instead of failing cleanly.

Recommendation: use t.Errorf in the handler and write a non-2xx status:

if r.Method != http.MethodPost || r.URL.Path != "/sandboxes" { t.Errorf("request = %s %s", r.Method, r.URL.Path) w.WriteHeader(http.StatusBadRequest) return }

cubesandboxbot · 2026-06-25T13:21:13Z

+		}
+		reqBody["metadata"] = metaMap
+	}
+	body, _ := json.Marshal(reqBody)


json.Marshal error silently discarded (pre-existing pattern). While json.Marshal on the current types never fails, a future change to the body structure could make this fail silently. Consider returning the error as defense-in-depth.

cubesandboxbot · 2026-06-25T13:21:25Z

+	if cfg.Metadata != "" {
+		metaMap := cfg.metadataMap
+		if metaMap == nil {
+			var err error
+			metaMap, err = normalizeMetadata(cfg.Metadata)
+			if err != nil {
+				r.Err = fmt.Sprintf("metadata: %v", err)
+				return r
+			}
+		}
+		reqBody["metadata"] = metaMap
+	}


Dead fallback code on hot path. parseConfig() (main.go:151-162) already guarantees that when cfg.Metadata != "", cfg.metadataMap is populated — or the program has called os.Exit(1). The nil-check and re-normalization on lines 32–39 are unreachable in production.

This matters because benchOne is on the benchmark hot path (every warmup + measured iteration). The dead branch adds a map lookup + conditional branch to every call for no benefit.

Recommendation: remove lines 32–40 and simplify to:

if cfg.Metadata != "" { reqBody["metadata"] = cfg.metadataMap }

cubesandboxbot · 2026-06-25T13:21:26Z

+		if r.Method != http.MethodPost || r.URL.Path != "/sandboxes" {
+			t.Fatalf("request = %s %s", r.Method, r.URL.Path)


t.Fatalf inside httptest handler goroutine can hang the test. The handler runs on the server goroutine, not the test goroutine. If t.Fatal/t.Fatalf fires here, it calls runtime.Goexit() on the handler goroutine only — no HTTP response is written, and the test's main goroutine blocks forever in client.Do(req). The test times out silently (typically 10 min) instead of failing cleanly.

This applies to all t.Fatalf calls inside the handler (lines 21, 24, 27).

Recommendation: use t.Errorf instead and send a non-2xx status so benchOne returns an error result that can be asserted on the main goroutine:

if r.Method != http.MethodPost || r.URL.Path != "/sandboxes" { t.Errorf("request = %s %s", r.Method, r.URL.Path) w.WriteHeader(http.StatusBadRequest) return }

cubesandboxbot · 2026-06-25T13:21:27Z

+		}
+		reqBody["metadata"] = metaMap
+	}
+	body, _ := json.Marshal(reqBody)


Silent json.Marshal error discard (pre-existing pattern). While json.Marshal on the current types is safe, a future change to the body structure could make this fail silently. Consider checking and returning the error instead as defense-in-depth.

cubesandboxbot · 2026-06-25T13:37:34Z

+
+func TestBenchOneSendsMetadataAsStringMap(t *testing.T) {
+	rawMetadata := `{"scenario":"integration-execution","host-mount":[{"hostPath":"/tmp/data","mountPath":"/mnt/data","readOnly":false}]}`
+	metaMap, err := normalizeMetadata(rawMetadata)


Integration test bypasses prepareMetadata — the test calls normalizeMetadata directly (line 13) and manually constructs the Config fields. This means the prepareMetadata wrapper — which adds the empty-object rejection (len(metadata) == 0 check) and the --metadata is not valid JSON error wrapping — is not exercised end-to-end.

Consider calling prepareMetadata(rawMetadata) instead of normalizeMetadata(rawMetadata) so the test validates the same path parseConfig uses. This would catch any future divergence between the two functions.

cubesandboxbot · 2026-06-25T13:37:36Z

+func buildCreateRequestBody(template string, metadata map[string]string) ([]byte, error) {
+	reqBody := map[string]interface{}{"templateID": template}
+	if len(metadata) > 0 {
+		reqBody["metadata"] = metadata
+	}
+	return json.Marshal(reqBody)
+}


Prefer a concrete struct over map[string]interface{} — using any here means the compiler can't help catch structural errors. A struct with json:"omitempty" would be more idiomatic Go:

type createRequest struct { TemplateID string `json:"templateID"` Metadata map[string]string `json:"metadata,omitempty"` }

This also guarantees that when metadata is empty the key is absent from the JSON output rather than present with a null value — which is the current intent (line 93-94) but is enforced only via conditional logic rather than the type system.

cubesandboxbot · 2026-06-25T13:37:38Z

@@ -27,11 +27,9 @@ func benchOne(client *http.Client, cfg *Config, seq int) IterResult {
 	apiURL := cfg.APIURL
 	headers := map[string]string{"Authorization": "Bearer " + cfg.APIKey}


Per-iteration headers map allocation could be cached — this map is heap-allocated on every single benchmark iteration (the hot path). The pre-computation philosophy already applied to cfg.requestBody could be extended here: pre-build the headers in parseConfig() and store them in Config. The savings per-iteration are small, but for a latency-measurement tool, reducing GC pressure at high concurrency is worthwhile.

ls-ggg · 2026-06-26T07:43:09Z

Thanks for the PR. Since we won't have many options similar to host-mount for benchmarks, could you just add host-mount as a standalone parameter?

Add a --host-mount JSON array flag to cube-bench and include the compacted host-mount payload in sandbox create requests. The benchmark helper now focuses on the concrete host-mount scenario, while preserving the original host-mount JSON for config display and exported reports. Signed-off-by: zhengyilei <zheng_yilei@qq.com>

zyl1121 · 2026-06-26T08:36:10Z

@ls-ggg Makes sense. I narrowed this PR to a dedicated --host-mount flag and updated the title/body/document accordingly.

ls-ggg · 2026-06-29T02:17:47Z

LGTM

zyl1121 requested a review from tinklone as a code owner June 25, 2026 13:13

cubesandboxbot Bot reviewed Jun 25, 2026

View reviewed changes

zyl1121 force-pushed the feat/cube-bench-metadata branch from a2b5df9 to aaf7b83 Compare June 25, 2026 13:33

cubesandboxbot Bot reviewed Jun 25, 2026

View reviewed changes

zyl1121 force-pushed the feat/cube-bench-metadata branch from aaf7b83 to d26ba7f Compare June 26, 2026 08:26

zyl1121 changed the title ~~feat(cube-bench): add metadata flag for create requests~~ feat(cube-bench): add host-mount flag for create requests Jun 26, 2026

cubesandboxbot Bot reviewed Jun 26, 2026

View reviewed changes

Comment thread examples/cube-bench/runner.go

ls-ggg merged commit a1b4fcc into TencentCloud:master Jun 29, 2026
5 checks passed

		if r.Method != http.MethodPost \|\| r.URL.Path != "/sandboxes" {
		t.Fatalf("request = %s %s", r.Method, r.URL.Path)

		@@ -27,11 +27,9 @@ func benchOne(client http.Client, cfg Config, seq int) IterResult {
		apiURL := cfg.APIURL
		headers := map[string]string{"Authorization": "Bearer " + cfg.APIKey}

Uh oh!

Conversation

zyl1121 commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

What Changed

Validation

Uh oh!

cubesandboxbot Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review: PR #651

Findings

Test coverage gaps

Positive

Uh oh!

cubesandboxbot Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubesandboxbot Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubesandboxbot Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubesandboxbot Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubesandboxbot Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubesandboxbot Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubesandboxbot Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubesandboxbot Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

cubesandboxbot Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

ls-ggg commented Jun 26, 2026

Uh oh!

Uh oh!

zyl1121 commented Jun 26, 2026

Uh oh!

ls-ggg commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zyl1121 commented Jun 25, 2026 •

edited

Loading

cubesandboxbot Bot commented Jun 25, 2026 •

edited

Loading