Fix TestTaskRunRetry for k8s 1.22.9 and later #4944
Conversation
tekton-robot
added
do-not-merge/work-in-progress
tekton-robot
added
the
size/S
tekton-robot
removed
the
do-not-merge/work-in-progress
|
/assign @vdemeester @lbernick @afrittoli @jerop |
|
/assign @pritidesai |
There was a problem hiding this comment.
Thank you @abayer for digging into this issue, and the quick fix!
TestPipelineTaskTimeout is failing this time and it's a known flake - #4427
/test pull-tekton-pipeline-integration-tests
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jerop The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
|
/test pull-tekton-pipeline-integration-tests |
|
/test pull-tekton-pipeline-alpha-integration-tests |
We just switched to k8s 1.22.9 on CI last night, and `TestTaskRunRetry` started failing. After a bunch of investigation, I determined this was due to kubernetes/kubernetes#108366, which went into k8s 1.22.9. This resulted in the `pod.Status.Phase` that `TestTaskRunRetry` expected to be updated instantly having a delay. I could have fixed this with a sleep, but decided verifying that each container in the pod had terminated with the expected exit code was cleaner. Signed-off-by: Andrew Bayer <[email protected]>
abayer
force-pushed
the
fix-e2e-retry-test-for-k8s-1.22.8-and-later
branch
from
3b6e994 to
dcfd1da
Compare
| @@ -146,8 +146,14 @@ spec: | |||
| if _, found := podNames[p.Name]; !found { | |||
| t.Errorf("BUG: TaskRunStatus.RetriesStatus did not report pod name %q", p.Name) | |||
| } | |||
| if p.Status.Phase != corev1.PodFailed { | |||
| t.Errorf("BUG: Pod %q is not failed: %v", p.Name, p.Status.Phase) | |||
| // Check each container in the pod, rather than the phase, since the phase doesn't update to a terminal state instantly. | |||
There was a problem hiding this comment.
🎉 thank you for adding the explicit check!
There was a problem hiding this comment.
If the check against p.Status.Phase still worked (without a sleep - in my local testing, it did work fine with a 2s delay before the Pods(namespace).List(...) call), I would find the each-container check to be overkill, but hey. =)
|
Thanks @abayer 🎉 |
Changes
We just switched to k8s 1.22.9 on CI last night, and
TestTaskRunRetrystarted failing. After a bunch of investigation, I determined this was due to kubernetes/kubernetes#108366, which went into k8s 1.22.9. This resulted in thepod.Status.PhasethatTestTaskRunRetryexpected to be updated instantly having a delay. I could have fixed this with a sleep, but decided verifying that each container in the pod had terminated with the expected exit code was cleaner./kind misc
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
(if there are no user facing changes, use release note "NONE")
Release Notes