fix(operator): Remove duplicate conditions from status (#13497)

grafana · Jul 12, 2024 · 527510d · 527510d
1 parent ac284ca
commit 527510d
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 30 deletions.
diff --git a/operator/internal/status/conditions.go b/operator/internal/status/conditions.go
@@ -2,36 +2,35 @@ package status
 
 import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+type conditionKey struct {
+	Type   string
+	Reason string
+}
+
 func mergeConditions(old, active []metav1.Condition, now metav1.Time) []metav1.Condition {
+	conditions := map[conditionKey]bool{}
 	merged := make([]metav1.Condition, 0, len(old)+len(active))
-	for len(old) > 0 {
-		c := old[0]
-		found := -1
-		for i, ac := range active {
-			if c.Type == ac.Type && c.Reason == ac.Reason {
-				found = i
-				break
-			}
-		}
+	for _, c := range active {
+		c.Status = metav1.ConditionTrue
+		c.LastTransitionTime = now
+
+		merged = append(merged, c)
+		conditions[conditionKey{Type: c.Type, Reason: c.Reason}] = true
+	}
 
-		if found != -1 {
-			c = active[found]
-			active = append(active[:found], active[found+1:]...)
+	for _, c := range old {
+		if conditions[conditionKey{c.Type, c.Reason}] {
+			continue
+		}
 
-			c.Status = metav1.ConditionTrue
-		} else {
+		if c.Status != metav1.ConditionFalse {
+			c.LastTransitionTime = now
 			c.Status = metav1.ConditionFalse
 		}
 
-		c.LastTransitionTime = now
 		merged = append(merged, c)
-		old = old[1:]
+		conditions[conditionKey{c.Type, c.Reason}] = true
 	}
 
-	for _, c := range active {
-		c.Status = metav1.ConditionTrue
-		c.LastTransitionTime = now
-		merged = append(merged, c)
-	}
 	return merged
 }
diff --git a/operator/internal/status/conditions_test.go b/operator/internal/status/conditions_test.go
@@ -11,7 +11,8 @@ import (
 )
 
 func TestMergeConditions(t *testing.T) {
-	now := metav1.NewTime(time.Unix(0, 0))
+	oldTime := metav1.NewTime(time.Unix(0, 0))
+	now := metav1.NewTime(time.Unix(10, 0))
 	tt := []struct {
 		desc       string
 		old        []metav1.Condition
@@ -37,25 +38,53 @@ func TestMergeConditions(t *testing.T) {
 		{
 			desc: "reset old condition",
 			old: []metav1.Condition{
-				conditionPending,
+				{
+					Type:               conditionPending.Type,
+					Status:             metav1.ConditionTrue,
+					LastTransitionTime: oldTime,
+					Reason:             conditionPending.Reason,
+					Message:            conditionPending.Message,
+				},
 			},
 			active: []metav1.Condition{
 				conditionReady,
 			},
 			wantMerged: []metav1.Condition{
+				{
+					Type:               conditionReady.Type,
+					Status:             metav1.ConditionTrue,
+					LastTransitionTime: now,
+					Reason:             conditionReady.Reason,
+					Message:            conditionReady.Message,
+				},
 				{
 					Type:               conditionPending.Type,
 					Status:             metav1.ConditionFalse,
 					LastTransitionTime: now,
 					Reason:             conditionPending.Reason,
 					Message:            conditionPending.Message,
 				},
+			},
+		},
+		{
+			desc: "keep transition time of old condition",
+			old: []metav1.Condition{
 				{
-					Type:               conditionReady.Type,
-					Status:             metav1.ConditionTrue,
-					LastTransitionTime: now,
-					Reason:             conditionReady.Reason,
-					Message:            conditionReady.Message,
+					Type:               conditionPending.Type,
+					Status:             metav1.ConditionFalse,
+					LastTransitionTime: oldTime,
+					Reason:             conditionPending.Reason,
+					Message:            conditionPending.Message,
+				},
+			},
+			active: []metav1.Condition{},
+			wantMerged: []metav1.Condition{
+				{
+					Type:               conditionPending.Type,
+					Status:             metav1.ConditionFalse,
+					LastTransitionTime: oldTime,
+					Reason:             conditionPending.Reason,
+					Message:            conditionPending.Message,
 				},
 			},
 		},
@@ -72,7 +101,7 @@ func TestMergeConditions(t *testing.T) {
 				{
 					Type:               conditionPending.Type,
 					Status:             metav1.ConditionFalse,
-					LastTransitionTime: now,
+					LastTransitionTime: oldTime,
 					Reason:             conditionPending.Reason,
 					Message:            conditionPending.Message,
 				},
@@ -93,13 +122,56 @@ func TestMergeConditions(t *testing.T) {
 					Reason:             conditionReady.Reason,
 					Message:            conditionReady.Message,
 				},
+				{
+					Type:               string(lokiv1.ConditionWarning),
+					Status:             metav1.ConditionTrue,
+					LastTransitionTime: now,
+					Reason:             "test-warning",
+					Message:            "test-warning-message",
+				},
 				{
 					Type:               conditionPending.Type,
 					Status:             metav1.ConditionFalse,
-					LastTransitionTime: now,
+					LastTransitionTime: oldTime,
 					Reason:             conditionPending.Reason,
 					Message:            conditionPending.Message,
 				},
+			},
+		},
+		{
+			desc: "remove duplicates",
+			old: []metav1.Condition{
+				{
+					Type:               conditionReady.Type,
+					Status:             metav1.ConditionTrue,
+					LastTransitionTime: now,
+					Reason:             conditionReady.Reason,
+					Message:            conditionReady.Message,
+				},
+				{
+					Type:               conditionReady.Type,
+					Status:             metav1.ConditionTrue,
+					LastTransitionTime: now,
+					Reason:             conditionReady.Reason,
+					Message:            conditionReady.Message,
+				},
+			},
+			active: []metav1.Condition{
+				conditionReady,
+				{
+					Type:    string(lokiv1.ConditionWarning),
+					Reason:  "test-warning",
+					Message: "test-warning-message",
+				},
+			},
+			wantMerged: []metav1.Condition{
+				{
+					Type:               conditionReady.Type,
+					Status:             metav1.ConditionTrue,
+					LastTransitionTime: now,
+					Reason:             conditionReady.Reason,
+					Message:            conditionReady.Message,
+				},
 				{
 					Type:               string(lokiv1.ConditionWarning),
 					Status:             metav1.ConditionTrue,