From 6b3fd9672de12a757be8a0d32cfa252f71a26172 Mon Sep 17 00:00:00 2001 From: James Thompson Date: Tue, 7 Apr 2026 14:47:32 -0700 Subject: [PATCH] feat: support injected clock in ConditionSet for testability ConditionSet.Set() used metav1.Now() (real clock) for LastTransitionTime. Controllers that use a fake clock for time-dependent logic (e.g., checking elapsed time since a condition was set) get incorrect results because the condition timestamp and the controller clock diverge. Add a WithClock option to ConditionTypes.For() that injects a clock.Clock into the ConditionSet. Defaults to clock.RealClock{}, so existing callers are unaffected. Tests can pass a fake clock to get deterministic LastTransitionTime values. This fixes a class of test flakes in downstream projects (e.g., kubernetes-sigs/karpenter#2951) where controllers compare clock.Since(condition.LastTransitionTime) using a fake clock but the condition was stamped with real time. --- status/condition_set.go | 33 +++++++++++++++++++++++++++++---- status/condition_set_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) diff --git a/status/condition_set.go b/status/condition_set.go index a58c3e4..330bd4d 100644 --- a/status/condition_set.go +++ b/status/condition_set.go @@ -10,6 +10,7 @@ import ( "github.com/samber/lo" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/clock" ) // ConditionTypes is an abstract collection of the possible ConditionType values @@ -47,12 +48,31 @@ func newConditionTypes(root string, dependents ...string) ConditionTypes { type ConditionSet struct { ConditionTypes object Object + clock clock.Clock +} + +// ForOptions configures a ConditionSet. +type ForOptions struct { + Clock clock.Clock +} + +// ForOption is a functional option for For. +type ForOption func(*ForOptions) + +// WithClock sets the clock used for LastTransitionTime. Defaults to the real +// clock. Inject a fake clock in tests to avoid real-time dependencies. +func WithClock(c clock.Clock) ForOption { + return func(o *ForOptions) { o.Clock = c } } // 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) ConditionSet { - cs := ConditionSet{object: object, ConditionTypes: r} +func (r ConditionTypes) For(object Object, opts ...ForOption) ConditionSet { + o := ForOptions{Clock: clock.RealClock{}} + for _, opt := range opts { + opt(&o) + } + cs := ConditionSet{object: object, ConditionTypes: r, clock: o.Clock} // 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...) { @@ -123,7 +143,7 @@ func (c ConditionSet) Set(condition Condition) (modified bool) { if condition.Status == cond.Status { condition.LastTransitionTime = cond.LastTransitionTime } else { - condition.LastTransitionTime = metav1.Now() + condition.LastTransitionTime = c.now() } if reflect.DeepEqual(condition, cond) { return false @@ -136,7 +156,7 @@ func (c ConditionSet) Set(condition Condition) (modified bool) { if c.IsDependentCondition(condition.Type) { condition.LastTransitionTime = c.object.GetCreationTimestamp() } else { - condition.LastTransitionTime = metav1.Now() + condition.LastTransitionTime = c.now() } } conditions = append(conditions, condition) @@ -260,6 +280,11 @@ func (c ConditionSet) recomputeRootCondition(conditionType string) { } } +// now returns the current time from the injected clock as a metav1.Time. +func (c ConditionSet) now() metav1.Time { + return metav1.NewTime(c.clock.Now()) +} + func (c ConditionSet) findUnhealthyDependents() []Condition { if len(c.dependents) == 0 { return nil diff --git a/status/condition_set_test.go b/status/condition_set_test.go index 80c0af6..6fb2e26 100644 --- a/status/condition_set_test.go +++ b/status/condition_set_test.go @@ -10,6 +10,7 @@ import ( "github.com/samber/lo" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clocktesting "k8s.io/utils/clock/testing" ) var _ = Describe("Conditions", func() { @@ -195,4 +196,28 @@ var _ = Describe("Conditions", func() { Expect(conditions.Root().Status).To(Equal(metav1.ConditionTrue)) Expect(conditions.Root().ObservedGeneration).To(Equal(int64(2))) }) + It("should use injected clock for LastTransitionTime", func() { + baseTime := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + fakeClock := clocktesting.NewFakeClock(baseTime) + testObject := test.Object(&test.CustomObject{}) + conditions := status.NewReadyConditions(test.ConditionTypeFoo, test.ConditionTypeBar).For(testObject, status.WithClock(fakeClock)) + + // Set an independent condition; its LastTransitionTime should use the fake clock + conditions.SetUnknownWithReason("MyCondition", "reason", "message") + cond := conditions.Get("MyCondition") + Expect(cond).ToNot(BeNil()) + Expect(cond.LastTransitionTime.Time).To(Equal(baseTime)) + + // Advance the fake clock and transition the condition + fakeClock.Step(10 * time.Second) + conditions.SetTrue("MyCondition") + cond = conditions.Get("MyCondition") + Expect(cond.LastTransitionTime.Time).To(Equal(baseTime.Add(10 * time.Second))) + + // Setting the same status again should preserve the LastTransitionTime + fakeClock.Step(5 * time.Second) + conditions.SetTrueWithReason("MyCondition", "reason2", "message2") + cond = conditions.Get("MyCondition") + Expect(cond.LastTransitionTime.Time).To(Equal(baseTime.Add(10 * time.Second))) + }) })