fix(scylla_client): fix strategy calculation in DescribeRing

Fixes #3745

pkg/scyllaclient/client_scylla.go

@@ -20,6 +20,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/scylladb/go-set/strset"
 	"github.com/scylladb/scylla-manager/v3/pkg/dht"
+	"github.com/scylladb/scylla-manager/v3/pkg/util/maputil"
 	"github.com/scylladb/scylla-manager/v3/pkg/util/slice"
 	"go.uber.org/multierr"
 
@@ -393,6 +394,8 @@ func (c *Client) DescribeRing(ctx context.Context, keyspace string) (Ring, error
 	replicaTokens := make(map[uint64][]TokenRange)
 	replicaHash := make(map[uint64][]string)
 
+	isNetworkTopologyStrategy := true
+	var dcRF map[string]int
 	for _, p := range resp.Payload {
 		// Parse tokens
 		startToken, err := strconv.ParseInt(p.StartToken, 10, 64)
@@ -415,6 +418,18 @@ func (c *Client) DescribeRing(ctx context.Context, keyspace string) (Ring, error
 			EndToken:   endToken,
 		})
 
+		ring.RF = max(ring.RF, len(p.Endpoints))
+		tokenDCrf := make(map[string]int)
+		for _, e := range p.EndpointDetails {
+			tokenDCrf[e.Datacenter]++
+		}
+		// NetworkTopologyStrategy -> all token ranges have the same dc to rf mapping
+		if dcRF == nil || maputil.Equal(dcRF, tokenDCrf) {
+			dcRF = tokenDCrf
+		} else {
+			isNetworkTopologyStrategy = false
+		}
+
 		// Update host to DC mapping
 		for _, e := range p.EndpointDetails {
 			ring.HostDC[e.Host] = e.Datacenter
@@ -443,16 +458,14 @@ func (c *Client) DescribeRing(ctx context.Context, keyspace string) (Ring, error
 	}
 
 	// Detect replication strategy
-	if len(ring.HostDC) == 1 {
+	switch {
+	case len(ring.HostDC) == 1:
 		ring.Replication = LocalStrategy
-	} else {
+	case isNetworkTopologyStrategy:
 		ring.Replication = NetworkTopologyStrategy
-		for _, tokens := range dcTokens {
-			if tokens != len(resp.Payload) {
-				ring.Replication = SimpleStrategy
-				break
-			}
-		}
+		ring.DCrf = dcRF
+	default:
+		ring.Replication = SimpleStrategy
 	}
 
 	return ring, nil

pkg/scyllaclient/client_scylla_integration_test.go

@@ -13,7 +13,9 @@ import (
 	"testing"
 	"time"
 
+	"github.com/scylladb/scylla-manager/v3/pkg/testutils/db"
 	. "github.com/scylladb/scylla-manager/v3/pkg/testutils/testconfig"
+	"github.com/scylladb/scylla-manager/v3/pkg/util/maputil"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/scylladb/go-log"
@@ -71,6 +73,77 @@ func TestClientStatusIntegration(t *testing.T) {
 	}
 }
 
+func TestClientDescribeRingIntegration(t *testing.T) {
+	testCases := []struct {
+		replicationStmt string
+		replication     scyllaclient.ReplicationStrategy
+		rf              int
+		dcRF            map[string]int
+	}{
+		{
+			replicationStmt: "{'class': 'SimpleStrategy', 'replication_factor': 4}",
+			replication:     scyllaclient.SimpleStrategy,
+			rf:              4,
+		},
+		{
+			replicationStmt: "{'class': 'NetworkTopologyStrategy', 'dc1': 1}",
+			replication:     scyllaclient.NetworkTopologyStrategy,
+			rf:              1,
+			dcRF: map[string]int{
+				"dc1": 1,
+			},
+		},
+		{
+			replicationStmt: "{'class': 'NetworkTopologyStrategy', 'dc1': 2, 'dc2': 2}",
+			replication:     scyllaclient.NetworkTopologyStrategy,
+			rf:              4,
+			dcRF: map[string]int{
+				"dc1": 2,
+				"dc2": 2,
+			},
+		},
+		{
+			replicationStmt: "{'class': 'NetworkTopologyStrategy', 'dc1': 3, 'dc2': 3}",
+			replication:     scyllaclient.NetworkTopologyStrategy,
+			rf:              6,
+			dcRF: map[string]int{
+				"dc1": 3,
+				"dc2": 3,
+			},
+		},
+	}
+
+	client, err := scyllaclient.NewClient(scyllaclient.TestConfig(ManagedClusterHosts(), AgentAuthToken()), log.NewDevelopment())
+	if err != nil {
+		t.Fatal(err)
+	}
+	clusterSession := db.CreateSessionAndDropAllKeyspaces(t, client)
+	defer clusterSession.Close()
+
+	for i := range testCases {
+		tc := testCases[i]
+		if err := clusterSession.ExecStmt("DROP KEYSPACE IF EXISTS test_ks"); err != nil {
+			t.Fatal(err)
+		}
+		if err := clusterSession.ExecStmt("CREATE KEYSPACE test_ks WITH replication = " + tc.replicationStmt); err != nil {
+			t.Fatal(err)
+		}
+		ring, err := client.DescribeRing(context.Background(), "test_ks")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if tc.replication != ring.Replication {
+			t.Fatalf("Replication: expected %s, got %s", tc.replication, ring.Replication)
+		}
+		if tc.rf != ring.RF {
+			t.Fatalf("RF: expected %d, got %d", tc.rf, ring.RF)
+		}
+		if !maputil.Equal(tc.dcRF, ring.DCrf) {
+			t.Fatalf("DCrf: expected %v, got %v", tc.dcRF, ring.DCrf)
+		}
+	}
+}
+
 func TestClientActiveRepairsIntegration(t *testing.T) {
 	client, err := scyllaclient.NewClient(scyllaclient.TestConfig(ManagedClusterHosts(), AgentAuthToken()), log.NewDevelopment())
 	if err != nil {

pkg/scyllaclient/model.go

@@ -179,6 +179,8 @@ type Ring struct {
 	ReplicaTokens []ReplicaTokenRanges
 	HostDC        map[string]string
 	Replication   ReplicationStrategy
+	RF            int
+	DCrf          map[string]int // initialized only for NetworkTopologyStrategy
 }
 
 // Datacenters returns a list of datacenters the keyspace is replicated in.

pkg/util/maputil/map.go

@@ -0,0 +1,16 @@
+// Copyright (C) 2024 ScyllaDB
+
+package maputil
+
+// Equal checks if m1 and m2 are the same mapping.
+func Equal[K, V comparable](m1, m2 map[K]V) bool {
+	if len(m1) != len(m2) {
+		return false
+	}
+	for k, v1 := range m1 {
+		if v2, ok := m2[k]; !ok || v1 != v2 {
+			return false
+		}
+	}
+	return true
+}