New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Node E2E: Change the disk eviction test to pull images again after the test. #33201
Node E2E: Change the disk eviction test to pull images again after the test. #33201
Conversation
if !isImageSupported() { | ||
framework.Logf("test skipped because the image is not supported by the test") | ||
AfterEach(func() { | ||
if !isImageSupported() || !evictionOptionIsSet() { // Skip the after each |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will AfterEach
run if the test was already skipped?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I've tried. So I've to check and skip again here.
if !evictionOptionIsSet() { | ||
framework.Logf("test skipped because eviction option is not set") | ||
return | ||
podClient.Delete(busyPodName, &api.DeleteOptions{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of manually deleting this, can we delete the namespace and wait until all pods are gone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yujuhong Do you mean that deleting the pod gracefully (maybe with a longer graceful period), and wait for the pod to be deleted instead of deleting containers manually here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean we can rely on the normal cleanup process, instead of the manual cleanup. I don't think longer grace period is required. kubelet should be able to clean up normally, no?
78e126f
to
fcfe426
Compare
@yujuhong Addressed comments. |
Jenkins GCE e2e failed for commit fcfe426. The magic incantation to run this job again is |
Marked p1 to help with the test flakes. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
Automatic merge from submit-queue |
Fixes #32022 (comment).
This PR changes the disk eviction test to pull test images again in
AfterEach
, because images may be evicted during the test.@yujuhong
/cc @kubernetes/sig-node
This change is