Skip to content

Commit 02ff405

Browse files
authored
feat: support injected clock in ConditionSet for testability (#204)
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.
1 parent 7a13865 commit 02ff405

2 files changed

Lines changed: 54 additions & 4 deletions

File tree

status/condition_set.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"github.com/samber/lo"
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/utils/clock"
1314
)
1415

1516
// ConditionTypes is an abstract collection of the possible ConditionType values
@@ -47,12 +48,31 @@ func newConditionTypes(root string, dependents ...string) ConditionTypes {
4748
type ConditionSet struct {
4849
ConditionTypes
4950
object Object
51+
clock clock.Clock
52+
}
53+
54+
// ForOptions configures a ConditionSet.
55+
type ForOptions struct {
56+
Clock clock.Clock
57+
}
58+
59+
// ForOption is a functional option for For.
60+
type ForOption func(*ForOptions)
61+
62+
// WithClock sets the clock used for LastTransitionTime. Defaults to the real
63+
// clock. Inject a fake clock in tests to avoid real-time dependencies.
64+
func WithClock(c clock.Clock) ForOption {
65+
return func(o *ForOptions) { o.Clock = c }
5066
}
5167

5268
// For creates a ConditionSet from an object using the original
5369
// ConditionTypes as a reference. Status must be a pointer to a struct.
54-
func (r ConditionTypes) For(object Object) ConditionSet {
55-
cs := ConditionSet{object: object, ConditionTypes: r}
70+
func (r ConditionTypes) For(object Object, opts ...ForOption) ConditionSet {
71+
o := ForOptions{Clock: clock.RealClock{}}
72+
for _, opt := range opts {
73+
opt(&o)
74+
}
75+
cs := ConditionSet{object: object, ConditionTypes: r, clock: o.Clock}
5676
// Set known conditions Unknown if not set.
5777
// Set the root condition first to get consistent timing for LastTransitionTime
5878
for _, t := range append([]string{r.root}, r.dependents...) {
@@ -123,7 +143,7 @@ func (c ConditionSet) Set(condition Condition) (modified bool) {
123143
if condition.Status == cond.Status {
124144
condition.LastTransitionTime = cond.LastTransitionTime
125145
} else {
126-
condition.LastTransitionTime = metav1.Now()
146+
condition.LastTransitionTime = c.now()
127147
}
128148
if reflect.DeepEqual(condition, cond) {
129149
return false
@@ -136,7 +156,7 @@ func (c ConditionSet) Set(condition Condition) (modified bool) {
136156
if c.IsDependentCondition(condition.Type) {
137157
condition.LastTransitionTime = c.object.GetCreationTimestamp()
138158
} else {
139-
condition.LastTransitionTime = metav1.Now()
159+
condition.LastTransitionTime = c.now()
140160
}
141161
}
142162
conditions = append(conditions, condition)
@@ -260,6 +280,11 @@ func (c ConditionSet) recomputeRootCondition(conditionType string) {
260280
}
261281
}
262282

283+
// now returns the current time from the injected clock as a metav1.Time.
284+
func (c ConditionSet) now() metav1.Time {
285+
return metav1.NewTime(c.clock.Now())
286+
}
287+
263288
func (c ConditionSet) findUnhealthyDependents() []Condition {
264289
if len(c.dependents) == 0 {
265290
return nil

status/condition_set_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/samber/lo"
1111

1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
clocktesting "k8s.io/utils/clock/testing"
1314
)
1415

1516
var _ = Describe("Conditions", func() {
@@ -195,4 +196,28 @@ var _ = Describe("Conditions", func() {
195196
Expect(conditions.Root().Status).To(Equal(metav1.ConditionTrue))
196197
Expect(conditions.Root().ObservedGeneration).To(Equal(int64(2)))
197198
})
199+
It("should use injected clock for LastTransitionTime", func() {
200+
baseTime := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
201+
fakeClock := clocktesting.NewFakeClock(baseTime)
202+
testObject := test.Object(&test.CustomObject{})
203+
conditions := status.NewReadyConditions(test.ConditionTypeFoo, test.ConditionTypeBar).For(testObject, status.WithClock(fakeClock))
204+
205+
// Set an independent condition; its LastTransitionTime should use the fake clock
206+
conditions.SetUnknownWithReason("MyCondition", "reason", "message")
207+
cond := conditions.Get("MyCondition")
208+
Expect(cond).ToNot(BeNil())
209+
Expect(cond.LastTransitionTime.Time).To(Equal(baseTime))
210+
211+
// Advance the fake clock and transition the condition
212+
fakeClock.Step(10 * time.Second)
213+
conditions.SetTrue("MyCondition")
214+
cond = conditions.Get("MyCondition")
215+
Expect(cond.LastTransitionTime.Time).To(Equal(baseTime.Add(10 * time.Second)))
216+
217+
// Setting the same status again should preserve the LastTransitionTime
218+
fakeClock.Step(5 * time.Second)
219+
conditions.SetTrueWithReason("MyCondition", "reason2", "message2")
220+
cond = conditions.Get("MyCondition")
221+
Expect(cond.LastTransitionTime.Time).To(Equal(baseTime.Add(10 * time.Second)))
222+
})
198223
})

0 commit comments

Comments
 (0)