*: review fixup (will be squashed in)

pkg/kv/kvclient/kvcoord/send_test.go

@@ -104,7 +104,7 @@ func (n Node) UpdateSpanConfigs(
 func (n Node) SpanConfigConformance(
 	context.Context, *roachpb.SpanConfigConformanceRequest,
 ) (*roachpb.SpanConfigConformanceResponse, error) {
-	panic("implement me")
+	panic("unimplemented")
 }
 
 func (n Node) TenantSettings(

pkg/kv/kvserver/allocator/allocatorimpl/allocator_test.go

@@ -3975,7 +3975,7 @@ func TestRemoveCandidatesNumReplicasConstraints(t *testing.T) {
 				StoreID: storeID,
 			}
 		}
-		analyzed := constraint.AnalyzeConstraints(a.StorePool, existingRepls, 0, tc.constraints)
+		analyzed := constraint.AnalyzeConstraints(a.StorePool, existingRepls, 0 /* numReplicas */, tc.constraints)
 
 		// Check behavior in a span config where `voter_constraints` are empty.
 		checkFn := voterConstraintsCheckerForRemoval(analyzed, constraint.EmptyAnalyzedConstraints)

pkg/kv/kvserver/replica_metrics.go

@@ -223,7 +223,7 @@ func calcRangeCounter(
 			// needed{Voters,NonVoters} - we don't care about the
 			// under/over-replication determinations from the report because
 			// it's too magic. We'll do our own determination below.
-			0, 0)
+			0, -1)
 		unavailable = !status.Available
 		liveVoters := calcLiveVoterReplicas(desc, livenessMap)
 		liveNonVoters := calcLiveNonVoterReplicas(desc, livenessMap)

pkg/kv/kvserver/reports/replication_stats_report.go

@@ -404,7 +404,7 @@ func (v *replicationStatsVisitor) countRange(
 		// NB: this reporting code was written before ReplicationStatus reported
 		// on non-voting replicas. This code will also soon be removed in favor
 		// of something that works with multi-tenancy (#89987).
-	}, replicationFactor, 0)
+	}, replicationFactor, -1 /* neededNonVoters */)
 	// Note that a range can be under-replicated and over-replicated at the same
 	// time if it has many replicas, but sufficiently many of them are on dead
 	// nodes.

pkg/roachpb/metadata_replicas.go

