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.

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

[vtadmin] Add Options struct to wrap grpc/http options #8461

Merged
merged 1 commit into from
Jul 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion go/cmd/vtadmin/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,10 @@ func run(cmd *cobra.Command, args []string) {
clusters[i] = cluster
}

s := vtadmin.NewAPI(clusters, opts, httpOpts)
s := vtadmin.NewAPI(clusters, vtadmin.Options{
GRPCOpts: opts,
HTTPOpts: httpOpts,
})
bootSpan.Finish()

if err := s.ListenAndServe(); err != nil {
Expand Down
31 changes: 19 additions & 12 deletions go/vt/vtadmin/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,21 @@ type API struct {
vtexplainLock sync.Mutex
}

// Options wraps the configuration options for different components of the
// vtadmin API.
type Options struct {
GRPCOpts grpcserver.Options
HTTPOpts vtadminhttp.Options
}

// NewAPI returns a new API, configured to service the given set of clusters,
// and configured with the given gRPC and HTTP server options.
// and configured with the given options.
//
// If opts.Services is nil, NewAPI will automatically add
// If opts.GRPCOpts.Services is nil, NewAPI will automatically add
// "vtadmin.VTAdminServer" to the list of services queryable in the healthcheck
// service. Callers can opt-out of this behavior by explicitly setting this
// value to the empty slice.
func NewAPI(clusters []*cluster.Cluster, opts grpcserver.Options, httpOpts vtadminhttp.Options) *API {
func NewAPI(clusters []*cluster.Cluster, opts Options) *API {
clusterMap := make(map[string]*cluster.Cluster, len(clusters))
for _, cluster := range clusters {
clusterMap[cluster.ID] = cluster
Expand All @@ -86,11 +93,11 @@ func NewAPI(clusters []*cluster.Cluster, opts grpcserver.Options, httpOpts vtadm
return c1.ID < c2.ID
}).Sort(clusters)

if opts.Services == nil {
opts.Services = []string{"vtadmin.VTAdminServer"}
if opts.GRPCOpts.Services == nil {
opts.GRPCOpts.Services = []string{"vtadmin.VTAdminServer"}
}

serv := grpcserver.New("vtadmin", opts)
serv := grpcserver.New("vtadmin", opts.GRPCOpts)
serv.Router().HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte("ok\n"))
})
Expand All @@ -106,7 +113,7 @@ func NewAPI(clusters []*cluster.Cluster, opts grpcserver.Options, httpOpts vtadm

vtadminpb.RegisterVTAdminServer(serv.GRPCServer(), api)

httpAPI := vtadminhttp.NewAPI(api, httpOpts)
httpAPI := vtadminhttp.NewAPI(api, opts.HTTPOpts)

router.HandleFunc("/backups", httpAPI.Adapt(vtadminhttp.GetBackups)).Name("API.GetBackups")
router.HandleFunc("/clusters", httpAPI.Adapt(vtadminhttp.GetClusters)).Name("API.GetClusters")
Expand All @@ -129,7 +136,7 @@ func NewAPI(clusters []*cluster.Cluster, opts grpcserver.Options, httpOpts vtadm
experimentalRouter := router.PathPrefix("/experimental").Subrouter()
experimentalRouter.HandleFunc("/tablet/{tablet}/debug/vars", httpAPI.Adapt(experimental.TabletDebugVarsPassthrough)).Name("API.TabletDebugVarsPassthrough")

if !httpOpts.DisableDebug {
if !opts.HTTPOpts.DisableDebug {
// Due to the way net/http/pprof insists on registering its handlers, we
// have to put these on the root router, and not on the /debug prefixed
// subrouter, which would make way more sense, but alas. Additional
Expand All @@ -153,16 +160,16 @@ func NewAPI(clusters []*cluster.Cluster, opts grpcserver.Options, httpOpts vtadm
// 3. Tracing
middlewares := []mux.MiddlewareFunc{}

if len(httpOpts.CORSOrigins) > 0 {
if len(opts.HTTPOpts.CORSOrigins) > 0 {
serv.Router().Use(handlers.CORS(
handlers.AllowCredentials(), handlers.AllowedOrigins(httpOpts.CORSOrigins)))
handlers.AllowCredentials(), handlers.AllowedOrigins(opts.HTTPOpts.CORSOrigins)))
}

if !httpOpts.DisableCompression {
if !opts.HTTPOpts.DisableCompression {
middlewares = append(middlewares, handlers.CompressHandler)
}

if httpOpts.EnableTracing {
if opts.HTTPOpts.EnableTracing {
middlewares = append(middlewares, vthandlers.TraceHandler)
}

Expand Down
48 changes: 19 additions & 29 deletions go/vt/vtadmin/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ import (
"vitess.io/vitess/go/vt/vtadmin/cluster"
"vitess.io/vitess/go/vt/vtadmin/cluster/discovery/fakediscovery"
vtadminerrors "vitess.io/vitess/go/vt/vtadmin/errors"
"vitess.io/vitess/go/vt/vtadmin/grpcserver"
"vitess.io/vitess/go/vt/vtadmin/http"
vtadmintestutil "vitess.io/vitess/go/vt/vtadmin/testutil"
"vitess.io/vitess/go/vt/vtadmin/vtctldclient/fakevtctldclient"
"vitess.io/vitess/go/vt/vtctl/grpcvtctldserver"
Expand Down Expand Up @@ -517,7 +515,7 @@ func TestFindSchema(t *testing.T) {
clusters[i] = vtadmintestutil.BuildCluster(t, cfg)
}

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

resp, err := api.FindSchema(ctx, tt.req)
if tt.shouldErr {
Expand Down Expand Up @@ -726,7 +724,7 @@ func TestFindSchema(t *testing.T) {
},
)

api := NewAPI([]*cluster.Cluster{c1, c2}, grpcserver.Options{}, http.Options{})
api := NewAPI([]*cluster.Cluster{c1, c2}, Options{})
schema, err := api.FindSchema(ctx, &vtadminpb.FindSchemaRequest{
Table: "testtable",
TableSizeOptions: &vtadminpb.GetSchemaTableSizeOptions{
Expand Down Expand Up @@ -822,7 +820,7 @@ func TestGetClusters(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

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

resp, err := api.GetClusters(ctx, &vtadminpb.GetClustersRequest{})
assert.NoError(t, err)
Expand Down Expand Up @@ -900,7 +898,7 @@ func TestGetGates(t *testing.T) {
},
}

api := NewAPI([]*cluster.Cluster{cluster1, cluster2}, grpcserver.Options{}, http.Options{})
api := NewAPI([]*cluster.Cluster{cluster1, cluster2}, Options{})
ctx := context.Background()

resp, err := api.GetGates(ctx, &vtadminpb.GetGatesRequest{})
Expand Down Expand Up @@ -1038,7 +1036,7 @@ func TestGetKeyspace(t *testing.T) {
})
}

api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
api := NewAPI(clusters, Options{})
ks, err := api.GetKeyspace(ctx, tt.req)
if tt.shouldErr {
assert.Error(t, err)
Expand Down Expand Up @@ -1291,7 +1289,7 @@ func TestGetKeyspaces(t *testing.T) {
}),
}

api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
api := NewAPI(clusters, Options{})
resp, err := api.GetKeyspaces(ctx, tt.req)
require.NoError(t, err)

Expand Down Expand Up @@ -1515,7 +1513,7 @@ func TestGetSchema(t *testing.T) {
VtctldClient: client,
Tablets: tt.tablets,
})
api := NewAPI([]*cluster.Cluster{c}, grpcserver.Options{}, http.Options{})
api := NewAPI([]*cluster.Cluster{c}, Options{})

resp, err := api.GetSchema(ctx, tt.req)
if tt.shouldErr {
Expand Down Expand Up @@ -1641,7 +1639,7 @@ func TestGetSchema(t *testing.T) {
},
)

api := NewAPI([]*cluster.Cluster{c1, c2}, grpcserver.Options{}, http.Options{})
api := NewAPI([]*cluster.Cluster{c1, c2}, Options{})
schema, err := api.GetSchema(ctx, &vtadminpb.GetSchemaRequest{
ClusterId: c1.ID,
Keyspace: "testkeyspace",
Expand Down Expand Up @@ -2191,7 +2189,7 @@ func TestGetSchemas(t *testing.T) {
})
}

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

resp, err := api.GetSchemas(ctx, tt.req)
require.NoError(t, err)
Expand Down Expand Up @@ -2411,7 +2409,7 @@ func TestGetSchemas(t *testing.T) {
},
)

api := NewAPI([]*cluster.Cluster{c1, c2}, grpcserver.Options{}, http.Options{})
api := NewAPI([]*cluster.Cluster{c1, c2}, Options{})
resp, err := api.GetSchemas(ctx, &vtadminpb.GetSchemasRequest{
TableSizeOptions: &vtadminpb.GetSchemaTableSizeOptions{
AggregateSizes: true,
Expand Down Expand Up @@ -2642,7 +2640,7 @@ func TestGetSrvVSchema(t *testing.T) {
}),
}

api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
api := NewAPI(clusters, Options{})
resp, err := api.GetSrvVSchema(ctx, tt.req)

if tt.shouldErr {
Expand Down Expand Up @@ -2936,7 +2934,7 @@ func TestGetSrvVSchemas(t *testing.T) {
}),
}

api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
api := NewAPI(clusters, Options{})
resp, err := api.GetSrvVSchemas(ctx, tt.req)

if tt.shouldErr {
Expand Down Expand Up @@ -3192,7 +3190,7 @@ func TestGetTablet(t *testing.T) {
})
}

api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
api := NewAPI(clusters, Options{})
resp, err := api.GetTablet(ctx, tt.req)
if tt.shouldErr {
assert.Error(t, err)
Expand Down Expand Up @@ -3387,7 +3385,7 @@ func TestGetTablets(t *testing.T) {
})
}

api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
api := NewAPI(clusters, Options{})
resp, err := api.GetTablets(ctx, tt.req)
if tt.shouldErr {
assert.Error(t, err)
Expand Down Expand Up @@ -3518,7 +3516,7 @@ func TestGetVSchema(t *testing.T) {
t.Parallel()

clusters := []*cluster.Cluster{vtadmintestutil.BuildCluster(t, tt.clusterCfg)}
api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
api := NewAPI(clusters, Options{})

resp, err := api.GetVSchema(ctx, tt.req)
if tt.shouldErr {
Expand Down Expand Up @@ -3848,7 +3846,7 @@ func TestGetVSchemas(t *testing.T) {
}

clusters := vtadmintestutil.BuildClusters(t, tt.clusterCfgs...)
api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
api := NewAPI(clusters, Options{})

resp, err := api.GetVSchemas(ctx, tt.req)
if tt.shouldErr {
Expand Down Expand Up @@ -3977,11 +3975,7 @@ func TestGetWorkflow(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

api := NewAPI(
vtadmintestutil.BuildClusters(t, tt.cfgs...),
grpcserver.Options{},
http.Options{},
)
api := NewAPI(vtadmintestutil.BuildClusters(t, tt.cfgs...), Options{})

resp, err := api.GetWorkflow(ctx, tt.req)
if tt.shouldErr {
Expand Down Expand Up @@ -4420,11 +4414,7 @@ func TestGetWorkflows(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

api := NewAPI(
vtadmintestutil.BuildClusters(t, tt.cfgs...),
grpcserver.Options{},
http.Options{},
)
api := NewAPI(vtadmintestutil.BuildClusters(t, tt.cfgs...), Options{})

resp, err := api.GetWorkflows(ctx, tt.req)
if tt.shouldErr {
Expand Down Expand Up @@ -4718,7 +4708,7 @@ func TestVTExplain(t *testing.T) {
}),
}

api := NewAPI(clusters, grpcserver.Options{}, http.Options{})
api := NewAPI(clusters, Options{})
resp, err := api.VTExplain(ctx, tt.req)

if tt.expectedError != nil {
Expand Down