From cb251306ceb4adfd4d6a4c2e356da714c3ee8a7d Mon Sep 17 00:00:00 2001 From: OsoriAndOmori Date: Mon, 10 Aug 2026 11:52:59 +0900 Subject: [PATCH] fix: populate involvedObject name/namespace on Finalized events for already-deleted objects cleanupNotFound records a "Finalized" event for each finalizer that was removed, but the object passed in was only obtained via object.New[T]() and was never populated by Get() (which returned NotFound). As a result the event's involvedObject.Name was always empty, which downstream log processors (e.g. Grafana Alloy's loki.source.kubernetes_events) reject with "no involved object for event". req.Name/req.Namespace are already known at this point, so set them on o before it's used to record the event. --- status/controller.go | 7 +++++ status/controller_test.go | 61 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/status/controller.go b/status/controller.go index c30198e..984b24d 100644 --- a/status/controller.go +++ b/status/controller.go @@ -246,6 +246,13 @@ func (c *Controller[T]) reconcile(ctx context.Context, req reconcile.Request, o } func (c *Controller[T]) cleanupNotFound(req reconcile.Request, o Object) error { + // o was never populated by a successful Get (the object is already gone), + // so it still carries the zero-value Name/Namespace from object.New[T](). + // Fill them in from req so any event recorded against o below has a valid + // involvedObject reference instead of an empty Name. + o.SetName(req.Name) + o.SetNamespace(req.Namespace) + c.observedConditions.Delete(req) c.observedGaugeLabels.Delete(req) c.deletePartialMatchGaugeMetric(c.ConditionCount, ConditionCount, map[string]string{ diff --git a/status/controller_test.go b/status/controller_test.go index 1a15d87..902ee16 100644 --- a/status/controller_test.go +++ b/status/controller_test.go @@ -27,10 +27,37 @@ import ( ) var ctx context.Context -var recorder *record.FakeRecorder +var recorder *capturingRecorder var kubeClient client.Client var registry = metrics.Registry +// capturingRecorder wraps a FakeRecorder but also captures the involved +// object for each recorded event, keyed by reason. This lets tests assert on +// the object identity (e.g. Name) that events were recorded against, which +// FakeRecorder alone discards. +type capturingRecorder struct { + *record.FakeRecorder + mu sync.Mutex + objects map[string][]runtime.Object +} + +func newCapturingRecorder() *capturingRecorder { + return &capturingRecorder{FakeRecorder: record.NewFakeRecorder(10), objects: map[string][]runtime.Object{}} +} + +func (r *capturingRecorder) Event(object runtime.Object, eventtype, reason, message string) { + r.mu.Lock() + r.objects[reason] = append(r.objects[reason], object) + r.mu.Unlock() + r.FakeRecorder.Event(object, eventtype, reason, message) +} + +func (r *capturingRecorder) objectsFor(reason string) []runtime.Object { + r.mu.Lock() + defer r.mu.Unlock() + return r.objects[reason] +} + // spyClient wraps a client.Client and counts how many Get calls are made with // a runtime.Unstructured object, which would bypass the controller-runtime cache. type spyClient struct { @@ -65,11 +92,11 @@ var _ = AfterEach(func() { var _ = Describe("Controller", func() { var ctx context.Context - var recorder *record.FakeRecorder + var recorder *capturingRecorder var controller *status.Controller[*test.CustomObject] var kubeClient client.Client BeforeEach(func() { - recorder = record.NewFakeRecorder(10) + recorder = newCapturingRecorder() kubeClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).WithStatusSubresource(&test.CustomObject{}).Build() ctx = log.IntoContext(context.Background(), GinkgoLogr) controller = status.NewController[*test.CustomObject](kubeClient, recorder, status.EmitDeprecatedMetrics) @@ -110,6 +137,19 @@ var _ = Describe("Controller", func() { testObject.SetFinalizers([]string{}) Expect(client.IgnoreNotFound(kubeClient.Patch(ctx, testObject, mergeFrom))).To(Succeed()) ExpectReconciled(ctx, controller, testObject) + + // The object no longer exists by the time the finalizer removal is + // observed, so the event must still carry the object's identity + // rather than an empty involvedObject name. + finalizedEvents := recorder.objectsFor("Finalized") + Expect(finalizedEvents).ToNot(BeEmpty()) + for _, obj := range finalizedEvents { + co, ok := obj.(client.Object) + Expect(ok).To(BeTrue()) + Expect(co.GetName()).To(Equal(testObject.Name)) + Expect(co.GetNamespace()).To(Equal(testObject.Namespace)) + } + Expect(GetMetric("operator_termination_current_time_seconds", map[string]string{status.MetricLabelName: testObject.Name})).To(BeNil()) Expect(GetMetric("operator_customobject_termination_current_time_seconds", map[string]string{status.MetricLabelName: testObject.Name})).To(BeNil()) metric = GetMetric("operator_termination_duration_seconds", map[string]string{}) @@ -832,7 +872,7 @@ var _ = Describe("Generic Controller", func() { var genericController *status.GenericObjectController[*TestGenericObject] var spy *spyClient BeforeEach(func() { - recorder = record.NewFakeRecorder(10) + recorder = newCapturingRecorder() spy = &spyClient{Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()} kubeClient = spy ctx = log.IntoContext(context.Background(), GinkgoLogr) @@ -874,6 +914,19 @@ var _ = Describe("Generic Controller", func() { testObject.SetFinalizers([]string{}) Expect(client.IgnoreNotFound(kubeClient.Patch(ctx, testObject, mergeFrom))).To(Succeed()) ExpectReconciled(ctx, genericController, testObject) + + // The object no longer exists by the time the finalizer removal is + // observed, so the event must still carry the object's identity + // rather than an empty involvedObject name. + finalizedEvents := recorder.objectsFor("Finalized") + Expect(finalizedEvents).ToNot(BeEmpty()) + for _, obj := range finalizedEvents { + co, ok := obj.(client.Object) + Expect(ok).To(BeTrue()) + Expect(co.GetName()).To(Equal(testObject.Name)) + Expect(co.GetNamespace()).To(Equal(testObject.Namespace)) + } + Expect(GetMetric("operator_termination_current_time_seconds", map[string]string{status.MetricLabelName: testObject.Name})).To(BeNil()) Expect(GetMetric("operator_testgenericobject_termination_current_time_seconds", map[string]string{status.MetricLabelName: testObject.Name})).To(BeNil()) metric = GetMetric("operator_termination_duration_seconds", map[string]string{})