From c525e6b4f3de9b1d51aaafee6932381825b770a6 Mon Sep 17 00:00:00 2001 From: Sanchit2662 Date: Wed, 15 Apr 2026 18:25:39 +0530 Subject: [PATCH 1/2] fix: honor MaxSessionDuration in placeholder expiry ZSet score Signed-off-by: Sanchit2662 --- pkg/workloadmanager/sandbox_helper.go | 6 +- pkg/workloadmanager/sandbox_helper_test.go | 96 +++++++++++++++++++++- 2 files changed, 98 insertions(+), 4 deletions(-) diff --git a/pkg/workloadmanager/sandbox_helper.go b/pkg/workloadmanager/sandbox_helper.go index 17d565bc..407771e4 100644 --- a/pkg/workloadmanager/sandbox_helper.go +++ b/pkg/workloadmanager/sandbox_helper.go @@ -27,12 +27,16 @@ import ( ) func buildSandboxPlaceHolder(sandboxCR *sandboxv1alpha1.Sandbox, entry *sandboxEntry) *types.SandboxInfo { + expiresAt := time.Now().Add(DefaultSandboxTTL) + if sandboxCR.Spec.Lifecycle.ShutdownTime != nil { + expiresAt = sandboxCR.Spec.Lifecycle.ShutdownTime.Time + } return &types.SandboxInfo{ Kind: entry.Kind, SessionID: entry.SessionID, SandboxNamespace: sandboxCR.GetNamespace(), Name: sandboxCR.GetName(), - ExpiresAt: time.Now().Add(DefaultSandboxTTL), + ExpiresAt: expiresAt, Status: "creating", } } diff --git a/pkg/workloadmanager/sandbox_helper_test.go b/pkg/workloadmanager/sandbox_helper_test.go index c58ade10..8c0aa02f 100644 --- a/pkg/workloadmanager/sandbox_helper_test.go +++ b/pkg/workloadmanager/sandbox_helper_test.go @@ -30,9 +30,99 @@ import ( const sandboxHelperTestPodIP = "10.0.0.1" -// Note: TestBuildSandboxPlaceHolder and TestBuildSandboxPlaceHolder_CodeInterpreter -// removed - they only verified that struct fields match input parameters, which is -// trivial field copying behavior. +func TestBuildSandboxPlaceHolder_TableDriven(t *testing.T) { + now := time.Now() + + tests := []struct { + name string + setupSandbox func() *sandboxv1alpha1.Sandbox + entry *sandboxEntry + validate func(t *testing.T, result *types.SandboxInfo) + }{ + { + name: "no ShutdownTime falls back to DefaultSandboxTTL", + setupSandbox: func() *sandboxv1alpha1.Sandbox { + return &sandboxv1alpha1.Sandbox{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sandbox", + Namespace: "default", + }, + } + }, + entry: &sandboxEntry{ + Kind: types.SandboxKind, + SessionID: "session-123", + }, + validate: func(t *testing.T, result *types.SandboxInfo) { + expected := now.Add(DefaultSandboxTTL) + assert.WithinDuration(t, expected, result.ExpiresAt, 2*time.Second) + assert.Equal(t, "creating", result.Status) + assert.Equal(t, "session-123", result.SessionID) + }, + }, + { + name: "ShutdownTime set to 24h is used as ExpiresAt", + setupSandbox: func() *sandboxv1alpha1.Sandbox { + shutdownTime := now.Add(24 * time.Hour) + return &sandboxv1alpha1.Sandbox{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sandbox", + Namespace: "default", + }, + Spec: sandboxv1alpha1.SandboxSpec{ + Lifecycle: sandboxv1alpha1.Lifecycle{ + ShutdownTime: &metav1.Time{Time: shutdownTime}, + }, + }, + } + }, + entry: &sandboxEntry{ + Kind: types.SandboxKind, + SessionID: "session-456", + }, + validate: func(t *testing.T, result *types.SandboxInfo) { + expected := now.Add(24 * time.Hour) + assert.WithinDuration(t, expected, result.ExpiresAt, 2*time.Second) + }, + }, + { + name: "ShutdownTime set to 30m overrides DefaultSandboxTTL", + setupSandbox: func() *sandboxv1alpha1.Sandbox { + shutdownTime := now.Add(30 * time.Minute) + return &sandboxv1alpha1.Sandbox{ + ObjectMeta: metav1.ObjectMeta{ + Name: "short-sandbox", + Namespace: "default", + }, + Spec: sandboxv1alpha1.SandboxSpec{ + Lifecycle: sandboxv1alpha1.Lifecycle{ + ShutdownTime: &metav1.Time{Time: shutdownTime}, + }, + }, + } + }, + entry: &sandboxEntry{ + Kind: types.SandboxClaimsKind, + SessionID: "session-789", + }, + validate: func(t *testing.T, result *types.SandboxInfo) { + expected := now.Add(30 * time.Minute) + assert.WithinDuration(t, expected, result.ExpiresAt, 2*time.Second) + // Must NOT be 8h (DefaultSandboxTTL) + assert.True(t, result.ExpiresAt.Before(now.Add(DefaultSandboxTTL)), + "ExpiresAt should be 30m, not the 8h default") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sandbox := tt.setupSandbox() + result := buildSandboxPlaceHolder(sandbox, tt.entry) + tt.validate(t, result) + }) + } +} func TestBuildSandboxInfo_TableDriven(t *testing.T) { now := time.Now() From 5a641f8842fd42b19fc55082819fa6adbc3c454c Mon Sep 17 00:00:00 2001 From: Sanchit2662 Date: Wed, 15 Apr 2026 23:19:16 +0530 Subject: [PATCH 2/2] fix: address copilot review comments on MaxSessionDuration expiry fix - refactor buildSandboxPlaceHolder to use if/else instead of assign-then-overwrite, avoiding a redundant time.Now() call - set ShutdownTime on simpleSandbox in the warm-pool CodeInterpreter path so MaxSessionDuration is honored in the GC expiry ZSet for warm-pool sessions too - switch assert.WithinDuration to assert.Equal for ShutdownTime-set test cases since those are exact values, not time.Now()-based - add warm-pool regression test case to prevent the bug recurring Signed-off-by: Sanchit2662 --- pkg/workloadmanager/sandbox_helper.go | 4 ++- pkg/workloadmanager/sandbox_helper_test.go | 37 ++++++++++++++++++++-- pkg/workloadmanager/workload_builder.go | 4 +++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/pkg/workloadmanager/sandbox_helper.go b/pkg/workloadmanager/sandbox_helper.go index 407771e4..36b392dd 100644 --- a/pkg/workloadmanager/sandbox_helper.go +++ b/pkg/workloadmanager/sandbox_helper.go @@ -27,9 +27,11 @@ import ( ) func buildSandboxPlaceHolder(sandboxCR *sandboxv1alpha1.Sandbox, entry *sandboxEntry) *types.SandboxInfo { - expiresAt := time.Now().Add(DefaultSandboxTTL) + var expiresAt time.Time if sandboxCR.Spec.Lifecycle.ShutdownTime != nil { expiresAt = sandboxCR.Spec.Lifecycle.ShutdownTime.Time + } else { + expiresAt = time.Now().Add(DefaultSandboxTTL) } return &types.SandboxInfo{ Kind: entry.Kind, diff --git a/pkg/workloadmanager/sandbox_helper_test.go b/pkg/workloadmanager/sandbox_helper_test.go index 8c0aa02f..7ff9e40d 100644 --- a/pkg/workloadmanager/sandbox_helper_test.go +++ b/pkg/workloadmanager/sandbox_helper_test.go @@ -82,7 +82,7 @@ func TestBuildSandboxPlaceHolder_TableDriven(t *testing.T) { }, validate: func(t *testing.T, result *types.SandboxInfo) { expected := now.Add(24 * time.Hour) - assert.WithinDuration(t, expected, result.ExpiresAt, 2*time.Second) + assert.Equal(t, expected, result.ExpiresAt) }, }, { @@ -107,12 +107,45 @@ func TestBuildSandboxPlaceHolder_TableDriven(t *testing.T) { }, validate: func(t *testing.T, result *types.SandboxInfo) { expected := now.Add(30 * time.Minute) - assert.WithinDuration(t, expected, result.ExpiresAt, 2*time.Second) + assert.Equal(t, expected, result.ExpiresAt) // Must NOT be 8h (DefaultSandboxTTL) assert.True(t, result.ExpiresAt.Before(now.Add(DefaultSandboxTTL)), "ExpiresAt should be 30m, not the 8h default") }, }, + { + name: "warm-pool path: ShutdownTime set on simpleSandbox reflects MaxSessionDuration", + setupSandbox: func() *sandboxv1alpha1.Sandbox { + // Simulates the simpleSandbox built by the warm-pool CodeInterpreter path + // after the fix in workload_builder.go sets ShutdownTime from MaxSessionDuration. + shutdownTime := now.Add(24 * time.Hour) + return &sandboxv1alpha1.Sandbox{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "ci-warmpool-abc", + Labels: map[string]string{ + SessionIdLabelKey: "session-wp-001", + }, + }, + Spec: sandboxv1alpha1.SandboxSpec{ + Lifecycle: sandboxv1alpha1.Lifecycle{ + ShutdownTime: &metav1.Time{Time: shutdownTime}, + }, + }, + } + }, + entry: &sandboxEntry{ + Kind: types.SandboxClaimsKind, + SessionID: "session-wp-001", + }, + validate: func(t *testing.T, result *types.SandboxInfo) { + expected := now.Add(24 * time.Hour) + assert.Equal(t, expected, result.ExpiresAt, + "warm-pool placeholder ExpiresAt must reflect MaxSessionDuration, not the 8h default") + assert.Equal(t, "creating", result.Status) + assert.Equal(t, types.SandboxClaimsKind, result.Kind) + }, + }, } for _, tt := range tests { diff --git a/pkg/workloadmanager/workload_builder.go b/pkg/workloadmanager/workload_builder.go index e7d06dde..dc426d77 100644 --- a/pkg/workloadmanager/workload_builder.go +++ b/pkg/workloadmanager/workload_builder.go @@ -351,6 +351,10 @@ func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string, }, }, } + if codeInterpreterObj.Spec.MaxSessionDuration != nil { + shutdownTime := metav1.NewTime(time.Now().Add(codeInterpreterObj.Spec.MaxSessionDuration.Duration)) + simpleSandbox.Spec.Lifecycle.ShutdownTime = &shutdownTime + } sandboxEntry.Kind = types.SandboxClaimsKind return simpleSandbox, sandboxClaim, sandboxEntry, nil }