Skip to content

Commit 8c379e7

Browse files
Sayan-claude
andcommitted
Make stop_timeout configurable at create and stop time
The graceful stop grace period was hardcoded to a 5s default: the StopTimeout field existed on instance metadata but nothing ever set it, so it was always 0 and fell back to the default. Expose it as an optional stop_timeout on the create-instance request (persisted on the instance and also used by delete's graceful teardown) and as an optional per-call override in the stop request body. Precedence: stop-call override > instance-configured value > default. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 8ce342f commit 8c379e7

19 files changed

Lines changed: 510 additions & 335 deletions

cmd/api/api/instances.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,17 @@ func (s *ApiService) CreateInstance(ctx context.Context, request oapi.CreateInst
302302
}, nil
303303
}
304304

305+
stopTimeout := 0
306+
if request.Body.StopTimeout != nil {
307+
if *request.Body.StopTimeout < 1 {
308+
return oapi.CreateInstance400JSONResponse{
309+
Code: "invalid_stop_timeout",
310+
Message: "stop_timeout must be at least 1 second",
311+
}, nil
312+
}
313+
stopTimeout = *request.Body.StopTimeout
314+
}
315+
305316
domainReq := instances.CreateInstanceRequest{
306317
Name: request.Body.Name,
307318
Image: request.Body.Image,
@@ -326,6 +337,7 @@ func (s *ApiService) CreateInstance(ctx context.Context, request oapi.CreateInst
326337
Cmd: cmd,
327338
SkipKernelHeaders: request.Body.SkipKernelHeaders != nil && *request.Body.SkipKernelHeaders,
328339
SkipGuestAgent: request.Body.SkipGuestAgent != nil && *request.Body.SkipGuestAgent,
340+
StopTimeout: stopTimeout,
329341
AutoStandby: autoStandby,
330342
HealthCheck: healthCheck,
331343
RestartPolicy: restartPolicy,
@@ -754,7 +766,18 @@ func (s *ApiService) StopInstance(ctx context.Context, request oapi.StopInstance
754766
}
755767
log := logger.FromContext(ctx)
756768

757-
result, err := s.InstanceManager.StopInstance(ctx, inst.Id)
769+
var stopTimeout *int
770+
if request.Body != nil && request.Body.StopTimeout != nil {
771+
if *request.Body.StopTimeout < 1 {
772+
return oapi.StopInstance400JSONResponse{
773+
Code: "invalid_stop_timeout",
774+
Message: "stop_timeout must be at least 1 second",
775+
}, nil
776+
}
777+
stopTimeout = request.Body.StopTimeout
778+
}
779+
780+
result, err := s.InstanceManager.StopInstance(ctx, inst.Id, stopTimeout)
758781
if err != nil {
759782
switch {
760783
case errors.Is(err, instances.ErrInvalidState):

lib/builds/manager_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ func (m *mockInstanceManager) RestoreSnapshot(ctx context.Context, id string, sn
121121
return nil, instances.ErrNotSupported
122122
}
123123

124-
func (m *mockInstanceManager) StopInstance(ctx context.Context, id string) (*instances.Instance, error) {
124+
func (m *mockInstanceManager) StopInstance(ctx context.Context, id string, stopTimeout *int) (*instances.Instance, error) {
125125
if m.stopFunc != nil {
126126
return m.stopFunc(ctx, id)
127127
}

lib/instances/create.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,7 @@ func (m *manager) createInstance(
358358
Cmd: req.Cmd,
359359
SkipKernelHeaders: req.SkipKernelHeaders,
360360
SkipGuestAgent: req.SkipGuestAgent,
361+
StopTimeout: req.StopTimeout,
361362
EnableRosetta: enableRosetta,
362363
SnapshotPolicy: cloneSnapshotPolicy(req.SnapshotPolicy),
363364
AutoStandby: cloneAutoStandbyPolicy(req.AutoStandby),

lib/instances/delete.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (m *manager) deleteInstance(
6161
// 4. If active, try graceful guest shutdown before force kill.
6262
gracefulShutdown := false
6363
if inst.State == StateRunning || inst.State == StateInitializing {
64-
stopTimeout := resolveStopTimeout(stored)
64+
stopTimeout := resolveStopTimeout(stored, nil)
6565
if stopTimeout > deleteGracefulShutdownTimeout {
6666
stopTimeout = deleteGracefulShutdownTimeout
6767
}

lib/instances/firecracker_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ func TestFirecrackerStopClearsStaleSnapshot(t *testing.T) {
341341
require.NoError(t, err)
342342
require.True(t, beforeStop.HasSnapshot, "test setup should create visible stale snapshot")
343343

344-
inst, err = mgr.StopInstance(ctx, inst.Id)
344+
inst, err = mgr.StopInstance(ctx, inst.Id, nil)
345345
require.NoError(t, err)
346346
assert.Equal(t, StateStopped, inst.State)
347347
assert.False(t, inst.HasSnapshot, "stopped instances should not retain stale snapshots")

lib/instances/fork.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ func (m *manager) applyForkTargetState(ctx context.Context, forkID string, targe
562562
inst, err := m.standbyInstance(ctx, forkID, StandbyInstanceRequest{}, false)
563563
return returnWithReadiness(inst, err, false)
564564
case StateStopped:
565-
inst, err := m.stopInstance(ctx, forkID)
565+
inst, err := m.stopInstance(ctx, forkID, nil)
566566
return returnWithReadiness(inst, err, false)
567567
}
568568
}

lib/instances/fork_speed_darwin_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func TestVZForkSpeed(t *testing.T) {
9898
require.Equalf(t, 0, code, "dd to fill overlay should succeed")
9999

100100
// Stop the source so each fork copies a stable on-disk state without booting.
101-
_, err = mgr.StopInstance(ctx, source.Id)
101+
_, err = mgr.StopInstance(ctx, source.Id, nil)
102102
require.NoError(t, err)
103103

104104
// Reset the global reflink flag however the test exits — a require failure

lib/instances/lifecycle_noop_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func TestLifecycleNoopTransitionsReturnCurrentInstanceWithoutEvent(t *testing.T)
106106
name: "stop already stopped",
107107
state: StateStopped,
108108
action: func(ctx context.Context, m *manager, id string) (*Instance, error) {
109-
return m.StopInstance(ctx, id)
109+
return m.StopInstance(ctx, id, nil)
110110
},
111111
},
112112
}

lib/instances/manager.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ type Manager interface {
4242
StandbyInstance(ctx context.Context, id string, req StandbyInstanceRequest) (*Instance, error)
4343
RestoreInstance(ctx context.Context, id string) (*Instance, error)
4444
RestoreSnapshot(ctx context.Context, id string, snapshotID string, req RestoreSnapshotRequest) (*Instance, error)
45-
StopInstance(ctx context.Context, id string) (*Instance, error)
45+
StopInstance(ctx context.Context, id string, stopTimeout *int) (*Instance, error)
4646
StartInstance(ctx context.Context, id string, req StartInstanceRequest) (*Instance, error)
4747
UpdateInstance(ctx context.Context, id string, req UpdateInstanceRequest) (*Instance, error)
4848
StreamInstanceLogs(ctx context.Context, id string, tail int, follow bool, source LogSource) (<-chan string, error)
@@ -609,8 +609,9 @@ func (m *manager) RestoreSnapshot(ctx context.Context, id string, snapshotID str
609609
return inst, err
610610
}
611611

612-
// StopInstance gracefully stops a running instance
613-
func (m *manager) StopInstance(ctx context.Context, id string) (*Instance, error) {
612+
// StopInstance gracefully stops a running instance. A non-nil stopTimeout
613+
// overrides the instance's configured grace period for this call only.
614+
func (m *manager) StopInstance(ctx context.Context, id string, stopTimeout *int) (*Instance, error) {
614615
lock := m.getInstanceLock(id)
615616
lock.Lock()
616617
defer lock.Unlock()
@@ -631,7 +632,7 @@ func (m *manager) StopInstance(ctx context.Context, id string) (*Instance, error
631632
if err := m.markRestartManualStopLocked(ctx, id); err != nil {
632633
return nil, err
633634
}
634-
inst, err := m.stopInstance(ctx, id)
635+
inst, err := m.stopInstance(ctx, id, stopTimeout)
635636
if err == nil {
636637
m.notifyLifecycleEvent(ctx, LifecycleEventStop, inst)
637638
}

lib/instances/manager_darwin_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ func TestVZBasicLifecycle(t *testing.T) {
182182

183183
// Graceful shutdown test
184184
t.Log("Stopping instance (graceful shutdown)...")
185-
inst, err = mgr.StopInstance(ctx, inst.Id)
185+
inst, err = mgr.StopInstance(ctx, inst.Id, nil)
186186
require.NoError(t, err)
187187
assert.Equal(t, StateStopped, inst.State)
188188
t.Log("Instance stopped")
@@ -255,7 +255,7 @@ func TestVZBasicLifecycle(t *testing.T) {
255255

256256
// Stop again before delete
257257
t.Log("Stopping instance before delete...")
258-
inst, err = mgr.StopInstance(ctx, inst.Id)
258+
inst, err = mgr.StopInstance(ctx, inst.Id, nil)
259259
require.NoError(t, err)
260260
assert.Equal(t, StateStopped, inst.State)
261261

@@ -359,7 +359,7 @@ func TestVZExecAndShutdown(t *testing.T) {
359359

360360
// Graceful shutdown
361361
t.Log("Stopping instance...")
362-
inst, err = mgr.StopInstance(ctx, inst.Id)
362+
inst, err = mgr.StopInstance(ctx, inst.Id, nil)
363363
require.NoError(t, err)
364364
assert.Equal(t, StateStopped, inst.State)
365365
t.Log("Instance stopped gracefully")

0 commit comments

Comments
 (0)