diff --git a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml index a6508b341..f8f378c73 100644 --- a/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml +++ b/manifests/charts/base/crds/runtime.agentcube.volcano.sh_agentruntimes.yaml @@ -44,10 +44,11 @@ spec: description: Spec defines the desired state of the AgentRuntime. properties: maxSessionDuration: - 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 @@ -8504,7 +8505,6 @@ spec: type: object type: array required: - - maxSessionDuration - podTemplate - sessionTimeout - targetPort diff --git a/pkg/apis/runtime/v1alpha1/agent_type.go b/pkg/apis/runtime/v1alpha1/agent_type.go index fe5413faa..0706260cd 100644 --- a/pkg/apis/runtime/v1alpha1/agent_type.go +++ b/pkg/apis/runtime/v1alpha1/agent_type.go @@ -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. + // 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"` } diff --git a/pkg/workloadmanager/garbage_collection.go b/pkg/workloadmanager/garbage_collection.go index 0de41d186..38f834473 100644 --- a/pkg/workloadmanager/garbage_collection.go +++ b/pkg/workloadmanager/garbage_collection.go @@ -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) diff --git a/pkg/workloadmanager/sandbox_helper.go b/pkg/workloadmanager/sandbox_helper.go index 0d0d467c9..8e783d5d6 100644 --- a/pkg/workloadmanager/sandbox_helper.go +++ b/pkg/workloadmanager/sandbox_helper.go @@ -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) @@ -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 { @@ -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 } diff --git a/pkg/workloadmanager/sandbox_helper_test.go b/pkg/workloadmanager/sandbox_helper_test.go index 1c8fc9176..7776991c2 100644 --- a/pkg/workloadmanager/sandbox_helper_test.go +++ b/pkg/workloadmanager/sandbox_helper_test.go @@ -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{ @@ -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) }, @@ -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 { diff --git a/pkg/workloadmanager/workload_builder.go b/pkg/workloadmanager/workload_builder.go index 2a42e4f07..609304c6a 100644 --- a/pkg/workloadmanager/workload_builder.go +++ b/pkg/workloadmanager/workload_builder.go @@ -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 + } // Allocate fresh maps for copied metadata so we never mutate informer-cached input. // Annotations are only copied when params.podAnnotations is non-nil. @@ -200,7 +201,7 @@ func buildSandboxObject(params *buildSandboxParams) *sandboxv1alpha1.Sandbox { }, }, Lifecycle: sandboxv1alpha1.Lifecycle{ - ShutdownTime: &shutdownTime, + ShutdownTime: shutdownTime, }, Replicas: ptr.To[int32](1), }, @@ -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 { @@ -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 } + shutdownTime := metav1.NewTime(time.Now().Add(maxDuration)) + simpleSandbox.Spec.Lifecycle.ShutdownTime = &shutdownTime sandboxEntry.Kind = types.SandboxClaimsKind return simpleSandbox, sandboxClaim, sandboxEntry, nil } @@ -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 diff --git a/pkg/workloadmanager/workload_builder_test.go b/pkg/workloadmanager/workload_builder_test.go index 035dd5c6b..5703c7bd5 100644 --- a/pkg/workloadmanager/workload_builder_test.go +++ b/pkg/workloadmanager/workload_builder_test.go @@ -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) + } + }) + + 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. @@ -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{ @@ -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{