helm: configure kube client-go exponential backoff by default

[wedaly]

Configure env vars KUBE_CLIENT_BACKOFF_BASE and
KUBE_CLIENT_BACKOFF_DURATION in cilium-agent.

This is used by client-go to enable exponential backoff to avoid a thundering herd of LIST requests when apiserver is overloaded.

Users can configure the backoff using these helmvalues:

k8sClientExponentialBackoff:
    enabled: true
    backoffBaseSeconds: 1
    backoffMaxDurationSeconds: 120

Fixes: #36525

Enable client-go exponential backoff in cilium agent by default.

[wedaly]

discussed in sig-scalability meeting this morning: TODO on me to add an upgrade note for this in the docs

[wedaly]

rebased and added upgrade note to docs. Please let me know if you'd prefer this to be in the same commit as the helm chart change.

[wedaly]

/test

[qmonnet]

Seems good to me, thank you!

[squeed]

One question, feel free to self-resolve. Nice!

[marseel]

I will take one more look early next week at this change to think a bit more about consequences for liveness probes of cilium-agent and other potential consequences etc.

[marseel]

I've started looking into it and noticed this:

So actually there was a PR in kubernetes to make it default: kubernetes#106272

It seems like this exponential backoff is sometimes doing the opposite of what we would expect:
from kubernetes/kubernetes#106272 (comment) :

For a server returning 503 and "retry after" with value "1" the new backoff manager takes effect.
However, for scenarios with connection refused errors, the behavior is the same.

Additionally, just last week, handling context in backoff was actually fixed: kubernetes/kubernetes#127709

[wedaly]

thanks for looking @marseel! I'm not an expert in client-go or apimachinery, but this is what I'm seeing in the code (it may have changed since kubernetes/kubernetes#106272):

1. When these env vars are set, then Request.backoff gets set.
2. In Request.request, there's a call to retry.IsNextRetry with isErrRetryableFunc https://github.com/kubernetes/kubernetes/blob/ed9572d9c7733602de43979caf886fd4092a7b0f/staging/src/k8s.io/client-go/rest/request.go#L808-L810
3. isErrRetryableFunc returns true for any GET request with connection reset, EOF, or HTTP2ConnectionLost: https://github.com/kubernetes/kubernetes/blob/ed9572d9c7733602de43979caf886fd4092a7b0f/staging/src/k8s.io/client-go/rest/request.go#L1227-L1238
4. If the error is retriable, then retry.IsNextRetry creates a "fake" retriable response hard-coded with Retry-After=1 then the logic below sets r.retryAfter.Wait: https://github.com/kubernetes/kubernetes/blob/ed9572d9c7733602de43979caf886fd4092a7b0f/staging/src/k8s.io/client-go/rest/with_retry.go#L169-L174
5. On the next iteration of the loop in Request.request, there's a call to retry.Before https://github.com/kubernetes/kubernetes/blob/ed9572d9c7733602de43979caf886fd4092a7b0f/staging/src/k8s.io/client-go/rest/request.go#L1244
6. retry.Before reaches line 218 and calls request.backoff.CalculateBackoffWithContext, which eventually returns the backoff configured by the env vars. https://github.com/kubernetes/kubernetes/blob/ed9572d9c7733602de43979caf886fd4092a7b0f/staging/src/k8s.io/client-go/rest/with_retry.go#L217-L224 As this backoff is longer than retryAfter.Wait (hardcoded 1 second), the env var backoff gets used.

[wedaly]

rebased to resolve merge conflict in upgrade documentation

[wedaly]

/test

[wedaly]

/ci-awscni