Skip to content

Commit e73b28b

Browse files
knzknz
committed
server/storage_api: fix tests and simplify test code
Release note: None
1 parent 9fe06cd commit e73b28b

File tree

11 files changed

+143
-41
lines changed

11 files changed

+143
-41
lines changed

‎pkg/server/storage_api/BUILD.bazel‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ go_test(
3838
"//pkg/kv/kvserver/kvserverpb",
3939
"//pkg/kv/kvserver/liveness",
4040
"//pkg/kv/kvserver/liveness/livenesspb",
41+
"//pkg/multitenant/tenantcapabilities",
4142
"//pkg/roachpb",
4243
"//pkg/security/securityassets",
4344
"//pkg/security/securitytest",

‎pkg/server/storage_api/engine_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,15 @@ func TestStatusEngineStatsJson(t *testing.T) {
3434
dir, cleanupFn := testutils.TempDir(t)
3535
defer cleanupFn()
3636

37-
s := serverutils.StartServerOnly(t, base.TestServerArgs{
37+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{
38+
DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(110020),
39+
3840
StoreSpecs: []base.StoreSpec{{
3941
Path: dir,
4042
}},
4143
})
42-
defer s.Stopper().Stop(context.Background())
44+
defer srv.Stopper().Stop(context.Background())
45+
s := srv.ApplicationLayer()
4346

4447
t.Logf("using admin URL %s", s.AdminURL())
4548

‎pkg/server/storage_api/enqueue_test.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,14 @@ func TestEnqueueRange(t *testing.T) {
2929
defer leaktest.AfterTest(t)()
3030
defer log.Scope(t).Close(t)
3131
testCluster := serverutils.StartCluster(t, 3, base.TestClusterArgs{
32+
ServerArgs: base.TestServerArgs{
33+
DefaultTestTenant: base.TestIsSpecificToStorageLayerAndNeedsASystemTenant,
34+
},
35+
3236
ReplicationMode: base.ReplicationManual,
3337
})
3438
defer testCluster.Stopper().Stop(context.Background())
39+
s0 := testCluster.Server(0).SystemLayer()
3540

3641
// Up-replicate r1 to all 3 nodes. We use manual replication to avoid lease
3742
// transfers causing temporary conditions in which no store is the
@@ -85,7 +90,7 @@ func TestEnqueueRange(t *testing.T) {
8590
RangeID: tc.rangeID,
8691
}
8792
var resp serverpb.EnqueueRangeResponse
88-
if err := srvtestutils.PostAdminJSONProto(testCluster.Server(0), "enqueue_range", req, &resp); err != nil {
93+
if err := srvtestutils.PostAdminJSONProto(s0, "enqueue_range", req, &resp); err != nil {
8994
t.Fatal(err)
9095
}
9196
if e, a := tc.expectedDetails, len(resp.Details); e != a {

‎pkg/server/storage_api/files_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,14 @@ func TestStatusGetFiles(t *testing.T) {
3636

3737
storeSpec := base.StoreSpec{Path: tempDir}
3838

39-
ts := serverutils.StartServerOnly(t, base.TestServerArgs{
39+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{
4040
StoreSpecs: []base.StoreSpec{
4141
storeSpec,
4242
},
4343
})
44-
defer ts.Stopper().Stop(context.Background())
44+
defer srv.Stopper().Stop(context.Background())
45+
46+
ts := srv.ApplicationLayer()
4547

4648
client := ts.GetStatusClient(t)
4749

‎pkg/server/storage_api/gossip_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,14 @@ func TestStatusGossipJson(t *testing.T) {
2828
defer leaktest.AfterTest(t)()
2929
defer log.Scope(t).Close(t)
3030

31-
s := serverutils.StartServerOnly(t, base.TestServerArgs{})
32-
defer s.Stopper().Stop(context.Background())
31+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{
32+
DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(110022),
33+
})
34+
defer srv.Stopper().Stop(context.Background())
35+
s := srv.ApplicationLayer()
36+
37+
// TODO(#110022): grant a special capability to the secondary tenant
38+
// before the endpoint can be accessed.
3339

3440
var data gossip.InfoStatus
3541
if err := srvtestutils.GetStatusJSONProto(s, "gossip/local", &data); err != nil {

‎pkg/server/storage_api/health_test.go‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/cockroachdb/cockroach/pkg/base"
1818
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness"
1919
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
20+
"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities"
2021
"github.com/cockroachdb/cockroach/pkg/roachpb"
2122
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
2223
"github.com/cockroachdb/cockroach/pkg/server/srvtestutils"
@@ -27,6 +28,7 @@ import (
2728
"github.com/cockroachdb/cockroach/pkg/util/log"
2829
"github.com/cockroachdb/errors"
2930
"github.com/stretchr/testify/assert"
31+
"github.com/stretchr/testify/require"
3032
)
3133

3234
func TestHealthAPI(t *testing.T) {
@@ -109,13 +111,26 @@ func TestLivenessAPI(t *testing.T) {
109111
tc := testcluster.StartTestCluster(t, 3, base.TestClusterArgs{})
110112
defer tc.Stopper().Stop(context.Background())
111113

112-
startTime := tc.Server(0).Clock().PhysicalNow()
114+
// The liveness endpoint needs a special tenant capability.
115+
if tc.Server(0).TenantController().StartedDefaultTestTenant() {
116+
// Enable access to the nodes endpoint for the test tenant.
117+
_, err := tc.SystemLayer(0).SQLConn(t, "").Exec(
118+
`ALTER TENANT [$1] GRANT CAPABILITY can_view_node_info=true`, serverutils.TestTenantID().ToUint64())
119+
require.NoError(t, err)
120+
121+
tc.WaitForTenantCapabilities(t, serverutils.TestTenantID(), map[tenantcapabilities.ID]string{
122+
tenantcapabilities.CanViewNodeInfo: "true",
123+
})
124+
}
125+
126+
ts := tc.Server(0).ApplicationLayer()
127+
startTime := ts.Clock().PhysicalNow()
113128

114129
// We need to retry because the gossiping of liveness status is an
115130
// asynchronous process.
116131
testutils.SucceedsSoon(t, func() error {
117132
var resp serverpb.LivenessResponse
118-
if err := serverutils.GetJSONProto(tc.Server(0), "/_admin/v1/liveness", &resp); err != nil {
133+
if err := serverutils.GetJSONProto(ts, "/_admin/v1/liveness", &resp); err != nil {
119134
return err
120135
}
121136
if a, e := len(resp.Livenesses), tc.NumServers(); a != e {
@@ -126,7 +141,7 @@ func TestLivenessAPI(t *testing.T) {
126141
livenessMap[l.NodeID] = l
127142
}
128143
for i := 0; i < tc.NumServers(); i++ {
129-
s := tc.Server(i)
144+
s := tc.Server(i).StorageLayer()
130145
sl, ok := livenessMap[s.NodeID()]
131146
if !ok {
132147
return errors.Errorf("found no liveness record for node %d", s.NodeID())

‎pkg/server/storage_api/logfiles_test.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,11 @@ func TestStatusLocalLogs(t *testing.T) {
5050
// there's just one.
5151
defer s.SetupSingleFileLogging()()
5252

53-
ts := serverutils.StartServerOnly(t, base.TestServerArgs{})
54-
defer ts.Stopper().Stop(context.Background())
53+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{})
54+
defer srv.Stopper().Stop(context.Background())
55+
ts := srv.ApplicationLayer()
56+
57+
logCtx := ts.AnnotateCtx(context.Background())
5558

5659
// Log an error of each main type which we expect to be able to retrieve.
5760
// The resolution of our log timestamps is such that it's possible to get
@@ -65,15 +68,15 @@ func TestStatusLocalLogs(t *testing.T) {
6568
const sleepBuffer = time.Microsecond * 20
6669
timestamp := timeutil.Now().UnixNano()
6770
time.Sleep(sleepBuffer)
68-
log.Errorf(context.Background(), "TestStatusLocalLogFile test message-Error")
71+
log.Errorf(logCtx, "TestStatusLocalLogFile test message-Error")
6972
time.Sleep(sleepBuffer)
7073
timestampE := timeutil.Now().UnixNano()
7174
time.Sleep(sleepBuffer)
72-
log.Warningf(context.Background(), "TestStatusLocalLogFile test message-Warning")
75+
log.Warningf(logCtx, "TestStatusLocalLogFile test message-Warning")
7376
time.Sleep(sleepBuffer)
7477
timestampEW := timeutil.Now().UnixNano()
7578
time.Sleep(sleepBuffer)
76-
log.Infof(context.Background(), "TestStatusLocalLogFile test message-Info")
79+
log.Infof(logCtx, "TestStatusLocalLogFile test message-Info")
7780
time.Sleep(sleepBuffer)
7881
timestampEWI := timeutil.Now().UnixNano()
7982

@@ -205,8 +208,11 @@ func TestStatusLocalLogsTenantFilter(t *testing.T) {
205208
// there's just one.
206209
defer sc.SetupSingleFileLogging()()
207210

208-
ts := serverutils.StartServerOnly(t, base.TestServerArgs{})
209-
defer ts.Stopper().Stop(context.Background())
211+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{
212+
DefaultTestTenant: base.TestControlsTenantsExplicitly,
213+
})
214+
defer srv.Stopper().Stop(context.Background())
215+
ts := srv.ApplicationLayer()
210216

211217
appTenantID := roachpb.MustMakeTenantID(uint64(2))
212218
ctxSysTenant, ctxAppTenant := server.TestingMakeLoggingContexts(appTenantID)
@@ -336,11 +342,13 @@ func TestStatusLogRedaction(t *testing.T) {
336342
// Apply the redactable log boolean for this test.
337343
defer log.TestingSetRedactable(redactableLogs)()
338344

339-
ts := serverutils.StartServerOnly(t, base.TestServerArgs{})
340-
defer ts.Stopper().Stop(context.Background())
345+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{})
346+
defer srv.Stopper().Stop(context.Background())
347+
ts := srv.ApplicationLayer()
341348

342349
// Log something.
343-
log.Infof(context.Background(), "THISISSAFE %s", "THISISUNSAFE")
350+
logCtx := ts.AnnotateCtx(context.Background())
351+
log.Infof(logCtx, "THISISSAFE %s", "THISISUNSAFE")
344352

345353
// Determine the log file name.
346354
var wrapper serverpb.LogFilesListResponse

‎pkg/server/storage_api/network_test.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,21 @@ func TestNetworkConnectivity(t *testing.T) {
3131
defer log.Scope(t).Close(t)
3232
numNodes := 3
3333
testCluster := serverutils.StartCluster(t, numNodes, base.TestClusterArgs{
34+
ServerArgs: base.TestServerArgs{
35+
DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(110024),
36+
},
37+
3438
ReplicationMode: base.ReplicationManual,
3539
})
3640
ctx := context.Background()
3741
defer testCluster.Stopper().Stop(ctx)
38-
ts := testCluster.Server(0)
42+
43+
// TODO(#110024): grant the appropriate capability to the test
44+
// tenant before the connectivity endpoint can be accessed. See
45+
// example in `TestNodeStatusResponse`.
46+
47+
s0 := testCluster.Server(0)
48+
ts := s0.ApplicationLayer()
3949

4050
var resp serverpb.NetworkConnectivityResponse
4151
// Should wait because endpoint relies on Gossip.
@@ -60,11 +70,11 @@ func TestNetworkConnectivity(t *testing.T) {
6070
return err
6171
}
6272
require.Equal(t, len(resp.Connections), numNodes-1)
63-
fmt.Printf("got status: %s", resp.Connections[ts.NodeID()].Peers[stoppedNodeID].Status.String())
64-
if resp.Connections[ts.NodeID()].Peers[stoppedNodeID].Status != serverpb.NetworkConnectivityResponse_ERROR {
73+
fmt.Printf("got status: %s", resp.Connections[s0.StorageLayer().NodeID()].Peers[stoppedNodeID].Status.String())
74+
if resp.Connections[s0.StorageLayer().NodeID()].Peers[stoppedNodeID].Status != serverpb.NetworkConnectivityResponse_ERROR {
6575
return errors.New("waiting for connection state to be changed.")
6676
}
67-
if latency := resp.Connections[ts.NodeID()].Peers[stoppedNodeID].Latency; latency > 0 {
77+
if latency := resp.Connections[s0.StorageLayer().NodeID()].Peers[stoppedNodeID].Latency; latency > 0 {
6878
return errors.Errorf("expected latency to be 0 but got %s", latency.String())
6979
}
7080
return nil

‎pkg/server/storage_api/nodes_test.go‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717

1818
"github.com/cockroachdb/cockroach/pkg/base"
1919
"github.com/cockroachdb/cockroach/pkg/build"
20+
"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities"
2021
"github.com/cockroachdb/cockroach/pkg/roachpb"
2122
"github.com/cockroachdb/cockroach/pkg/server"
2223
"github.com/cockroachdb/cockroach/pkg/server/apiconstants"
@@ -29,6 +30,7 @@ import (
2930
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
3031
"github.com/cockroachdb/cockroach/pkg/util/log"
3132
"github.com/pkg/errors"
33+
"github.com/stretchr/testify/require"
3234
)
3335

3436
// TestStatusJson verifies that status endpoints return expected Json results.
@@ -86,9 +88,25 @@ func TestStatusJson(t *testing.T) {
8688
func TestNodeStatusResponse(t *testing.T) {
8789
defer leaktest.AfterTest(t)()
8890
defer log.Scope(t).Close(t)
89-
s := serverutils.StartServerOnly(t, base.TestServerArgs{})
90-
defer s.Stopper().Stop(context.Background())
91-
node := s.Node().(*server.Node)
91+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{
92+
DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(110023),
93+
})
94+
defer srv.Stopper().Stop(context.Background())
95+
96+
if srv.TenantController().StartedDefaultTestTenant() {
97+
// Enable access to the nodes endpoint for the test tenant.
98+
_, err := srv.SystemLayer().SQLConn(t, "").Exec(
99+
`ALTER TENANT [$1] GRANT CAPABILITY can_view_node_info=true`, serverutils.TestTenantID().ToUint64())
100+
require.NoError(t, err)
101+
102+
serverutils.WaitForTenantCapabilities(t, srv, serverutils.TestTenantID(), map[tenantcapabilities.ID]string{
103+
tenantcapabilities.CanViewNodeInfo: "true",
104+
}, "")
105+
}
106+
107+
s := srv.ApplicationLayer()
108+
109+
node := srv.StorageLayer().Node().(*server.Node)
92110

93111
wrapper := serverpb.NodesResponse{}
94112

@@ -187,11 +205,25 @@ func TestNodesGRPCResponse(t *testing.T) {
187205

188206
ctx := context.Background()
189207

190-
s := serverutils.StartServerOnly(t, base.TestServerArgs{})
191-
defer s.Stopper().Stop(ctx)
208+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{
209+
DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(110023),
210+
})
211+
defer srv.Stopper().Stop(ctx)
212+
213+
if srv.TenantController().StartedDefaultTestTenant() {
214+
// Enable access to the nodes endpoint for the test tenant.
215+
_, err := srv.SystemLayer().SQLConn(t, "").Exec(
216+
`ALTER TENANT [$1] GRANT CAPABILITY can_view_node_info=true`, serverutils.TestTenantID().ToUint64())
217+
require.NoError(t, err)
218+
219+
serverutils.WaitForTenantCapabilities(t, srv, serverutils.TestTenantID(), map[tenantcapabilities.ID]string{
220+
tenantcapabilities.CanViewNodeInfo: "true",
221+
}, "")
222+
}
192223

193224
var request serverpb.NodesRequest
194225

226+
s := srv.ApplicationLayer()
195227
client := s.GetStatusClient(t)
196228

197229
response, err := client.Nodes(ctx, &request)

‎pkg/server/storage_api/raft_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@ import (
2727
func TestRaftDebug(t *testing.T) {
2828
defer leaktest.AfterTest(t)()
2929
defer log.Scope(t).Close(t)
30-
s := serverutils.StartServerOnly(t, base.TestServerArgs{})
31-
defer s.Stopper().Stop(context.Background())
30+
srv := serverutils.StartServerOnly(t, base.TestServerArgs{
31+
DefaultTestTenant: base.TestIsSpecificToStorageLayerAndNeedsASystemTenant,
32+
})
33+
defer srv.Stopper().Stop(context.Background())
34+
s := srv.SystemLayer()
3235

3336
var resp serverpb.RaftDebugResponse
3437
if err := srvtestutils.GetStatusJSONProto(s, "raft", &resp); err != nil {

0 commit comments

Comments
 (0)