From 2935f0859e80310fa01f949ac7b408754cf1ad95 Mon Sep 17 00:00:00 2001 From: Derek Frank Date: Wed, 8 Jul 2026 17:25:29 +0000 Subject: [PATCH] fix: don't fabricate a Ready condition for generic objects in metrics GenericObjectController wraps objects in an UnstructuredAdapter whose StatusConditions() called NewReadyConditions(...).For(), and For() initializes any missing root/dependent condition to Unknown. For an object that does not define a Ready condition, this fabricated a Ready=Unknown that was then emitted as a permanent operator_status_condition_*{type="Ready", status="Unknown"} metric, even though nothing is persisted to the object. Add a WithObservedOnly ForOption that builds the ConditionSet from the conditions already present without initializing absent ones, and use it from the UnstructuredAdapter (a read-only reflection used only for metrics). Owned objects driving reconcile are unaffected. --- status/condition_set.go | 15 +++++++++++++++ status/controller_test.go | 24 ++++++++++++++++++++++++ status/unstructured_adapter.go | 6 +++++- 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/status/condition_set.go b/status/condition_set.go index 330bd4d..237e45a 100644 --- a/status/condition_set.go +++ b/status/condition_set.go @@ -54,6 +54,9 @@ type ConditionSet struct { // ForOptions configures a ConditionSet. type ForOptions struct { Clock clock.Clock + // ObservedOnly builds the set from conditions already present, without + // initializing absent root/dependent conditions to Unknown. See WithObservedOnly. + ObservedOnly bool } // ForOption is a functional option for For. @@ -65,6 +68,15 @@ func WithClock(c clock.Clock) ForOption { return func(o *ForOptions) { o.Clock = c } } +// WithObservedOnly builds the ConditionSet from the conditions already present on +// the object, skipping initialization of missing root/dependent conditions to +// Unknown. Use this when the set is a read-only reflection of observed state (e.g. +// metrics emission for an object the caller does not own), so absent conditions are +// not fabricated. +func WithObservedOnly() ForOption { + return func(o *ForOptions) { o.ObservedOnly = true } +} + // For creates a ConditionSet from an object using the original // ConditionTypes as a reference. Status must be a pointer to a struct. func (r ConditionTypes) For(object Object, opts ...ForOption) ConditionSet { @@ -73,6 +85,9 @@ func (r ConditionTypes) For(object Object, opts ...ForOption) ConditionSet { opt(&o) } cs := ConditionSet{object: object, ConditionTypes: r, clock: o.Clock} + if o.ObservedOnly { + return cs + } // Set known conditions Unknown if not set. // Set the root condition first to get consistent timing for LastTransitionTime for _, t := range append([]string{r.root}, r.dependents...) { diff --git a/status/controller_test.go b/status/controller_test.go index ea49528..1a15d87 100644 --- a/status/controller_test.go +++ b/status/controller_test.go @@ -1162,6 +1162,30 @@ var _ = Describe("Generic Controller", func() { Expect(GetMetric("operator_testgenericobject_status_condition_current_status_seconds", conditionLabels(ConditionTypeBar, metav1.ConditionFalse))).To(BeNil()) Expect(GetMetric("operator_testgenericobject_status_condition_current_status_seconds", conditionLabels(ConditionTypeBar, metav1.ConditionUnknown))).To(BeNil()) }) + It("should not emit a synthetic Ready condition metric for objects without one", func() { + testObject := test.Object(&TestGenericObject{}) + gvk := object.GVK(testObject) + testObject.Status = TestGenericStatus{ + Conditions: []metav1.Condition{ + {Type: ConditionTypeFoo, Status: metav1.ConditionTrue}, + }, + } + + ExpectApplied(ctx, kubeClient, testObject) + ExpectReconciled(ctx, genericController, testObject) + + // The observed Foo condition is emitted. + Expect(GetMetric("operator_status_condition_count", conditionLabelsWithGroupKind(gvk, ConditionTypeFoo, metav1.ConditionTrue)).GetGauge().GetValue()).To(BeEquivalentTo(1)) + + // No Ready condition is present on the object, so the generic controller must not + // fabricate a Ready=Unknown metric for it, across either metric family. + Expect(GetMetric("operator_status_condition_count", conditionLabelsWithGroupKind(gvk, status.ConditionReady, metav1.ConditionTrue))).To(BeNil()) + Expect(GetMetric("operator_status_condition_count", conditionLabelsWithGroupKind(gvk, status.ConditionReady, metav1.ConditionFalse))).To(BeNil()) + Expect(GetMetric("operator_status_condition_count", conditionLabelsWithGroupKind(gvk, status.ConditionReady, metav1.ConditionUnknown))).To(BeNil()) + Expect(GetMetric("operator_status_condition_current_status_seconds", conditionLabelsWithGroupKind(gvk, status.ConditionReady, metav1.ConditionUnknown))).To(BeNil()) + Expect(GetMetric("operator_testgenericobject_status_condition_count", conditionLabels(status.ConditionReady, metav1.ConditionUnknown))).To(BeNil()) + Expect(GetMetric("operator_testgenericobject_status_condition_current_status_seconds", conditionLabels(status.ConditionReady, metav1.ConditionUnknown))).To(BeNil()) + }) It("should emit transition total metrics for abnormal conditions", func() { testObject := test.Object(&TestGenericObject{}) gvk := object.GVK(testObject) diff --git a/status/unstructured_adapter.go b/status/unstructured_adapter.go index 76620e5..69e7d92 100644 --- a/status/unstructured_adapter.go +++ b/status/unstructured_adapter.go @@ -87,5 +87,9 @@ func (u *UnstructuredAdapter[T]) StatusConditions(opts ...ForOption) ConditionSe conditionTypes := lo.Map(u.GetConditions(), func(condition Condition, _ int) string { return condition.Type }) - return NewReadyConditions(conditionTypes...).For(u, opts...) + // The adapter is a read-only reflection of an object the controller does not + // own, used to emit metrics for whatever conditions are present. Build the set + // from observed conditions only so we don't fabricate a root "Ready" condition + // (and a phantom Ready=Unknown metric) for objects that never define one. + return NewReadyConditions(conditionTypes...).For(u, append(opts, WithObservedOnly())...) }