xds: support istio grpc-agent

[naoh87]

I tried to use xDS with istio grpc-agent. But it doesn't work well.

Is your feature request related to a problem?

xDS doesn't work with istio grpc-agent because not supported NameResolver of unix:///
#4750

Then I write a simple NameResolverProvider that it just returns DomainSocketAddress, but it doesn't work.

Feb 02, 2022 7:56:38 PM io.grpc.internal.ManagedChannelImpl$NameResolverListener handleErrorInSyncContext
WARNING: [Channel<8>: (xds:///xxx.core.svc.cluster.local:9000)] Failed to resolve name. status=Status{code=UNAVAILABLE, description=NameResolver returned no usable address. addrs=[], attrs={}
io.grpc.xds.XdsNameResolver@7c6c48cc was used, cause=null}

Describe the solution you'd like

Enable xDS to handle unix domain socket with istio grpc-agent like grpc-go.

Additional context

https://istio.io/latest/blog/2021/proxyless-grpc/

[sanjaypujare]

I didn't see any references to unix domain socket in the additional context link you cited so I didn't see how it is related.

Are you saying the istio control plane returned unix: addresses as endpoints and the gRPC code dropped them even before using your NameResolver for UDS?

[ejona86]

Drat. Well, I guess this will be a forcing function to finishing #4750 and its dependencies. We've slowly been moving in that direction, so luckily the ChannelCredentials paved the way partly already.

Then I write a simple NameResolverProvider that it just returns DomainSocketAddress, but it doesn't work.

That should have worked. We know that would fail if OkHttp were in the mix or if Netty wasn't using epoll, and there are issues with grpc-netty vs grpc-netty-shaded. But you didn't hit any of those (yet) from the error. That error claims your name resolver didn't provide any addresses at all. @naoh87, can you paste code that prepares the data to return to the listener? I think there is something simple going on.

@sanjaypujare, FYI, fixing this means fixing #3085 which means methods like ServerProvider.provider() will no longer be used at all by gRPC, as we'll always need to be prepared to try multiple transports. We can leave it in place temporarily, but it will need to be deleted eventually.

@sanjaypujare, Istio uses a control plane proxy in the local environment and the client connects to it over UDS. See this diagram from the blog post: https://istio.io/latest/blog/2021/proxyless-grpc/architecture.svg

[sanjaypujare]

...
@sanjaypujare, Istio uses a control plane proxy in the local environment and the client connects to it over UDS. See this diagram from the blog post: https://istio.io/latest/blog/2021/proxyless-grpc/architecture.svg

Thanks, I did search for "UDS" on that page :-) Should be a good feature in Chrome (and other browsers) to OCR the embedded images in the text search.

[costinm]

I'll take a look, we may need to use TCP for java.

[ejona86]

@costinm, I should have noticed this earlier when looking at that blog post. It would certainly need a hack today (like making your own NR). If TCP is available, that'd be great. But it also seems like it is time for us to get the plumbing to allow UDS.

The NR itself is very easy and would be relatively (compared to others) small. But without the plumbing it would make multiple (mostly right) assumptions about the environment.

[costinm]

Yes, java support for UDS would be very nice.

Unfortunately the XDS proxy in the agent only support UDS, but at least for testing it should be possible to use a custom bootstrap file that connects directly to istiod.istio-system on the plain text port. The main purpose of the XDS proxy in the agent is to deal with the custom certificate and auth - ideally we shouldn't need it, but right now in Istio we just have too many ways to authenticate and get certificates.

[naoh87]

Sorry for my late reply @ejona86

can you paste code that prepares the data to return to the listener?

I tried this NR just call onAddress in start.

listener.onAddresses(
  List.of(new EquivalentAddressGroup(new DomainSocketAddress(targetUri.getPath()))),
  Attributes.EMPTY)

And I tried also add or not add epoll dependency. Both case cause same result.

"io.netty" % "netty-transport-native-epoll" % "4.1.73.Final" classifier "linux-x86_64"

It may not matter I used grpc-java from Scala.

