[kubeadm control plane]: etcd communication errors are being swallowed

[sethp-nr]

What steps did you take and what happened:

While testing an upgrade the etcd health checks were failing repeatedly. With the code from #2451 in place I could resolve it down one level:

failed to create etcd client: unable to create etcd client: context deadline exceeded

After some work, I found that my etcd ca secret was regenerated, changing the private key (see: #2454). It seems that GRPC has exactly one error message when the connection is misconfigured, and that's "context deadline exceeded." I haven't yet found a way to get more information on what happened via the API, but I'm continuing to dig.

What did you expect to happen:

When I set up the same condition with etcdctl and k port-forward I got a helpful error message:

{"level":"warn","ts":"2020-02-25T20:50:29.757-0800","caller":"clientv3/retry_interceptor.go:61","msg":"retrying of unary invoker failed","target":"endpoint://client-f67a0407-f684-4682-bb79-ec33c94b2178/127.0.0.1:63477","attempt":0,"error":"rpc error: code = DeadlineExceeded desc = latest connection error: connection error: desc = \"transport: authentication handshake failed: remote error: tls: bad certificate\""}
Error: context deadline exceeded

Note the Error: context deadline exceeded is what came back from clientv3.New, and the other is a log statement being printed to stderr.

Anything else you would like to add:

I found that any error sent back from the proxy dial function was being swallowed in the same way. It also looks like we're not using the errorStream we set up with the API Server, so it's possible that we'd miss important information about the proxy connection.

Environment:

• Cluster-api version: master
• Minikube/KIND version: kind v0.7.0 go1.13.6 darwin/amd64
• Kubernetes version: (use kubectl version): a mix of v1.15 and v1.16 control plane nodes
• OS (e.g. from /etc/os-release): ubuntu

/kind bug
/assign
/lifecycle active

[vincepri]

/milestone v0.3.0

[vincepri]

/milestone v0.3.x

[vincepri]

Bumping this given that it seems the upstream grpc fix might not go in soon

[vincepri]

@sethp-nr Not sure if you saw this response, should we leave things as they are if the upstream fix isn't merged?

[sethp-nr]

It ended up going in as an option under a different PR after some discussion in the linked thread (culminating here: grpc/grpc-go#2031 (comment) ).

There's still some work in getting it into a released version & getting etcd to be compatible with that version & picking that version up here and then we can finally replace WithBlock with WithReturnLastError and see clearer errors.

I'm working that in fits and starts when I have time, but if someone else wanted to do it I would not get in their way.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[vincepri]

/lifecycle frozen

[vincepri]

The PR changes with WithReturnConnectionError has been merged and available from v1.30.0

/milestone v0.4.0

[vincepri]

/help

[k8s-ci-robot]

@vincepri:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vincepri]

/priority important-longterm

[vincepri]

/milestone v1.0

[sbueringer]

/assign @killianmuldoon
to re-assess the current state

[timoreimann]

FWIW I submitted #4997 some time ago that addressed at least one occurrence of error hiding / swallowing. Not sure if this bug report is about more though.

[fabriziopandini]

/close

until we get more evidence that there are still other occurrences of this error after #4997 merged

[k8s-ci-robot]

@fabriziopandini: Closing this issue.

In response to this:

/close

until we get more evidence that there are still other occurrences of this error after #4997 merged

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.