stubserver: add support to optionally pass in a `grpc.Server` or `xds.GRPCServer`

[easwars]

There are tests which currently implement their own version of the test service instead of using the stub server because they have to register the test service on a server (grpc.Server or xds.GRPCServer) that they create in the test. If we add support to pass in an optional server to the stub server, these tests could start using the latter.

[Kailun2047]

@easwars Hi, can I contribute to this one? :)

[easwars]

Please go for it 👍

FYI: We have a ServiceRegistrar interface that is implemented by both the grpc.Server and xds.GRPCServer, and ideally, the stubserver package should accept this as an option, so that both the servers can be supported.

Please let us know if you have more questions.

[janardhanvissa]

@purnesh42H please assign me this issue

[easwars]

I actually have some WIP changes for this. Do you mind using that as a starting point? Else, I can try finishing them up when I find time.

[janardhanvissa]

Can you please provide the PR and some more context about this issue to work on top of that.

[easwars]

Let me go over what we are looking to do here:

• The StubServer type has a field in which it stores the actual underlying server, See: https://github.com/grpc/grpc-go/blob/8f920c6c56693bc53e526cd7f48e89ac250ff3a2/internal/stubserver/stubserver.go#L56C2-L56C3
• This field is initialized in setupServer here: https://github.com/grpc/grpc-go/blob/8f920c6c56693bc53e526cd7f48e89ac250ff3a2/internal/stubserver/stubserver.go#L135
• But as you can see, this is always initialized to a vanilla gRPC server. We want it to support an xds-enabled gRPC server as well. See: https://github.com/grpc/grpc-go/blob/8f920c6c56693bc53e526cd7f48e89ac250ff3a2/xds/server.go#L68
• What is want is to change the type of the field S to an interface that is defined locally to include only the methods that are used on the underlying server. And if the field is populated by the caller, then setupServer should not create a new one. Else it can continue to create a vanilla gRPC server.

I looked at the change that I currently have and it mostly handles the above. So, I'll send out a PR for that. Once that is merged though, we need to update tests that currently create their own test server to instead use the StubServer.

Something like this should give a list of files where the test service is implemented. So, we need some manual inspection to see how many of these can be changed to use the StubServer.

easwars@minetop: grpc-go$ grep -rl "func.*EmptyCall" * | grep -v .*pb.go
balancer/rls/helpers_test.go
balancer/grpclb/grpclb_test.go
internal/stubserver/stubserver.go
interop/xds/server/server.go
interop/test_utils.go
orca/service_test.go
rpc_util.go
test/end2end_test.go
test/xds/xds_server_integration_test.go
test/channelz_test.go
xds/internal/httpfilter/fault/fault_test.go
xds/internal/balancer/clustermanager/e2e_test/clustermanager_test.go
xds/server_ext_test.go
easwars@minetop: grpc-go$ 

[easwars]

Some more:

easwars@minetop: grpc-go$ grep -rl "func.*UnaryCall" * | grep -v .*pb.go
authz/grpc_authz_end2end_test.go
benchmark/benchmark.go
binarylog/binarylog_end2end_test.go
credentials/alts/alts_test.go
internal/stubserver/stubserver.go
interop/xds/server/server.go
interop/test_utils.go
orca/service_test.go
stats/stats_test.go
test/end2end_test.go
test/xds/xds_server_integration_test.go
test/channelz_test.go
xds/internal/balancer/clustermanager/e2e_test/clustermanager_test.go
xds/server_ext_test.go

easwars@minetop: grpc-go$ grep -rl "func.*StreamingOutputCall" * | grep -v .*pb.go
binarylog/binarylog_end2end_test.go
interop/test_utils.go
stats/stats_test.go
test/end2end_test.go
test/retry_test.go
test/channelz_test.go

easwars@minetop: grpc-go$ grep -rl "func.*StreamingInputCall" * | grep -v .*pb.go
authz/grpc_authz_end2end_test.go
binarylog/binarylog_end2end_test.go
interop/test_utils.go
stats/stats_test.go
test/end2end_test.go
test/gracefulstop_test.go
test/channelz_test.go

easwars@minetop: grpc-go$ grep -rl "func.*DuplexCall" * | grep -v .*pb.go
authz/audit/audit_logging_test.go
balancer/leastrequest/balancer_test.go
balancer/grpclb/grpclb_test.go
binarylog/binarylog_end2end_test.go
experimental/shared_buffer_pool_test.go
gcp/observability/observability_test.go
gcp/observability/logging_test.go
internal/idle/idle_e2e_test.go
internal/stubserver/stubserver.go
interop/test_utils.go
orca/call_metrics_test.go
producer_ext_test.go
server_ext_test.go
stats/opencensus/e2e_test.go
stats/opentelemetry/e2e_test.go
stats/opentelemetry/csm/observability_test.go
stats/stats_test.go
stream_test.go
test/end2end_test.go
test/interceptor_test.go
test/xds/xds_server_integration_test.go
test/stream_cleanup_test.go
test/retry_test.go
test/gracefulstop_test.go
test/server_test.go
test/context_canceled_test.go
test/channelz_test.go
test/metadata_test.go
test/transport_test.go
test/goaway_test.go
test/compressor_test.go
xds/internal/httpfilter/fault/fault_test.go
xds/server_ext_test.go

[easwars]

@janardhanvissa : Let me know what you feel about manually inspecting some of these files and figuring out which ones can be switched over to use the StubServer. Thanks.

[easwars]

@janardhanvissa : #7613 is now merged.

So, what remains is the manual determination of which tests can be switched over to use the stubserver instead of implementing the test service, creating a grpc server and registering the service on it.

It doesn't have to done in one shot. We can take piecemeal approach.

[janardhanvissa]

@purnesh42H In the testservice we have fields like fallback, security and metricsRecorder. For the replacement of stubserver we require those fields to update the test cases. Also StreamingOutputCall and StreamingInputCall are not defined in the StubServer struct. Can we include those? Please suggest this to me.

balancer/grpclb/grpclb_test.go Don't have a fallback field in the stubserver

test/channelz_test.go Don't have security field in the stubserver

interop/test_utils.go Don't have a metricsRecorder orca.ServerMetricsRecorder field in the stubserver

[easwars]

I would recommend starting with the simpler ones where you don't have to make such changes. Are there enough of those? Thanks.

[janardhanvissa]

Made changes on some of the test cases and raised PR's on the folk branch. Please find the PR's below.

https://github.com/janardhanvissa/grpc-go/pull/11 https://github.com/janardhanvissa/grpc-go/pull/12

[janardhanvissa]

orca/service_test.go

test/xds/xds_server_integration_test.go test/xds/xds_server_serving_mode_test.go test/xds/xds_client_ignore_resource_deletion_test.go test/xds/xds_server_certificate_providers_test.go test/xds/xds_server_test.go

xds/internal/httpfilter/fault/fault_test.go xds/server_ext_test.go