diff --git a/pkg/api/meta/conditions.go b/pkg/api/meta/conditions.go index 60c8209de..cbdf2eeb8 100644 --- a/pkg/api/meta/conditions.go +++ b/pkg/api/meta/conditions.go @@ -22,14 +22,15 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// SetStatusCondition sets the corresponding condition in conditions to newCondition. +// SetStatusCondition sets the corresponding condition in conditions to newCondition and returns true +// if the conditions are changed by this call. // conditions must be non-nil. // 1. if the condition of the specified type already exists (all fields of the existing condition are updated to // newCondition, LastTransitionTime is set to now if the new status differs from the old status) // 2. if a condition of the specified type does not exist (LastTransitionTime is set to now() if unset, and newCondition is appended) -func SetStatusCondition(conditions *[]metav1.Condition, newCondition metav1.Condition) { +func SetStatusCondition(conditions *[]metav1.Condition, newCondition metav1.Condition) (changed bool) { if conditions == nil { - return + return false } existingCondition := FindStatusCondition(*conditions, newCondition.Type) if existingCondition == nil { @@ -37,7 +38,7 @@ func SetStatusCondition(conditions *[]metav1.Condition, newCondition metav1.Cond newCondition.LastTransitionTime = metav1.NewTime(time.Now()) } *conditions = append(*conditions, newCondition) - return + return true } if existingCondition.Status != newCondition.Status { @@ -47,18 +48,31 @@ func SetStatusCondition(conditions *[]metav1.Condition, newCondition metav1.Cond } else { existingCondition.LastTransitionTime = metav1.NewTime(time.Now()) } + changed = true } - existingCondition.Reason = newCondition.Reason - existingCondition.Message = newCondition.Message - existingCondition.ObservedGeneration = newCondition.ObservedGeneration + if existingCondition.Reason != newCondition.Reason { + existingCondition.Reason = newCondition.Reason + changed = true + } + if existingCondition.Message != newCondition.Message { + existingCondition.Message = newCondition.Message + changed = true + } + if existingCondition.ObservedGeneration != newCondition.ObservedGeneration { + existingCondition.ObservedGeneration = newCondition.ObservedGeneration + changed = true + } + + return changed } -// RemoveStatusCondition removes the corresponding conditionType from conditions. +// RemoveStatusCondition removes the corresponding conditionType from conditions if present. Returns +// true if it was present and got removed. // conditions must be non-nil. -func RemoveStatusCondition(conditions *[]metav1.Condition, conditionType string) { +func RemoveStatusCondition(conditions *[]metav1.Condition, conditionType string) (removed bool) { if conditions == nil || len(*conditions) == 0 { - return + return false } newConditions := make([]metav1.Condition, 0, len(*conditions)-1) for _, condition := range *conditions { @@ -67,7 +81,10 @@ func RemoveStatusCondition(conditions *[]metav1.Condition, conditionType string) } } + removed = len(*conditions) != len(newConditions) *conditions = newConditions + + return removed } // FindStatusCondition finds the conditionType in conditions. diff --git a/pkg/api/meta/conditions_test.go b/pkg/api/meta/conditions_test.go index 72e5f680c..248c88580 100644 --- a/pkg/api/meta/conditions_test.go +++ b/pkg/api/meta/conditions_test.go @@ -29,10 +29,11 @@ func TestSetStatusCondition(t *testing.T) { oneHourAfter := time.Now().Add(1 * time.Hour) tests := []struct { - name string - conditions []metav1.Condition - toAdd metav1.Condition - expected []metav1.Condition + name string + conditions []metav1.Condition + toAdd metav1.Condition + expectChanged bool + expected []metav1.Condition }{ { name: "should-add", @@ -40,7 +41,8 @@ func TestSetStatusCondition(t *testing.T) { {Type: "first"}, {Type: "third"}, }, - toAdd: metav1.Condition{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message"}, + toAdd: metav1.Condition{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message"}, + expectChanged: true, expected: []metav1.Condition{ {Type: "first"}, {Type: "third"}, @@ -54,7 +56,8 @@ func TestSetStatusCondition(t *testing.T) { {Type: "second", Status: metav1.ConditionFalse}, {Type: "third"}, }, - toAdd: metav1.Condition{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message"}, + toAdd: metav1.Condition{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message"}, + expectChanged: true, expected: []metav1.Condition{ {Type: "first"}, {Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message"}, @@ -68,18 +71,36 @@ func TestSetStatusCondition(t *testing.T) { {Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}}, {Type: "third"}, }, - toAdd: metav1.Condition{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourAfter}, Reason: "reason", Message: "message", ObservedGeneration: 3}, + toAdd: metav1.Condition{Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourAfter}, Reason: "reason", Message: "message", ObservedGeneration: 3}, + expectChanged: true, expected: []metav1.Condition{ {Type: "first"}, {Type: "second", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}, Reason: "reason", Message: "message", ObservedGeneration: 3}, {Type: "third"}, }, }, + { + name: "nothing changes", + conditions: []metav1.Condition{{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Time{Time: oneHourBefore}, + }}, + toAdd: metav1.Condition{Type: "type", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Time{Time: oneHourBefore}}, + expected: []metav1.Condition{{ + Type: "type", + Status: metav1.ConditionTrue, + LastTransitionTime: metav1.Time{Time: oneHourBefore}, + }}, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - SetStatusCondition(&test.conditions, test.toAdd) + changed := SetStatusCondition(&test.conditions, test.toAdd) + if test.expectChanged != changed { + t.Errorf("expectChanged=%t != changed=%t", test.expectChanged, changed) + } if !reflect.DeepEqual(test.conditions, test.expected) { t.Error(test.conditions) } @@ -92,6 +113,7 @@ func TestRemoveStatusCondition(t *testing.T) { name string conditions []metav1.Condition conditionType string + expectRemoval bool expected []metav1.Condition }{ { @@ -102,6 +124,7 @@ func TestRemoveStatusCondition(t *testing.T) { {Type: "third"}, }, conditionType: "second", + expectRemoval: true, expected: []metav1.Condition{ {Type: "first"}, {Type: "third"}, @@ -131,7 +154,10 @@ func TestRemoveStatusCondition(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - RemoveStatusCondition(&test.conditions, test.conditionType) + removed := RemoveStatusCondition(&test.conditions, test.conditionType) + if test.expectRemoval != removed { + t.Errorf("expectRemoval=%t != removal=%t", test.expectRemoval, removed) + } if !reflect.DeepEqual(test.conditions, test.expected) { t.Error(test.conditions) }