Allow specifying GRPC load balancer

[d4l3k]

This is the easiest way I can see of adding in load balancing. I don't think there should be an unexpected side effects but please let me know if you can think of anything.

This adds in a round robin DNSResolver for load balancing purposes. The GRPC DNSResolver first checks if SRV records are present on the specified host and if so, uses them. If they don't exist it just resolves the DNS hostname as normal. If there's an IP specified it just uses that.

[d4l3k]

@pieterlouw Any thoughts on this?

[pieterlouw]

Ho @d4l3k ,
Thanks for this, it looks great. I need to find a moment to test this myself before I merge it in.

Few things:

How does this affect the Caddyfile setup? Can you perhaps post an example?

The only thing that bothers me is the gRPC Naming package is still experimental according to the documentation. I'm sure it's stable, it's just I'm reluctant to add an API that might change and break future Caddy builds.

[d4l3k]

Hey,

This shouldn't change the Caddyfile setup at all. It just checks for a GRPC SRV record first. and falls back to just normal DNS resolution.

And yeah, I totally missed the fact that that package was experimental and deprecated. I'll investigate a bit more and update the PR.

[d4l3k]

Okay, I just took a deeper look into this. Looks like load balancing is actually supported out of the box, but the default resolver is "passthrough" and load balancer is "pick_first".

For SRV resolving you can just do dns://service.examples.com.

[d4l3k]

I updated this PR to add a balancer config option which lets you specify which load balancer mode you want to do. Resolvers can just be specified by the scheme :// in the backend_addr field.

As for the balancing stuff being experimental, this code no longer depends on any package other than the main grpc one which should prevent most issues and it seems unlikely that they'd remove the grpc.WithBalancerName method without notice considering the old deprecated load balancing mechanism still works.

This in use would look like

grpc dns://service.foo {
  balancer round_robin
}

[pieterlouw]

Does this make "balancer" a required field in the Caddyfile?

"balancer" should be optional, especially important for backward compatibility.

[d4l3k]

I don't believe this makes it required. It just means if you specify balancer you need to provide an argument.

[pieterlouw]

Thanks @d4l3k

I added a comment on the changes you made in setup.go (https://github.com/pieterlouw/caddy-grpc/pull/5/files#diff-4d86aab957c347ca87f7021d57b17205)