[cfredri4]

Then I write a simple NameResolverProvider that it just returns DomainSocketAddress, but it doesn't work.

That should have worked.

Will it be enough with only a different NameResolverProvider? Because UDS requires a different socket channel as well (EpollDomainSocketChannel).

[sanjaypujare]

...
Will it be enough with only a different NameResolverProvider? Because UDS requires a different socket channel as well (EpollDomainSocketChannel).

yes, you can look at https://github.com/grpc/grpc-java/blob/v1.39.x/xds/src/main/java/io/grpc/xds/internal/sds/SdsClient.java#L116 to see how this channel is created.

Ideally this should be supported by Grpc.newChannelBuilder() by adding a new kind of ManagedChannelProvider which will use the "unix:///" prefix (and null creds) to return an appropriate UDS channel. Do you want to contribute a PR?

[ejona86]

Ideally this should be supported by Grpc.newChannelBuilder() by adding a new kind of ManagedChannelProvider which will use the "unix:///" prefix (and null creds) to return an appropriate UDS channel.

@sanjaypujare, that's a different design than we had been discussing earlier. Earlier we were talking about adding a NameResolver (along with #3085 (comment)). Using a ManagedChannelProvider that way could be made to work, but seems brittle and heavily reliant on having a high priority(). We could change how unknown target schemes are handled (to skip over that provider) to make that work better, but there's some important details to work out.

[cfredri4]

Short term, something like this should make it work with Istio?

[sanjaypujare]

@cfredri4 @ejona86 the NameResolver can return a DomainSocketAddress alright but requiring an EpollDomainSocketChannel type can only be done in the NettyChannelBuilder (or the provider calling that builder). Or should we modify NettyChannelBuilder to automatically use EpollDomainSocketChannel when the address returned is a DomainSocketAddress?

So @cfredri4 how does your code take care of EpollDomainSocketChannel ?

[cfredri4]

So @cfredri4 how does your code take care of EpollDomainSocketChannel ?

It does not, this would only be a short term fix for XDS (as you showed, SdsClient will detect and use a EpollDomainSocketChannel).

[sanjaypujare]

...
It does not, this would only be a short term fix for XDS (as you showed, SdsClient will detect and use a EpollDomainSocketChannel).

Okay so in your case you will have another place where you will set the channelType appropriately. Curious to see what that place will be.

[cfredri4]

Okay so in your case you will have another place where you will set the channelType appropriately. Curious to see what that place will be.

UDS is only needed for communication with the Istio agent/control plane. But then UDS is not needed when communicating with other services/data plane.
Right?

[sanjaypujare]

...
UDS is only needed for communication with the Istio agent/control plane. But then UDS is not needed when communicating with other services/data plane. Right?

