fix: added test for radis store, build sandox object and fix what reviewers ask volcano-sh#360

- CodeInterpreter now explicitly sets DefaultSandboxTTL (8h) when
  MaxSessionDuration is nil, in both the direct sandbox path and the
  warm-pool placeholder path, preserving the agreed-upon behavior while
  AgentRuntime remains unlimited by default.
- Add TestRedisStore_StoreSandbox_ZeroExpiresAt: verifies that a sandbox
  with zero ExpiresAt is stored and retrievable, absent from the expiry
  index (not hard-deleted), and still tracked in the last-activity index
  (idle-timeout GC still works).
- Add TestBuildSandboxObject_NoTTL: verifies that ttl==0 produces a
  Sandbox with nil ShutdownTime (AgentRuntime indefinite lifetime path).
- Replace DefaultSandboxTTL with time.Hour in workload_builder_test.go
  since DefaultSandboxTTL is no longer the implied default for every
  buildSandboxObject caller.
- Update example/agent-runtime/agent-runtime.yaml: remove hard-coded
  maxSessionDuration: 8h and replace with a commented-out example that
  documents the field is optional.
Signed-off-by: HarshitPal25 <harshit13082006@gmail.com>

Author: HarshitPal25

example/agent-runtime/agent-runtime.yaml

@@ -19,4 +19,7 @@ spec:
           image: busybox
           command: ["/bin/sh", "-c", "sleep 36000"]
   sessionTimeout: "15m"
-  maxSessionDuration: "8h"
+  # maxSessionDuration is optional. When omitted, the sandbox runs indefinitely
+  # and is only cleaned up by the idle timeout (sessionTimeout) above.
+  # Uncomment the line below to set a hard upper limit on session lifetime.
+  # maxSessionDuration: "24h"

pkg/store/store_redis_test.go

@@ -343,13 +343,70 @@ func TestUpdateSandboxLastActivity(t *testing.T) {
 	if int64(score) != newLastActivity.Unix() {
 		t.Fatalf("unexpected lastActivity score after update: got %v, want %v", score, newLastActivity.Unix())
 	}
+}
+
+// TestRedisStore_StoreSandbox_ZeroExpiresAt verifies the behavior for AgentRuntime
+// sandboxes that have no MaxSessionDuration (ExpiresAt == zero time):
+//   - The session is still stored and retrievable by session ID.
+//   - The session is NOT added to the expiry sorted-set (no hard cap).
+//   - The session IS tracked in the last-activity index after UpdateSessionLastActivity,
+//     so idle-timeout GC still works correctly.
+func TestRedisStore_StoreSandbox_ZeroExpiresAt(t *testing.T) {
+	ctx := context.Background()
+	c, mr := newTestRedisClient(t)
+
+	// Zero ExpiresAt — simulates an AgentRuntime sandbox with no MaxSessionDuration.
+	sb := &types.SandboxInfo{
+		SandboxID: "sb-no-expiry",
+		Name:      "test-sandbox-no-expiry",
+		SessionID: "sess-no-expiry",
+		CreatedAt: time.Now().UTC(),
+		ExpiresAt: time.Time{}, // zero
+		Status:    "running",
+	}
+
+	// StoreSandbox must succeed even with zero ExpiresAt.
+	if err := c.StoreSandbox(ctx, sb); err != nil {
+		t.Fatalf("StoreSandbox with zero ExpiresAt failed: %v", err)
+	}
+
+	// The session must be retrievable.
+	got, err := c.GetSandboxBySessionID(ctx, "sess-no-expiry")
+	if err != nil {
+		t.Fatalf("GetSandboxBySessionID failed: %v", err)
+	}
+	if got.SandboxID != "sb-no-expiry" {
+		t.Errorf("expected SandboxID sb-no-expiry, got %q", got.SandboxID)
+	}
 
-	// session not exists
-	err = c.UpdateSessionLastActivity(ctx, "sess-1-not-exist", newLastActivity)
+	// Must NOT be in the expiry sorted-set (zero ExpiresAt = no hard cap).
+	_, err = mr.ZScore(c.expiryIndexKey, "sess-no-expiry")
 	if err == nil {
-		t.Fatalf("expected error for non-existent session")
+		t.Error("expected sess-no-expiry to be absent from expiry index, but ZScore succeeded")
 	}
-	if !errors.Is(err, ErrNotFound) {
-		t.Fatalf("expected ErrNotFound, got %v", err)
+
+	// Must NOT appear in ListExpiredSandboxes even at a very late cutoff.
+	farFuture := time.Now().Add(100 * 365 * 24 * time.Hour)
+	expired, err := c.ListExpiredSandboxes(ctx, farFuture, 100)
+	if err != nil {
+		t.Fatalf("ListExpiredSandboxes error: %v", err)
+	}
+	for _, s := range expired {
+		if s.SessionID == "sess-no-expiry" {
+			t.Error("sess-no-expiry must not appear in ListExpiredSandboxes")
+		}
+	}
+
+	// After UpdateSessionLastActivity the session MUST be tracked in the activity index.
+	now := time.Now().UTC().Truncate(time.Second)
+	if err := c.UpdateSessionLastActivity(ctx, "sess-no-expiry", now); err != nil {
+		t.Fatalf("UpdateSessionLastActivity failed: %v", err)
+	}
+	score, err := mr.ZScore(c.lastActivityIndexKey, "sess-no-expiry")
+	if err != nil {
+		t.Fatalf("expected sess-no-expiry in last-activity index: %v", err)
+	}
+	if int64(score) != now.Unix() {
+		t.Errorf("unexpected last-activity score: got %v, want %v", int64(score), now.Unix())
 	}
 }

pkg/workloadmanager/workload_builder.go

@@ -401,10 +401,15 @@ func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string,
 				},
 			},
 		}
