Update set conditions to set LastTrasition time only when status changes

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
 		}
 	}

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 {