diff --git a/util/conditions/setter.go b/util/conditions/setter.go index 0b278eae1bd6..85da0426f4a1 100644 --- a/util/conditions/setter.go +++ b/util/conditions/setter.go @@ -36,8 +36,9 @@ type Setter interface { // Set sets the given condition. // -// NOTE: If a condition already exists, the LastTransitionTime is updated only if a change is detected -// in any of the following fields: Status, Reason, Severity and Message. +// 1. if the condition of the specified type already exists (all fields of the existing condition are updated to +// new condition, LastTransitionTime is set to current time if unset and new status differs from the old status) +// 2. if a condition of the specified type does not exist (LastTransitionTime is set to current time if unset, and newCondition is appended) func Set(to Setter, condition *clusterv1.Condition) { if to == nil || condition == nil { return @@ -52,11 +53,15 @@ func Set(to Setter, condition *clusterv1.Condition) { if existingCondition.Type == condition.Type { exists = true if !HasSameState(&existingCondition, condition) { - condition.LastTransitionTime = metav1.NewTime(time.Now().UTC().Truncate(time.Second)) + if existingCondition.Status != condition.Status { + if condition.LastTransitionTime.IsZero() { + condition.LastTransitionTime = metav1.NewTime(time.Now().UTC().Truncate(time.Second)) + } + } else { + condition.LastTransitionTime = existingCondition.LastTransitionTime + } conditions[i] = *condition - break } - condition.LastTransitionTime = existingCondition.LastTransitionTime break } } diff --git a/util/conditions/setter_test.go b/util/conditions/setter_test.go index cf34be02dfb2..5eb15f587f30 100644 --- a/util/conditions/setter_test.go +++ b/util/conditions/setter_test.go @@ -152,11 +152,15 @@ func TestSet(t *testing.T) { func TestSetLastTransitionTime(t *testing.T) { x := metav1.Date(2012, time.January, 1, 12, 15, 30, 5e8, time.UTC) + y := metav1.Date(2012, time.January, 2, 12, 15, 30, 5e8, time.UTC) foo := FalseCondition("foo", "reason foo", clusterv1.ConditionSeverityInfo, "message foo") + fooWithBarMessage := FalseCondition("foo", "reason foo", clusterv1.ConditionSeverityInfo, "message bar") fooWithLastTransitionTime := FalseCondition("foo", "reason foo", clusterv1.ConditionSeverityInfo, "message foo") fooWithLastTransitionTime.LastTransitionTime = x fooWithAnotherState := TrueCondition("foo") + fooWithAnotherStateWithLastTransitionTime := TrueCondition("foo") + fooWithAnotherStateWithLastTransitionTime.LastTransitionTime = y tests := []struct { name string @@ -196,6 +200,22 @@ func TestSetLastTransitionTime(t *testing.T) { g.Expect(lastTransitionTime).ToNot(Equal(x)) }, }, + { + name: "Set a condition that already exists but with different state should preserve the last transition time if defined", + to: setterWithConditions(fooWithLastTransitionTime), + new: fooWithAnotherStateWithLastTransitionTime, + LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) { + g.Expect(lastTransitionTime).To(Equal(y)) + }, + }, + { + name: "Set a condition that already exists but with different Message should preserve the last transition time", + to: setterWithConditions(fooWithLastTransitionTime), + new: fooWithBarMessage, + LastTransitionTimeCheck: func(g *WithT, lastTransitionTime metav1.Time) { + g.Expect(lastTransitionTime).To(Equal(x)) + }, + }, } for _, tt := range tests {