Skip to content

Commit a121afc

Browse files
authored
🌱 Update conditions.Set function to set LastTransitionTime only when status of condition changes (#11176)
* Update set conditions to set LastTrasition time only when status changes * Add new test to ignore last transition time even if set * Create seperate function for Set condition
1 parent 2dbfdf2 commit a121afc

File tree

2 files changed

+138
-0
lines changed

2 files changed

+138
-0
lines changed

‎util/conditions/setter.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,54 @@ func Set(to Setter, condition *clusterv1.Condition) {
7777
to.SetConditions(conditions)
7878
}
7979

80+
// SetWithCustomLastTransitionTime is similar to Set function which sets the given condition but following changes for LastTransitionTime.
81+
//
82+
// 1. if the condition of the specified type already exists (all fields of the existing condition are updated to
83+
// new condition, LastTransitionTime is set to current time if unset and new status differs from the old status)
84+
// 2. if a condition of the specified type does not exist (LastTransitionTime is set to current time if unset, and newCondition is appended)
85+
func SetWithCustomLastTransitionTime(to Setter, condition *clusterv1.Condition) {
86+
if to == nil || condition == nil {
87+
return
88+
}
89+
90+
// Check if the new conditions already exists, and change it only if there is a status
91+
// transition (otherwise we should preserve the current last transition time)-
92+
conditions := to.GetConditions()
93+
exists := false
94+
for i := range conditions {
95+
existingCondition := conditions[i]
96+
if existingCondition.Type == condition.Type {
97+
exists = true
98+
if !HasSameState(&existingCondition, condition) {
99+
if existingCondition.Status != condition.Status {
100+
if condition.LastTransitionTime.IsZero() {
101+
condition.LastTransitionTime = metav1.NewTime(time.Now().UTC().Truncate(time.Second))
102+
}
103+
} else {
104+
condition.LastTransitionTime = existingCondition.LastTransitionTime
105+
}
106+
conditions[i] = *condition
107+
}
108+
break
109+
}
110+
}
111+
112+
// If the condition does not exist, add it, setting the transition time only if not already set
113+
if !exists {
114+
if condition.LastTransitionTime.IsZero() {
115+
condition.LastTransitionTime = metav1.NewTime(time.Now().UTC().Truncate(time.Second))
116+
}
117+
conditions = append(conditions, *condition)
118+
}
119+
120+
// Sorts conditions for convenience of the consumer, i.e. kubectl.
121+
sort.Slice(conditions, func(i, j int) bool {
122+
return lexicographicLess(&conditions[i], &conditions[j])
123+
})
124+
125+
to.SetConditions(conditions)
126+
}
127+
80128
// TrueCondition returns a condition with Status=True and the given type.
81129
func TrueCondition(t clusterv1.ConditionType) *clusterv1.Condition {
82130
return &clusterv1.Condition{

‎util/conditions/setter_test.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,96 @@ func TestSetLastTransitionTime(t *testing.T) {
209209
}
210210
}
211211

212+
func TestSetWithCustomLastTransitionTime(t *testing.T) {
213+
x := metav1.Date(2012, time.January, 1, 12, 15, 30, 5e8, time.UTC)
214+
y := metav1.Date(2012, time.January, 2, 12, 15, 30, 5e8, time.UTC)
215+
216+
foo := FalseCondition("foo", "reason foo", clusterv1.ConditionSeverityInfo, "message foo")
217+
fooWithBarMessage := FalseCondition("foo", "reason foo", clusterv1.ConditionSeverityInfo, "message bar")
218+
fooWithLastTransitionTime := FalseCondition("foo", "reason foo", clusterv1.ConditionSeverityInfo, "message foo")
219+
fooWithLastTransitionTime.LastTransitionTime = x
220+
fooWithLastTransitionTimeWithBarMessage := FalseCondition("foo", "reason foo", clusterv1.ConditionSeverityInfo, "message bar")
221+
fooWithLastTransitionTimeWithBarMessage.LastTransitionTime = y
222+
223+
fooWithAnotherState := TrueCondition("foo")
224+
fooWithAnotherStateWithLastTransitionTime := TrueCondition("foo")
225+
fooWithAnotherStateWithLastTransitionTime.LastTransitionTime = y
226+
227+
tests := []struct {
228+
name string
229+
to Setter
230+
new *clusterv1.Condition
231+
LastTransitionTimeCheck func(*WithT, metav1.Time)
232+
}{
233+
{
234+
name: "Set a condition that does not exists should set the last transition time if not defined",
235+
to: setterWithConditions(),
236+
new: foo,
237+
LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) {
238+
g.Expect(lastTransitionTime).ToNot(BeZero())
239+
},
240+
},
241+
{
242+
name: "Set a condition that does not exists should preserve the last transition time if defined",
243+
to: setterWithConditions(),
244+
new: fooWithLastTransitionTime,
245+
LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) {
246+
g.Expect(lastTransitionTime).To(Equal(x))
247+
},
248+
},
249+
{
250+
name: "Set a condition that already exists with the same state should preserves the last transition time",
251+
to: setterWithConditions(fooWithLastTransitionTime),
252+
new: foo,
253+
LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) {
254+
g.Expect(lastTransitionTime).To(Equal(x))
255+
},
256+
},
257+
{
258+
name: "Set a condition that already exists but with different state should changes the last transition time",
259+
to: setterWithConditions(fooWithLastTransitionTime),
260+
new: fooWithAnotherState,
261+
LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) {
262+
g.Expect(lastTransitionTime).ToNot(Equal(x))
263+
},
264+
},
265+
{
266+
name: "Set a condition that already exists but with different state should preserve the last transition time if defined",
267+
to: setterWithConditions(fooWithLastTransitionTime),
268+
new: fooWithAnotherStateWithLastTransitionTime,
269+
LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) {
270+
g.Expect(lastTransitionTime).To(Equal(y))
271+
},
272+
},
273+
{
274+
name: "Set a condition that already exists but with different Message should preserve the last transition time",
275+
to: setterWithConditions(fooWithLastTransitionTime),
276+
new: fooWithBarMessage,
277+
LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) {
278+
g.Expect(lastTransitionTime).To(Equal(x))
279+
},
280+
},
281+
{
282+
name: "Set a condition that already exists, with different state but same Status should ignore the last transition even if defined",
283+
to: setterWithConditions(fooWithLastTransitionTime),
284+
new: fooWithLastTransitionTimeWithBarMessage,
285+
LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) {
286+
g.Expect(lastTransitionTime).To(Equal(x))
287+
},
288+
},
289+
}
290+
291+
for _, tt := range tests {
292+
t.Run(tt.name, func(t *testing.T) {
293+
g := NewWithT(t)
294+
295+
SetWithCustomLastTransitionTime(tt.to, tt.new)
296+
297+
tt.LastTransitionTimeCheck(g, Get(tt.to, "foo").LastTransitionTime)
298+
})
299+
}
300+
}
301+
212302
func TestMarkMethods(t *testing.T) {
213303
g := NewWithT(t)
214304

0 commit comments

Comments
 (0)