[Bug Fix] Garbage Collect Node e2e Failing #42661
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
This change is |
|
/release-note-none |
k8s-ci-robot
added
the
release-note-none
|
Flake PR: #38050 |
k8s-github-robot
added
the
size/XS
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED The following people have approved this PR: dashpole, dchen1107 Needs approval from an approver in each of these OWNERS Files:
We suggest the following people: |
k8s-github-robot
added
the
approved
dchen1107
added
the
kind/flake
|
@k8s-bot non-cri e2e test this |
| for _, pod := range test.testPods { | ||
| By(fmt.Sprintf("Deleting Pod %v", pod.podName)) | ||
| f.PodClient().DeleteSync(pod.podName, &metav1.DeleteOptions{}, defaultRuntimeRequestTimeoutDuration) | ||
| f.PodClient().DeleteSync(pod.podName, &metav1.DeleteOptions{}, podDisappearTimeout) |
There was a problem hiding this comment.
nit: The name is odd. Could it be podDeletionTimeout?
There was a problem hiding this comment.
The description is equally odd:
// podDisappearTimeout is the timeout to wait node disappear.
podDisappearTimeout = time.Minute * 2
There was a problem hiding this comment.
Might be worth just doing a quick pass over the e2e node suite and rename constants and move them to a single file
There was a problem hiding this comment.
There is also podWaitTimeout in lifecycle_hook_test.go, which is set to 3 minutes, and is used as a timeout for deletion.
There was a problem hiding this comment.
Yeah. I feel we can add default timeouts to DeleteSync() itself which can derive that from the framework. Less constants to deal with in such a workflow.
if this PR is urgent, don't block on these comments.
There was a problem hiding this comment.
#42734 creates a default and addresses these comments
|
@k8s-bot gce etcd3 e2e test this |
|
@dashpole: The following test(s) failed:
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
Automatic merge from submit-queue |
Automatic merge from submit-queue (batch tested with PRs 42734, 42745, 42758, 42814, 42694) Create DefaultPodDeletionTimeout for e2e tests In our e2e and e2e_node tests, we had a number of different timeouts for deletion. Recent changes to the way deletion works (kubernetes#41644, kubernetes#41456) have resulted in some timeouts in e2e tests. kubernetes#42661 was the most recent fix for this. Most of these tests are not meant to test pod deletion latency, but rather just to clean up pods after a test is finished. For this reason, we should change all these tests to use a standard, fairly high timeout for deletion. cc @vishh @Random-Liu
7 participants
This node e2e test uses its own deletion timeout (1 minute) instead of the default (3 minutes).
#41644 likely increased time for deletion. See that PR for analysis on that.
There may be other problems with this test, but those are difficult to pick apart from hitting this low timeout.
This PR changes the Garbage Collector test to use the default timeout. This should allow us to discern if there are any actual bugs to fix.
cc @kubernetes/sig-node-bugs @calebamiles @derekwaynecarr