Consider AZ awareness disabled if only 1 zone is present in list rules

Signed-off-by: Emmanuel Lodovice <[email protected]>

Author: emanlodovice

pkg/ruler/ruler_ring.go

@@ -144,7 +144,10 @@ func GetReplicationSetForListRule(r ring.ReadRing, cfg *RingConfig) (ring.Replic
 	// to 0, and then we update them whether zone-awareness is enabled or not.
 	maxErrors := 0
 	maxUnavailableZones := 0
-	if cfg.ZoneAwarenessEnabled {
+	// Because ring's Get method returns a number of ruler equal to the replication factor even if there is only 1 zone
+	// and ZoneAwarenessEnabled, we can consider that ZoneAwarenessEnabled is disabled if there is only 1 zone since
+	// rules are still replicated to rulers in the same zone.
+	if cfg.ZoneAwarenessEnabled && len(ringZones) > 1 {
 		numReplicatedZones := min(len(ringZones), r.ReplicationFactor())
 		// Given that quorum is not required, we only need at least one of the zone to be healthy to succeed. But we
 		// also need to handle case when RF < number of zones.

pkg/ruler/ruler_ring_test.go

@@ -180,6 +180,23 @@ func TestGetReplicationSetForListRule(t *testing.T) {
 				"z2": {},
 			},
 		},
+		"should succeed on 1 unhealthy instances in RF=3 zone replication enabled but only 1 zone": {
+			ringInstances: map[string]ring.InstanceDesc{
+				"instance-1": {Addr: "127.0.0.1", State: ring.ACTIVE, Timestamp: now.Unix(), Tokens: g.GenerateTokens(ring.NewDesc(), "instance-1", "z1", 128, true), Zone: "z1"},
+				"instance-2": {Addr: "127.0.0.2", State: ring.ACTIVE, Timestamp: now.Add(-10 * time.Second).Unix(), Tokens: g.GenerateTokens(ring.NewDesc(), "instance-2", "z1", 128, true), Zone: "z1"},
+				"instance-3": {Addr: "127.0.0.3", State: ring.ACTIVE, Timestamp: now.Add(-20 * time.Second).Unix(), Tokens: g.GenerateTokens(ring.NewDesc(), "instance-3", "z1", 128, true), Zone: "z1"},
+				"instance-4": {Addr: "127.0.0.4", State: ring.PENDING, Timestamp: now.Add(-30 * time.Second).Unix(), Tokens: g.GenerateTokens(ring.NewDesc(), "instance-4", "z1", 128, true), Zone: "z1"},
+			},
+			ringHeartbeatTimeout:  time.Minute,
+			ringReplicationFactor: 3,
+			expectedSet:           []string{"127.0.0.1", "127.0.0.2", "127.0.0.3"},
+			enableAZReplication:   true,
+			expectedFailedZones: map[string]struct{}{
+				"z1": {},
+			},
+			expectedMaxUnavailableZones: 0,
+			expectedMaxError:            1,
+		},
 	}
 
 	for testName, testData := range tests {

pkg/ruler/ruler_test.go

@@ -473,6 +473,11 @@ func TestGetRules(t *testing.T) {
 		"ruler2": "b",
 		"ruler3": "c",
 	}
+	rulerAZSingleZone := map[string]string{
+		"ruler1": "a",
+		"ruler2": "a",
+		"ruler3": "a",
+	}
 
 	expectedRules := expectedRulesMap{
 		"ruler1": map[string]rulespb.RuleGroupList{
@@ -711,6 +716,24 @@ func TestGetRules(t *testing.T) {
 			},
 			expectedClientCallCount: 2, // one of the ruler is pending, so we don't expect that ruler to be called
 		},
+		"Shuffle Sharding and ShardSize = 3 and AZ replication with API Rules backup enabled and one ruler in pending state and rulers are in same az": {
+			sharding:                   true,
+			shuffleShardSize:           3,
+			shardingStrategy:           util.ShardingStrategyShuffle,
+			enableZoneAwareReplication: true,
+			rulerStateMap:              rulerStateMapOnePending,
+			rulerAZMap:                 rulerAZSingleZone,
+			replicationFactor:          3,
+			rulesRequest: RulesRequest{
+				Type: recordingRuleFilter,
+			},
+			expectedCount: map[string]int{
+				"user1": 3,
+				"user2": 5,
+				"user3": 1,
+			},
+			expectedClientCallCount: 2, // one of the ruler is pending, so we don't expect that ruler to be called
+		},
 		"Shuffle Sharding and ShardSize = 3 and AZ replication with API Rules backup enabled and two ruler in pending state": {
 			sharding:                   true,
 			shuffleShardSize:           3,