Add a client flag to delete "--now" for grace period 0 #23756

smarterclayton · 2016-04-01T19:27:57Z

Is equivalent to --grace-period=0 but is more intuitive for end users.

Stuck pod?

kubectl delete pod foo --now

ncdc · 2016-04-01T19:41:31Z

Description should probably say kubectl instead of oc 😄

smarterclayton · 2016-04-01T19:46:08Z

We should have called kubectl kc

k8s-bot · 2016-04-01T20:02:28Z

GCE e2e build/test passed for commit 686a132dbea7ed0095614dbdd5fc76786823a1f4.

k8s-bot · 2016-04-04T04:59:38Z

GCE e2e build/test passed for commit 0cac01345dc9a152c4ea41abb1fc6f977f3d3eb7.

0xmichalis · 2016-04-04T09:22:13Z

Can you also add an example?

cc: @kubernetes/kubectl

k8s-bot · 2016-04-04T16:44:29Z

GCE e2e build/test passed for commit 5df2a394a443e38212c0c83202781216fcf980a1.

k8s-github-robot · 2016-04-05T23:54:44Z

@smarterclayton
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

k8s-bot · 2016-04-06T00:20:45Z

GCE e2e build/test passed for commit 5df2a394a443e38212c0c83202781216fcf980a1.

deads2k · 2016-04-06T12:39:56Z

I don't think I want to use the --force flag for this. I really want the idea of "protected" resources. These would be resources that are hard to delete, but should still be deletable if someone really means it. When we try to delete that resource, we could require them to add a "force" option to DeleteOptions.

As an example of things you want to protect, I could imagine someone deciding to protect serviceaccount/foo because they're using that one to run some maintenance script. I'd say this is also the exact case that was "solved" with immortalNamespaces.

smarterclayton · 2016-04-06T16:07:01Z

Force is saying "delete with grace period 0 without waiting for it to
gracefully shutdown". Nothing to do with immortal resources. Unless
you're saying you want to preserve --force for use with deletion protected
resources, which I can buy. In which case '--now' may be more appropriate.

The user expectation that needs to be managed is "I tried to delete this,
but it doesn't seem to be working. I want to override the normal behavior
and force deletion.". So force is probably aligned with the natural user
expectation.

On Wed, Apr 6, 2016 at 8:40 AM, David Eads notifications@github.com wrote:

I don't think I want to use the --force flag for this. I really want the
idea of "protected" resources. These would be resources that are hard to
delete, but should still be deletable if someone really means it. When we
try to delete that resource, we could require them to add a "force" option
to DeleteOptions.

As an example of things you want to protect, I could imagine someone
deciding to protect serviceaccount/foo because they're using that one to
run some maintenance script. I'd say this is also the exact case that was
"solved" with immortalNamespaces.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#23756 (comment)

deads2k · 2016-04-06T16:50:08Z

Unless
you're saying you want to preserve --force for use with deletion protected
resources, which I can buy.

Correct. Since the concept doesn't exist yet, I felt like I had to describe it.

I would prefer to have the concept of protected resources, in which case --force is a more natural fit for that. I'm fine with --now.

bgrant0607 · 2016-04-06T21:45:56Z

--force is about ignoring or working around conflicts.

bgrant0607 · 2016-04-06T21:47:46Z

--now would be similar to shutdown now, so I'm in favor of that.

bgrant0607 · 2016-04-06T21:48:03Z

cc @caesarxuchao @janetkuo

smarterclayton · 2016-04-06T22:04:21Z

Updated

On Wed, Apr 6, 2016 at 5:48 PM, Brian Grant notifications@github.com
wrote:

cc @caesarxuchao https://github.com/caesarxuchao @janetkuo
https://github.com/janetkuo

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#23756 (comment)

caesarxuchao · 2016-04-06T22:08:42Z

@smarterclayton @bgrant0607 in the future, do you expect "--grace-period=0" and "--now" to mean different things?

#23656 is proposing adding finalizers to resources. In that context, I would imagine for the deletion of a pod, "--now" means ignore all the finalizers, remove the Pod from etcd at once so that its name is released immediately; and "--grace-period=0" means containers receive a SIGKILL immediately, but Pod won't be deleted from etcd until all finalizers have completed.

