fix(executor): Deal with the pod watch API call timing out #4734
Conversation
|
|
hermanhobnob
changed the title
Signed-off-by: Paul Hirst <github@bearandgiraffe.org>
alexec
changed the title
| @@ -26,22 +26,32 @@ func UntilTerminated(kubernetesInterface kubernetes.Interface, namespace, podNam | |||
| } | |||
|
|
|||
| func untilTerminatedAux(podInterface v1.PodInterface, containerID string, listOptions metav1.ListOptions) (bool, error) { | |||
| for { | |||
| complete, done, err := doWatch(podInterface, containerID, listOptions) | |||
There was a problem hiding this comment.
In English, "complete" means the same thing as "done". Is there a better name to use?
There was a problem hiding this comment.
Very fair point. I couldn't think of a better synonym, so I went for something more explicit and swapped the booleans around to match.
Signed-off-by: Paul Hirst <github@bearandgiraffe.org>
|
LGTM |
|
@alexec , is there anything more I need to do with this PR or will you just merge it at some point, when appropriate? |
|
@simster7 this needs to be back-ported to v2.12. Would it be best if we label issues that need backporting? @sarabala1979 ? |
|
Sure, I am about to make an issue with all the commits since the previous release and was hoping to have you all checkbox which ones you wanted in the release |
|
ok, lets do your idea @simster7 |
|
Let's get this merged in |
Signed-off-by: Paul Hirst <github@bearandgiraffe.org>
Checklist:
Tested manually by adding a couple of lines to reduce the api timeout to ~20 seconds and then running a tweaked hello-world workflow with a sleep 60. Ideally this would be an e2e test, but in your youtube contributor video you mentioned needing all the e2e tests to run in under 10 minutes. I'm not sure how much you could compress the times and still have a reliable test for this, so I haven't added an end to end test. Opinions welcome.