Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add retries for kubeadm join / UpdateStatus #2092

Closed
fabriziopandini opened this issue Mar 30, 2020 · 6 comments · Fixed by kubernetes/kubernetes#91952
Closed

Add retries for kubeadm join / UpdateStatus #2092

fabriziopandini opened this issue Mar 30, 2020 · 6 comments · Fixed by kubernetes/kubernetes#91952
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.
Milestone

Comments

@fabriziopandini
Copy link
Member

Is this a BUG REPORT or FEATURE REQUEST?

BUG REPORT

Versions

kubeadm version: v1.17.*

What happened?

While executing Cluster API tests, in some cases it was observed kubeadm join failures when updating the kubeadm-config config map

xref kubernetes-sigs/cluster-api#2769

What you expected to happen?

To make update status more resilient by adding a retry loop to this operation

How to reproduce it (as minimally and precisely as possible)?

This error happens only sometimes, most probably when there is a temporary blackout of the load balancer that sits in front of the API servers (HA proxy reloading his configuration).
Also, the error might happen when the new API server enters the load balancing pool but the underlying etcd member is not yet available due to slow network/slow I/O causing delays in etcd getting online or in some cases, also change fo the etcd leader.

Anything else we need to know?

Important: if possible the change should be kept as small and possible and backported

@neolit123 neolit123 added this to the v1.19 milestone Mar 30, 2020
@neolit123 neolit123 added kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. labels Mar 30, 2020
@rosti
Copy link

rosti commented Apr 2, 2020

During the update status phase, we do the following 3 API calls:

  1. Create or mutate the kubeadm-config config map. This is hooked to a 20 step exponential backoff that totals in ~10s of wait time.
  2. Node role is created. No retries are performed.
  3. Node role binding is created. Again, no retries are performed.

The first question here is, should we consider a timeout for the whole phase or per API call?
What timeouts do we envision here?

Having too big timeouts on per-operation basis might frustrate end users. Having too short timeout will cause failures of the nature seen by the Cluster API folks.

@neolit123
Copy link
Member

neolit123 commented Apr 2, 2020

the ticket in CAPI, proposed that CAPI should be providing some metrics in terms of retires, yet this level of granularity will be hard to scope for them.

The first question here is, should we consider a timeout for the whole phase or per API call?

my vote goes for per-api call.

What timeouts do we envision here?

there is no sane answer for this.

a common timeout of exp back capping around 40 sec makes sense to me for general API calls.

BTW, at this point we seem to be applying a number of different backoffs, timeouts and different retry mechanics in different places which is increasing the tech dept in kubeadm.

@fabriziopandini
Copy link
Member Author

I have no strong opinions about applying retries per call or per phase, by considering the need for backporting I'm +1 for the simplest solution during this iteration

FYI current approach in clusterctl is to have retry loops for small groups of API calls (not for a single call) and everything is standardized around three backoff configurations:

  1. for group of operations with at least one write (~40s)
  2. for group of operations with only reads (~15s)
  3. specific for the connection to the API server (~15s)

https://github.com/kubernetes-sigs/cluster-api/blob/9fe8ad47e130758564d976dde7757d3edbba8c88/cmd/clusterctl/client/cluster/client.go#L206-L265

There are also special timeouts that apply to critical steps to the process (similar to wait for the API server or wait for TLS bootstrap in kubeadm)

@xlgao-zju
Copy link

I'd like to help with this.
/assign

@neolit123
Copy link
Member

@xlgao-zju hi, code freeze for 1.19 is June 25th.
would you be able to send a PR before that?

@xlgao-zju
Copy link

would you be able to send a PR before that?

will send the PR before June 15th, os that you reviewers will enough time to review the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now.
Projects
None yet
4 participants