Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,11 @@ spec:
description: Spec defines the desired state of the AgentRuntime.
properties:
maxSessionDuration:
Comment thread
safiya2610 marked this conversation as resolved.
default: 8h
description: |-
MaxSessionDuration describes the maximum duration for a session.
After this duration, the session will be terminated no matter active or inactive.
This field is optional for AgentRuntime; if omitted, sessions are not capped by a hard lifetime.
Non-positive values (e.g., 0s, -1h) are also treated as omitted and will not apply a hard lifetime cap.
type: string
podTemplate:
description: PodTemplate describes the template that will be used
Expand Down Expand Up @@ -8504,7 +8505,6 @@ spec:
type: object
type: array
required:
- maxSessionDuration
- podTemplate
- sessionTimeout
- targetPort
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/runtime/v1alpha1/agent_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ type AgentRuntimeSpec struct {

// MaxSessionDuration describes the maximum duration for a session.
// After this duration, the session will be terminated no matter active or inactive.
// +kubebuilder:validation:Required
// +kubebuilder:default="8h"
// This field is optional for AgentRuntime; if omitted, sessions are not capped by a hard lifetime.
Comment thread
safiya2610 marked this conversation as resolved.
Comment thread
safiya2610 marked this conversation as resolved.
Comment thread
safiya2610 marked this conversation as resolved.
// Non-positive values (e.g., 0s, -1h) are also treated as omitted and will not apply a hard lifetime cap.
MaxSessionDuration *metav1.Duration `json:"maxSessionDuration,omitempty" protobuf:"bytes,3,opt,name=maxSessionDuration"`
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/workloadmanager/garbage_collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func (gc *garbageCollector) once() {
}
}

