Merge pull request #113 from grafana/dimitar/compare-group-src-tenant…

…s-w-dups Account for repeating tenants when comparing rules
grafana · Jan 24, 2022 · 644605d · 644605d
2 parents bc7a802 + f05e435
commit 644605d
Show file tree

Hide file tree

Showing 2 changed files with 192 additions and 12 deletions.
diff --git a/rules/manager.go b/rules/manager.go
@@ -875,21 +875,26 @@ func (g *Group) Equals(ng *Group) bool {
 			return false
 		}
 	}
+	{
+		// compare source tenants
+		if len(g.sourceTenants) != len(ng.sourceTenants) {
+			return false
+		}
 
-	// compare source tenants ignoring their order
-	if len(g.sourceTenants) != len(ng.sourceTenants) {
-		return false
-	}
-
-	thisSourceTenants := make(map[string]struct{}, len(g.sourceTenants))
+		copyAndSort := func(x []string) []string {
+			copied := make([]string, len(x))
+			copy(copied, x)
+			sort.Strings(copied)
+			return copied
+		}
 
-	for _, tenant := range g.sourceTenants {
-		thisSourceTenants[tenant] = struct{}{}
-	}
+		ngSourceTenantsCopy := copyAndSort(ng.sourceTenants)
+		gSourceTenantsCopy := copyAndSort(g.sourceTenants)
 
-	for _, tenant := range ng.sourceTenants {
-		if _, ok := thisSourceTenants[tenant]; !ok {
-			return false
+		for i := range ngSourceTenantsCopy {
+			if gSourceTenantsCopy[i] != ngSourceTenantsCopy[i] {
+				return false
+			}
 		}
 	}
 

diff --git a/rules/manager_test.go b/rules/manager_test.go
@@ -1337,3 +1337,178 @@ func TestRuleHealthUpdates(t *testing.T) {
 	require.EqualError(t, rules.LastError(), storage.ErrOutOfOrderSample.Error())
 	require.Equal(t, HealthBad, rules.Health())
 }
+
+func TestGroup_Equals(t *testing.T) {
+	testExpression, err := parser.ParseExpr("up")
+	require.NoError(t, err)
+
+	tests := []struct {
+		name     string
+		groupOne *Group
+		groupTwo *Group
+		areEqual bool
+	}{
+		{
+			name: "identical configs",
+			groupOne: &Group{
+				name: "example_group",
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			groupTwo: &Group{
+				name: "example_group",
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			areEqual: true,
+		},
+		{
+			name: "differently ordered source tenants (should still be equivalent)",
+			groupOne: &Group{
+				name:          "example_group",
+				sourceTenants: []string{"tenant-2", "tenant-1"},
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			groupTwo: &Group{
+				name:          "example_group",
+				sourceTenants: []string{"tenant-1", "tenant-2"},
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			areEqual: true,
+		},
+		{
+			name: "different rule length",
+			groupOne: &Group{
+				name: "example_group",
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			groupTwo: &Group{
+				name: "example_group",
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			areEqual: false,
+		},
+		{
+			name: "different rule labels",
+			groupOne: &Group{
+				name: "example_group",
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			groupTwo: &Group{
+				name: "example_group",
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"1": "2", "3": "4"}),
+					},
+				},
+			},
+			areEqual: false,
+		},
+		{
+			name: "different source tenants",
+			groupOne: &Group{
+				name:          "example_group",
+				sourceTenants: []string{"tenant-1", "tenant-3"},
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			groupTwo: &Group{
+				name:          "example_group",
+				sourceTenants: []string{"tenant-1", "tenant-2"},
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			areEqual: false,
+		},
+		{
+			name: "repeating source tenants",
+			groupOne: &Group{
+				name:          "example_group",
+				sourceTenants: []string{"tenant-1", "tenant-2"},
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			groupTwo: &Group{
+				name:          "example_group",
+				sourceTenants: []string{"tenant-1", "tenant-1"},
+				rules: []Rule{
+					&RecordingRule{
+						name:   "one",
+						vector: testExpression,
+						labels: labels.FromMap(map[string]string{"a": "b", "c": "d"}),
+					},
+				},
+			},
+			areEqual: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.areEqual, tt.groupOne.Equals(tt.groupTwo))
+			require.Equal(t, tt.areEqual, tt.groupTwo.Equals(tt.groupOne))
+		})
+	}
+}