Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[vtctld | tests only] testtmclient refactor #7518

Merged
merged 8 commits into from
Feb 22, 2021
Merged

[vtctld | tests only] testtmclient refactor #7518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
329b70d
Add testutil vtctld constructor for fake TabletManagerClient impls
ajm188 Feb 18, 2021
9e6ee84
Rename global TabletManagerClient variable
ajm188 Feb 18, 2021
65fc52b
Move `emergencyReparenterTestTMClient` to `testutil.TabletManagerClient`
ajm188 Feb 18, 2021
9bdf61b
Move `ChangeType` into non-singleton test TabletManagerClient, update…
ajm188 Feb 18, 2021
af856e8
Remove `SetMaster` from the singleton tmclient implementation
ajm188 Feb 18, 2021
42f9ee3
Move `GetSchema` implementation to non-singleton, update tests
ajm188 Feb 18, 2021
00d157a
Remove the TODO comment now that it's TODONE
ajm188 Feb 18, 2021
12c6437
Fix tests
ajm188 Feb 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
161 changes: 100 additions & 61 deletions go/vt/vtadmin/api_test.go
View file
Open in desktop
Expand Up @@ -45,17 +45,14 @@ import (
"vitess.io/vitess/go/vt/vttablet/tmclient"

querypb "vitess.io/vitess/go/vt/proto/query"
"vitess.io/vitess/go/vt/proto/tabletmanagerdata"
tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
vtadminpb "vitess.io/vitess/go/vt/proto/vtadmin"
vtctldatapb "vitess.io/vitess/go/vt/proto/vtctldata"
vtctlservicepb "vitess.io/vitess/go/vt/proto/vtctlservice"
"vitess.io/vitess/go/vt/proto/vttime"
)

func init() {
*tmclient.TabletManagerProtocol = testutil.TabletManagerClientProtocol
}

