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.

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

Sleep before deleting the cluster to give the controller manager time to #8816

Merged
merged 1 commit into from
May 26, 2015

Conversation

a-robinson
Copy link
Contributor

delete any cloudprovider resources still around from the last test.

Should (hopefully) fix #7753.

@quinton-hoole @zmerlynn

@saad-ali saad-ali added this to the v1.0 milestone May 26, 2015
@saad-ali
Copy link
Member

Marking this for v1.0 milestone since it fixes test instability. Assigning @quinton-hoole

@saad-ali saad-ali assigned ghost May 26, 2015
@@ -222,5 +222,8 @@ fi

### Clean up ###
if [[ "${E2E_DOWN,,}" == "true" ]]; then
# Sleep before deleting the cluster to give the controller manager time to
# delete any cloudprovider resources still around from the last test.
sleep 60
Copy link

Choose a reason for hiding this comment

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

Please add a note as to how the number 60 was arrived at (presumably based on the ELB retry loop frequency), and that it should not be changed independently of that basis.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The deletions are managed using a watch and failed attempts to reconcile are retried every 5 seconds, so 60 is sort of an arbitrary bound. I've added a comment in the next push.

@ghost
Copy link

ghost commented May 26, 2015

One nit. Other than that, LGTM.

delete any cloudprovider resources still around from the last test.
@a-robinson a-robinson added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 26, 2015
saad-ali added a commit that referenced this pull request May 26, 2015
Sleep before deleting the cluster to give the controller manager time to
@saad-ali saad-ali merged commit 3975f6e into kubernetes:master May 26, 2015
@a-robinson a-robinson unassigned ghost Aug 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Target pool leak in GCE project running our jenkins e2e tests
3 participants