Skip to content

Commit ea7ed17

Browse files
committed
fix(sandboxes): honor the configured request deadline
1 parent 4e3d995 commit ea7ed17

4 files changed

Lines changed: 92 additions & 4 deletions

File tree

docs/sandboxes.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@ volcano cloud sandboxes executions cancel <sandbox-id> <execution-id> --generati
5656
```
5757

5858
Platform errors and unknown/pending command outcomes exit 1. Ctrl-C or the overall
59-
`--timeout` stops the client request, not necessarily the server command. The
59+
`--timeout` stops the client request, not necessarily the server command. It
60+
defaults to 10 minutes and accepts positive durations up to 24 hours. It controls
61+
the overall command request or log stream, including response-body reads. The
6062
request key is printed to stderr before mutations; retain it alongside operation
6163
IDs when investigating a disconnect. The CLI never retries writes. Do not create
6264
a new request key and replay an uncertain command. Use an existing key only with

internal/cmd/sandboxes/sandboxes_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"os"
1111
"path/filepath"
1212
"testing"
13+
"time"
1314

1415
"github.com/spf13/cobra"
1516
"github.com/stretchr/testify/assert"
@@ -22,6 +23,39 @@ import (
2223

2324
const projectID = "22222222-2222-4222-8222-222222222222"
2425

26+
type deadlineDoer func(*http.Request) (*http.Response, error)
27+
28+
func (d deadlineDoer) Do(r *http.Request) (*http.Response, error) { return d(r) }
29+
30+
func TestOverallTimeoutReachesRequest(t *testing.T) {
31+
for _, tc := range []struct {
32+
name string
33+
args []string
34+
want time.Duration
35+
}{
36+
{"default", []string{"list"}, 10 * time.Minute},
37+
{"longer than transport fallback", []string{"list", "--timeout", "5m"}, 5 * time.Minute},
38+
{"shorter than transport fallback", []string{"list", "--timeout", "30s"}, 30 * time.Second},
39+
} {
40+
t.Run(tc.name, func(t *testing.T) {
41+
hits := 0
42+
deps := cliruntime.Deps{ConfigLoader: func() (*config.Config, error) {
43+
return &config.Config{UserToken: "platform-token", IgnoreEnv: true, CurrentProject: &config.ProjectConfig{ID: projectID}}, nil
44+
}, HTTPClient: deadlineDoer(func(r *http.Request) (*http.Response, error) {
45+
hits++
46+
deadline, ok := r.Context().Deadline()
47+
assert.True(t, ok)
48+
assert.WithinDuration(t, time.Now().Add(tc.want), deadline, time.Second)
49+
return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"X-Volcano-Sandbox-Version": []string{sandbox.Version}}, Body: io.NopCloser(bytes.NewBufferString(`{"items":[]}`))}, nil
50+
})}
51+
t.Setenv("VOLCANO_SANDBOX_URL", "https://sandbox.example")
52+
_, _, err := execute(t, New(deps), tc.args...)
53+
require.NoError(t, err)
54+
assert.Equal(t, 1, hits)
55+
})
56+
}
57+
}
58+
2559
func fixture(t *testing.T, handler http.HandlerFunc) cliruntime.Deps {
2660
t.Helper()
2761
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

internal/sandbox/client.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,14 @@ func (e *Error) Error() string {
8888
return fmt.Sprintf("sandbox %s (HTTP %d, operation %s)", e.Code, e.Status, e.OperationID)
8989
}
9090

91-
// Do performs a bounded request without replaying unknown outcomes.
91+
// Do honors the caller's deadline without replaying unknown outcomes. Calls
92+
// without a deadline use a bounded fallback; CLI commands set their own timeout.
9293
func (c *Client) Do(ctx context.Context, input Request) (Response, error) {
93-
ctx, cancel := context.WithTimeout(ctx, 65*time.Second)
94-
defer cancel()
94+
if _, ok := ctx.Deadline(); !ok {
95+
var cancel context.CancelFunc
96+
ctx, cancel = context.WithTimeout(ctx, 65*time.Second)
97+
defer cancel()
98+
}
9599
response, err := c.send(ctx, input)
96100
if err != nil {
97101
return Response{}, err

internal/sandbox/client_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,54 @@ import (
1414
"github.com/stretchr/testify/require"
1515
)
1616

17+
type requestDoer func(*http.Request) (*http.Response, error)
18+
19+
func (d requestDoer) Do(r *http.Request) (*http.Response, error) { return d(r) }
20+
21+
func TestRequestHonorsCallerDeadline(t *testing.T) {
22+
for _, duration := range []time.Duration{30 * time.Second, 5 * time.Minute, 10 * time.Minute} {
23+
t.Run(duration.String(), func(t *testing.T) {
24+
ctx, cancel := context.WithTimeout(t.Context(), duration)
25+
defer cancel()
26+
want, _ := ctx.Deadline()
27+
hits := 0
28+
client, err := New("https://sandbox.example", "secret", "p1", requestDoer(func(r *http.Request) (*http.Response, error) {
29+
hits++
30+
got, ok := r.Context().Deadline()
31+
assert.True(t, ok)
32+
assert.Equal(t, want, got)
33+
cancel()
34+
return nil, r.Context().Err()
35+
}))
36+
require.NoError(t, err)
37+
_, err = client.Do(ctx, Request{Method: http.MethodPost, Path: "/one-shot"})
38+
require.ErrorIs(t, err, context.Canceled)
39+
assert.Equal(t, 1, hits)
40+
})
41+
}
42+
}
43+
44+
func TestRequestWithoutDeadlineRetainsBoundedFallback(t *testing.T) {
45+
var requestDone <-chan struct{}
46+
start := time.Now()
47+
client, err := New("https://sandbox.example", "secret", "p1", requestDoer(func(r *http.Request) (*http.Response, error) {
48+
requestDone = r.Context().Done()
49+
deadline, ok := r.Context().Deadline()
50+
assert.True(t, ok)
51+
assert.WithinDuration(t, start.Add(65*time.Second), deadline, time.Second)
52+
return &http.Response{StatusCode: http.StatusNoContent, Header: http.Header{"X-Volcano-Sandbox-Version": []string{Version}}, Body: http.NoBody}, nil
53+
}))
54+
require.NoError(t, err)
55+
_, err = client.Do(context.Background(), Request{Method: http.MethodGet, Path: "/capabilities"})
56+
require.NoError(t, err)
57+
require.NotNil(t, requestDone)
58+
select {
59+
case <-requestDone:
60+
default:
61+
t.Fatal("fallback deadline was not released after the response")
62+
}
63+
}
64+
1765
func TestRequestPinsContractIdentityAndNeverReplays(t *testing.T) {
1866
for _, status := range []int{200, 401, 403, 409, 429, 501, 503} {
1967
t.Run(http.StatusText(status), func(t *testing.T) {

0 commit comments

Comments
 (0)