smarterclayton · 2016-04-06T22:23:52Z

Would removing finalizers be a thing regular users can do?

On Wed, Apr 6, 2016 at 6:09 PM, Chao Xu notifications@github.com wrote:

@smarterclayton https://github.com/smarterclayton @bgrant0607
https://github.com/bgrant0607 in the future, do you expect
"--grace-period=0" and "--now" to mean different things?

#23656 #23656 is proposing
adding finalizers to resources. In that context, I would imagine for the
deletion of a pod, "--now" means ignore all the finalizers, remove the
Pod from etcd at once so that its name is released immediately; and
"--grace-period=0" means containers receive a SIGKILL immediately, but
Pod won't be deleted from etcd until all finalizers have completed.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#23756 (comment)

k8s-bot · 2016-04-06T22:44:15Z

GCE e2e build/test passed for commit 148157211f020fcb0ad1b3d104f4d3eab8a8495a.

caesarxuchao · 2016-04-06T22:51:02Z

Would removing finalizers be a thing regular users can do?

Yes, I think it's necessary. If a deletion is stuck because a finalizer is not responsive, a user should be able to remove the finalizer manually.

bgrant0607 · 2016-04-06T23:28:38Z

@caesarxuchao Good point, but may need to work through some use cases to understand whether removing finalizers would be acceptable. I suggest we work that out in the cascading deletion proposal rather than here. For example, blanket finalizer removal could conflict with another flag to indicate whether children should be orphaned.

Finalizers might need to be handled on a case-by-case basis.

smarterclayton · 2016-04-06T23:39:57Z

What if the finalizer is a link to an external system of record? I had assumed initializers are owned by the platform and not the end user, but had not considered finalizers as user driven

Is equivalent to --grace-period=0 but is more intuitive for end users.

k8s-bot · 2016-04-08T04:19:28Z

GCE e2e build/test passed for commit ea3467f.

janetkuo · 2016-04-08T05:45:33Z

pkg/kubectl/cmd/delete.go

