xds: Add server features support to Bootstrapper

[dapengzhang0]

In preparation for xds-v3 support.

[voidzcy]

It doesn't make sense to put such an API here. This makes a factory class stateful.

Maybe it is time to eliminate XdsChannelFactory, instead we do need something like Helper.createResolvingOobChannel(), but accessible inside NameResolver.

Last time when I was doing some local e2e test for traffic splitting and routing, the gRPC channel is using plaintext, but the xDS channel is fixed to use TLS (if not google_default) instead of falling back to use the same configuration as its parent channel. This is broken for Java. It is not bitting us because we are always using TLS if not google_default in production.

We should support such a feature correctly. Basically, the creation of xDS channel will be done outside XdsClientImpl. For EDS only's code path, we are roughly able to do it with Helper.createResolvingOobChannel(), so not a problem for migrating.

For now, I think a better way is to change the createChannel API for XdsChannelFactory. You could define a simple class that contains a Channel and server features.

[dapengzhang0]

The problem is coming from the factory or whatever structure being used for XdsClientIml consumes a list of server_urls. When you create the channel from it, you must know the index of the server_url that is selected, so that you can know the corresponding server feature. If it's only a single server_url, there is no problem at all.

[dapengzhang0]

A workaround is we pass just one ServerInfo instead of List<ServerInfo> to XdsClientImpl as well as XdsChannelFactory. Because we are actually not supporting multiple management server yet, and the current XdsChannelFactory has a hack to pick the hard coded first ServerInfo in the list.

[voidzcy]

This is what I mean:

class XdsChannel {
  ManagedChannel channel;
  boolean useProtocolV3;  // or something similar if you have a better idea
}

XdsChannel createChannel(List<ServerInfo> servers) {
  ...
}

The output of creating channel contains the server feature.

[dapengzhang0]

I also thought about that, which introduces yet another class and I had thought you wouldn't like it. I'm totally okay with it.

[voidzcy]

:( Why would you think introducing a new class is bad? 😢 This looks fairly natural with channel and protocol feature encapsulated together.

[dapengzhang0]

Done.

[voidzcy]

Given that the format of server_features is undefined. Maybe it's better to just parse it to a String list.

The parser should not try to interpret what is being parsed. It's just a list of string. "xds_v3" should be defined in the consumer class, which is XdsChannelFactory (e.g., String PROTOCOL_VERSION = "xds_v3). The channel factory looks up the parsed server features and check if the expected protocol version setting is there.

[voidzcy]

The log here should just print out what is parsed literally.

[dapengzhang0]

Given that the format of server_features is undefined. Maybe it's better to just parse it to a String list.

What do you mean actually? The following?

List<String> serverFeatures = JsonUtil.getListOfStrings(serverConfig, "server_features");

Or just pick all string type items in the list?

[voidzcy]

Yeah, just use JsonUtil.getListOfStrings.

[dapengzhang0]

Done.

[dapengzhang0]

Added reading env variable for XdsChannelFactory. PTAL.