From 6550c9918954ca875201521ee317395093575bd7 Mon Sep 17 00:00:00 2001 From: Sri Roopa Ramesh Babu Date: Mon, 3 Aug 2026 23:14:59 -0400 Subject: [PATCH] OLS-3566 Add TTL lifecycle CRD fields and reconciler enforcement Add automatic garbage collection of terminal AgenticRun resources following the Kubernetes Jobs ttlSecondsAfterFinished pattern. CRD changes: - AgenticOLSConfig.spec.lifecycle.terminalTTL (cluster-wide default) - AgenticRun.spec.ttlAfterTerminal (per-run override, mutable) - AgenticRun.status.terminalTime (stamped once at terminal state) Reconciler changes: - Stamp terminalTime and ttlAfterTerminal on terminal state - Delete expired runs; RequeueAfter for non-expired - ttlAfterTerminal=0 disables auto-deletion - Pre-set ttlAfterTerminal preserved (not overwritten) - No AgenticOLSConfig CR = no auto-deletion (backwards-compatible) Co-Authored-By: Claude Opus 4.6 (1M context) --- api/v1alpha1/agenticolsconfig_types.go | 25 ++ api/v1alpha1/agenticrun_types.go | 25 ++ ...gentic.openshift.io_agenticolsconfigs.yaml | 23 ++ .../agentic.openshift.io_agenticruns.yaml | 27 ++ controller/agenticrun/helpers.go | 13 + controller/agenticrun/reconciler.go | 95 ++++- controller/agenticrun/ttl_test.go | 357 ++++++++++++++++++ 7 files changed, 563 insertions(+), 2 deletions(-) create mode 100644 controller/agenticrun/ttl_test.go diff --git a/api/v1alpha1/agenticolsconfig_types.go b/api/v1alpha1/agenticolsconfig_types.go index 869de448..671643e5 100644 --- a/api/v1alpha1/agenticolsconfig_types.go +++ b/api/v1alpha1/agenticolsconfig_types.go @@ -29,6 +29,26 @@ const ( AgenticOLSConfigConditionSuspended = "Suspended" ) +// LifecycleConfig controls automatic cleanup of terminal AgenticRun resources. +// +// +kubebuilder:validation:MinProperties=1 +type LifecycleConfig struct { + // terminalTTL is the default time-to-live in seconds for terminal + // AgenticRun resources (Completed, Failed, Denied, Escalated, + // EmergencyStopped, NoActionRequired). After a run reaches a terminal + // state and this many seconds elapse, the operator deletes the + // AgenticRun CR. Kubernetes garbage collection cascades deletion to + // owned resources via owner references. + // + // Per-run overrides via AgenticRun.spec.ttlAfterTerminal take + // precedence over this cluster-wide default. + // + // When omitted (nil), no automatic deletion occurs. + // +optional + // +kubebuilder:validation:Minimum=0 + TerminalTTL *int32 `json:"terminalTTL,omitempty"` +} + // AgenticOLSConfigSpec defines the desired state of AgenticOLSConfig. // // +kubebuilder:validation:MinProperties=1 @@ -41,6 +61,11 @@ type AgenticOLSConfigSpec struct { // +optional // +default=false Suspended bool `json:"suspended,omitempty"` //nolint:kubeapilinter // kill switch is genuinely binary; bool is the right type + + // lifecycle controls automatic cleanup of terminal AgenticRun resources. + // When omitted, no automatic deletion occurs (backwards-compatible). + // +optional + Lifecycle LifecycleConfig `json:"lifecycle,omitzero"` } // AgenticOLSConfigStatus defines the observed state of AgenticOLSConfig. diff --git a/api/v1alpha1/agenticrun_types.go b/api/v1alpha1/agenticrun_types.go index 311c4d83..781546b5 100644 --- a/api/v1alpha1/agenticrun_types.go +++ b/api/v1alpha1/agenticrun_types.go @@ -378,6 +378,23 @@ type AgenticRunSpec struct { // +kubebuilder:validation:MinLength=1 // +kubebuilder:validation:MaxLength=32768 RevisionFeedback string `json:"revisionFeedback,omitempty"` + + // ttlAfterTerminal is the time-to-live in seconds for this AgenticRun + // after it reaches a terminal state (Completed, Failed, Denied, + // Escalated, EmergencyStopped, NoActionRequired). When the TTL expires, + // the operator deletes the AgenticRun CR and Kubernetes garbage + // collection cascades deletion to owned resources. + // + // Overrides the cluster-wide default from + // AgenticOLSConfig.spec.lifecycle.terminalTTL for this run. + // + // Set to 0 to disable automatic deletion for this run. + // + // Mutable: adapters or admins may pre-set this before the run reaches + // terminal state. The operator will not overwrite a pre-set value. + // +optional + // +kubebuilder:validation:Minimum=0 + TTLAfterTerminal *int32 `json:"ttlAfterTerminal,omitempty"` } // AgenticRunStatus defines the observed state of AgenticRun. All fields are @@ -406,6 +423,14 @@ type AgenticRunStatus struct { // info, and references to result CRs. // +optional Steps StepsStatus `json:"steps,omitzero"` + + // terminalTime is the timestamp when the run first reached a terminal + // state (Completed, Failed, Denied, Escalated, EmergencyStopped, + // NoActionRequired). Set once by the operator and never updated. + // Used together with spec.ttlAfterTerminal to compute when the run + // should be garbage-collected. + // +optional + TerminalTime *metav1.Time `json:"terminalTime,omitempty"` } // +kubebuilder:object:root=true diff --git a/config/crd/bases/agentic.openshift.io_agenticolsconfigs.yaml b/config/crd/bases/agentic.openshift.io_agenticolsconfigs.yaml index 3c3a2189..0e3ad021 100644 --- a/config/crd/bases/agentic.openshift.io_agenticolsconfigs.yaml +++ b/config/crd/bases/agentic.openshift.io_agenticolsconfigs.yaml @@ -54,6 +54,29 @@ spec: description: spec defines the desired system configuration. minProperties: 1 properties: + lifecycle: + description: |- + lifecycle controls automatic cleanup of terminal AgenticRun resources. + When omitted, no automatic deletion occurs (backwards-compatible). + minProperties: 1 + properties: + terminalTTL: + description: |- + terminalTTL is the default time-to-live in seconds for terminal + AgenticRun resources (Completed, Failed, Denied, Escalated, + EmergencyStopped, NoActionRequired). After a run reaches a terminal + state and this many seconds elapse, the operator deletes the + AgenticRun CR. Kubernetes garbage collection cascades deletion to + owned resources via owner references. + + Per-run overrides via AgenticRun.spec.ttlAfterTerminal take + precedence over this cluster-wide default. + + When omitted (nil), no automatic deletion occurs. + format: int32 + minimum: 0 + type: integer + type: object suspended: default: false description: |- diff --git a/config/crd/bases/agentic.openshift.io_agenticruns.yaml b/config/crd/bases/agentic.openshift.io_agenticruns.yaml index 17e1fd83..9e8c58ef 100644 --- a/config/crd/bases/agentic.openshift.io_agenticruns.yaml +++ b/config/crd/bases/agentic.openshift.io_agenticruns.yaml @@ -1275,6 +1275,24 @@ spec: - image x-kubernetes-list-type: map type: object + ttlAfterTerminal: + description: |- + ttlAfterTerminal is the time-to-live in seconds for this AgenticRun + after it reaches a terminal state (Completed, Failed, Denied, + Escalated, EmergencyStopped, NoActionRequired). When the TTL expires, + the operator deletes the AgenticRun CR and Kubernetes garbage + collection cascades deletion to owned resources. + + Overrides the cluster-wide default from + AgenticOLSConfig.spec.lifecycle.terminalTTL for this run. + + Set to 0 to disable automatic deletion for this run. + + Mutable: adapters or admins may pre-set this before the run reaches + terminal state. The operator will not overwrite a pre-set value. + format: int32 + minimum: 0 + type: integer verification: description: |- verification defines per-step configuration for the verification step. @@ -2268,6 +2286,15 @@ spec: type: object type: object type: object + terminalTime: + description: |- + terminalTime is the timestamp when the run first reached a terminal + state (Completed, Failed, Denied, Escalated, EmergencyStopped, + NoActionRequired). Set once by the operator and never updated. + Used together with spec.ttlAfterTerminal to compute when the run + should be garbage-collected. + format: date-time + type: string type: object required: - spec diff --git a/controller/agenticrun/helpers.go b/controller/agenticrun/helpers.go index 104477c6..feabf3b0 100644 --- a/controller/agenticrun/helpers.go +++ b/controller/agenticrun/helpers.go @@ -83,6 +83,19 @@ func isSuspended(ctx context.Context, c client.Client) (bool, error) { return config.Spec.Suspended, nil } +// getTerminalTTL returns the cluster-wide default TTL from AgenticOLSConfig, +// or nil if no config exists or no TTL is configured. +func getTerminalTTL(ctx context.Context, c client.Client) (*int32, error) { + var config agenticv1alpha1.AgenticOLSConfig + if err := c.Get(ctx, client.ObjectKey{Name: "cluster"}, &config); err != nil { + if client.IgnoreNotFound(err) == nil { + return nil, nil + } + return nil, err + } + return config.Spec.Lifecycle.TerminalTTL, nil +} + // failStep marks a step as failed and creates a failure result CR. // The caller must have set the step condition to ConditionUnknown before // calling failStep so that conditionTime can extract the start time. diff --git a/controller/agenticrun/reconciler.go b/controller/agenticrun/reconciler.go index 3828d933..a00c158b 100644 --- a/controller/agenticrun/reconciler.go +++ b/controller/agenticrun/reconciler.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "strconv" + "time" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" @@ -24,6 +25,8 @@ const ( ErrAddFinalizer = "add finalizer" ErrPatchTemplogCleanupAttempts = "patch templog cleanup attempts" ErrPatchRBACCleanupAttempts = "patch rbac cleanup attempts" + ErrStampTerminalTTL = "stamp terminal TTL" + ErrDeleteExpiredRun = "delete expired run" ) // TempLogCleaner is the interface for deleting templog records on CR deletion. @@ -115,6 +118,9 @@ func (r *AgenticRunReconciler) Reconcile(ctx context.Context, req ctrl.Request) r.Audit.EmitTerminalSpan(ctx, &run, string(phase), terminalReason(&run)) r.Audit.Cleanup(&run) } + if result, requeue, err := r.handleTerminalTTL(ctx, &run); requeue || err != nil { + return result, err + } return ctrl.Result{}, nil } @@ -129,6 +135,9 @@ func (r *AgenticRunReconciler) Reconcile(ctx context.Context, req ctrl.Request) r.Audit.EmitTerminalSpan(ctx, &run, string(phase), terminalReason(&run)) r.Audit.Cleanup(&run) } + if result, requeue, err := r.handleTerminalTTL(ctx, &run); requeue || err != nil { + return result, err + } return ctrl.Result{}, nil } @@ -144,11 +153,20 @@ func (r *AgenticRunReconciler) Reconcile(ctx context.Context, req ctrl.Request) r.Audit.EmitTerminalSpan(ctx, &run, string(phase), terminalReason(&run)) r.Audit.Cleanup(&run) } + if result, requeue, err := r.handleTerminalTTL(ctx, &run); requeue || err != nil { + return result, err + } return ctrl.Result{}, nil case agenticv1alpha1.AgenticRunPhaseFailed: if !(run.Spec.Execution.IsZero() && needsRevision(&run)) { - return r.handleFailed(ctx, &run) + if result, err := r.handleFailed(ctx, &run); err != nil { + return result, err + } + if result, requeue, err := r.handleTerminalTTL(ctx, &run); requeue || err != nil { + return result, err + } + return ctrl.Result{}, nil } } @@ -257,7 +275,10 @@ func (r *AgenticRunReconciler) SetupWithManager(mgr ctrl.Manager) error { } var reqs []ctrl.Request for _, p := range runs.Items { - if !isTerminal(agenticv1alpha1.DerivePhase(p.Status.Conditions)) { + phase := agenticv1alpha1.DerivePhase(p.Status.Conditions) + // Enqueue non-terminal runs (normal workflow) and terminal runs + // that may need TTL stamping (terminalTime set but no ttlAfterTerminal yet). + if !isTerminal(phase) || (p.Status.TerminalTime != nil && p.Spec.TTLAfterTerminal == nil) { reqs = append(reqs, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(&p)}) } } @@ -286,6 +307,76 @@ func (r *AgenticRunReconciler) SetupWithManager(mgr ctrl.Manager) error { Complete(r) } +// handleTerminalTTL stamps terminalTime and ttlAfterTerminal on a terminal run, +// then checks whether the TTL has expired. If expired, it deletes the AgenticRun +// CR (Kubernetes GC cascades to owned resources). If not expired, it returns a +// RequeueAfter for the remaining TTL. Returns (result, requeue, error) where +// requeue=true means the caller should return the result instead of continuing. +func (r *AgenticRunReconciler) handleTerminalTTL(ctx context.Context, run *agenticv1alpha1.AgenticRun) (ctrl.Result, bool, error) { + log := logf.FromContext(ctx) + now := metav1.Now() + + // --- Stamp terminalTime if not yet set --- + if run.Status.TerminalTime == nil { + base := run.DeepCopy() + run.Status.TerminalTime = &now + if err := r.statusPatch(ctx, run, base); err != nil { + log.Error(err, "failed to stamp terminalTime") + return ctrl.Result{}, false, err + } + } + + // --- Stamp ttlAfterTerminal from cluster config if not already set --- + if run.Spec.TTLAfterTerminal == nil { + clusterTTL, err := getTerminalTTL(ctx, r.Client) + if err != nil { + return ctrl.Result{}, false, fmt.Errorf("%s: %w", ErrStampTerminalTTL, err) + } + if clusterTTL != nil { + // Re-fetch to avoid conflicts after the status patch above. + if err := r.Get(ctx, client.ObjectKeyFromObject(run), run); err != nil { + return ctrl.Result{}, false, client.IgnoreNotFound(err) + } + original := run.DeepCopy() + run.Spec.TTLAfterTerminal = clusterTTL + if err := r.Patch(ctx, run, client.MergeFrom(original)); err != nil { + log.Error(err, "failed to stamp ttlAfterTerminal") + return ctrl.Result{}, false, err + } + } + } + + // --- Evaluate TTL --- + if run.Spec.TTLAfterTerminal == nil { + // No TTL configured — no auto-deletion. + return ctrl.Result{}, false, nil + } + + ttlSeconds := *run.Spec.TTLAfterTerminal + if ttlSeconds == 0 { + // TTL=0 explicitly disables auto-deletion for this run. + return ctrl.Result{}, false, nil + } + + terminalTime := run.Status.TerminalTime.Time + expiry := terminalTime.Add(time.Duration(ttlSeconds) * time.Second) + remaining := time.Until(expiry) + + if remaining <= 0 { + log.Info("TTL expired, deleting AgenticRun", LogKeyName, run.Name) + if err := r.Delete(ctx, run); err != nil { + if client.IgnoreNotFound(err) == nil { + return ctrl.Result{}, true, nil + } + return ctrl.Result{}, false, fmt.Errorf("%s: %w", ErrDeleteExpiredRun, err) + } + return ctrl.Result{}, true, nil + } + + log.V(1).Info("TTL not yet expired, requeueing", LogKeyName, run.Name, "remaining", remaining) + return ctrl.Result{RequeueAfter: remaining}, true, nil +} + // handleTemplogCleanup deletes audit logs from the Collector's Postgres store // for this AgenticRun. Retries up to templogMaxCleanupAttempts, then removes // the finalizer regardless to unblock CR deletion. diff --git a/controller/agenticrun/ttl_test.go b/controller/agenticrun/ttl_test.go new file mode 100644 index 00000000..c4b7e18e --- /dev/null +++ b/controller/agenticrun/ttl_test.go @@ -0,0 +1,357 @@ +package agenticrun + +import ( + "context" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + agenticv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1" +) + +// ptr32 is defined in helpers_test.go + +func TestGetTerminalTTL(t *testing.T) { + tests := []struct { + name string + objects []client.Object + want *int32 + }{ + { + name: "no config CR returns nil", + objects: nil, + want: nil, + }, + { + name: "config without lifecycle returns nil", + objects: []client.Object{&agenticv1alpha1.AgenticOLSConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: agenticv1alpha1.AgenticOLSConfigSpec{}, + }}, + want: nil, + }, + { + name: "config with terminalTTL returns value", + objects: []client.Object{&agenticv1alpha1.AgenticOLSConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: agenticv1alpha1.AgenticOLSConfigSpec{ + Lifecycle: agenticv1alpha1.LifecycleConfig{TerminalTTL: ptr32(3600)}, + }, + }}, + want: ptr32(3600), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + objects := tt.objects + if objects == nil { + objects = []client.Object{} + } + fc := fake.NewClientBuilder(). + WithScheme(testScheme()). + WithObjects(objects...). + Build() + got, err := getTerminalTTL(context.Background(), fc) + if err != nil { + t.Fatalf("getTerminalTTL() error = %v", err) + } + if (got == nil) != (tt.want == nil) { + t.Fatalf("getTerminalTTL() = %v, want %v", got, tt.want) + } + if got != nil && *got != *tt.want { + t.Fatalf("getTerminalTTL() = %d, want %d", *got, *tt.want) + } + }) + } +} + +func TestHandleTerminalTTL_StampsTerminalTimeAndTTL(t *testing.T) { + config := &agenticv1alpha1.AgenticOLSConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: agenticv1alpha1.AgenticOLSConfigSpec{ + Lifecycle: agenticv1alpha1.LifecycleConfig{TerminalTTL: ptr32(3600)}, + }, + } + + run := testAgenticRun() + run.Status.Conditions = []metav1.Condition{{ + Type: agenticv1alpha1.AgenticRunConditionVerified, + Status: metav1.ConditionTrue, + Reason: "Complete", + }} + + objs := append([]client.Object{run, config}, defaultObjects()...) + fc := fake.NewClientBuilder().WithScheme(testScheme()).WithObjects(objs...). + WithStatusSubresource(run).Build() + + r := &AgenticRunReconciler{Client: fc, Agent: newTestAgentCaller(), Namespace: "default"} + + result, err := reconcileOnce(r, "fix-crash") + if err != nil { + t.Fatalf("reconcile: %v", err) + } + + got, _ := getAgenticRun(r, "fix-crash") + + if got.Status.TerminalTime == nil { + t.Fatal("terminalTime should be stamped") + } + if got.Spec.TTLAfterTerminal == nil { + t.Fatal("ttlAfterTerminal should be stamped from config") + } + if *got.Spec.TTLAfterTerminal != 3600 { + t.Errorf("ttlAfterTerminal = %d, want 3600", *got.Spec.TTLAfterTerminal) + } + if result.RequeueAfter <= 0 { + t.Error("expected RequeueAfter > 0 for non-expired TTL") + } +} + +func TestHandleTerminalTTL_PresetTTLNotOverwritten(t *testing.T) { + config := &agenticv1alpha1.AgenticOLSConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: agenticv1alpha1.AgenticOLSConfigSpec{ + Lifecycle: agenticv1alpha1.LifecycleConfig{TerminalTTL: ptr32(3600)}, + }, + } + + run := testAgenticRun() + run.Spec.TTLAfterTerminal = ptr32(7200) // pre-set by adapter + run.Status.Conditions = []metav1.Condition{{ + Type: agenticv1alpha1.AgenticRunConditionVerified, + Status: metav1.ConditionTrue, + Reason: "Complete", + }} + + objs := append([]client.Object{run, config}, defaultObjects()...) + fc := fake.NewClientBuilder().WithScheme(testScheme()).WithObjects(objs...). + WithStatusSubresource(run).Build() + + r := &AgenticRunReconciler{Client: fc, Agent: newTestAgentCaller(), Namespace: "default"} + + _, err := reconcileOnce(r, "fix-crash") + if err != nil { + t.Fatalf("reconcile: %v", err) + } + + got, _ := getAgenticRun(r, "fix-crash") + if got.Spec.TTLAfterTerminal == nil || *got.Spec.TTLAfterTerminal != 7200 { + t.Errorf("ttlAfterTerminal = %v, want 7200 (pre-set should not be overwritten)", got.Spec.TTLAfterTerminal) + } +} + +func TestHandleTerminalTTL_ZeroDisablesAutoDeletion(t *testing.T) { + run := testAgenticRun() + run.Spec.TTLAfterTerminal = ptr32(0) // explicitly disable + now := metav1.NewTime(time.Now().Add(-1 * time.Hour)) + run.Status.TerminalTime = &now + run.Status.Conditions = []metav1.Condition{{ + Type: agenticv1alpha1.AgenticRunConditionVerified, + Status: metav1.ConditionTrue, + Reason: "Complete", + }} + + objs := append([]client.Object{run}, defaultObjects()...) + fc := fake.NewClientBuilder().WithScheme(testScheme()).WithObjects(objs...). + WithStatusSubresource(run).Build() + + r := &AgenticRunReconciler{Client: fc, Agent: newTestAgentCaller(), Namespace: "default"} + + result, err := reconcileOnce(r, "fix-crash") + if err != nil { + t.Fatalf("reconcile: %v", err) + } + if result.RequeueAfter != 0 { + t.Error("ttl=0 should not requeue") + } + + // Run should still exist. + got, getErr := getAgenticRun(r, "fix-crash") + if getErr != nil { + t.Fatalf("run should not be deleted when ttl=0: %v", getErr) + } + if got == nil { + t.Fatal("run should still exist when ttl=0") + } +} + +func TestHandleTerminalTTL_ExpiredRunDeleted(t *testing.T) { + run := testAgenticRun() + run.Spec.TTLAfterTerminal = ptr32(60) // 60 seconds TTL + pastTime := metav1.NewTime(time.Now().Add(-2 * time.Minute)) + run.Status.TerminalTime = &pastTime + run.Status.Conditions = []metav1.Condition{{ + Type: agenticv1alpha1.AgenticRunConditionVerified, + Status: metav1.ConditionTrue, + Reason: "Complete", + }} + + objs := append([]client.Object{run}, defaultObjects()...) + fc := fake.NewClientBuilder().WithScheme(testScheme()).WithObjects(objs...). + WithStatusSubresource(run).Build() + + r := &AgenticRunReconciler{Client: fc, Agent: newTestAgentCaller(), Namespace: "default"} + + _, err := reconcileOnce(r, "fix-crash") + if err != nil { + t.Fatalf("reconcile: %v", err) + } + + // Run should be marked for deletion (DeletionTimestamp set). + // With finalizers present, Kubernetes keeps the object until finalizers + // are removed — Delete sets DeletionTimestamp rather than removing it. + var updated agenticv1alpha1.AgenticRun + getErr := fc.Get(context.Background(), types.NamespacedName{Name: "fix-crash", Namespace: "default"}, &updated) + if getErr != nil { + if client.IgnoreNotFound(getErr) == nil { + // Object was fully deleted (no finalizers case) — also acceptable. + return + } + t.Fatalf("unexpected error: %v", getErr) + } + if updated.DeletionTimestamp.IsZero() { + t.Fatal("expired run should have DeletionTimestamp set") + } +} + +func TestHandleTerminalTTL_NotExpiredRequeues(t *testing.T) { + run := testAgenticRun() + run.Spec.TTLAfterTerminal = ptr32(3600) // 1 hour TTL + now := metav1.NewTime(time.Now()) + run.Status.TerminalTime = &now + run.Status.Conditions = []metav1.Condition{{ + Type: agenticv1alpha1.AgenticRunConditionVerified, + Status: metav1.ConditionTrue, + Reason: "Complete", + }} + + objs := append([]client.Object{run}, defaultObjects()...) + fc := fake.NewClientBuilder().WithScheme(testScheme()).WithObjects(objs...). + WithStatusSubresource(run).Build() + + r := &AgenticRunReconciler{Client: fc, Agent: newTestAgentCaller(), Namespace: "default"} + + result, err := reconcileOnce(r, "fix-crash") + if err != nil { + t.Fatalf("reconcile: %v", err) + } + if result.RequeueAfter <= 0 { + t.Error("non-expired TTL should requeue with remaining time") + } + if result.RequeueAfter > 1*time.Hour { + t.Errorf("RequeueAfter = %v, should be <= 1h", result.RequeueAfter) + } + + // Run should still exist. + _, getErr := getAgenticRun(r, "fix-crash") + if getErr != nil { + t.Fatalf("non-expired run should still exist: %v", getErr) + } +} + +func TestHandleTerminalTTL_NoConfigNoAutoDeletion(t *testing.T) { + // No AgenticOLSConfig CR exists — backwards-compatible, no auto-deletion. + run := testAgenticRun() + run.Status.Conditions = []metav1.Condition{{ + Type: agenticv1alpha1.AgenticRunConditionVerified, + Status: metav1.ConditionTrue, + Reason: "Complete", + }} + + objs := append([]client.Object{run}, defaultObjects()...) + fc := fake.NewClientBuilder().WithScheme(testScheme()).WithObjects(objs...). + WithStatusSubresource(run).Build() + + r := &AgenticRunReconciler{Client: fc, Agent: newTestAgentCaller(), Namespace: "default"} + + result, err := reconcileOnce(r, "fix-crash") + if err != nil { + t.Fatalf("reconcile: %v", err) + } + if result.RequeueAfter != 0 || result.Requeue { + t.Error("no config should not cause requeue for TTL") + } + + // Run should still exist and have terminalTime stamped but no ttlAfterTerminal. + got, _ := getAgenticRun(r, "fix-crash") + if got.Status.TerminalTime == nil { + t.Error("terminalTime should still be stamped") + } + if got.Spec.TTLAfterTerminal != nil { + t.Errorf("ttlAfterTerminal should be nil when no config, got %d", *got.Spec.TTLAfterTerminal) + } +} + +func TestHandleTerminalTTL_DeniedPhase(t *testing.T) { + config := &agenticv1alpha1.AgenticOLSConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: agenticv1alpha1.AgenticOLSConfigSpec{ + Lifecycle: agenticv1alpha1.LifecycleConfig{TerminalTTL: ptr32(60)}, + }, + } + + run := testAgenticRun() + run.Status.Conditions = []metav1.Condition{ + {Type: agenticv1alpha1.AgenticRunConditionAnalyzed, Status: metav1.ConditionTrue, Reason: "AnalysisComplete"}, + {Type: agenticv1alpha1.AgenticRunConditionDenied, Status: metav1.ConditionTrue, Reason: "UserDenied"}, + } + + objs := append([]client.Object{run, config}, defaultObjects()...) + fc := fake.NewClientBuilder().WithScheme(testScheme()).WithObjects(objs...). + WithStatusSubresource(run).Build() + + r := &AgenticRunReconciler{Client: fc, Agent: newTestAgentCaller(), Namespace: "default"} + + result, err := reconcileOnce(r, "fix-crash") + if err != nil { + t.Fatalf("reconcile: %v", err) + } + + got, _ := getAgenticRun(r, "fix-crash") + if got.Status.TerminalTime == nil { + t.Fatal("Denied run should have terminalTime stamped") + } + if got.Spec.TTLAfterTerminal == nil || *got.Spec.TTLAfterTerminal != 60 { + t.Errorf("ttlAfterTerminal should be 60 for Denied run, got %v", got.Spec.TTLAfterTerminal) + } + if result.RequeueAfter <= 0 { + t.Error("expected RequeueAfter > 0 for non-expired Denied run") + } +} + +func TestHandleTerminalTTL_FailedPhase(t *testing.T) { + config := &agenticv1alpha1.AgenticOLSConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: agenticv1alpha1.AgenticOLSConfigSpec{ + Lifecycle: agenticv1alpha1.LifecycleConfig{TerminalTTL: ptr32(120)}, + }, + } + + run := testAgenticRun() + run.Status.Conditions = []metav1.Condition{ + {Type: agenticv1alpha1.AgenticRunConditionAnalyzed, Status: metav1.ConditionFalse, Reason: "Failed", Message: "analysis error"}, + } + + objs := append([]client.Object{run, config}, defaultObjects()...) + fc := fake.NewClientBuilder().WithScheme(testScheme()).WithObjects(objs...). + WithStatusSubresource(run).Build() + + r := &AgenticRunReconciler{Client: fc, Agent: newTestAgentCaller(), Namespace: "default"} + + result, err := reconcileOnce(r, "fix-crash") + if err != nil { + t.Fatalf("reconcile: %v", err) + } + + got, _ := getAgenticRun(r, "fix-crash") + if got.Status.TerminalTime == nil { + t.Fatal("Failed run should have terminalTime stamped") + } + if result.RequeueAfter <= 0 { + t.Error("expected RequeueAfter > 0 for non-expired Failed run") + } +}