From 505e9272291e035233fbf0e5c6ce55d909cc2556 Mon Sep 17 00:00:00 2001 From: Mariusz Sabath Date: Tue, 1 Sep 2026 12:10:36 -0400 Subject: [PATCH] fix(agentruntime): degrade gracefully when target workload is missing (#2490) When an AgentRuntime's spec.targetRef workload (Sandbox for agents, Deployment for tools) is deleted but the AgentRuntime CR remains, the operator previously logged an Error and emitted a Warning event every ~30s while leaving a stale Ready: True. resolveTargetRef now wraps the IsNotFound case with a sentinel; Reconcile branches on it and treats a missing target as a recoverable degraded state: sets Ready=False and TargetResolved=False (reason TargetNotFound), clears the now-stale status.Card, logs at V(1) instead of Error, emits the Warning event only on transition into the degraded state, and requeues at 60s (recovery is watch-driven, not bound by the interval). Genuine (non-IsNotFound) API errors keep the loud Error path with a distinct reason, TargetResolveError. Target-resolution reasons are extracted to constants. Adds envtest coverage: degraded sets Ready=False; the Warning event is deduped to once across cycles; status.Card is cleared on degrade; recovery to Ready=True when the target reappears. The context's AfterEach now drains the kagenti.io/cleanup finalizer so specs do not leak state. Assisted-By: Claude (Anthropic AI) Signed-off-by: Mariusz Sabath --- .../controller/agentruntime_controller.go | 65 +++++++++- .../agentruntime_controller_test.go | 119 ++++++++++++++++-- 2 files changed, 172 insertions(+), 12 deletions(-) diff --git a/operator/internal/controller/agentruntime_controller.go b/operator/internal/controller/agentruntime_controller.go index ff4e1135..f0442d57 100644 --- a/operator/internal/controller/agentruntime_controller.go +++ b/operator/internal/controller/agentruntime_controller.go @@ -21,6 +21,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "strconv" "strings" @@ -79,6 +80,11 @@ const ( ConditionTypeConfigResolved = "ConfigResolved" ConditionTypeMTLSReady = "MTLSReady" + // Condition reasons for AgentRuntime target resolution. + ReasonTargetFound = "TargetFound" + ReasonTargetNotFound = "TargetNotFound" + ReasonTargetResolveError = "TargetResolveError" + // AnnotationLastCardFetchHash stores the change-detection key used to skip // redundant card fetches when the workload's pod template has not changed. AnnotationLastCardFetchHash = "agent.rossoctl.dev/last-card-fetch-hash" @@ -100,6 +106,12 @@ var sandboxGVK = schema.GroupVersionKind{ Kind: KindSandbox, } +// errTargetNotFound is a sentinel wrapped by resolveTargetRef when the target +// workload referenced by spec.targetRef does not exist. Reconcile treats this +// as a recoverable degraded state rather than an error, to avoid log/event +// spam while the target is absent. +var errTargetNotFound = errors.New("target workload not found") + // AgentRuntimeReconciler reconciles AgentRuntime objects. type AgentRuntimeReconciler struct { client.Client @@ -174,16 +186,37 @@ func (r *AgentRuntimeReconciler) Reconcile(ctx context.Context, req ctrl.Request // 4. Resolve targetRef (existence check) if err := r.resolveTargetRef(ctx, rt); err != nil { + if errors.Is(err, errTargetNotFound) { + // Recoverable: the AgentRuntime outlives its target workload (e.g. the + // child Sandbox/Deployment was deleted directly). Treat as a degraded + // state instead of spamming Error logs / Warning events every cycle. + // Emit the Warning event only on transition into the degraded state, + // deduped via the pre-existing TargetResolved=False/TargetNotFound + // condition. Recovery is automatic once the target reappears. + prev := meta.FindStatusCondition(rt.Status.Conditions, ConditionTypeTargetResolved) + alreadyDegraded := prev != nil && + prev.Status == metav1.ConditionFalse && + prev.Reason == ReasonTargetNotFound + + logger.V(1).Info("Target workload not found; AgentRuntime degraded", "error", err.Error()) + r.setDegradedTargetNotFound(ctx, req.NamespacedName, err.Error()) + if r.Recorder != nil && !alreadyDegraded { + r.Recorder.Eventf(rt, nil, corev1.EventTypeWarning, ReasonTargetNotFound, + "ResolveTarget", err.Error()) + } + return ctrl.Result{RequeueAfter: 60 * time.Second}, nil + } + // Genuine API error resolving the target: keep the loud error path. logger.Error(err, "Failed to resolve targetRef") - r.updateErrorStatus(ctx, req.NamespacedName, ConditionTypeTargetResolved, "TargetNotFound", err.Error()) + r.updateErrorStatus(ctx, req.NamespacedName, ConditionTypeTargetResolved, ReasonTargetResolveError, err.Error()) if r.Recorder != nil { - r.Recorder.Eventf(rt, nil, corev1.EventTypeWarning, "TargetNotFound", + r.Recorder.Eventf(rt, nil, corev1.EventTypeWarning, ReasonTargetResolveError, "ResolveTarget", err.Error()) } return ctrl.Result{RequeueAfter: 30 * time.Second}, nil } - r.setCondition(rt, ConditionTypeTargetResolved, metav1.ConditionTrue, "TargetFound", + r.setCondition(rt, ConditionTypeTargetResolved, metav1.ConditionTrue, ReasonTargetFound, fmt.Sprintf("%s %s resolved", rt.Spec.TargetRef.Kind, rt.Spec.TargetRef.Name)) // 4.1. Complete two-phase Sandbox restart if pending. @@ -348,7 +381,8 @@ func (r *AgentRuntimeReconciler) resolveTargetRef(ctx context.Context, rt *agent key := client.ObjectKey{Namespace: rt.Namespace, Name: ref.Name} if err := r.Get(ctx, key, acc.obj); err != nil { if apierrors.IsNotFound(err) { - return fmt.Errorf("%s/%s %s not found in namespace %s", ref.APIVersion, ref.Kind, ref.Name, rt.Namespace) + return fmt.Errorf("%s/%s %s not found in namespace %s: %w", + ref.APIVersion, ref.Kind, ref.Name, rt.Namespace, errTargetNotFound) } return err } @@ -889,6 +923,29 @@ func (r *AgentRuntimeReconciler) updateErrorStatus(ctx context.Context, key type } } +// setDegradedTargetNotFound marks the AgentRuntime as degraded because its +// target workload is missing: Ready=False and TargetResolved=False, both with +// reason "TargetNotFound". It also clears status.Card, which was discovered +// from the now-absent target and is therefore stale. Other conditions are +// preserved. Recovery is automatic once the target reappears (see +// SetupWithManager workload watches). +func (r *AgentRuntimeReconciler) setDegradedTargetNotFound(ctx context.Context, key types.NamespacedName, message string) { + logger := log.FromContext(ctx) + if statusErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { + latest := &agentv1alpha1.AgentRuntime{} + if err := r.Get(ctx, key, latest); err != nil { + return err + } + r.setCondition(latest, ConditionTypeTargetResolved, metav1.ConditionFalse, ReasonTargetNotFound, message) + r.setCondition(latest, ConditionTypeReady, metav1.ConditionFalse, ReasonTargetNotFound, message) + // The card was discovered from the now-absent target workload; clear it. + latest.Status.Card = nil + return r.Status().Update(ctx, latest) + }); statusErr != nil { + logger.Error(statusErr, "Failed to update degraded status", "reason", ReasonTargetNotFound) + } +} + // fetchAndUpdateCard discovers the agent card from the workload's Service endpoint // and populates status.card. Skips fetch when the feature flag is disabled or // when the workload's change-detection key has not changed. diff --git a/operator/internal/controller/agentruntime_controller_test.go b/operator/internal/controller/agentruntime_controller_test.go index 0554ef84..88ea7a84 100644 --- a/operator/internal/controller/agentruntime_controller_test.go +++ b/operator/internal/controller/agentruntime_controller_test.go @@ -19,17 +19,20 @@ package controller import ( "context" "fmt" + "strings" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/events" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -382,10 +385,23 @@ var _ = Describe("AgentRuntime Controller", func() { }) AfterEach(func() { + // The AgentRuntime carries the kagenti.io/cleanup finalizer, so a bare + // Delete only sets a DeletionTimestamp; drive reconciles until the + // deletion reconcile removes the finalizer and the object is gone, so + // specs in this context do not leak state into each other. + r := newReconciler() _ = k8sClient.Delete(ctx, rt) + Eventually(func() bool { + _, _ = r.Reconcile(ctx, reconcile.Request{ + NamespacedName: types.NamespacedName{Name: "rt-no-target", Namespace: namespace}, + }) + err := k8sClient.Get(ctx, types.NamespacedName{Name: "rt-no-target", Namespace: namespace}, + &agentv1alpha1.AgentRuntime{}) + return apierrors.IsNotFound(err) + }, "10s", "100ms").Should(BeTrue()) }) - It("should set TargetNotFound condition", func() { + It("should set TargetNotFound condition and Ready=False", func() { r := newReconciler() // First reconcile: adds finalizer @@ -402,16 +418,103 @@ var _ = Describe("AgentRuntime Controller", func() { updated := &agentv1alpha1.AgentRuntime{} Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "rt-no-target", Namespace: namespace}, updated)).To(Succeed()) - var targetCond *metav1.Condition - for i := range updated.Status.Conditions { - if updated.Status.Conditions[i].Type == ConditionTypeTargetResolved { - targetCond = &updated.Status.Conditions[i] - break - } - } + targetCond := meta.FindStatusCondition(updated.Status.Conditions, ConditionTypeTargetResolved) Expect(targetCond).NotTo(BeNil()) Expect(targetCond.Status).To(Equal(metav1.ConditionFalse)) Expect(targetCond.Reason).To(Equal("TargetNotFound")) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, ConditionTypeReady) + Expect(readyCond).NotTo(BeNil()) + Expect(readyCond.Status).To(Equal(metav1.ConditionFalse)) + Expect(readyCond.Reason).To(Equal("TargetNotFound")) + }) + + It("should emit the TargetNotFound Warning event at most once across cycles", func() { + fakeRecorder := events.NewFakeRecorder(10) + r := &AgentRuntimeReconciler{ + Client: k8sClient, + APIReader: k8sClient, + Scheme: scheme.Scheme, + Recorder: fakeRecorder, + } + nn := types.NamespacedName{Name: "rt-no-target", Namespace: namespace} + + // First reconcile: adds finalizer (no target resolution yet) + _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + // Second reconcile: transitions into degraded -> should emit one event + _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + // Third reconcile: already degraded -> should NOT emit another event + _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + + // Drain the channel and count TargetNotFound Warning events. + count := 0 + for len(fakeRecorder.Events) > 0 { + evt := <-fakeRecorder.Events + if strings.Contains(evt, "TargetNotFound") { + count++ + } + } + Expect(count).To(Equal(1), "TargetNotFound event should be emitted only on transition") + }) + + It("should recover to Ready when the target is created", func() { + r := newReconciler() + nn := types.NamespacedName{Name: "rt-no-target", Namespace: namespace} + + // Drive into degraded state. + _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + + // Create the missing target, then reconcile again. + dep := newDeployment("nonexistent-deploy", namespace) + Expect(k8sClient.Create(ctx, dep)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, dep) }() + + result, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(BeZero(), "healthy reconcile should not requeue") + + updated := &agentv1alpha1.AgentRuntime{} + Expect(k8sClient.Get(ctx, nn, updated)).To(Succeed()) + + targetCond := meta.FindStatusCondition(updated.Status.Conditions, ConditionTypeTargetResolved) + Expect(targetCond).NotTo(BeNil()) + Expect(targetCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(targetCond.Reason).To(Equal("TargetFound")) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, ConditionTypeReady) + Expect(readyCond).NotTo(BeNil()) + Expect(readyCond.Status).To(Equal(metav1.ConditionTrue)) + Expect(readyCond.Reason).To(Equal("Configured")) + }) + + It("should clear stale status.Card when the target is missing", func() { + r := newReconciler() + nn := types.NamespacedName{Name: "rt-no-target", Namespace: namespace} + + // First reconcile: adds finalizer. + _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + + // Simulate a card left over from when the target existed. + seed := &agentv1alpha1.AgentRuntime{} + Expect(k8sClient.Get(ctx, nn, seed)).To(Succeed()) + seed.Status.Card = &agentv1alpha1.CardStatus{ + AgentCardData: agentv1alpha1.AgentCardData{Name: "stale-agent"}, + CardHash: "sha256:deadbeef", + } + Expect(k8sClient.Status().Update(ctx, seed)).To(Succeed()) + + // Second reconcile: target still missing -> degraded path runs. + _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + + updated := &agentv1alpha1.AgentRuntime{} + Expect(k8sClient.Get(ctx, nn, updated)).To(Succeed()) + Expect(updated.Status.Card).To(BeNil()) + + readyCond := meta.FindStatusCondition(updated.Status.Conditions, ConditionTypeReady) + Expect(readyCond).NotTo(BeNil()) + Expect(readyCond.Status).To(Equal(metav1.ConditionFalse)) + Expect(readyCond.Reason).To(Equal("TargetNotFound")) }) })