Consider an absent condition to be Unknown

go.mod

@@ -17,7 +17,6 @@ require (
 	k8s.io/apimachinery v0.31.2
 	k8s.io/client-go v0.31.2
 	k8s.io/klog/v2 v2.130.1
-	k8s.io/kubernetes v1.31.3
 	k8s.io/utils v0.0.0-20240711033017-18e509b52bc8
 	sigs.k8s.io/controller-runtime v0.19.1
 	sigs.k8s.io/yaml v1.4.0

go.sum

@@ -190,8 +190,6 @@ k8s.io/klog/v2 v2.130.1 h1:n9Xl7H1Xvksem4KFG4PYbdQCQxqc/tTUyrgXaOhHSzk=
 k8s.io/klog/v2 v2.130.1/go.mod h1:3Jpz1GvMt720eyJH1ckRHK1EDfpxISzJ7I9OYgaDtPE=
 k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 h1:BZqlfIlq5YbRMFko6/PM7FjZpUb45WallggurYhKGag=
 k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340/go.mod h1:yD4MZYeKMBwQKVht279WycxKyM84kkAx2DPrTXaeb98=
-k8s.io/kubernetes v1.31.3 h1:oqb7HdfnTelrGlZ6ziNugvQ/L/aJWR704114EAhUn9Q=
-k8s.io/kubernetes v1.31.3/go.mod h1:9xmT2buyTYj8TRKwRae7FcuY8k5+xlxv7VivvO0KKfs=
 k8s.io/utils v0.0.0-20240711033017-18e509b52bc8 h1:pUdcCO1Lk/tbT5ztQWOBi5HBgbBP1J8+AsQnQCKsi8A=
 k8s.io/utils v0.0.0-20240711033017-18e509b52bc8/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
 sigs.k8s.io/controller-runtime v0.19.1 h1:Son+Q40+Be3QWb+niBXAg2vFiYWolDjjRfO8hn/cxOk=

object/object.go

@@ -10,8 +10,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/dump"
 	"k8s.io/client-go/kubernetes/scheme"
-	"k8s.io/kubernetes/pkg/util/hash"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/yaml"
@@ -56,6 +56,7 @@ func Unmarshal[T any](raw []byte) *T {
 
 func Hash(o any) string {
 	h := fnv.New64a()
-	hash.DeepHashObject(h, o)
-	return strconv.FormatUint(uint64(h.Sum64()), 10)
+	h.Reset()
+	fmt.Fprintf(h, "%v", dump.ForHash(o))
+	return strconv.FormatUint(h.Sum64(), 10)
 }

status/condition_set.go

@@ -54,9 +54,16 @@ type ConditionSet struct {
 func (r ConditionTypes) For(object Object) ConditionSet {
 	cs := ConditionSet{object: object, ConditionTypes: r}
 	// Set known conditions Unknown if not set.
-	for _, t := range append(r.dependents, r.root) {
+	// Set the root condition first to get consistent timing for LastTransitionTime
+	for _, t := range append([]string{r.root}, r.dependents...) {
 		if cs.Get(t) == nil {
-			cs.SetUnknown(t)
+			cs.Set(Condition{
+				Type:               t,
+				Status:             metav1.ConditionUnknown,
+				Reason:             "AwaitingReconciliation",
+				Message:            "object is awaiting reconciliation",
+				LastTransitionTime: object.GetCreationTimestamp(),
+			})
 		}
 	}
 	return cs
@@ -99,12 +106,22 @@ func (c ConditionSet) IsTrue(conditionTypes ...string) bool {
 	return true
 }
 
+func (c ConditionSet) IsNormalCondition(t string) bool {
+	if t == c.root || lo.Contains(c.dependents, t) {
+		return true
+	}
+	return false
+}
+
 // Set sets or updates the Condition on Conditions for Condition.Type.
 // If there is an update, Conditions are stored back sorted.
 func (c ConditionSet) Set(condition Condition) (modified bool) {
 	conditionType := condition.Type
 	var conditions []Condition
-	condition.LastTransitionTime = metav1.Now()
+	// If we get passed a LastTransitionTime, we should respect it
+	if condition.LastTransitionTime.IsZero() {
+		condition.LastTransitionTime = metav1.Now()
+	}
 	condition.ObservedGeneration = c.object.GetGeneration()
 	for _, cond := range c.object.GetConditions() {
 		if cond.Type != conditionType {
@@ -143,7 +160,7 @@ func (c ConditionSet) Clear(t string) error {
 		return nil
 	}
 	// Normal conditions are not handled as they can't be nil
-	if t == c.root || lo.Contains(c.dependents, t) {
+	if c.IsNormalCondition(t) {
 		return fmt.Errorf("clearing normal conditions not implemented")
 	}
 	cond := c.Get(t)

status/controller.go

@@ -225,17 +225,27 @@ func (c *Controller[T]) reconcile(ctx context.Context, req reconcile.Request, o
 			MetricLabelConditionStatus: string(condition.Status),
 			pmetrics.LabelReason:       condition.Reason,
 		})
+		var duration time.Duration
+		var status metav1.ConditionStatus
 		if observedCondition == nil {
-			continue
+			// We only treat the CreationTimestamp as the LastTransitionTime for normal conditions
+			// We can't know the LastTransitionTime if the condition is absent for abnormal conditions
+			if !o.StatusConditions().IsNormalCondition(condition.Type) {
+				continue
+			}
+			duration = condition.LastTransitionTime.Time.Sub(o.GetCreationTimestamp().Time)
+			status = metav1.ConditionUnknown
+		} else {
+			duration = condition.LastTransitionTime.Time.Sub(observedCondition.LastTransitionTime.Time)
+			status = observedCondition.Status
 		}
-		duration := condition.LastTransitionTime.Time.Sub(observedCondition.LastTransitionTime.Time).Seconds()
-		c.observeHistogram(c.ConditionDuration, ConditionDuration, duration, map[string]string{
-			pmetrics.LabelType:         observedCondition.Type,
-			MetricLabelConditionStatus: string(observedCondition.Status),
+		c.observeHistogram(c.ConditionDuration, ConditionDuration, duration.Seconds(), map[string]string{
+			pmetrics.LabelType:         condition.Type,
+			MetricLabelConditionStatus: string(status),
 		})
 		c.eventRecorder.Event(o, v1.EventTypeNormal, condition.Type, fmt.Sprintf("Status condition transitioned, Type: %s, Status: %s -> %s, Reason: %s%s",
 			condition.Type,
-			observedCondition.Status,
+			status,
 			condition.Status,
 			condition.Reason,
 			lo.Ternary(condition.Message != "", fmt.Sprintf(", Message: %s", condition.Message), ""),

status/controller_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/awslabs/operatorpkg/status"
 	"github.com/awslabs/operatorpkg/test"
 	. "github.com/awslabs/operatorpkg/test/expectations"
-	"github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -44,7 +43,7 @@ var _ = Describe("Controller", func() {
 	BeforeEach(func() {
 		recorder = record.NewFakeRecorder(10)
 		kubeClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
-		ctx = log.IntoContext(context.Background(), ginkgo.GinkgoLogr)
+		ctx = log.IntoContext(context.Background(), GinkgoLogr)
 		controller = status.NewController[*test.CustomObject](kubeClient, recorder, status.EmitDeprecatedMetrics)
 	})
 	AfterEach(func() {
@@ -517,14 +516,50 @@ var _ = Describe("Controller", func() {
 			}()
 		}
 	})
+	It("should set LastTransitionTime for status conditions on initialization to CreationTimestamp", func() {
+		testObject := test.Object(&test.CustomObject{})
+		testObject.StatusConditions() // initialize conditions after applying and setting CreationTimestamp
+
+		Expect(testObject.StatusConditions().Get(test.ConditionTypeFoo).LastTransitionTime.Time).To(Equal(testObject.GetCreationTimestamp().Time))
+		Expect(testObject.StatusConditions().Get(test.ConditionTypeBar).LastTransitionTime.Time).To(Equal(testObject.GetCreationTimestamp().Time))
+		Expect(testObject.StatusConditions().Get(status.ConditionReady).LastTransitionTime.Time).To(Equal(testObject.GetCreationTimestamp().Time))
+	})
+	It("should consider status conditions that aren't set as unknown", func() {
+		testObject := test.Object(&test.CustomObject{})
+		testObject.StatusConditions().SetTrue(test.ConditionTypeFoo) // initialize all conditions, set Foo to true
+
+		// conditions not set
+		ExpectApplied(ctx, kubeClient, testObject)
+		ExpectReconciled(ctx, controller, testObject)
+
+		Expect(GetMetric("operator_customobject_status_condition_transition_seconds", conditionLabels(status.ConditionReady, metav1.ConditionTrue))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transition_seconds", conditionLabels(status.ConditionReady, metav1.ConditionFalse))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transition_seconds", conditionLabels(status.ConditionReady, metav1.ConditionUnknown))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transition_seconds", conditionLabels(ConditionTypeFoo, metav1.ConditionTrue))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transition_seconds", conditionLabels(ConditionTypeFoo, metav1.ConditionFalse))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transition_seconds", conditionLabels(ConditionTypeFoo, metav1.ConditionUnknown)).GetHistogram().GetSampleCount()).To(BeNumerically(">", 0))
+		Expect(GetMetric("operator_customobject_status_condition_transition_seconds", conditionLabels(ConditionTypeBar, metav1.ConditionTrue))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transition_seconds", conditionLabels(ConditionTypeBar, metav1.ConditionFalse))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transition_seconds", conditionLabels(ConditionTypeBar, metav1.ConditionUnknown))).To(BeNil())
+
+		Expect(GetMetric("operator_customobject_status_condition_transitions_total", conditionLabels(status.ConditionReady, metav1.ConditionTrue))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transitions_total", conditionLabels(status.ConditionReady, metav1.ConditionFalse))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transitions_total", conditionLabels(status.ConditionReady, metav1.ConditionUnknown))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transitions_total", conditionLabels(ConditionTypeFoo, metav1.ConditionTrue)).GetCounter().GetValue()).To(BeEquivalentTo(1))
+		Expect(GetMetric("operator_customobject_status_condition_transitions_total", conditionLabels(ConditionTypeFoo, metav1.ConditionFalse))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transitions_total", conditionLabels(ConditionTypeFoo, metav1.ConditionUnknown))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transitions_total", conditionLabels(ConditionTypeBar, metav1.ConditionTrue))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transitions_total", conditionLabels(ConditionTypeBar, metav1.ConditionFalse))).To(BeNil())
+		Expect(GetMetric("operator_customobject_status_condition_transitions_total", conditionLabels(ConditionTypeBar, metav1.ConditionUnknown))).To(BeNil())
+	})
 })
 
 var _ = Describe("Generic Controller", func() {
 	var genericController *status.GenericObjectController[*TestGenericObject]
 	BeforeEach(func() {
 		recorder = record.NewFakeRecorder(10)
 		kubeClient = fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
-		ctx = log.IntoContext(context.Background(), ginkgo.GinkgoLogr)
+		ctx = log.IntoContext(context.Background(), GinkgoLogr)
 		genericController = status.NewGenericObjectController[*TestGenericObject](kubeClient, recorder, status.EmitDeprecatedMetrics)
 	})
 	AfterEach(func() {

test/object.go

@@ -33,6 +33,7 @@ func Object[T client.Object](base T, overrides ...T) T {
 	for _, src := range append([]T{base}, overrides...) {
 		lo.Must0(mergo.Merge(dest, src, mergo.WithOverride))
 	}
+	dest.SetCreationTimestamp(metav1.Now())
 	return dest
 }