// List sandboxes that have reached their expiry deadline
// List sandboxes that have reached their expiry deadline.
expiredSandboxes, err := gc.storeClient.ListExpiredSandboxes(ctx, now, gcCandidateLimit)
if err != nil {
klog.Errorf("garbage collector error listing expired sandboxes: %v", err)
Expand Down
11 changes: 9 additions & 2 deletions pkg/workloadmanager/sandbox_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ const (
sandboxStatusNotReady = "not-ready"
)

// noExpiryDeadline represents a sandbox with no hard expiry deadline
// (used for AgentRuntime without maxSessionDuration).
// Setting ExpiresAt to this value ensures the sandbox won't be selected by ListExpiredSandboxes queries.
var noExpiryDeadline = time.Date(9999, 12, 31, 23, 59, 59, 0, time.UTC)

var sandboxEntrypointDial = func(ctx context.Context, endpoint string, timeout time.Duration) error {
dialer := &net.Dialer{Timeout: timeout}
conn, err := dialer.DialContext(ctx, "tcp", endpoint)
Expand All @@ -52,7 +57,9 @@ func buildSandboxPlaceHolder(sandboxCR *sandboxv1alpha1.Sandbox, entry *sandboxE
if sandboxCR.Spec.Lifecycle.ShutdownTime != nil {
expiresAt = sandboxCR.Spec.Lifecycle.ShutdownTime.Time
} else {
expiresAt = time.Now().Add(DefaultSandboxTTL)
// If no ShutdownTime is set (AgentRuntime without maxSessionDuration),
// use noExpiryDeadline to indicate no hard expiry deadline.
expiresAt = noExpiryDeadline
}
idleTimeout := entry.IdleTimeout
if idleTimeout == 0 {
Expand All @@ -72,7 +79,7 @@ func buildSandboxPlaceHolder(sandboxCR *sandboxv1alpha1.Sandbox, entry *sandboxE

func buildSandboxInfo(sandbox *sandboxv1alpha1.Sandbox, podIP string, entry *sandboxEntry) *types.SandboxInfo {
createdAt := sandbox.GetCreationTimestamp().Time
expiresAt := createdAt.Add(DefaultSandboxTTL)
expiresAt := noExpiryDeadline
if sandbox.Spec.Lifecycle.ShutdownTime != nil {
expiresAt = sandbox.Spec.Lifecycle.ShutdownTime.Time
}
Comment thread
safiya2610 marked this conversation as resolved.
Expand Down
38 changes: 35 additions & 3 deletions pkg/workloadmanager/sandbox_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestBuildSandboxPlaceHolder_TableDriven(t *testing.T) {
validate func(t *testing.T, result *types.SandboxInfo)
}{
{
name: "no ShutdownTime falls back to DefaultSandboxTTL",
name: "no ShutdownTime sets ExpiresAt to noExpiryDeadline (AgentRuntime without maxSessionDuration)",
setupSandbox: func() *sandboxv1alpha1.Sandbox {
return &sandboxv1alpha1.Sandbox{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -54,8 +54,9 @@ func TestBuildSandboxPlaceHolder_TableDriven(t *testing.T) {
SessionID: "session-123",
},
validate: func(t *testing.T, result *types.SandboxInfo) {
expected := now.Add(DefaultSandboxTTL)
assert.WithinDuration(t, expected, result.ExpiresAt, 2*time.Second)
// When ShutdownTime is nil (AgentRuntime without maxSessionDuration),
// ExpiresAt should be set to noExpiryDeadline to indicate no hard expiry.
assert.Equal(t, noExpiryDeadline, result.ExpiresAt)
assert.Equal(t, "creating", result.Status)
assert.Equal(t, "session-123", result.SessionID)
},
Expand Down Expand Up @@ -317,6 +318,37 @@ func TestBuildSandboxInfo_TableDriven(t *testing.T) {
assert.Equal(t, ":8080", result.EntryPoints[0].Endpoint)
},
},
{
name: "sandbox with nil ShutdownTime defaults to noExpiryDeadline",
setupSandbox: func() *sandboxv1alpha1.Sandbox {
return &sandboxv1alpha1.Sandbox{
ObjectMeta: metav1.ObjectMeta{
Name: "test-sandbox-no-ttl",
Namespace: "default",
UID: "test-uid-456",
CreationTimestamp: metav1.NewTime(now),
},
Status: sandboxv1alpha1.SandboxStatus{
Conditions: []metav1.Condition{
{
Type: string(sandboxv1alpha1.SandboxConditionReady),
Status: metav1.ConditionTrue,
},
},
},
}
},
podIP: sandboxHelperTestPodIP,
entry: &sandboxEntry{
Kind: types.AgentRuntimeKind,
SessionID: "test-session-no-ttl",
Ports: []runtimev1alpha1.TargetPort{},
},
validateResult: func(t *testing.T, result *types.SandboxInfo) {
// When ShutdownTime is nil, ExpiresAt should default to noExpiryDeadline
assert.Equal(t, noExpiryDeadline, result.ExpiresAt)
},
},
}

for _, tt := range tests {
Expand Down
29 changes: 17 additions & 12 deletions pkg/workloadmanager/workload_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,15 @@ type buildSandboxClaimParams struct {

// buildSandboxObject builds a Sandbox object from parameters
func buildSandboxObject(params *buildSandboxParams) *sandboxv1alpha1.Sandbox {
if params.ttl == 0 {
params.ttl = DefaultSandboxTTL
}
if params.idleTimeout == 0 {
params.idleTimeout = DefaultSandboxIdleTimeout
}

shutdownTime := metav1.NewTime(time.Now().Add(params.ttl))
var shutdownTime *metav1.Time
if params.ttl != 0 {
st := metav1.NewTime(time.Now().Add(params.ttl))
shutdownTime = &st
}
Comment thread
safiya2610 marked this conversation as resolved.
Comment thread
safiya2610 marked this conversation as resolved.

// Allocate fresh maps for copied metadata so we never mutate informer-cached input.
// Annotations are only copied when params.podAnnotations is non-nil.
Expand Down Expand Up @@ -200,7 +201,7 @@ func buildSandboxObject(params *buildSandboxParams) *sandboxv1alpha1.Sandbox {
},
},
Lifecycle: sandboxv1alpha1.Lifecycle{
ShutdownTime: &shutdownTime,
ShutdownTime: shutdownTime,
},
Replicas: ptr.To[int32](1),
},
Expand Down Expand Up @@ -289,8 +290,8 @@ func buildSandboxByAgentRuntime(namespace string, name string, ownerID string, i
if agentRuntimeObj.Spec.Template.Annotations != nil {
buildParams.podAnnotations = agentRuntimeObj.Spec.Template.Annotations
}
if agentRuntimeObj.Spec.MaxSessionDuration != nil {
buildParams.ttl = agentRuntimeObj.Spec.MaxSessionDuration.Duration
if d := agentRuntimeObj.Spec.MaxSessionDuration; d != nil && d.Duration > 0 {
buildParams.ttl = d.Duration
}
idleTimeout := DefaultSandboxIdleTimeout
if agentRuntimeObj.Spec.SessionTimeout != nil {
Expand Down Expand Up @@ -386,10 +387,12 @@ 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
maxDuration := DefaultSandboxTTL
if d := codeInterpreterObj.Spec.MaxSessionDuration; d != nil && d.Duration > 0 {
maxDuration = d.Duration
}
Comment thread
safiya2610 marked this conversation as resolved.
shutdownTime := metav1.NewTime(time.Now().Add(maxDuration))
simpleSandbox.Spec.Lifecycle.ShutdownTime = &shutdownTime
sandboxEntry.Kind = types.SandboxClaimsKind
return simpleSandbox, sandboxClaim, sandboxEntry, nil
}
Expand Down Expand Up @@ -430,8 +433,10 @@ func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string,
idleTimeout: idleTimeout,
}

if codeInterpreterObj.Spec.MaxSessionDuration != nil {
buildParams.ttl = codeInterpreterObj.Spec.MaxSessionDuration.Duration
if d := codeInterpreterObj.Spec.MaxSessionDuration; d != nil && d.Duration > 0 {
buildParams.ttl = d.Duration
} else {
buildParams.ttl = DefaultSandboxTTL
}
sandbox := buildSandboxObject(buildParams)
return sandbox, nil, sandboxEntry, nil
Expand Down
146 changes: 146 additions & 0 deletions pkg/workloadmanager/workload_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,41 @@ func TestBuildSandboxObject_NilLabels(t *testing.T) {
}
}

func TestBuildSandboxObject_UsesShutdownTimeForNonZeroTTL(t *testing.T) {
t.Run("negative ttl sets an already-expired shutdown time", func(t *testing.T) {
params := &buildSandboxParams{
namespace: "default",
sandboxName: "sandbox-negative-ttl",
sessionID: "session-negative-ttl",
ttl: -time.Hour,
idleTimeout: 15 * time.Minute,
}

sandbox := buildSandboxObject(params)
if sandbox.Spec.Lifecycle.ShutdownTime == nil {
t.Fatal("expected shutdown time to be set for negative ttl")
}
if !sandbox.Spec.Lifecycle.ShutdownTime.Time.Before(time.Now()) {
t.Fatalf("expected shutdown time to be in the past, got %s", sandbox.Spec.Lifecycle.ShutdownTime.Time)
}
})
Comment thread
safiya2610 marked this conversation as resolved.

t.Run("zero ttl leaves shutdown time unset", func(t *testing.T) {
params := &buildSandboxParams{
namespace: "default",
sandboxName: "sandbox-zero-ttl",
sessionID: "session-zero-ttl",
ttl: 0,
idleTimeout: 15 * time.Minute,
}

sandbox := buildSandboxObject(params)
if sandbox.Spec.Lifecycle.ShutdownTime != nil {
t.Fatalf("expected shutdown time to remain unset for zero ttl, got %v", sandbox.Spec.Lifecycle.ShutdownTime)
}
})
}

// TestBuildSandboxObject_WorkloadNameLabel verifies that the WorkloadNameLabelKey
// label is correctly set on the Sandbox object metadata from the workloadName param.
// This covers the bug where CodeInterpreter sandboxes were missing this label.
Expand Down Expand Up @@ -473,9 +508,58 @@ func TestBuildSandboxByAgentRuntime_DefaultTimeouts(t *testing.T) {

assert.Equal(t, types.SandboxKind, entry.Kind)
assert.Equal(t, DefaultSandboxIdleTimeout, entry.IdleTimeout)
assert.Nil(t, sandbox.Spec.Lifecycle.ShutdownTime)
assert.Equal(t, sandbox.Name, sandbox.Spec.PodTemplate.ObjectMeta.Labels[SandboxNameLabelKey])
}

func TestBuildSandboxByAgentRuntime_DoesNotSetShutdownTimeForNonPositiveMaxSessionDuration(t *testing.T) {
tests := []struct {
name string
duration *metav1.Duration
}{
{name: "nil", duration: nil},
{name: "zero", duration: &metav1.Duration{Duration: 0}},
{name: "negative", duration: &metav1.Duration{Duration: -time.Hour}},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ar := &runtimev1alpha1.AgentRuntime{
ObjectMeta: metav1.ObjectMeta{
Namespace: testNamespace,
Name: testAgentRuntimeName,
},
Spec: runtimev1alpha1.AgentRuntimeSpec{
SessionTimeout: &metav1.Duration{Duration: 30 * time.Minute},
MaxSessionDuration: tt.duration,
Template: &runtimev1alpha1.SandboxTemplate{
Spec: corev1.PodSpec{
Containers: []corev1.Container{{Name: "main", Image: "nginx"}},
},
},
},
}

fakeClient := cubefake.NewSimpleClientset(ar)
factory := cubeinformers.NewSharedInformerFactory(fakeClient, 0)
agentRuntimeInformer := factory.Runtime().V1alpha1().AgentRuntimes()
err := agentRuntimeInformer.Informer().GetStore().Add(ar)
assert.NoError(t, err)

ifm := &Informers{
AgentRuntimeLister: agentRuntimeInformer.Lister(),
AgentRuntimeInformer: agentRuntimeInformer.Informer(),
informerFactory: newFactory(),
cubeInformerFactory: factory,
}

sandbox, _, err := buildSandboxByAgentRuntime(testNamespace, testAgentRuntimeName, "", ifm)
assert.NoError(t, err)
assert.Nil(t, sandbox.Spec.Lifecycle.ShutdownTime)
})
}
}

func TestBuildSandboxByAgentRuntime_CustomTimeouts(t *testing.T) {
ar := &runtimev1alpha1.AgentRuntime{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -571,6 +655,68 @@ func TestBuildSandboxByCodeInterpreter_PicodAuthFailsWithoutKey(t *testing.T) {
}
}

func TestBuildSandboxByCodeInterpreter_FallsBackToDefaultTTLForNonPositiveMaxSessionDuration(t *testing.T) {
tests := []struct {
name string
warmPoolSize *int32
maxDuration *metav1.Duration
wantClaim bool
}{
{
name: "warm pool uses default ttl for negative max session duration",
warmPoolSize: ptr.To(int32(2)),
maxDuration: &metav1.Duration{Duration: -time.Hour},
wantClaim: true,
},
{
name: "sandbox uses default ttl for zero max session duration",
warmPoolSize: nil,
maxDuration: &metav1.Duration{Duration: 0},
wantClaim: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
codeInterpreter := &runtimev1alpha1.CodeInterpreter{
ObjectMeta: metav1.ObjectMeta{
Name: "ci-default-ttl",
Namespace: testNamespace,
},
Spec: runtimev1alpha1.CodeInterpreterSpec{
AuthMode: runtimev1alpha1.AuthModeNone,
WarmPoolSize: tt.warmPoolSize,
Template: &runtimev1alpha1.CodeInterpreterSandboxTemplate{
Image: "my-ci-image:latest",
},
MaxSessionDuration: tt.maxDuration,
SessionTimeout: &metav1.Duration{Duration: 10 * time.Minute},
},
}

fakeClient := cubefake.NewSimpleClientset(codeInterpreter)
factory := cubeinformers.NewSharedInformerFactory(fakeClient, 0)
codeInterpreterInformer := factory.Runtime().V1alpha1().CodeInterpreters()
err := codeInterpreterInformer.Informer().GetStore().Add(codeInterpreter)
assert.NoError(t, err)

ifm := &Informers{
CodeInterpreterLister: codeInterpreterInformer.Lister(),
CodeInterpreterInformer: codeInterpreterInformer.Informer(),
informerFactory: newFactory(),
cubeInformerFactory: factory,
}

sandbox, claim, _, err := buildSandboxByCodeInterpreter(testNamespace, "ci-default-ttl", "", ifm)
assert.NoError(t, err)
assert.NotNil(t, sandbox)
assert.Equal(t, tt.wantClaim, claim != nil)
assert.NotNil(t, sandbox.Spec.Lifecycle.ShutdownTime)
assert.WithinDuration(t, time.Now().Add(DefaultSandboxTTL), sandbox.Spec.Lifecycle.ShutdownTime.Time, 5*time.Second)
})
}
}

func TestBuildSandboxByCodeInterpreter_SuccessNoWarmPool(t *testing.T) {
codeInterpreter := &runtimev1alpha1.CodeInterpreter{
ObjectMeta: metav1.ObjectMeta{
Expand Down