+		// CodeInterpreter always has a hard cap: use explicit MaxSessionDuration or
+		// fall back to DefaultSandboxTTL (8h). This differs from AgentRuntime where
+		// omitting MaxSessionDuration means no hard expiry.
+		ttl := DefaultSandboxTTL
 		if codeInterpreterObj.Spec.MaxSessionDuration != nil {
-			shutdownTime := metav1.NewTime(time.Now().Add(codeInterpreterObj.Spec.MaxSessionDuration.Duration))
-			simpleSandbox.Spec.Lifecycle.ShutdownTime = &shutdownTime
+			ttl = codeInterpreterObj.Spec.MaxSessionDuration.Duration
 		}
+		shutdownTime := metav1.NewTime(time.Now().Add(ttl))
+		simpleSandbox.Spec.Lifecycle.ShutdownTime = &shutdownTime
 		sandboxEntry.Kind = types.SandboxClaimsKind
 		return simpleSandbox, sandboxClaim, sandboxEntry, nil
 	}
@@ -444,8 +449,13 @@ func buildSandboxByCodeInterpreter(namespace string, codeInterpreterName string,
 		idleTimeout:    idleTimeout,
 	}
 
+	// CodeInterpreter always has a hard cap: use explicit MaxSessionDuration or
+	// fall back to DefaultSandboxTTL (8h). This differs from AgentRuntime where
+	// omitting MaxSessionDuration means no hard expiry.
 	if codeInterpreterObj.Spec.MaxSessionDuration != nil {
 		buildParams.ttl = codeInterpreterObj.Spec.MaxSessionDuration.Duration
+	} else {
+		buildParams.ttl = DefaultSandboxTTL
 	}
 	sandbox := buildSandboxObject(buildParams)
 	return sandbox, nil, sandboxEntry, nil

pkg/workloadmanager/workload_builder_test.go

@@ -51,7 +51,7 @@ func TestBuildSandboxObject_DoesNotMutateCallerLabels(t *testing.T) {
 		namespace:   "default",
 		sandboxName: "sandbox-abc",
 		sessionID:   "session-123",
-		ttl:         DefaultSandboxTTL,
+		ttl:         time.Hour,
 		idleTimeout: DefaultSandboxIdleTimeout,
 		podLabels:   original,
 	}
@@ -115,6 +115,26 @@ func TestBuildSandboxObject_NilLabels(t *testing.T) {
 	}
 }
 
+// TestBuildSandboxObject_NoTTL verifies that when ttl==0 (AgentRuntime with no
+// MaxSessionDuration configured), ShutdownTime is nil — the sandbox runs
+// indefinitely and is only cleaned up by idle timeout.
+func TestBuildSandboxObject_NoTTL(t *testing.T) {
+	params := &buildSandboxParams{
+		namespace:   "default",
+		sandboxName: "sandbox-no-ttl",
+		sessionID:   "session-no-ttl",
+		ttl:         0, // explicitly no TTL — AgentRuntime without MaxSessionDuration
+		idleTimeout: 15 * time.Minute,
+	}
+
+	sandbox := buildSandboxObject(params)
+
+	if sandbox.Spec.Lifecycle.ShutdownTime != nil {
+		t.Errorf("expected ShutdownTime to be nil for zero-ttl sandbox, 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.