func TestGetClusters(t *testing.T) {
tests := []struct {
name string
Expand Down Expand Up @@ -277,11 +274,13 @@ func TestGetKeyspaces(t *testing.T) {
}

func TestGetSchemas(t *testing.T) {
t.Parallel()

tests := []struct {
name string
clusterTablets [][]*vtadminpb.Tablet
// Indexed by tablet alias
tabletSchemas map[string]*tabletmanagerdata.SchemaDefinition
tabletSchemas map[string]*tabletmanagerdatapb.SchemaDefinition
req *vtadminpb.GetSchemasRequest
expected *vtadminpb.GetSchemasResponse
}{
Expand Down Expand Up @@ -315,10 +314,10 @@ func TestGetSchemas(t *testing.T) {
},
},
},
tabletSchemas: map[string]*tabletmanagerdata.SchemaDefinition{
tabletSchemas: map[string]*tabletmanagerdatapb.SchemaDefinition{
"c0_cell1-0000000100": {
DatabaseSchema: "CREATE DATABASE vt_testkeyspace",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t1",
Schema: `CREATE TABLE t1 (id int(11) not null,PRIMARY KEY (id));`,
Expand All @@ -345,7 +344,7 @@ func TestGetSchemas(t *testing.T) {
Name: "cluster0",
},
Keyspace: "commerce",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t1",
Schema: `CREATE TABLE t1 (id int(11) not null,PRIMARY KEY (id));`,
Expand Down Expand Up @@ -395,10 +394,10 @@ func TestGetSchemas(t *testing.T) {
},
},
},
tabletSchemas: map[string]*tabletmanagerdata.SchemaDefinition{
tabletSchemas: map[string]*tabletmanagerdatapb.SchemaDefinition{
"c0_cell1-0000000100": {
DatabaseSchema: "CREATE DATABASE vt_testkeyspace",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t1",
Schema: `CREATE TABLE t1 (id int(11) not null,PRIMARY KEY (id));`,
Expand All @@ -417,7 +416,7 @@ func TestGetSchemas(t *testing.T) {
},
"c1_cell1-0000000100": {
DatabaseSchema: "CREATE DATABASE vt_testkeyspace",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t2",
Schema: `CREATE TABLE t2 (id int(11) not null,PRIMARY KEY (id));`,
Expand All @@ -444,7 +443,7 @@ func TestGetSchemas(t *testing.T) {
Name: "cluster0",
},
Keyspace: "commerce",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t1",
Schema: `CREATE TABLE t1 (id int(11) not null,PRIMARY KEY (id));`,
Expand All @@ -467,7 +466,7 @@ func TestGetSchemas(t *testing.T) {
Name: "cluster1",
},
Keyspace: "commerce",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t2",
Schema: `CREATE TABLE t2 (id int(11) not null,PRIMARY KEY (id));`,
Expand Down Expand Up @@ -517,10 +516,10 @@ func TestGetSchemas(t *testing.T) {
},
},
},
tabletSchemas: map[string]*tabletmanagerdata.SchemaDefinition{
tabletSchemas: map[string]*tabletmanagerdatapb.SchemaDefinition{
"c0_cell1-0000000100": {
DatabaseSchema: "CREATE DATABASE vt_testkeyspace",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t1",
Schema: `CREATE TABLE t1 (id int(11) not null,PRIMARY KEY (id));`,
Expand All @@ -539,7 +538,7 @@ func TestGetSchemas(t *testing.T) {
},
"c1_cell1-0000000100": {
DatabaseSchema: "CREATE DATABASE vt_testkeyspace",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t2",
Schema: `CREATE TABLE t2 (id int(11) not null,PRIMARY KEY (id));`,
Expand Down Expand Up @@ -568,7 +567,7 @@ func TestGetSchemas(t *testing.T) {
Name: "cluster1",
},
Keyspace: "commerce",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t2",
Schema: `CREATE TABLE t2 (id int(11) not null,PRIMARY KEY (id));`,
Expand Down Expand Up @@ -605,10 +604,10 @@ func TestGetSchemas(t *testing.T) {
},
},
},
tabletSchemas: map[string]*tabletmanagerdata.SchemaDefinition{
tabletSchemas: map[string]*tabletmanagerdatapb.SchemaDefinition{
"c0_cell1-0000000100": {
DatabaseSchema: "CREATE DATABASE vt_testkeyspace",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t1",
Schema: `CREATE TABLE t1 (id int(11) not null,PRIMARY KEY (id));`,
Expand Down Expand Up @@ -650,7 +649,7 @@ func TestGetSchemas(t *testing.T) {
},
},
},
tabletSchemas: map[string]*tabletmanagerdata.SchemaDefinition{},
tabletSchemas: map[string]*tabletmanagerdatapb.SchemaDefinition{},
req: &vtadminpb.GetSchemasRequest{},
expected: &vtadminpb.GetSchemasResponse{
Schemas: []*vtadminpb.Schema{},
Expand All @@ -673,10 +672,10 @@ func TestGetSchemas(t *testing.T) {
},
},
},
tabletSchemas: map[string]*tabletmanagerdata.SchemaDefinition{
tabletSchemas: map[string]*tabletmanagerdatapb.SchemaDefinition{
"c0_cell1-0000000100": {
DatabaseSchema: "CREATE DATABASE vt_testkeyspace",
TableDefinitions: []*tabletmanagerdata.TableDefinition{
TableDefinitions: []*tabletmanagerdatapb.TableDefinition{
{
Name: "t1",
Schema: `CREATE TABLE t1 (id int(11) not null,PRIMARY KEY (id));`,
Expand All @@ -702,50 +701,76 @@ func TestGetSchemas(t *testing.T) {
}

for _, tt := range tests {
testutil.TabletManagerClient.Schemas = map[string]*tabletmanagerdata.SchemaDefinition{}

topos := []*topo.Server{
memorytopo.NewServer("c0_cell1"),
memorytopo.NewServer("c1_cell1"),
}

// Setting up WithTestServer in a generic, recursive way is... unpleasant,
// so all tests are set-up and run in the context of these two clusters.
testutil.WithTestServer(t, grpcvtctldserver.NewVtctldServer(topos[0]), func(t *testing.T, cluster0Client vtctldclient.VtctldClient) {
testutil.WithTestServer(t, grpcvtctldserver.NewVtctldServer(topos[1]), func(t *testing.T, cluster1Client vtctldclient.VtctldClient) {
// Put 'em in a slice so we can look them up by index
clusterClients := []vtctldclient.VtctldClient{cluster0Client, cluster1Client}

// Build the clusters
clusters := make([]*cluster.Cluster, len(topos))
for cdx, toposerver := range topos {
// Handle when a test doesn't define any tablets for a given cluster.
var cts []*vtadminpb.Tablet
if cdx < len(tt.clusterTablets) {
cts = tt.clusterTablets[cdx]
}
tt := tt

for _, tablet := range cts {
// AddTablet also adds the keyspace + shard for us.
testutil.AddTablet(context.Background(), t, toposerver, tablet.Tablet, nil)
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

// Adds each SchemaDefinition to the fake TabletManagerClient, or nil
// if there are no schemas for that tablet. (All tablet aliases must
// exist in the map. Otherwise, TabletManagerClient will return an error when
// looking up the schema with tablet alias that doesn't exist.)
alias := topoproto.TabletAliasString(tablet.Tablet.Alias)
testutil.TabletManagerClient.Schemas[alias] = tt.tabletSchemas[alias]
}
topos := []*topo.Server{
memorytopo.NewServer("c0_cell1"),
memorytopo.NewServer("c1_cell1"),
}

clusters[cdx] = buildCluster(cdx, clusterClients[cdx], cts, nil)
}
tmc := testutil.TabletManagerClient{
GetSchemaResults: map[string]struct {
Schema *tabletmanagerdatapb.SchemaDefinition
Error error
}{},
}

vtctlds := []vtctlservicepb.VtctldServer{
testutil.NewVtctldServerWithTabletManagerClient(t, topos[0], &tmc, func(ts *topo.Server) vtctlservicepb.VtctldServer {
return grpcvtctldserver.NewVtctldServer(ts)
}),
testutil.NewVtctldServerWithTabletManagerClient(t, topos[1], &tmc, func(ts *topo.Server) vtctlservicepb.VtctldServer {
return grpcvtctldserver.NewVtctldServer(ts)
}),
}

api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
// Setting up WithTestServer in a generic, recursive way is... unpleasant,
// so all tests are set-up and run in the context of these two clusters.
testutil.WithTestServer(t, vtctlds[0], func(t *testing.T, cluster0Client vtctldclient.VtctldClient) {
testutil.WithTestServer(t, vtctlds[1], func(t *testing.T, cluster1Client vtctldclient.VtctldClient) {
// Put 'em in a slice so we can look them up by index
clusterClients := []vtctldclient.VtctldClient{cluster0Client, cluster1Client}

resp, err := api.GetSchemas(context.Background(), tt.req)
require.NoError(t, err)
// Build the clusters
clusters := make([]*cluster.Cluster, len(topos))
for cdx, toposerver := range topos {
// Handle when a test doesn't define any tablets for a given cluster.
var cts []*vtadminpb.Tablet
if cdx < len(tt.clusterTablets) {
cts = tt.clusterTablets[cdx]
}

vtadmintestutil.AssertSchemaSlicesEqual(t, tt.expected.Schemas, resp.Schemas, tt.name)
for _, tablet := range cts {
// AddTablet also adds the keyspace + shard for us.
testutil.AddTablet(context.Background(), t, toposerver, tablet.Tablet, nil)

// Adds each SchemaDefinition to the fake TabletManagerClient, or nil
// if there are no schemas for that tablet. (All tablet aliases must
// exist in the map. Otherwise, TabletManagerClient will return an error when
// looking up the schema with tablet alias that doesn't exist.)
alias := topoproto.TabletAliasString(tablet.Tablet.Alias)
tmc.GetSchemaResults[alias] = struct {
Schema *tabletmanagerdatapb.SchemaDefinition
Error error
}{
Schema: tt.tabletSchemas[alias],
Error: nil,
}
}

clusters[cdx] = buildCluster(cdx, clusterClients[cdx], cts, nil)
}

api := NewAPI(clusters, grpcserver.Options{}, http.Options{})

resp, err := api.GetSchemas(context.Background(), tt.req)
require.NoError(t, err)

vtadmintestutil.AssertSchemaSlicesEqual(t, tt.expected.Schemas, resp.Schemas, tt.name)
})
})
})
}
Expand Down Expand Up @@ -1234,3 +1259,17 @@ func buildCluster(i int, vtctldClient vtctldclient.VtctldClient, tablets []*vtad

return cluster
}

func init() {
// For tests that don't actually care about mocking the tmclient (i.e. they
// call grpcvtctldserver.NewVtctldServer to initialize the unit under test),
// this needs to be set.
//
// Tests that do care about the tmclient should use
// testutil.NewVtctldServerWithTabletManagerClient to initialize their
// VtctldServer.
*tmclient.TabletManagerProtocol = "vtadmin.test"
tmclient.RegisterTabletManagerClientFactory("vtadmin.test", func() tmclient.TabletManagerClient {
return nil
})
}
31 changes: 17 additions & 14 deletions go/vt/vtctl/endtoend/get_schema_test.go
View file
Open in desktop
Expand Up @@ -138,14 +138,26 @@ func TestGetSchema(t *testing.T) {
},
}

tmc := testutil.TabletManagerClient
tmc.Schemas[topoproto.TabletAliasString(tablet.Alias)] = sd
tmc := testutil.TabletManagerClient{
GetSchemaResults: map[string]struct {
Schema *tabletmanagerdatapb.SchemaDefinition
Error error
}{
topoproto.TabletAliasString(tablet.Alias): {
Schema: sd,
Error: nil,
},
},
}

*tmclient.TabletManagerProtocol = testutil.TabletManagerClientProtocol
tmclient.RegisterTabletManagerClientFactory(t.Name(), func() tmclient.TabletManagerClient {
return &tmc
})
*tmclient.TabletManagerProtocol = t.Name()

logger := logutil.NewMemoryLogger()

err := vtctl.RunCommand(ctx, wrangler.New(logger, topo, tmc), []string{
err := vtctl.RunCommand(ctx, wrangler.New(logger, topo, &tmc), []string{
"GetSchema",
topoproto.TabletAliasString(tablet.Alias),
})
Expand Down Expand Up @@ -185,7 +197,7 @@ func TestGetSchema(t *testing.T) {
},
}

err = vtctl.RunCommand(ctx, wrangler.New(logger, topo, tmc), []string{
err = vtctl.RunCommand(ctx, wrangler.New(logger, topo, &tmc), []string{
"GetSchema",
"-table_sizes_only",
topoproto.TabletAliasString(tablet.Alias),
Expand All @@ -202,12 +214,3 @@ func TestGetSchema(t *testing.T) {

assert.Equal(t, sd, actual)
}

func init() {
// enforce we will use the right protocol (gRPC) (note the
// client is unused, but it is initialized, so it needs to exist)
*tmclient.TabletManagerProtocol = "grpc"
tmclient.RegisterTabletManagerClientFactory("grpc", func() tmclient.TabletManagerClient {
return nil
})
}