diff --git a/internal/controller/conditionsutil/conditions_util.go b/internal/controller/conditionsutil/conditions_util.go index 436f4ac81..63cde9f75 100644 --- a/internal/controller/conditionsutil/conditions_util.go +++ b/internal/controller/conditionsutil/conditions_util.go @@ -29,17 +29,17 @@ const ( // since Pinniped should always check every condition. // It returns true if any resulting condition has non-true status. func MergeConditions( - conditions []*metav1.Condition, - conditionsToUpdate *[]metav1.Condition, + newConditions []*metav1.Condition, + existingConditionsToUpdate *[]metav1.Condition, observedGeneration int64, lastTransitionTime metav1.Time, log plog.MinLogger, ) bool { - for i := range conditions { - cond := conditions[i].DeepCopy() + for i := range newConditions { + cond := newConditions[i].DeepCopy() cond.LastTransitionTime = lastTransitionTime cond.ObservedGeneration = observedGeneration - if mergeCondition(conditionsToUpdate, cond) { + if mergeCondition(existingConditionsToUpdate, cond) { log.Info("updated condition", "type", cond.Type, "status", cond.Status, @@ -47,39 +47,39 @@ func MergeConditions( "message", cond.Message) } } - sort.SliceStable(*conditionsToUpdate, func(i, j int) bool { - return (*conditionsToUpdate)[i].Type < (*conditionsToUpdate)[j].Type + sort.SliceStable(*existingConditionsToUpdate, func(i, j int) bool { + return (*existingConditionsToUpdate)[i].Type < (*existingConditionsToUpdate)[j].Type }) - return HadErrorCondition(conditions) + return HadErrorCondition(newConditions) } // mergeCondition merges a new metav1.Condition into a slice of existing conditions. It returns true -// if the condition has meaningfully changed. -func mergeCondition(existing *[]metav1.Condition, new *metav1.Condition) bool { +// if something other than the LastTransitionTime has been updated. +func mergeCondition(existingConditionsToUpdate *[]metav1.Condition, newCondition *metav1.Condition) bool { // Find any existing condition with a matching type. var old *metav1.Condition - for i := range *existing { - if (*existing)[i].Type == new.Type { - old = &(*existing)[i] + for i := range *existingConditionsToUpdate { + if (*existingConditionsToUpdate)[i].Type == newCondition.Type { + old = &(*existingConditionsToUpdate)[i] continue } } // If there is no existing condition of this type, append this one and we're done. if old == nil { - *existing = append(*existing, *new) + *existingConditionsToUpdate = append(*existingConditionsToUpdate, *newCondition) return true } // Set the LastTransitionTime depending on whether the status has changed. - new = new.DeepCopy() - if old.Status == new.Status { - new.LastTransitionTime = old.LastTransitionTime + newCondition = newCondition.DeepCopy() + if old.Status == newCondition.Status { + newCondition.LastTransitionTime = old.LastTransitionTime } // If anything has actually changed, update the entry and return true. - if !equality.Semantic.DeepEqual(old, new) { - *old = *new + if !equality.Semantic.DeepEqual(old, newCondition) { + *old = *newCondition return true } diff --git a/internal/controller/conditionsutil/conditions_util_test.go b/internal/controller/conditionsutil/conditions_util_test.go index f5402286d..7c34a81b4 100644 --- a/internal/controller/conditionsutil/conditions_util_test.go +++ b/internal/controller/conditionsutil/conditions_util_test.go @@ -21,13 +21,13 @@ func TestMergeIDPConditions(t *testing.T) { testTime := metav1.Now() tests := []struct { - name string - newConditions []*metav1.Condition - conditionsToUpdate *[]metav1.Condition - observedGeneration int64 - wantResult bool - wantLogSnippets []string - wantConditions []metav1.Condition + name string + newConditions []*metav1.Condition + existingConditionsToUpdate *[]metav1.Condition + observedGeneration int64 + wantResult bool + wantLogSnippets []string + wantConditions []metav1.Condition }{ { name: "Adding a new condition with status=True returns false", @@ -39,8 +39,8 @@ func TestMergeIDPConditions(t *testing.T) { Message: "new message", }, }, - observedGeneration: int64(999), - conditionsToUpdate: &[]metav1.Condition{}, + observedGeneration: int64(999), + existingConditionsToUpdate: &[]metav1.Condition{}, wantLogSnippets: []string{ `"message":"updated condition","type":"NewType","status":"True"`, }, @@ -78,7 +78,7 @@ func TestMergeIDPConditions(t *testing.T) { Message: "new message", }, }, - conditionsToUpdate: &[]metav1.Condition{ + existingConditionsToUpdate: &[]metav1.Condition{ { Type: "UnchangedType", Status: metav1.ConditionTrue, @@ -140,7 +140,7 @@ func TestMergeIDPConditions(t *testing.T) { Message: "unchanged message", }, }, - conditionsToUpdate: &[]metav1.Condition{ + existingConditionsToUpdate: &[]metav1.Condition{ { Type: "UnchangedType", Status: metav1.ConditionFalse, @@ -165,26 +165,26 @@ func TestMergeIDPConditions(t *testing.T) { }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { var log bytes.Buffer logger := plog.TestLogger(t, &log) result := MergeConditions( - tt.newConditions, - tt.conditionsToUpdate, - tt.observedGeneration, + test.newConditions, + test.existingConditionsToUpdate, + test.observedGeneration, testTime, logger, ) logString := log.String() - require.Equal(t, len(tt.wantLogSnippets), strings.Count(logString, "\n")) - for _, wantLog := range tt.wantLogSnippets { + require.Equal(t, len(test.wantLogSnippets), strings.Count(logString, "\n")) + for _, wantLog := range test.wantLogSnippets { require.Contains(t, logString, wantLog) } - require.Equal(t, tt.wantResult, result) - require.Equal(t, tt.wantConditions, *tt.conditionsToUpdate) + require.Equal(t, test.wantResult, result) + require.Equal(t, test.wantConditions, *test.existingConditionsToUpdate) }) } }