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

Add --force to kubectl delete and explain force deletion #35484

Merged
merged 1 commit into from Nov 5, 2016
Merged

Add --force to kubectl delete and explain force deletion #35484

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion hack/lib/test.sh
View file
Open in desktop
Expand Up @@ -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
Expand Down
37 changes: 24 additions & 13 deletions hack/make-rules/test-cmd.sh
View file
Open in desktop
Expand Up @@ -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}}" ''

Expand All @@ -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
Expand All @@ -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}}" ''

Expand All @@ -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}}' ''

Expand Down Expand Up @@ -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}}' ''

Expand Down Expand Up @@ -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}}" ''

Expand Down Expand Up @@ -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}}" ''

Expand Down Expand Up @@ -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}}" ''

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

##############
Expand Down Expand Up @@ -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
Expand Down Expand 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}}" ''

Expand Down
36 changes: 30 additions & 6 deletions pkg/kubectl/cmd/delete.go
View file
Open in desktop
Expand Up @@ -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.

Only one type of the arguments may be specified: filenames, resources and names, or resources and label selector
JSON and YAML formats are accepted. 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
Expand All @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'd place a WARNING: prefix to this message!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, but we can't do that today without changes to the command stuff. I'll open a follow up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Excuse my ignorance, why is it not possible to add a string prefix to the error message here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All errors are prefixed with error:. So if we added a prefix it would be error: WARNING: blah which doesn't make much sense.

}

shortOutput := cmdutil.GetFlagString(cmd, "output") == "name"
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/kubectl.go
View file
Open in desktop
Expand Up @@ -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...)
}

Expand Down