+	gracePeriod := cmdutil.GetFlagInt(cmd, "grace-period")
+	if cmdutil.GetFlagBool(cmd, "now") {
+		if gracePeriod != -1 {
+			return fmt.Errorf("--now and --grace-period cannot be specified together")


we can allow --now and --grace-period=0 to be set together (although it's redundant)

janetkuo · 2016-04-08T05:46:28Z

Please update the PR title and description to --now

smarterclayton · 2016-04-08T15:54:14Z

Do we want to encourage users to do that? Why would a user ever set them
together? We generally don't allow combinations that are equivalent to be
specified together (which is fairly consistent from a unix perspective)

On Fri, Apr 8, 2016 at 1:46 AM, Janet Kuo notifications@github.com wrote:

Please update the PR title and description to --now

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#23756 (comment)

janetkuo · 2016-04-08T17:18:12Z

@smarterclayton I agree.

smarterclayton · 2016-04-11T23:03:42Z

Any other comments on this?

deads2k · 2016-04-12T11:47:57Z

lgtm

deads2k · 2016-04-12T11:48:56Z

@smarterclayton how picky are we on release notes? I labelled it that way just in case we want to note every CLI change, but let me know if that's not the case.

k8s-bot · 2016-04-12T12:24:09Z

GCE e2e build/test passed for commit ea3467f.

smarterclayton · 2016-04-12T14:34:25Z

It's a usability improvement so I think it's deserved.

k8s-bot · 2016-04-14T12:56:19Z

GCE e2e build/test passed for commit ea3467f.

…ed_again Bug 1747377: UPSTREAM: 73863: Fix code to handle delayed binding volumes Origin-commit: b05811ee34b081c7f3504d0701f02d25181f3e42

googlebot added the cla: yes label Apr 1, 2016

smarterclayton mentioned this pull request Apr 1, 2016

Reconcile graceful termination #20497

Closed

k8s-github-robot assigned ikehz Apr 1, 2016

k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Apr 1, 2016

smarterclayton force-pushed the force_delete branch from 686a132 to 0cac013 Compare April 4, 2016 04:26

smarterclayton force-pushed the force_delete branch from 0cac013 to 5df2a39 Compare April 4, 2016 16:03

k8s-github-robot added the kind/old-docs label Apr 4, 2016

ikehz added the team/ux label Apr 4, 2016

ikehz assigned bgrant0607 and unassigned ikehz Apr 4, 2016

bgrant0607 assigned deads2k and unassigned bgrant0607 Apr 6, 2016

smarterclayton force-pushed the force_delete branch from 5df2a39 to 1481572 Compare April 6, 2016 22:04

caesarxuchao mentioned this pull request Apr 7, 2016

[GarbageCollector] Adding a proposal for server-side cascading deletion #23656

Merged

Add a client flag for --now delete

ea3467f

Is equivalent to --grace-period=0 but is more intuitive for end users.

smarterclayton force-pushed the force_delete branch from 1481572 to ea3467f Compare April 8, 2016 03:45

janetkuo reviewed Apr 8, 2016
View reviewed changes

smarterclayton changed the title ~~Add a client flag for --force delete~~ Add a client flag to delete "--now" for grace period 0 Apr 8, 2016

deads2k added lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Apr 12, 2016

lavalamp merged commit a42d7fa into kubernetes:master Apr 14, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a client flag to delete "--now" for grace period 0 #23756

Add a client flag to delete "--now" for grace period 0 #23756

smarterclayton commented Apr 1, 2016

ncdc commented Apr 1, 2016

smarterclayton commented Apr 1, 2016

k8s-bot commented Apr 1, 2016

k8s-bot commented Apr 4, 2016

0xmichalis commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

k8s-github-robot commented Apr 5, 2016

k8s-bot commented Apr 6, 2016

deads2k commented Apr 6, 2016

smarterclayton commented Apr 6, 2016

deads2k commented Apr 6, 2016

bgrant0607 commented Apr 6, 2016

bgrant0607 commented Apr 6, 2016

bgrant0607 commented Apr 6, 2016

smarterclayton commented Apr 6, 2016

caesarxuchao commented Apr 6, 2016

smarterclayton commented Apr 6, 2016

k8s-bot commented Apr 6, 2016

caesarxuchao commented Apr 6, 2016

bgrant0607 commented Apr 6, 2016

smarterclayton commented Apr 6, 2016 via email

k8s-bot commented Apr 8, 2016

janetkuo Apr 8, 2016

janetkuo commented Apr 8, 2016

smarterclayton commented Apr 8, 2016

janetkuo commented Apr 8, 2016

smarterclayton commented Apr 11, 2016

deads2k commented Apr 12, 2016

deads2k commented Apr 12, 2016

k8s-bot commented Apr 12, 2016

smarterclayton commented Apr 12, 2016

k8s-bot commented Apr 14, 2016

Add a client flag to delete "--now" for grace period 0 #23756

Add a client flag to delete "--now" for grace period 0 #23756

Conversation

smarterclayton commented Apr 1, 2016

ncdc commented Apr 1, 2016

smarterclayton commented Apr 1, 2016

k8s-bot commented Apr 1, 2016

k8s-bot commented Apr 4, 2016

0xmichalis commented Apr 4, 2016

k8s-bot commented Apr 4, 2016

k8s-github-robot commented Apr 5, 2016

k8s-bot commented Apr 6, 2016

deads2k commented Apr 6, 2016

smarterclayton commented Apr 6, 2016

deads2k commented Apr 6, 2016

bgrant0607 commented Apr 6, 2016

bgrant0607 commented Apr 6, 2016

bgrant0607 commented Apr 6, 2016

smarterclayton commented Apr 6, 2016

caesarxuchao commented Apr 6, 2016

smarterclayton commented Apr 6, 2016

k8s-bot commented Apr 6, 2016

caesarxuchao commented Apr 6, 2016

bgrant0607 commented Apr 6, 2016

smarterclayton commented Apr 6, 2016 via email

k8s-bot commented Apr 8, 2016

janetkuo Apr 8, 2016

Choose a reason for hiding this comment

janetkuo commented Apr 8, 2016

smarterclayton commented Apr 8, 2016

janetkuo commented Apr 8, 2016

smarterclayton commented Apr 11, 2016

deads2k commented Apr 12, 2016

deads2k commented Apr 12, 2016

k8s-bot commented Apr 12, 2016

smarterclayton commented Apr 12, 2016

k8s-bot commented Apr 14, 2016