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

don't gracefully delete terminated pods #15900

Merged
merged 1 commit into from Oct 22, 2015
Merged

don't gracefully delete terminated pods #15900

merged 1 commit into from Oct 22, 2015

Conversation

mikedanese
Copy link
Member

No description provided.

@k8s-github-robot
Copy link

Labelling this PR as size/XS

@k8s-github-robot k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 19, 2015
@k8s-bot
Copy link

k8s-bot commented Oct 19, 2015

GCE e2e build/test failed for commit fb5b1380029e73abd439b744cd64f56ba7616b07.

@k8s-bot
Copy link

k8s-bot commented Oct 19, 2015

GCE e2e test build/test passed for commit 5b1f36f613f60925b8c9c273dc96daf38620142d.

@mikedanese mikedanese mentioned this pull request Oct 20, 2015
Closed
@smarterclayton
Copy link
Contributor

LGTM

@smarterclayton
Copy link
Contributor

Add a test please though

@bgrant0607 bgrant0607 added this to the v1.1 milestone Oct 20, 2015
bgrant0607
bgrant0607 reviewed Oct 20, 2015
View reviewed changes
pkg/registry/pod/strategy.go
@@ -109,6 +109,10 @@ func (podStrategy) CheckGracefulDelete(obj runtime.Object, options *api.DeleteOp
if len(pod.Spec.NodeName) == 0 {
period = 0
}
// if the pod is already terminated, delete immediately
if pod.Status.Phase == api.PodFailed && pod.Status.Phase == api.PodSucceeded {
Copy link
Member

Choose a reason for hiding this comment

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

OR, not AND

@mikedanese
Copy link
Member Author

Added test for defaulted DeleteOptions

@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 20, 2015
@smarterclayton smarterclayton added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2015
ncdc
ncdc reviewed Oct 20, 2015
View reviewed changes
pkg/registry/pod/strategy_test.go
func TestCheckGracefulDelete(t *testing.T) {

defaultGracePeriod := int64(30)
otherGracePeriod := int64(15)
Copy link
Member

Choose a reason for hiding this comment

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

unused?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@k8s-bot
Copy link

k8s-bot commented Oct 20, 2015

GCE e2e test build/test passed for commit d0be6afb6417705689aa58c545b890a51618959e.

@k8s-github-robot
Copy link

PR changed after LGTM, removing LGTM.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2015
@k8s-bot
Copy link

k8s-bot commented Oct 20, 2015

GCE e2e test build/test passed for commit fbebf0e63f0f123e47009a4e630f6692bc9afdd0.

@k8s-bot
Copy link

k8s-bot commented Oct 20, 2015

GCE e2e build/test failed for commit c2c507e368cc1e750d3bef394e0561a7e00886a5.

@smarterclayton
Copy link
Contributor

ok to test

@k8s-bot
Copy link

k8s-bot commented Oct 20, 2015

GCE e2e build/test failed for commit c2c507e368cc1e750d3bef394e0561a7e00886a5.

@mikedanese
Copy link
Member Author

Flake Kubernetes e2e suite.Probing container with readiness probe should not be ready before initial delay and never restart [Conformance]

@k8s-bot ok to test

@k8s-bot
Copy link

k8s-bot commented Oct 20, 2015

GCE e2e test build/test passed for commit c2c507e368cc1e750d3bef394e0561a7e00886a5.

@mikedanese
Copy link
Member Author

@ncdc PTAL

@ncdc
Copy link
Member

ncdc commented Oct 20, 2015

@mikedanese looks good, thanks!

@k8s-bot
Copy link

k8s-bot commented Oct 20, 2015

GCE e2e test build/test passed for commit ac686d7.

@mikedanese mikedanese added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2015
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Oct 22, 2015

GCE e2e test build/test passed for commit ac686d7.

saad-ali added a commit that referenced this pull request Oct 22, 2015
@saad-ali
Merge pull request #15900 from mikedanese/graceterm-1
a47a790 
don't gracefully delete terminated pods
@saad-ali saad-ali merged commit a47a790 into kubernetes:master Oct 22, 2015
@mikedanese mikedanese deleted the graceterm-1 branch October 22, 2015 19:44
@mikedanese mikedanese mentioned this pull request Oct 22, 2015
Closed
@smarterclayton smarterclayton mentioned this pull request Oct 22, 2015
Closed
@mikedanese mikedanese mentioned this pull request Oct 23, 2015
Merged
@ikehz
Copy link
Contributor

ikehz commented Oct 24, 2015

@mikedanese Can you please file an issue under kind/workaround-removal for removing this when we drop support for v1.0? Thanks!

@mikedanese
Copy link
Member Author

@ihmccreery we want this forever.

k8s-github-robot pushed a commit that referenced this pull request Oct 24, 2015
Merge pull request #16169 from mikedanese/automated-cherry-pick-of-#1…
50b4808 
…5153-#15900-#15930-upstream-release-1.1

Auto commit by PR queue bot
@mikedanese mikedanese mentioned this pull request Oct 30, 2015
Merged
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
Merge pull request kubernetes#16169 from mikedanese/automated-cherry-…
694127e 
…pick-of-#15153-kubernetes#15900-kubernetes#15930-upstream-release-1.1

Auto commit by PR queue bot
shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017
Merge pull request kubernetes#16169 from mikedanese/automated-cherry-…
8f359c5 
…pick-of-#15153-kubernetes#15900-kubernetes#15930-upstream-release-1.1

Auto commit by PR queue bot
@cofyc cofyc mentioned this pull request Feb 21, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@smarterclayton smarterclayton

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.1
Development

Successfully merging this pull request may close these issues.

None yet