fix(CubeMaster): stably sort sandbox list to stop reordering on refresh

[yingjun8]

Fixes #742.

Problem

The Sandboxes list (:12088/sandboxes) reorders on every ~auto-refresh. ListSandbox fans out to per-node goroutines and appends results to rsp.Data in goroutine-completion order, which varies between calls, so the WebUI/SDK receive a reshuffled list each poll. No layer (CubeMaster, CubeAPI, frontend) imposed a stable order.

Root cause

The request path is:
WebUI → CubeAPI GET /v2/sandboxes → CubeMaster ListSandbox (gRPC) → per-node Cubelet.
In CubeMaster/pkg/service/sandbox/sandbox_list.go, ListSandbox fans out to one goroutine per node and aggregates the results through a channel:

for res := range resChan {
    rsp.Data = append(rsp.Data, res)
}   

Results are appended in goroutine-completion order, which is non-deterministic and varies between calls and across nodes. No layer (CubeMaster, CubeAPI, or the frontend) applied any sort, so each refresh returned the same set of sandboxes in a different order.

Fix

Add a stable sort after aggregation in ListSandbox, before returning:

• primary key: CreateAt descending (newest first)
• tie-breaker: SandboxID ascending (deterministic)

This is consistent with the existing cubecli list command, which already sorts by CreatedAt descending. The fix is contained to the CubeMaster aggregation layer, so all consumers (WebUI and SDK) get a stable order without frontend changes.

sort.Slice(rsp.Data, func(i, j int) bool {
  if rsp.Data[i].CreateAt != rsp.Data[j].CreateAt {
       return rsp.Data[i].CreateAt > rsp.Data[j].CreateAt
  }
  return rsp.Data[i].SandboxID < rsp.Data[j].SandboxID
})

[cubesandboxbot]

Suggestion: "stable" has a specific meaning in Go's sort package — sort.Slice is explicitly not a stable sort (see Go docs: "The sort is not guaranteed to be stable"). Since the comparator defines a total order via the SandboxID tiebreaker, the result is deterministic regardless of algorithm stability. Consider rewording to:

Suggested change

	// falling back to SandboxID for a deterministic, stable order.
	// falling back to SandboxID for a deterministic order.

This avoids confusion with sort.SliceStable semantics.

[cubesandboxbot]

Suggestion: Consider extracting this sort closure into a named function (sortSandboxBriefData) for testability. Currently it can only be tested via the full ListSandbox function (requiring mocked cubelet RPCs and goroutine orchestration). A package-level function would allow direct table-driven unit tests covering: distinct timestamps, equal timestamps (tiebreaker order), single element, and empty slice — without the integration overhead.

[cubesandboxbot]

Review: PR #762 — fix(CubeMaster): stably sort sandbox list to stop reordering on refresh

Files changed: 1 file, +12/-0

Summary

The PR correctly addresses the root cause of the sandbox list reordering on every refresh: ListSandbox fan-outs to per-node goroutines, and results are appended in goroutine-completion order. Adding a deterministic sort by CreateAt descending (with SandboxID tiebreaker) after aggregation is the right fix at the right layer.

Positive findings

• Correct root cause identification — The channel-based fan-out pattern means goroutine-completion order varies across calls, making the sort essential for a stable list view.
• Correct comparison — CreateAt is int64 (Unix timestamp from protobuf), not a string, so the > operator is a proper numeric comparison. No lexicographic ordering pitfalls.
• SandboxID tiebreaker — Ensures a total order even when CreateAt timestamps are identical. Without it, sort.Slice (which is not a stable sort) could still produce inconsistent ordering for equal-timestamp entries.
• Performance — O(n log n) sort on the aggregated result set is negligible compared to the network I/O to all nodes. No blocking concerns.
• Security — No security implications. The comparator is hardcoded, no injection vectors, no data integrity issues.

Noteworthy feedback

1. "stable" terminology — Comment line 108
   The comment says "deterministic, stable order" but sort.Slice is explicitly not a stable sort in Go. "Stable" in Go has a specific meaning (sort.SliceStable preserves original order of equal elements). Since the SandboxID tiebreaker makes the output fully deterministic regardless, consider rewording to avoid confusion: "deterministic order" only.

2. Extract sort for testability — Lines 109-114
   The sort is an inline closure inside ListSandbox, making it hard to unit-test independently of the goroutine fan-out and cubelet RPCs. The existing sandbox_list_test.go only tests parseCPUCount and parseMemoryMB — there are zero tests for ListSandbox itself or the sorting logic. Extracting the comparator to a named function (e.g., sortSandboxBriefData) would enable direct table-driven unit tests.

3. Optional: nil guard
   rsp.Data is []*types.SandboxBriefData (pointer slice). While current code paths never send nil pointers, a defensive nil check in the less function would guard against future maintenance errors. Minor concern.

Inline comments

Two inline comments have been posted on the diff corresponding to feedback items 1 and 2 above.

This review was generated with automated tools. Suggestions should be evaluated in context.

[chenhengqi]

LGTM. Thanks.