Correct. But what I am saying is even to open that UDS channel to the Istio agent 2 things need to happen (from https://github.com/grpc/grpc-java/blob/v1.39.x/xds/src/main/java/io/grpc/xds/internal/sds/SdsClient.java#L116)

• the NettyChannelBuilder needs to get a DomainSocketAddress - which can be achieved thru your UdsNameResolver
• the NettyChannelBuilder also needs to be provided with the channelType of EpollDomainSocketChannel - and this one cannot be done thru the NameResolver unless I am missing something.

If you trace the flow from the Grpc.newChannelBuilder() call I don't see where you can provide the hook for supplying the correct channelType.

[cfredri4]

Now I'm confused. 😅 I asked if it was enough with only a different NameResolverProvider, and you said yes, showing that SdsClient is already providing an EpollDomainSocketChannel...

...
Will it be enough with only a different NameResolverProvider? Because UDS requires a different socket channel as well (EpollDomainSocketChannel).

yes, ...

But then I do actually need a ManagedChannelProvider as well?

[sanjaypujare]

Now I'm confused. 😅 I asked if it was enough with only a different NameResolverProvider, and you said yes, showing that SdsClient is already providing an EpollDomainSocketChannel...

SdsClient has been removed and is a different client (not the XdsClient that we want here). My link was to an older version (1.40.x) if you notice...

...
Will it be enough with only a different NameResolverProvider? Because UDS requires a different socket channel as well (EpollDomainSocketChannel).

yes, ...

But then I do actually need a ManagedChannelProvider as well?

Looks like that because that's the only one that can provide the channelType we were talking about.

[cfredri4]

Thanks, now it's clear.
Something like this seems to work. but like @ejona86 says it's a bit brittle.
What would a "proper" solution be? I may attempt a PR.

[sanjaypujare]

Thanks, now it's clear. Something like this seems to work.

But you also need to register this in ManagedChannelRegistry and would you do that thru the Service Provider interface?

but like @ejona86 says it's a bit brittle. What would a "proper" solution be? I may attempt a PR.

I'll let @ejona86 answer that. It will be good if you can contribute a PR - since this functionality is sorely needed.

[cfredri4]

But you also need to register this in ManagedChannelRegistry and would you do that thru the Service Provider interface?

Yes.

I'll let @ejona86 answer that. It will be good if you can contribute a PR - since this functionality is sorely needed.

A partly related question; since this will give the client the capability to connect over UDS, should the same be done for server as well? I.e. something like this would be needed somewhere:

NettyServerBuilder.forAddress(new DomainSocketAddress(path))
  .channelType(EpollServerDomainSocketChannel.class)
  ...

[sanjaypujare]

...
A partly related question; since this will give the client the capability to connect over UDS, should the same be done for server as well? I.e. something like this would be needed somewhere:

NettyServerBuilder.forAddress(new DomainSocketAddress(path))
  .channelType(EpollServerDomainSocketChannel.class)
  ...

"client" in this context can be confusing. In this case gRPC (the library) only needs the XdsClient to be able to instantiate a gRPC client over UDS to talk to the Istio agent (which is the XDS server although just a proxy). Even if one is creating an XDS managed gRPC server (check out io.grpc.xds.XdsServerBuilder) it still needs only the XdsClient (containing a gRPC client) to talk to the Istio agent (over UDS in this case).

So something like what you are describing i.e. creating a NettyServer using UDS to receive incoming RPCs is not needed.

[cfredri4]

So something like what you are describing i.e. creating a NettyServer using UDS to receive incoming RPCs is not needed.

Well it wouldn't be needed for the issue discussed here (xDS over UDS), but the fixes discussed here would also allow any other grpc-java client to connect to any server over UDS.

So my question is rather if there should be an easier way to create a server for UDS? As this will allow out-of-the-box support for creating both a client and server over UDS. E.g. something like?

Grpc.newServerBuilderForAddress("unix:/foo.bar", InsecureServerCredentials.create())

[sanjaypujare]

...

So my question is rather if there should be an easier way to create a server for UDS? As this will allow out-of-the-box support for creating both a client and server over UDS. E.g. something like?

Grpc.newServerBuilderForAddress("unix:/foo.bar", InsecureServerCredentials.create())

Okay I see what you are saying. But:

• at least I can't think of an easier way or a common change that gives us UDS support for both client & server
• UDS on the client side is more urgent whereas we don't immediately need UDS on the server side.

[cfredri4]

but like @ejona86 says it's a bit brittle. What would a "proper" solution be? I may attempt a PR.

I'll let @ejona86 answer that. It will be good if you can contribute a PR - since this functionality is sorely needed.

@ejona86 any thoughts?

[ejona86]

The design for #3085 is essentially add Collection<SocketAddress> getSupportedSocketAddresses() to NameResolverProvider and ManagedChannelProvider. ManagedChannelRegistry would use the target string to determine which NameResolverProvider would be used. It will then loop through ManagedChannelProviders in order until one supports all the addresses returned by the NameResolverProvider. And then for this issue we add unix name resolver and a unix variant for Netty.

I think @sanjaypujare was looking at implementing this, although I'd imagine he'd be happy handing it off. He has some stuff written up in a random doc; he could potentially share it with you.

[sanjaypujare]

@cfredri4 if you are interested in submitting a PR I can share my doc with you. Let me know