evaluate the retry logic for API calls

[neolit123]

with the merge of PR:
kubernetes/kubernetes#78915

we added retry logic to kubeadm when fetching ConfigMaps.

The PR also added a TODO to evaluate if this can be done better.
This work should happen in the 1.16 cycle.

---

update:

we are seeing users having random API server downtime which trips our API calls without retry, such as the ones in uplaod-certs phase of init:

• kubeadm on version 1.29.1 with upload-certs times out during the configMap POST #3022
• kubeadm init fails to create ConfigMap when --upload certs is set kubernetes#112411

this is tracked for a fix in 1.30, but no backports are planned.
as a first step we should add retries for all calls in idempotency.go

[fabriziopandini]

I think that we need to arrange for a code-walkthrough and add retries to everything is accessing api-server/etcd during join and fix it consistently

[ereslibre]

In the meantime, I'll give this one a go.

/assign

[xlgao-zju]

I'd like to help this. cloud I assign?

[neolit123]

i think @rosti was working on an idea on how to create a generic retry client.

[xlgao-zju]

create a generic retry client.

that sounds cool, ping me if you @rosti need help.

[neolit123]

/kind design feature
/remove-kind bug

[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

[pacoxu]

i think @rosti was working on an idea on how to create a generic retry client.

I cannot find new PR from @rosti about this. Any update for this?

https://github.com/kubernetes/kubernetes/blame/e777f721638cf585b4e9e5d933d27e753a35fabe/cmd/kubeadm/app/util/apiclient/idempotency.go#L342-L362

[neolit123]

I don't think we had a POC. Are you seeing any particular problems?

…

On Feb 17, 2022 12:05, "Paco Xu" ***@***.***> wrote: i think @rosti <https://github.com/rosti> was working on an idea on how to create a generic retry client. I cannot find new PR from @rosti <https://github.com/rosti> about this. Any update for this? https://github.com/kubernetes/kubernetes/blame/ e777f721638cf585b4e9e5d933d27e753a35fabe/cmd/kubeadm/app/ util/apiclient/idempotency.go#L342-L362 — Reply to this email directly, view it on GitHub <#1606 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACRATHIWXJDJBZOV5E6VF3U3TCABANCNFSM4HXKNECQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you were assigned.Message ID: ***@***.***>

[pacoxu]

After walking through the thread in #78915, it works well. The discussion focus on whether there are any hidden problems in the apiserver side or proxy/LB side.

To summarize it:

1. we want to know if there are any random failures.
2. retry helps in some potential unknown/unsure cases

Why not log the unknown error as a warning and keep the retry logic. Or the current retry logic is acceptable.

[neolit123]

• we want to know if there are any random failures.
• retry helps in some potential unknown/unsure cases

retry does help and IIRC the ideas was to have all API calls behind a client that retries for a per-use-case time.
not sure how doable that is...maybe doable but it's also a lot of work.

Why not log the unknown error as a warning and keep the retry logic. Or the current retry logic is acceptable.

some of these warnings in polls can spam the logs a lot.
high verbosity logs are better with V(x), but this is also inconsistent.
one of the discussed topics was to make the API client logic consistent.

i wouldn't mind closing this ticket until further notice.

[pacoxu]

If so, I think the priority would be priority/backlog 😄.

[neolit123]

moving to 1.30 with priority soon.
there was another report of failed API call without retry, so i think we should retry everywhere.

xref

• kubeadm on version 1.29.1 with upload-certs times out during the configMap POST #3022
• kubeadm init fails to create ConfigMap when --upload certs is set kubernetes#112411

[neolit123]

WIP PR
kubernetes/kubernetes#123271

[meezaan]

@neolit123 regarding kubernetes/kubernetes#112411, I was able to figure out what the problem was, at least in my case.

I had installed containerd using apt install containerd and not using the instructions in the docker repo which installs containerd.io. Once I installed the latter, the API was up and all the API calls were successful. I caught this when following kubeadm init phase step by step and eventually saw something in the containerd logs related to telemetry failures.

[neolit123]

@neolit123 regarding kubernetes/kubernetes#112411, I was able to figure out what the problem was, at least in my case.

I had installed containerd using apt install containerd and not using the instructions in the docker repo which installs containerd.io. Once I installed the latter, the API was up and all the API calls were successful. I caught this when following kubeadm init phase step by step and eventually saw something in the containerd logs related to telemetry failures.

thanks for the update. so it's bound to a specific containerd version or config; i'd assume the alternative version is much newer?

[neolit123]

we recommend to users to install containerd using their guide in the containerd repo
https://kubernetes.io/docs/setup/production-environment/container-runtimes/#containerd

[meezaan]

@neolit123 regarding kubernetes/kubernetes#112411, I was able to figure out what the problem was, at least in my case.
I had installed containerd using apt install containerd and not using the instructions in the docker repo which installs containerd.io. Once I installed the latter, the API was up and all the API calls were successful. I caught this when following kubeadm init phase step by step and eventually saw something in the containerd logs related to telemetry failures.

thanks for the update. so it's bound to a specific containerd version or config; i'd assume the alternative version is much newer?

Unfortunately I did not check the version that came as the default Debian package, but I subsequently used https://docs.docker.com/engine/install/debian/ to install containerd.io.