From 6e258303781c69b877e535aed503d77ef4e4bd64 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 24 Oct 2016 22:43:37 -0400 Subject: [PATCH] Add --force to kubectl delete and explain force deletion --force is required for --grace-period=0. --now is == --grace-period=1. Improve command help to explain what graceful deletion is and warn about force deletion. --- hack/lib/test.sh | 2 +- hack/make-rules/test-cmd.sh | 37 ++++++++++++++++++++++++------------- pkg/kubectl/cmd/delete.go | 34 +++++++++++++++++++++++++++++----- test/e2e/kubectl.go | 2 +- 4 files changed, 55 insertions(+), 20 deletions(-) diff --git a/hack/lib/test.sh b/hack/lib/test.sh index 00f7ceb6c85..5f6bff88d88 100644 --- a/hack/lib/test.sh +++ b/hack/lib/test.sh @@ -23,7 +23,7 @@ readonly red=$(tput setaf 1) readonly green=$(tput setaf 2) kube::test::clear_all() { - kubectl delete "${kube_flags[@]}" rc,pods --all --grace-period=0 + kubectl delete "${kube_flags[@]}" rc,pods --all --grace-period=0 --force } # Force exact match of a returned result for a object query. Wrap this with || to support multiple diff --git a/hack/make-rules/test-cmd.sh b/hack/make-rules/test-cmd.sh index 70e3c9a4da2..37e49d78b40 100755 --- a/hack/make-rules/test-cmd.sh +++ b/hack/make-rules/test-cmd.sh @@ -473,7 +473,7 @@ runTests() { # Pre-condition: valid-pod POD exists kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' # Command - kubectl delete pod valid-pod "${kube_flags[@]}" --grace-period=0 + kubectl delete pod valid-pod "${kube_flags[@]}" --grace-period=0 --force # Post-condition: valid-pod POD doesn't exist kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' @@ -486,6 +486,17 @@ runTests() { # Post-condition: valid-pod POD doesn't exist kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' + ### Delete POD valid-pod by id with --grace-period=0 + # Pre-condition: valid-pod POD exists + kubectl create "${kube_flags[@]}" -f test/fixtures/doc-yaml/admin/limitrange/valid-pod.yaml + kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' + # Command fails without --force + ! kubectl delete pod valid-pod "${kube_flags[@]}" --grace-period=0 + # Command succeds with --force + kubectl delete pod valid-pod "${kube_flags[@]}" --grace-period=0 --force + # Post-condition: valid-pod POD doesn't exist + kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' + ### Create POD valid-pod from dumped YAML # Pre-condition: no POD exists create_and_use_new_namespace @@ -499,7 +510,7 @@ runTests() { # Pre-condition: valid-pod POD exists kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' # Command - kubectl delete -f test/fixtures/doc-yaml/admin/limitrange/valid-pod.yaml "${kube_flags[@]}" --grace-period=0 + kubectl delete -f test/fixtures/doc-yaml/admin/limitrange/valid-pod.yaml "${kube_flags[@]}" --grace-period=0 --force # Post-condition: valid-pod POD doesn't exist kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' @@ -516,7 +527,7 @@ runTests() { # Pre-condition: valid-pod POD exists kube::test::get_object_assert "pods -l'name in (valid-pod)'" '{{range.items}}{{$id_field}}:{{end}}' 'valid-pod:' # Command - kubectl delete pods -l'name in (valid-pod)' "${kube_flags[@]}" --grace-period=0 + kubectl delete pods -l'name in (valid-pod)' "${kube_flags[@]}" --grace-period=0 --force # Post-condition: valid-pod POD doesn't exist kube::test::get_object_assert "pods -l'name in (valid-pod)'" '{{range.items}}{{$id_field}}:{{end}}' '' @@ -549,7 +560,7 @@ runTests() { # Pre-condition: valid-pod POD exists kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' # Command - kubectl delete --all pods "${kube_flags[@]}" --grace-period=0 # --all remove all the pods + kubectl delete --all pods "${kube_flags[@]}" --grace-period=0 --force # --all remove all the pods # Post-condition: no POD exists kube::test::get_object_assert "pods -l'name in (valid-pod)'" '{{range.items}}{{$id_field}}:{{end}}' '' @@ -607,7 +618,7 @@ runTests() { # Pre-condition: valid-pod and redis-proxy PODs exist kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'redis-proxy:valid-pod:' # Command - kubectl delete pods valid-pod redis-proxy "${kube_flags[@]}" --grace-period=0 # delete multiple pods at once + kubectl delete pods valid-pod redis-proxy "${kube_flags[@]}" --grace-period=0 --force # delete multiple pods at once # Post-condition: no POD exists kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' @@ -669,7 +680,7 @@ runTests() { # Pre-condition: valid-pod POD exists kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' # Command - kubectl delete pods -lnew-name=new-valid-pod --grace-period=0 "${kube_flags[@]}" + kubectl delete pods -lnew-name=new-valid-pod --grace-period=0 --force "${kube_flags[@]}" # Post-condition: valid-pod POD doesn't exist kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' @@ -896,7 +907,7 @@ __EOF__ # Pre-condition: valid-pod POD exists kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' # Command - kubectl delete pods -l'name in (valid-pod-super-sayan)' --grace-period=0 "${kube_flags[@]}" + kubectl delete pods -l'name in (valid-pod-super-sayan)' --grace-period=0 --force "${kube_flags[@]}" # Post-condition: valid-pod POD doesn't exist kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' @@ -1462,7 +1473,7 @@ __EOF__ # Pre-condition: busybox0 & busybox1 PODs exist kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'busybox0:busybox1:' # Command - output_message=$(! kubectl delete -f hack/testdata/recursive/pod --recursive --grace-period=0 2>&1 "${kube_flags[@]}") + output_message=$(! kubectl delete -f hack/testdata/recursive/pod --recursive --grace-period=0 --force 2>&1 "${kube_flags[@]}") # Post-condition: busybox0 & busybox1 PODs are deleted, and since busybox2 is malformed, it should error kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' kube::test::if_has_string "${output_message}" "Object 'Kind' is missing" @@ -1521,7 +1532,7 @@ __EOF__ # Pre-condition: busybox0 & busybox1 PODs exist kube::test::get_object_assert rc "{{range.items}}{{$id_field}}:{{end}}" 'busybox0:busybox1:' # Command - output_message=$(! kubectl delete -f hack/testdata/recursive/rc --recursive --grace-period=0 2>&1 "${kube_flags[@]}") + output_message=$(! kubectl delete -f hack/testdata/recursive/rc --recursive --grace-period=0 --force 2>&1 "${kube_flags[@]}") # Post-condition: busybox0 & busybox1 replication controllers are deleted, and since busybox2 is malformed, it should error kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' kube::test::if_has_string "${output_message}" "Object 'Kind' is missing" @@ -1561,7 +1572,7 @@ __EOF__ # Clean up unset PRESERVE_ERR_FILE rm "${ERROR_FILE}" - ! kubectl delete -f hack/testdata/recursive/deployment --recursive "${kube_flags[@]}" --grace-period=0 + ! kubectl delete -f hack/testdata/recursive/deployment --recursive "${kube_flags[@]}" --grace-period=0 --force sleep 1 ### Rollout on multiple replication controllers recursively - these tests ensure that rollouts cannot be performed on resources that don't support it @@ -1590,7 +1601,7 @@ __EOF__ kube::test::if_has_string "${output_message}" 'replicationcontrollers "busybox0" resuming is not supported' kube::test::if_has_string "${output_message}" 'replicationcontrollers "busybox0" resuming is not supported' # Clean up - ! kubectl delete -f hack/testdata/recursive/rc --recursive "${kube_flags[@]}" --grace-period=0 + ! kubectl delete -f hack/testdata/recursive/rc --recursive "${kube_flags[@]}" --grace-period=0 --force sleep 1 ############## @@ -1637,7 +1648,7 @@ __EOF__ # Pre-condition: valid-pod POD exists kube::test::get_object_assert 'pods --namespace=other' "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' # Command - kubectl delete "${kube_flags[@]}" pod --namespace=other valid-pod --grace-period=0 + kubectl delete "${kube_flags[@]}" pod --namespace=other valid-pod --grace-period=0 --force # Post-condition: valid-pod POD doesn't exist kube::test::get_object_assert 'pods --namespace=other' "{{range.items}}{{$id_field}}:{{end}}" '' # Clean up @@ -2848,7 +2859,7 @@ __EOF__ # Pre-condition: valid-pod exists kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' # Command - kubectl delete "${kube_flags[@]}" pod valid-pod --grace-period=0 + kubectl delete "${kube_flags[@]}" pod valid-pod --grace-period=0 --force # Post-condition: valid-pod doesn't exist kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" '' diff --git a/pkg/kubectl/cmd/delete.go b/pkg/kubectl/cmd/delete.go index 1bcc01e7d58..c8445a22d33 100644 --- a/pkg/kubectl/cmd/delete.go +++ b/pkg/kubectl/cmd/delete.go @@ -36,9 +36,26 @@ var ( delete_long = templates.LongDesc(` Delete resources by filenames, stdin, resources and names, or by resources and label selector. - JSON and YAML formats are accepted. + JSON and YAML formats are accepted. Only one type of the arguments may be specified: filenames, + resources and names, or resources and label selector. - Only one type of the arguments may be specified: filenames, resources and names, or resources and label selector + Some resources, such as pods, support graceful deletion. These resources define a default period + before they are forcibly terminated (the grace period) but you may override that value with + the --grace-period flag, or pass --now to set a grace-period of 1. Because these resources often + represent entities in the cluster, deletion may not be acknowledged immediately. If the node + hosting a pod is down or cannot reach the API server, termination may take significantly longer + than the grace period. To force delete a resource, you must pass a grace period of 0 and specify + the --force flag. + + IMPORTANT: Force deleting pods does not wait for confirmation that the pod's processes have been + terminated, which can leave those processes running until the node detects the deletion and + completes graceful deletion. If your processes use shared storage or talk to a remote API and + depend on the name of the pod to identify themselves, force deleting those pods may result in + multiple processes running on different machines using the same identification which may lead + to data corruption or inconsistency. Only force delete pods when you are sure the pod is + terminated, or if your application can tolerate multiple copies of the same pod running at once. + Also, if you force delete pods the scheduler may place new pods on those nodes before the node + has released those resources and causing those pods to be evicted immediately. Note that the delete command does NOT do resource version checks, so if someone submits an update to a resource right when you submit a delete, their update @@ -57,9 +74,12 @@ var ( # Delete pods and services with label name=myLabel. kubectl delete pods,services -l name=myLabel - # Delete a pod immediately (no graceful shutdown) + # Delete a pod with minimal delay kubectl delete pod foo --now + # Force delete a pod on a dead node + kubectl delete pod foo --grace-period=0 --force + # Delete a pod with UID 1234-56-7890-234234-456456. kubectl delete pod 1234-56-7890-234234-456456 @@ -102,7 +122,8 @@ func NewCmdDelete(f cmdutil.Factory, out io.Writer) *cobra.Command { cmd.Flags().Bool("ignore-not-found", false, "Treat \"resource not found\" as a successful delete. Defaults to \"true\" when --all is specified.") cmd.Flags().Bool("cascade", true, "If true, cascade the deletion of the resources managed by this resource (e.g. Pods created by a ReplicationController). Default true.") cmd.Flags().Int("grace-period", -1, "Period of time in seconds given to the resource to terminate gracefully. Ignored if negative.") - cmd.Flags().Bool("now", false, "If true, resources are force terminated without graceful deletion (same as --grace-period=0).") + cmd.Flags().Bool("now", false, "If true, resources are signaled for immediate shutdown (same as --grace-period=1).") + cmd.Flags().Bool("force", false, "Immediate deletion of some resources may result in inconsistency or data loss and requires confirmation.") cmd.Flags().Duration("timeout", 0, "The length of time to wait before giving up on a delete, zero means determine a timeout from the size of the object") cmdutil.AddOutputFlagsForMutation(cmd) cmdutil.AddInclude3rdPartyFlags(cmd) @@ -148,7 +169,10 @@ func RunDelete(f cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []stri if gracePeriod != -1 { return fmt.Errorf("--now and --grace-period cannot be specified together") } - gracePeriod = 0 + gracePeriod = 1 + } + if gracePeriod == 0 && !cmdutil.GetFlagBool(cmd, "force") { + return fmt.Errorf("Immediate deletion does not wait for confirmation that the running resource has been terminated. The resource may continue to run on the cluster indefinitely. You must pass --force to delete with grace period 0.") } shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" diff --git a/test/e2e/kubectl.go b/test/e2e/kubectl.go index ce04e54d4a6..1d016b76578 100644 --- a/test/e2e/kubectl.go +++ b/test/e2e/kubectl.go @@ -144,7 +144,7 @@ func cleanupKubectlInputs(fileContents string, ns string, selectors ...string) { } // support backward compatibility : file paths or raw json - since we are removing file path // dependencies from this test. - framework.RunKubectlOrDieInput(fileContents, "delete", "--grace-period=0", "-f", "-", nsArg) + framework.RunKubectlOrDieInput(fileContents, "delete", "--grace-period=0", "--force", "-f", "-", nsArg) framework.AssertCleanup(ns, selectors...) }