@@ -378,7 +378,7 @@ func (d ReplicaSet) HasReplicaOnNode(nodeID NodeID) bool {
 // the replication layer. This is more complicated than just counting the number
 // of replicas due to the existence of joint quorums.
 func (d ReplicaSet) CanMakeProgress(liveFunc func(descriptor ReplicaDescriptor) bool) bool {
-	return d.ReplicationStatus(liveFunc, 0 /* neededVoters */, 0 /* neededNonVoters*/).Available
+	return d.ReplicationStatus(liveFunc, 0 /* neededVoters */, -1 /* neededNonVoters*/).Available
 }
 
 // RangeStatusReport contains info about a range's replication status. Returned
@@ -412,7 +412,8 @@ type RangeStatusReport struct {
 // neededVoters is the replica's desired replication for purposes of determining
 // over/under-replication of voters. If the caller is only interested in
 // availability of voting replicas, 0 can be passed in. neededNonVoters is the
-// counterpart for non-voting replicas.
+// counterpart for non-voting replicas but with -1 as the sentinel value (unlike
+// voters, it's possible to expect 0 non-voters).
 func (d ReplicaSet) ReplicationStatus(
 	liveFunc func(descriptor ReplicaDescriptor) bool, neededVoters int, neededNonVoters int,
 ) RangeStatusReport {
@@ -454,7 +455,7 @@ func (d ReplicaSet) ReplicationStatus(
 	overReplicatedNewGroup := len(votersNewGroup) > neededVoters
 	res.UnderReplicated = underReplicatedOldGroup || underReplicatedNewGroup
 	res.OverReplicated = overReplicatedOldGroup || overReplicatedNewGroup
-	if neededNonVoters == 0 {
+	if neededNonVoters == -1 {
 		return res
 	}
 

pkg/roachpb/span_config.proto

@@ -278,15 +278,23 @@ message SpanConfigEntry {
   SpanConfig config = 2 [(gogoproto.nullable) = false];
 };
 
-// SpanConfigConformanceReport lists out ranges that (i) don't conform to span
-// configs that apply over them, and (ii) are unavailable.
+// SpanConfigConformanceReport reports ranges that (i) don't conform to span
+// configs that apply over them, and (ii) are unavailable. Also included in this
+// report are the IDs of unavailable nodes (possibly contributing to
+// under-replication or range-unavailability).
 message SpanConfigConformanceReport {
-  repeated RangeDescriptor under_replicated = 1 [(gogoproto.nullable) = false];
-  repeated RangeDescriptor over_replicated = 2 [(gogoproto.nullable) = false];
-  repeated RangeDescriptor violating_constraints = 3 [(gogoproto.nullable) = false];
-  repeated RangeDescriptor unavailable = 4 [(gogoproto.nullable) = false];
+  repeated ConformanceReportedRange under_replicated = 1 [(gogoproto.nullable) = false];
+  repeated ConformanceReportedRange over_replicated = 2 [(gogoproto.nullable) = false];
+  repeated ConformanceReportedRange violating_constraints = 3 [(gogoproto.nullable) = false];
+  repeated ConformanceReportedRange unavailable = 4 [(gogoproto.nullable) = false];
+  repeated int32 unavailable_node_ids = 5 [(gogoproto.customname) = "UnavailableNodeIDs"];
 };
 
+message ConformanceReportedRange {
+  RangeDescriptor range_descriptor = 1 [(gogoproto.nullable) = false];
+  SpanConfig config = 2 [(gogoproto.nullable) = false];
+}
+
 // GetSpanConfigsRequest is used to fetch the span configurations and system
 // span configurations.
 message GetSpanConfigsRequest {

pkg/rpc/auth_tenant.go

@@ -115,6 +115,9 @@ func (a tenantAuthorizer) authorize(
 	case "/cockroach.roachpb.Internal/GetSpanConfigs":
 		return a.authGetSpanConfigs(tenID, req.(*roachpb.GetSpanConfigsRequest))
 
+	case "/cockroach.roachpb.Internal/SpanConfigConformance":
+		return a.authSpanConfigConformance(tenID, req.(*roachpb.SpanConfigConformanceRequest))
+
 	case "/cockroach.roachpb.Internal/GetAllSystemSpanConfigsThatApply":
 		return a.authGetAllSystemSpanConfigsThatApply(tenID, req.(*roachpb.GetAllSystemSpanConfigsThatApplyRequest))
 
@@ -339,6 +342,19 @@ func (a tenantAuthorizer) authUpdateSpanConfigs(
 	return nil
 }
 
+// authSpanConfigConformance authorizes the provided tenant to invoke the
+// SpanConfigConformance RPC with the provided args.
+func (a tenantAuthorizer) authSpanConfigConformance(
+	tenID roachpb.TenantID, args *roachpb.SpanConfigConformanceRequest,
+) error {
+	for _, sp := range args.Spans {
+		if err := validateSpan(tenID, sp); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // validateSpanConfigTarget validates that the tenant is authorized to interact
 // with the supplied span config target. In particular, span targets must be
 // wholly contained within the tenant keyspace and system span config targets
@@ -371,28 +387,28 @@ func validateSpanConfigTarget(
 		return nil
 	}
 
-	validateSpan := func(sp roachpb.Span) error {
-		tenSpan := tenantPrefix(tenID)
-		rSpan, err := keys.SpanAddr(sp)
-		if err != nil {
-			return authError(err.Error())
-		}
-		if !tenSpan.ContainsKeyRange(rSpan.Key, rSpan.EndKey) {
-			return authErrorf("requested key span %s not fully contained in tenant keyspace %s", rSpan, tenSpan)
-		}
-		return nil
-	}
-
 	switch spanConfigTarget.Union.(type) {
 	case *roachpb.SpanConfigTarget_Span:
-		return validateSpan(*spanConfigTarget.GetSpan())
+		return validateSpan(tenID, *spanConfigTarget.GetSpan())
 	case *roachpb.SpanConfigTarget_SystemSpanConfigTarget:
 		return validateSystemTarget(*spanConfigTarget.GetSystemSpanConfigTarget())
 	default:
 		return errors.AssertionFailedf("unknown span config target type")
 	}
 }
 
+func validateSpan(tenID roachpb.TenantID, sp roachpb.Span) error {
+	tenSpan := tenantPrefix(tenID)
+	rSpan, err := keys.SpanAddr(sp)
+	if err != nil {
+		return authError(err.Error())
+	}
+	if !tenSpan.ContainsKeyRange(rSpan.Key, rSpan.EndKey) {
+		return authErrorf("requested key span %s not fully contained in tenant keyspace %s", rSpan, tenSpan)
+	}
+	return nil
+}
+
 func contextWithTenant(ctx context.Context, tenID roachpb.TenantID) context.Context {
 	ctx = roachpb.NewContextForTenant(ctx, tenID)
 	ctx = logtags.AddTag(ctx, "tenant", tenID.String())

pkg/rpc/auth_test.go

@@ -221,6 +221,11 @@ func TestTenantAuthRequest(t *testing.T) {
 			},
 		}}
 	}
+	makeSpanConfigConformanceReq := func(
+		span roachpb.Span,
+	) *roachpb.SpanConfigConformanceRequest {
+		return &roachpb.SpanConfigConformanceRequest{Spans: []roachpb.Span{span}}
+	}
 
 	const noError = ""
 	for method, tests := range map[string][]struct {
@@ -700,6 +705,36 @@ func TestTenantAuthRequest(t *testing.T) {
 				expErr: `secondary tenants cannot target the entire keyspace`,
 			},
 		},
+		"/cockroach.roachpb.Internal/SpanConfigConformance": {
+			{
+				req:    &roachpb.SpanConfigConformanceRequest{},
+				expErr: noError,
+			},
+			{
+				req:    makeSpanConfigConformanceReq(makeSpan("a", "b")),
+				expErr: `requested key span {a-b} not fully contained in tenant keyspace /Tenant/1{0-1}`,
+			},
+			{
+				req:    makeSpanConfigConformanceReq(makeSpan(prefix(5, "a"), prefix(5, "b"))),
+				expErr: `requested key span /Tenant/5{a-b} not fully contained in tenant keyspace /Tenant/1{0-1}`,
+			},
+			{
+				req:    makeSpanConfigConformanceReq(makeSpan(prefix(10, "a"), prefix(10, "b"))),
+				expErr: noError,
+			},
+			{
+				req:    makeSpanConfigConformanceReq(makeSpan(prefix(50, "a"), prefix(50, "b"))),
+				expErr: `requested key span /Tenant/50{a-b} not fully contained in tenant keyspace /Tenant/1{0-1}`,
+			},
+			{
+				req:    makeSpanConfigConformanceReq(makeSpan("a", prefix(10, "b"))),
+				expErr: `requested key span {a-/Tenant/10b} not fully contained in tenant keyspace /Tenant/1{0-1}`,
+			},
+			{
+				req:    makeSpanConfigConformanceReq(makeSpan(prefix(10, "a"), prefix(20, "b"))),
+				expErr: `requested key span /Tenant/{10a-20b} not fully contained in tenant keyspace /Tenant/1{0-1}`,
+			},
+		},
 		"/cockroach.roachpb.Internal/GetAllSystemSpanConfigsThatApply": {
 			{
 				req:    &roachpb.GetAllSystemSpanConfigsThatApplyRequest{},

pkg/spanconfig/spanconfigreporter/BUILD.bazel

@@ -11,11 +11,11 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/kv/kvserver/constraint",
+        "//pkg/kv/kvserver/liveness/livenesspb",
         "//pkg/roachpb",
         "//pkg/settings",
         "//pkg/settings/cluster",
         "//pkg/spanconfig",
-        "//pkg/util/log",
         "//pkg/util/rangedesciter",
         "@com_github_cockroachdb_errors//:errors",
     ],
@@ -31,7 +31,9 @@ go_test(
     data = glob(["testdata/**"]),
     deps = [
         ":spanconfigreporter",
+        "//pkg/keys",
         "//pkg/kv/kvserver/constraint",
+        "//pkg/kv/kvserver/liveness/livenesspb",
         "//pkg/roachpb",
         "//pkg/security/securityassets",
         "//pkg/security/securitytest",

pkg/spanconfig/spanconfigreporter/datadriven_test.go

@@ -17,7 +17,9 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/constraint"
+	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	clustersettings "github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
@@ -156,6 +158,9 @@ func TestDataDriven(t *testing.T) {
 					}
 					spans = append(spans, spanconfigtestutils.ParseSpan(t, line))
 				}
+				if len(spans) == 0 {
+					spans = append(spans, keys.EverythingSpan)
+				}
 				report, err := reporter.SpanConfigConformance(ctx, spans)
 				require.NoError(t, err)
 				printRangeDesc := func(r roachpb.RangeDescriptor) string {
@@ -176,13 +181,14 @@ func TestDataDriven(t *testing.T) {
 					buf.WriteString("]")
 					return buf.String()
 				}
-				printList := func(tag string, descs []roachpb.RangeDescriptor) string {
+				printList := func(tag string, ranges []roachpb.ConformanceReportedRange) string {
 					var buf strings.Builder
-					for i, desc := range descs {
+					for i, r := range ranges {
 						if i == 0 {
 							buf.WriteString(fmt.Sprintf("%s:\n", tag))
 						}
-						buf.WriteString(fmt.Sprintf("  %s\n", printRangeDesc(desc)))
+						buf.WriteString(fmt.Sprintf("  %s applying %s\n", printRangeDesc(r.RangeDescriptor),
+							spanconfigtestutils.PrintSpanConfigDiffedAgainstDefaults(r.Config)))
 					}
 					return buf.String()
 				}
@@ -231,11 +237,15 @@ func newMockCluster(
 	}
 }
 
-// IsLive implements spanconfigreporter.Liveness.
-func (s *mockCluster) IsLive(id roachpb.NodeID) (bool, error) {
-	live, found := s.liveness[id]
-	require.True(s.t, found, "undeclared node n%d", id)
-	return live, nil
+// GetIsLiveMap implements spanconfigreporter.Liveness.
+func (s *mockCluster) GetIsLiveMap() livenesspb.IsLiveMap {
+	isLiveMap := livenesspb.IsLiveMap{}
+	for nid, isLive := range s.liveness {
+		isLiveMap[nid] = livenesspb.IsLiveMapEntry{
+			IsLive: isLive,
+		}
+	}
+	return isLiveMap
 }
 
 // GetStoreDescriptor implements constraint.StoreResolver.
@@ -251,7 +261,7 @@ func (s *mockCluster) GetStoreDescriptor(storeID roachpb.StoreID) (roachpb.Store
 
 // Iterate implements rangedesciter.Iterator.
 func (s *mockCluster) Iterate(
-	_ context.Context, _ int, _ func(), fn func(...roachpb.RangeDescriptor) error,
+	_ context.Context, _ int, _ func(), _ roachpb.Span, fn func(...roachpb.RangeDescriptor) error,
 ) error {
 	var descs []roachpb.RangeDescriptor
 	for _, d := range s.ranges {