federation e2e: Adding retries to fetching secret in controller manager #27823
Conversation
nikhiljindal
changed the title
| ) | ||
|
|
||
| const ( | ||
| KubeAPIQPS = 20.0 | ||
| KubeAPIBurst = 30 | ||
| KubeconfigSecretDataKey = "kubeconfig" | ||
| getSecretTimeout = 1 * time.Minute |
There was a problem hiding this comment.
I have kept the timeout to 1 min.
Happy to reduce it but want to first make sure that the test passes on jenkins.
We can look at the log and see how much time is it taking and adjust timeout based on that.
There was a problem hiding this comment.
nit: Please use standard timeouts from the utils package.
There was a problem hiding this comment.
Did you mean: https://github.com/kubernetes/kubernetes/blob/master/test/e2e/framework/util.go?
Thats the package that has standard timeouts.
But I cannot depend on test code from here. We can perhaps refactor that library to move generic parts out of test.
k8s-github-robot
added
size/S
nikhiljindal
added
release-note-none
| @@ -101,9 +105,16 @@ var KubeconfigGetterForSecret = func(secretName string) clientcmd.KubeconfigGett | |||
| return nil, fmt.Errorf("error in creating in-cluster client: %s", err) | |||
| } | |||
| data = []byte{} | |||
| secret, err := client.Secrets(namespace).Get(secretName) | |||
| var secret *api.Secret | |||
| for start := time.Now(); time.Since(start) < getSecretTimeout; time.Sleep(2 * time.Second) { | |||
There was a problem hiding this comment.
nit: Please use standard retry periods from the utils package. Better still, use wait.Poll (or whatever it's called) from the utils package.
There was a problem hiding this comment.
updated the code to use wait.Poll
|
Some minor nits, then LGTM. Feel free to apply the label yourself once addressed. Thanks @nikhiljindal |
nikhiljindal
force-pushed
the
waitForSecret
branch
from
ab16487 to
c59de79
Compare
|
Thanks! Updated code as per comments. |
nikhiljindal
added
priority/important-soon
|
GCE e2e build/test passed for commit c59de79. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit c59de79. |
|
Automatic merge from submit-queue |
Ref: #27708
Trying to fix the following failure in fed controller manager:
I am not sure why that error is happening in the first place. kube-proxy should have been configured before fed controller manager pod comes up. I didnt find anything wrong in kube-proxy logs.
The request never reaches fed apiserver.
Lets see if adding retries helps.
cc @kubernetes/sig-cluster-federation @mml @colhom