From 1ff5f9fa1aaa146ba0f49e1227adbd65a6a4fc89 Mon Sep 17 00:00:00 2001 From: Robert Jacob Date: Wed, 10 Jul 2024 21:03:55 +0200 Subject: [PATCH] fix(operator): Remove duplicate conditions from status --- operator/internal/status/conditions.go | 41 +++++----- operator/internal/status/conditions_test.go | 90 ++++++++++++++++++--- 2 files changed, 101 insertions(+), 30 deletions(-) diff --git a/operator/internal/status/conditions.go b/operator/internal/status/conditions.go index 637a50e6f89f..e23597c1253f 100644 --- 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 index 3d85942df875..a3fd76dff5cf 100644 --- 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,12 +38,25 @@ 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, @@ -50,12 +64,27 @@ func TestMergeConditions(t *testing.T) { 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,