diff --git a/kagenti-operator/GETTING_STARTED.md b/kagenti-operator/GETTING_STARTED.md index 96ce99c6..c495f856 100644 --- a/kagenti-operator/GETTING_STARTED.md +++ b/kagenti-operator/GETTING_STARTED.md @@ -170,8 +170,8 @@ spec: ### Deleting an AgentRuntime When you delete the AgentRuntime CR, the controller performs a graceful cleanup: -- Preserves the `kagenti.io/type` label (so AgentCard discovery continues) -- Updates the config hash to defaults-only (triggers a rollback to default sidecar configuration) +- Removes the `kagenti.io/type` label from the workload metadata and PodTemplateSpec +- Removes the `kagenti.io/config-hash` annotation from the PodTemplateSpec (triggers a rolling update so existing injected pods are replaced) - Removes the `app.kubernetes.io/managed-by` label ```bash diff --git a/kagenti-operator/docs/architecture.md b/kagenti-operator/docs/architecture.md index 4b3096ad..2e646deb 100644 --- a/kagenti-operator/docs/architecture.md +++ b/kagenti-operator/docs/architecture.md @@ -72,7 +72,7 @@ The Kagenti Operator is a Kubernetes controller that implements the [Operator Pa - Computes config hash from 3-layer merged configuration (cluster defaults → namespace defaults → CR overrides) - Discovers linked skills by reading the `kagenti.io/skills` annotation from target workloads when the `skillDiscovery` feature gate is enabled - Triggers rolling updates when configuration changes -- On CR deletion: preserves type label, updates config-hash to defaults-only, removes managed-by label +- On CR deletion: removes type label, managed-by label and config-hash annotation (causing the workload to lose sidecars) - Coordinates with the AuthBridge mutating webhook (in-process) which injects sidecars at Pod CREATE time ### Supporting Components @@ -194,8 +194,8 @@ The AgentRuntime Controller reconciles AgentRuntime CRs by resolving the target ``` 1. Fetch AgentRuntime CR 2. Handle deletion (if marked for deletion): - a. Preserve kagenti.io/type label on workload - b. Update config-hash to defaults-only (triggers rollback) + a. Remove kagenti.io/type label from workload metadata and PodTemplateSpec + b. Remove kagenti.io/config-hash annotation from PodTemplateSpec (triggers rolling update) c. Remove managed-by label d. Remove finalizer 3. Ensure kagenti.io/cleanup finalizer is present diff --git a/kagenti-operator/docs/controller-webhook-interaction.md b/kagenti-operator/docs/controller-webhook-interaction.md index db0f9542..519c04db 100644 --- a/kagenti-operator/docs/controller-webhook-interaction.md +++ b/kagenti-operator/docs/controller-webhook-interaction.md @@ -116,9 +116,9 @@ sequenceDiagram Note over Ctrl: Finalizer kagenti.io/cleanup is present - Ctrl->>API: Patch Deployment:
- Preserve kagenti.io/type label
- Update config-hash to defaults-only
- Remove managed-by label + Ctrl->>API: Patch Deployment:
- Remove kagenti.io/type label
- Remove kagenti.io/config-hash annotation
- Remove managed-by label API-->>K8s: PodTemplateSpec changed → rolling update - Note over K8s: New Pods get sidecars with default config only + Note over K8s: New Pods lack the type label — webhook skips injection Ctrl->>API: Remove finalizer from AgentRuntime CR API->>API: CR garbage collected diff --git a/kagenti-operator/internal/controller/agentruntime_controller.go b/kagenti-operator/internal/controller/agentruntime_controller.go index 27a146e8..19b5b62c 100644 --- a/kagenti-operator/internal/controller/agentruntime_controller.go +++ b/kagenti-operator/internal/controller/agentruntime_controller.go @@ -656,7 +656,9 @@ func isPodOwnedByWorkload(pod *corev1.Pod, workloadName string) bool { } // handleDeletion runs finalizer logic when an AgentRuntime is deleted. -// It preserves the kagenti.io/type label and updates the config-hash to defaults-only. +// It removes the kagenti.io/type label and kagenti.io/config-hash annotation so that +// the next rolling update creates pods without sidecars, returning the workload to its +// pre-AR state. func (r *AgentRuntimeReconciler) handleDeletion(ctx context.Context, rt *agentv1alpha1.AgentRuntime) (ctrl.Result, error) { logger := log.FromContext(ctx) @@ -669,12 +671,6 @@ func (r *AgentRuntimeReconciler) handleDeletion(ctx context.Context, rt *agentv1 ref := rt.Spec.TargetRef acc, ok := newRuntimePodTemplateAccessor(ref.Kind) if ok { - defaultsHash, err := ComputeDefaultsOnlyHash(ctx, r.Client, rt.Namespace) - if err != nil { - logger.V(1).Info("Failed to compute defaults-only hash, using empty", "error", err) - defaultsHash = "" - } - key := types.NamespacedName{Name: ref.Name, Namespace: rt.Namespace} updateErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { if err := r.Get(ctx, key, acc.obj); err != nil { @@ -684,21 +680,31 @@ func (r *AgentRuntimeReconciler) handleDeletion(ctx context.Context, rt *agentv1 return err } - // Preserve kagenti.io/type label (workload stays classified) - // Update config-hash to defaults-only - podAnnotations := acc.getPodAnnotations(acc.obj) - if podAnnotations == nil { - podAnnotations = make(map[string]string) - } - podAnnotations[AnnotationConfigHash] = defaultsHash - acc.setPodAnnotations(acc.obj, podAnnotations) - - // Remove managed-by label from workload metadata + // Remove kagenti.io/type and kagenti.io/managed-by from workload metadata. workloadLabels := acc.obj.GetLabels() + delete(workloadLabels, LabelAgentType) delete(workloadLabels, LabelManagedBy) acc.obj.SetLabels(workloadLabels) - logger.Info("Updated workload to defaults-only config on AgentRuntime deletion", + // Remove skills annotation from workload metadata. + workloadAnnotations := acc.obj.GetAnnotations() + delete(workloadAnnotations, AnnotationSkills) + acc.obj.SetAnnotations(workloadAnnotations) + + // Remove kagenti.io/type from PodTemplateSpec pod labels so future pods + // are not presented to the webhook with the type label. + podLabels := acc.getPodLabels(acc.obj) + delete(podLabels, LabelAgentType) + acc.setPodLabels(acc.obj, podLabels) + + // Remove kagenti.io/config-hash from PodTemplateSpec pod annotations. + // This triggers the rolling update that replaces existing injected pods, + // and leaves the workload annotation-clean for any future AR. + podAnnotations := acc.getPodAnnotations(acc.obj) + delete(podAnnotations, AnnotationConfigHash) + acc.setPodAnnotations(acc.obj, podAnnotations) + + logger.Info("Removed kagenti labels and config-hash from workload on AgentRuntime deletion", "workload", ref.Name, "kind", ref.Kind) return r.Update(ctx, acc.obj) }) diff --git a/kagenti-operator/internal/controller/agentruntime_controller_test.go b/kagenti-operator/internal/controller/agentruntime_controller_test.go index 66f8b7ae..3ae6c0aa 100644 --- a/kagenti-operator/internal/controller/agentruntime_controller_test.go +++ b/kagenti-operator/internal/controller/agentruntime_controller_test.go @@ -378,7 +378,7 @@ var _ = Describe("AgentRuntime Controller", func() { _ = k8sClient.Delete(ctx, dep) }) - It("should preserve type label, remove managed-by, and update config-hash on deletion", func() { + It("should remove type label and config-hash, and remove managed-by on deletion", func() { r := newReconciler() // Reconcile to add finalizer + apply config @@ -389,11 +389,12 @@ var _ = Describe("AgentRuntime Controller", func() { NamespacedName: types.NamespacedName{Name: "del-rt", Namespace: namespace}, }) - // Get hash before deletion + // Confirm labels and hash are set before deletion depBefore := &appsv1.Deployment{} Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "del-deploy", Namespace: namespace}, depBefore)).To(Succeed()) - hashBefore := depBefore.Spec.Template.Annotations[AnnotationConfigHash] - Expect(hashBefore).NotTo(BeEmpty()) + Expect(depBefore.Spec.Template.Annotations[AnnotationConfigHash]).NotTo(BeEmpty()) + Expect(depBefore.Labels[LabelAgentType]).NotTo(BeEmpty()) + Expect(depBefore.Spec.Template.Labels[LabelAgentType]).NotTo(BeEmpty()) // Delete the AgentRuntime Expect(k8sClient.Delete(ctx, rt)).To(Succeed()) @@ -408,16 +409,15 @@ var _ = Describe("AgentRuntime Controller", func() { depAfter := &appsv1.Deployment{} Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "del-deploy", Namespace: namespace}, depAfter)).To(Succeed()) - // Type label preserved - Expect(depAfter.Labels[LabelAgentType]).To(Equal("agent")) - Expect(depAfter.Spec.Template.Labels[LabelAgentType]).To(Equal("agent")) + // kagenti.io/type removed from both workload metadata and PodTemplateSpec + Expect(depAfter.Labels).NotTo(HaveKey(LabelAgentType)) + Expect(depAfter.Spec.Template.Labels).NotTo(HaveKey(LabelAgentType)) - // Managed-by removed + // kagenti.io/managed-by removed Expect(depAfter.Labels).NotTo(HaveKey(LabelManagedBy)) - // Config-hash updated to defaults-only (different from before) - hashAfter := depAfter.Spec.Template.Annotations[AnnotationConfigHash] - Expect(hashAfter).NotTo(Equal(hashBefore), "config-hash should change to defaults-only on deletion") + // kagenti.io/config-hash removed from PodTemplateSpec + Expect(depAfter.Spec.Template.Annotations).NotTo(HaveKey(AnnotationConfigHash)) // Finalizer removed — AgentRuntime should be gone deletedRT := &agentv1alpha1.AgentRuntime{} diff --git a/kagenti-operator/test/e2e/e2e_test.go b/kagenti-operator/test/e2e/e2e_test.go index 7e380e0c..7bdb93d0 100644 --- a/kagenti-operator/test/e2e/e2e_test.go +++ b/kagenti-operator/test/e2e/e2e_test.go @@ -1117,8 +1117,6 @@ rules: SetDefaultEventuallyPollingInterval(time.Second) Context("Agent lifecycle", Ordered, func() { - var initialConfigHash string - It("should apply labels and config-hash to target Deployment", func() { By("deploying the agent target workload") _, err := utils.KubectlApplyStdin(runtimeTargetDeploymentFixture(), agentRuntimeTestNamespace) @@ -1162,7 +1160,6 @@ rules: g.Expect(err).NotTo(HaveOccurred()) g.Expect(hash).NotTo(BeEmpty()) g.Expect(hash).To(HaveLen(64)) - initialConfigHash = hash }).Should(Succeed()) By("verifying AgentCard is auto-created by AgentCardSync") @@ -1224,9 +1221,6 @@ rules: }, 30*time.Second, 5*time.Second).Should(Succeed()) }) - // Note: the AgentCard auto-created by AgentCardSync (runtime-agent-target-deployment-card) - // persists after AgentRuntime deletion because kagenti.io/type=agent is preserved on the - // Deployment and AgentCardSync owns the card independently of the AgentRuntime lifecycle. It("should clean up on deletion", func() { By("deleting the AgentRuntime CR") cmd := exec.Command("kubectl", "delete", "agentruntime", "test-agent-runtime", @@ -1248,12 +1242,20 @@ rules: _, err = utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) - By("verifying kagenti.io/type label is preserved") + By("verifying kagenti.io/type label is removed from workload metadata") Eventually(func(g Gomega) { typeLabel, err := utils.KubectlGetJsonpath("deployment", "runtime-agent-target", agentRuntimeTestNamespace, "{.metadata.labels['kagenti\\.io/type']}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(typeLabel).To(Equal("agent")) + g.Expect(typeLabel).To(BeEmpty()) + }).Should(Succeed()) + + By("verifying kagenti.io/type label is removed from PodTemplateSpec") + Eventually(func(g Gomega) { + typeLabel, err := utils.KubectlGetJsonpath("deployment", "runtime-agent-target", + agentRuntimeTestNamespace, "{.spec.template.metadata.labels['kagenti\\.io/type']}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(typeLabel).To(BeEmpty()) }).Should(Succeed()) By("verifying managed-by label is removed") @@ -1264,15 +1266,13 @@ rules: g.Expect(managedBy).To(BeEmpty()) }).Should(Succeed()) - By("verifying config-hash changed to defaults-only hash") + By("verifying config-hash annotation is removed from PodTemplateSpec") Eventually(func(g Gomega) { hash, err := utils.KubectlGetJsonpath("deployment", "runtime-agent-target", agentRuntimeTestNamespace, "{.spec.template.metadata.annotations['kagenti\\.io/config-hash']}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(hash).NotTo(BeEmpty()) - g.Expect(hash).To(HaveLen(64)) - g.Expect(hash).NotTo(Equal(initialConfigHash)) + g.Expect(hash).To(BeEmpty()) }).Should(Succeed()) }) }) @@ -1476,7 +1476,6 @@ var _ = Describe("Combined AgentRuntime + AgentCard + Auth Bridge E2E", Ordered, const controllerDeployment = "kagenti-operator-controller-manager" var origArgs []string - var initialConfigHash string BeforeAll(func() { By("ensuring mlflow-operator ClusterRole exists for ServiceAccount informer") @@ -1704,7 +1703,6 @@ rules: "{.spec.template.metadata.annotations['kagenti\\.io/config-hash']}") g.Expect(err).NotTo(HaveOccurred()) g.Expect(hash).To(HaveLen(64)) - initialConfigHash = hash }).Should(Succeed()) }) @@ -1840,7 +1838,7 @@ rules: }).Should(Succeed()) }) - It("should clean up on AgentRuntime deletion and maintain injection", func() { + It("should clean up on AgentRuntime deletion and stop injection", func() { By("deleting the AgentRuntime CR") cmd := exec.Command("kubectl", "delete", "agentruntime", "combined-agent", "-n", combinedTestNamespace) @@ -1861,12 +1859,20 @@ rules: _, err = utils.Run(cmd) Expect(err).NotTo(HaveOccurred()) - By("verifying kagenti.io/type=agent label preserved") + By("verifying kagenti.io/type label removed from workload metadata") Eventually(func(g Gomega) { typeLabel, err := utils.KubectlGetJsonpath("deployment", "combined-agent", combinedTestNamespace, "{.metadata.labels['kagenti\\.io/type']}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(typeLabel).To(Equal("agent")) + g.Expect(typeLabel).To(BeEmpty()) + }).Should(Succeed()) + + By("verifying kagenti.io/type label removed from PodTemplateSpec") + Eventually(func(g Gomega) { + typeLabel, err := utils.KubectlGetJsonpath("deployment", "combined-agent", + combinedTestNamespace, "{.spec.template.metadata.labels['kagenti\\.io/type']}") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(typeLabel).To(BeEmpty()) }).Should(Succeed()) By("verifying managed-by label removed") @@ -1877,14 +1883,13 @@ rules: g.Expect(managedBy).To(BeEmpty()) }).Should(Succeed()) - By("verifying config-hash changed from initial") + By("verifying config-hash annotation removed from PodTemplateSpec") Eventually(func(g Gomega) { hash, err := utils.KubectlGetJsonpath("deployment", "combined-agent", combinedTestNamespace, "{.spec.template.metadata.annotations['kagenti\\.io/config-hash']}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(hash).To(HaveLen(64)) - g.Expect(hash).NotTo(Equal(initialConfigHash)) + g.Expect(hash).To(BeEmpty()) }).Should(Succeed()) By("verifying AgentCard still exists") @@ -1896,58 +1901,27 @@ rules: g.Expect(name).To(Equal(cardName)) }).Should(Succeed()) - By("getting current pod name") - var oldPodName string - Eventually(func(g Gomega) { - cmd := exec.Command("kubectl", "get", "pods", - "-l", "app.kubernetes.io/name=combined-agent", - "-n", combinedTestNamespace, - "-o", "jsonpath={.items[0].metadata.name}") - output, err := utils.Run(cmd) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(output).NotTo(BeEmpty()) - oldPodName = output - }).Should(Succeed()) - - By("deleting pod to verify re-injection") - cmd = exec.Command("kubectl", "delete", "pod", oldPodName, "-n", combinedTestNamespace) - _, err = utils.Run(cmd) - Expect(err).NotTo(HaveOccurred()) - - By("waiting for replacement pod with sidecars") - Eventually(func(g Gomega) { - cmd := exec.Command("kubectl", "get", "pods", - "-l", "app.kubernetes.io/name=combined-agent", - "-n", combinedTestNamespace, - "-o", "jsonpath={.items[0].metadata.name}") - output, err := utils.Run(cmd) - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(output).NotTo(BeEmpty()) - g.Expect(output).NotTo(Equal(oldPodName), "new pod should have a different name") - - phase, err := utils.KubectlGetJsonpath("pod", output, combinedTestNamespace, "{.status.phase}") - g.Expect(err).NotTo(HaveOccurred()) - g.Expect(phase).To(Equal("Running")) - }, 3*time.Minute, 2*time.Second).Should(Succeed()) + By("waiting for rolling update to complete after label removal") + Expect(utils.WaitForDeploymentReady("combined-agent", combinedTestNamespace, 3*time.Minute)).To(Succeed()) - By("verifying replacement pod has sidecars (spiffe-helper bundled in envoy-proxy)") + By("verifying replacement pods have no sidecars") Eventually(func(g Gomega) { containers, err := utils.KubectlGetJsonpath("pod", "", combinedTestNamespace, "{.items[?(@.metadata.labels.app\\.kubernetes\\.io/name=='combined-agent')].spec.containers[*].name}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(containers).To(ContainSubstring("envoy-proxy")) - g.Expect(containers).NotTo(ContainSubstring("spiffe-helper"), - "spiffe-helper is bundled inside envoy-proxy, not a separate container") + g.Expect(containers).NotTo(ContainSubstring("envoy-proxy"), + "envoy-proxy should not be injected after AR deletion") }).Should(Succeed()) - By("verifying replacement pod has proxy-init") + By("verifying replacement pods have no init containers") Eventually(func(g Gomega) { initContainers, err := utils.KubectlGetJsonpath("pod", "", combinedTestNamespace, "{.items[?(@.metadata.labels.app\\.kubernetes\\.io/name=='combined-agent')].spec.initContainers[*].name}") g.Expect(err).NotTo(HaveOccurred()) - g.Expect(initContainers).To(ContainSubstring("proxy-init")) + g.Expect(initContainers).NotTo(ContainSubstring("proxy-init"), + "proxy-init should not be injected after AR deletion") }).Should(Succeed()) }) }) diff --git a/kagenti-operator/test/e2e/fixtures.go b/kagenti-operator/test/e2e/fixtures.go index 9ce35247..2cd67eff 100644 --- a/kagenti-operator/test/e2e/fixtures.go +++ b/kagenti-operator/test/e2e/fixtures.go @@ -1137,7 +1137,6 @@ metadata: name: combined-agent namespace: ` + combinedTestNamespace + ` labels: - kagenti.io/type: agent protocol.kagenti.io/a2a: "" app.kubernetes.io/name: combined-agent spec: @@ -1145,12 +1144,10 @@ spec: selector: matchLabels: app.kubernetes.io/name: combined-agent - kagenti.io/type: agent template: metadata: labels: app.kubernetes.io/name: combined-agent - kagenti.io/type: agent protocol.kagenti.io/a2a: "" spec: serviceAccountName: combined-agent