wait for balancer to have some address available on gateway #76

sp-joseluis-ledesma · 2018-06-01T08:46:12Z

When running the health command, more often than not, the output is:

WARN[0000] error checking health: rpc error: code = Unavailable desc = there is no address available 
INFO[0000] healthy: ok

running with GRPC_GO_LOG_SEVERITY_LEVEL=info GRPC_GO_LOG_VERBOSITY_LEVEL=8 the problem is that the balancer has not been notifiied of the address before it tries to connect:

INFO: 2018/06/01 06:55:06 ccBalancerWrapper: updating state and picker called by balancer: IDLE, 0xc420205320
INFO: 2018/06/01 06:55:06 dialing to target with scheme: ""
INFO: 2018/06/01 06:55:06 could not get resolver for scheme: ""
WARN[0000] error checking health: rpc error: code = Unavailable desc = there is no address available 
INFO: 2018/06/01 06:55:06 balancerWrapper: is pickfirst: false
INFO: 2018/06/01 06:55:06 grpc: failed dns SRV record lookup due to lookup _grpclb._tcp.localhost on 100.64.0.10:53: no such host.
[...]

This PR adds a "wait" for the balancer to have address on the NewGateway function, so when it returns the KiamGateway we ensure there is some address in the balancer

pingles

I'd prefer to see the timeout/context specified as an arg to NewGateway rather than inside NewGateway- to allow whatever is consuming it to specify/propagate a timeout.

Then, we can place balancer.Get inside a retry loop that can just keep retrying until the balancer resolves- again allowing whatever is consuming NewGateway to determine how long they're happy to wait.

Thanks for the PR though! Be nice to have this integrated.

sp-joseluis-ledesma · 2018-06-01T10:14:03Z

I added a parameter to the NewGateway function, and passed it from the agent and health commands. Is it ok? or do you prefer another aproach?

Perhaps the NewGateway parameters could be converted to the "WithOption" pattern, but I leave this for another PR :)

pingles

Thanks- I've made some more comments in-line to help.

Sorry for not explaining before but I'd like the context.Context to be constructed by the commands and passed to NewGateway rather than the wait duration (and add cli flags to set defaults).

Thanks again so much for adding this.

pingles · 2018-06-01T11:16:04Z

pkg/server/gateway.go

@@ -45,7 +45,7 @@ type GatewayConfig struct {
 	TLS     *TLSConfig
 }

-func NewGateway(address string, refresh time.Duration, caFile, certificateFile, keyFile string) (*KiamGateway, error) {
+func NewGateway(address string, refresh time.Duration, waitForBalancer time.Duration, caFile, certificateFile, keyFile string) (*KiamGateway, error) {


Could we pass the context.Context through as the first arg to NewGateway, rather than the wait duration.

pingles · 2018-06-01T11:16:38Z

pkg/server/gateway.go

@@ -80,6 +80,13 @@ func NewGateway(address string, refresh time.Duration, caFile, certificateFile,
 		return nil, err
 	}

+	ctx, cancel := context.WithTimeout(context.Background(), waitForBalancer)
+	defer cancel()
+	_, _, err = balancer.Get(ctx, grpc.BalancerGetOptions{BlockingWait: true})


If we pass the context through then this doesn't need to create context.WithTimeout it can just cancel according to the context it's provided.

pingles · 2018-06-01T11:18:25Z

cmd/agent/main.go

@@ -129,7 +129,7 @@ func main() {
 	config := http.NewConfig(opts.port)
 	config.AllowIPQuery = opts.allowIPQuery

-	gateway, err := kiamserver.NewGateway(opts.serverAddress, opts.serverAddressRefresh, opts.caPath, opts.certificatePath, opts.keyPath)
+	gateway, err := kiamserver.NewGateway(opts.serverAddress, opts.serverAddressRefresh, 50*time.Millisecond, opts.caPath, opts.certificatePath, opts.keyPath)


Instead of passing the time we'll pass a context.Context through. We should also add a CLI flag to control what the wait duration is.

pingles · 2018-06-01T11:18:55Z

cmd/health/main.go

@@ -63,7 +63,7 @@ func main() {
 		log.SetLevel(log.ErrorLevel)
 	}

-	gateway, err := kiamserver.NewGateway(opts.serverAddress, opts.serverAddressRefresh, opts.caPath, opts.certificatePath, opts.keyPath)
+	gateway, err := kiamserver.NewGateway(opts.serverAddress, opts.serverAddressRefresh, 50*time.Millisecond, opts.caPath, opts.certificatePath, opts.keyPath)


As with the agent's main, we should construct a context and pass it to NewGateway and set the timeout on that context from a cli flag.

sp-joseluis-ledesma · 2018-06-01T12:41:12Z

Ok, updated following your comments. Let me know if there is anything more to change.

mikesplain · 2018-06-01T12:42:03Z

Thanks @sp-joseluis-ledesma! We run into this as well, this is great.

pingles · 2018-06-01T12:44:37Z

Thanks @sp-joseluis-ledesma

pingles · 2018-06-01T12:58:17Z

I've tagged it as v2.8 and the image is now up on quay.io, I'll create a separate PR with the CHANGELOG updates but you can update now if you like!

TattiQ · 2018-08-03T12:15:16Z

Hi guys, sorry for bothering you in this thread but my issues seems a bit connected to this one. I am using version 2.7 now with kops , it ran fine in dev and today I tried it out in prod (hoping to substitute kube2iam) and got an issue with some of the agents when they can not seem to connect to the server. The error suggests that there are no addresses to connect to and unlike for Jose Luis, I get a fatal error. and this is reproduced constantly on those agents even if the ping succeeds after all. I wonder if anyone has any thoughts on that.

then the agent reports in its logs - {"addr":"100.99.186.53:33766","level":"error","method":"GET","msg":"error processing request: rpc error: code = Unavailable desc = there is no address available","path":"/latest/meta-data/iam/security-credentials/","status":500,"time":"2018-08-03T07:35:45Z"} .

and the healthchecks on all servers run fine + nothing suspicious in servers logs.

pingles · 2018-08-03T12:20:13Z

@TattiQ only thing I can recommend for now is increasing the time limits on your health check and see if that helps- do liveness probes always fail or intermittently? Does it do the same if you specify the FQDN for the server?

Could you perhaps open as a new issue and then we can track it?

pingles suggested changes Jun 1, 2018

View reviewed changes

sp-joseluis-ledesma force-pushed the master branch from 426f352 to 3a5dba1 Compare June 1, 2018 10:12

pingles suggested changes Jun 1, 2018

View reviewed changes

wait for balancer to have some address available on gateway

5c78c6d

sp-joseluis-ledesma force-pushed the master branch from 3a5dba1 to 5c78c6d Compare June 1, 2018 12:39

pingles approved these changes Jun 1, 2018

View reviewed changes

pingles merged commit 1d6d618 into uswitch:master Jun 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wait for balancer to have some address available on gateway #76

wait for balancer to have some address available on gateway #76

wait for balancer to have some address available on gateway #76

wait for balancer to have some address available on gateway #76

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment