From 2d76f3d74a81c1d1368a16fe3d28bc116e4770c0 Mon Sep 17 00:00:00 2001 From: Daniels Nagornuks Date: Wed, 27 May 2026 14:51:01 +0100 Subject: [PATCH 1/3] fix: remove kagenti.io/type label and config-hash on AgentRuntime deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the finalizer preserved kagenti.io/type on the target workload and replaced the config-hash with a defaults-only value, leaving the workload labelled after the AR was removed and sidecars still injecting with platform defaults. On AR deletion the controller now removes: - kagenti.io/type from workload metadata labels and PodTemplateSpec pod labels — the webhook's pre-filter requires this label, so future pods are no longer mutated - kagenti.io/config-hash from PodTemplateSpec pod annotations — its removal still triggers a rolling update so existing injected pods are replaced, and leaves the workload clean for any future AR The managed-by label, skills annotation, and skill volumes continue to be removed as before. Update docs/architecture.md, docs/controller-webhook-interaction.md, and GETTING_STARTED.md to reflect the new deletion behaviour. Signed-off-by: Daniels Nagornuks Assisted-By: Claude (Anthropic AI) --- kagenti-operator/GETTING_STARTED.md | 4 +- kagenti-operator/docs/architecture.md | 6 +-- .../docs/controller-webhook-interaction.md | 4 +- .../controller/agentruntime_controller.go | 42 +++++++++++-------- .../agentruntime_controller_test.go | 22 +++++----- 5 files changed, 42 insertions(+), 36 deletions(-) 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{} From e09bb18ee917b2f12924221923bc25accabbadd9 Mon Sep 17 00:00:00 2001 From: Daniels Nagornuks Date: Wed, 27 May 2026 15:58:14 +0100 Subject: [PATCH 2/3] test(e2e): update deletion assertions to reflect label and config-hash removal The e2e deletion tests were asserting the old behaviour where kagenti.io/type was preserved and config-hash was updated to a defaults-only value after AR deletion. Update both deletion tests (AgentRuntime context and combined context) to assert that: - kagenti.io/type is absent from workload metadata and PodTemplateSpec - kagenti.io/config-hash is absent from PodTemplateSpec Update the combined-agent deletion test to verify that the rolling update triggered by the PodTemplate change produces pods without sidecars, rather than pods with defaults-only sidecar config. Signed-off-by: Daniels Nagornuks Assisted-By: Claude (Anthropic AI) --- kagenti-operator/test/e2e/e2e_test.go | 92 ++++++++++----------------- 1 file changed, 33 insertions(+), 59 deletions(-) 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()) }) }) From 1efe0fddc34fcf7e554f753729eeb20184fde17f Mon Sep 17 00:00:00 2001 From: Daniels Nagornuks Date: Tue, 2 Jun 2026 12:35:05 +0100 Subject: [PATCH 3/3] Fix: remove kagenti.io/type from combined-agent selector to unblock deletion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The combined-agent e2e fixture had kagenti.io/type: agent in its spec.selector.matchLabels. Kubernetes rejects updates that remove a selector-required label from spec.template.metadata.labels (422 Unprocessable Entity). handleDeletion was failing on every attempt because RetryOnConflict only retries on 409 Conflict, not 422, causing the finalizer to never be cleared and kubectl delete to block until the 30-minute test timeout. Fix: remove kagenti.io/type from the fixture's selector and pod-template labels. The AgentRuntime controller owns that label lifecycle — it adds the label on AR creation and removes it on deletion. The fixture now represents a clean pre-AR deployment, which is the typical use case. Signed-off-by: Daniels Nagornuks Assisted-By: Claude (Anthropic AI) --- kagenti-operator/test/e2e/fixtures.go | 3 --- 1 file changed, 3 deletions(-) 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