Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added API node ready check after PD test deleting a GCE instance. #46746

Merged
merged 1 commit into from Jun 1, 2017
Merged

Added API node ready check after PD test deleting a GCE instance. #46746

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion test/e2e/storage/pd.go
View file
Open in desktop
Expand Up @@ -43,7 +43,7 @@ import (
const (
gcePDDetachTimeout = 10 * time.Minute
gcePDDetachPollTime = 10 * time.Second
nodeStatusTimeout = 1 * time.Minute
nodeStatusTimeout = 3 * time.Minute
nodeStatusPollTime = 1 * time.Second
maxReadRetry = 3
)
Expand Down Expand Up @@ -428,6 +428,8 @@ var _ = framework.KubeDescribe("Pod Disks", func() {
By("Cleaning up PD-RW test env")
podClient.Delete(host0Pod.Name, metav1.NewDeleteOptions(0))
detachAndDeletePDs(diskName, []types.NodeName{host0Name})
framework.WaitForNodeToBeReady(f.ClientSet, string(host0Name), nodeStatusTimeout)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this go in the AfterEach() for the test suite instead of the specific test case, so that we make sure all the tests will properly wait?

Copy link
Contributor Author

@verult verult Jun 1, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a good idea. This is one of the only two tests that require nodes to become ready again, and given that I didn't spend time understanding the implications for the other tests yet, I decided to keep the scope small.

Another possibility is to put an "all nodes ready" check in the general AfterEach inside framework.go, possibly reducing flakes from other tests as well, but again it'd be harder to understand the scope of this. @kubernetes/sig-testing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving this inside "defer" since it waits for a specific deleted node in the test to be ready.

Expect(len(nodes.Items)).To(Equal(initialGroupSize))
Copy link
Member

@msau42 msau42 Jun 1, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to refresh nodes again? Otherwise, this will pick up the old nodes array?

}()

By("submitting host0Pod to kubernetes")
Expand Down