github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Default grace period to 0 when --force is used to delete an object

Browse Source
This commit is contained in:
Brian Pursley
2020-02-24 09:30:56 -05:00
parent ca23b07dd4
commit 0f31bef94c
3 changed files with 149 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
staging/src/k8s.io/kubectl/pkg/cmd/delete/delete.go
View File

@@ -52,8 +52,7 @@ var (
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.
than the grace period. To force delete a resource, you must specify the --force flag.
Note: only a subset of resources support graceful deletion. In absence of the support, --grace-period is ignored.
IMPORTANT: Force deleting pods does not wait for confirmation that the pod's processes have been
@@ -90,7 +89,7 @@ var (
kubectl delete pod foo --now
# Force delete a pod on a dead node
kubectl delete pod foo --grace-period=0 --force
kubectl delete pod foo --force
# Delete all pods
kubectl delete pods --all`))
@@ -176,6 +175,9 @@ func (o *DeleteOptions) Complete(f cmdutil.Factory, args []string, cmd *cobra.Co
// into --grace-period=1. Users may provide --force to bypass this conversion.
o.GracePeriod = 1
}
if o.ForceDeletion && o.GracePeriod < 0 {
o.GracePeriod = 0
}
o.DryRunStrategy, err = cmdutil.GetDryRunStrategy(cmd)
if err != nil {
@@ -239,8 +241,8 @@ func (o *DeleteOptions) Validate() error {
switch {
case o.GracePeriod == 0 && o.ForceDeletion:
fmt.Fprintf(o.ErrOut, "warning: Immediate deletion does not wait for confirmation that the running resource has been terminated. The resource may continue to run on the cluster indefinitely.\n")
case o.ForceDeletion:
fmt.Fprintf(o.ErrOut, "warning: --force is ignored because --grace-period is not 0.\n")
case o.GracePeriod > 0 && o.ForceDeletion:
return fmt.Errorf("--force and --grace-period greater than 0 cannot be specified together")
}
if len(o.Raw) > 0 {

2
staging/src/k8s.io/kubectl/pkg/cmd/delete/delete_flags.go
View File

@@ -116,7 +116,7 @@ func (f *DeleteFlags) AddFlags(cmd *cobra.Command) {
cmd.Flags().BoolVarP(f.AllNamespaces, "all-namespaces", "A", *f.AllNamespaces, "If present, list the requested object(s) across all namespaces. Namespace in current context is ignored even if specified with --namespace.")
}
if f.Force != nil {
cmd.Flags().BoolVar(f.Force, "force", *f.Force, "Only used when grace-period=0. If true, immediately remove resources from API and bypass graceful deletion. Note that immediate deletion of some resources may result in inconsistency or data loss and requires confirmation.")
cmd.Flags().BoolVar(f.Force, "force", *f.Force, "If true, immediately remove resources from API and bypass graceful deletion. Note that immediate deletion of some resources may result in inconsistency or data loss and requires confirmation.")
}
if f.Cascade != nil {
cmd.Flags().BoolVar(f.Cascade, "cascade", *f.Cascade, "If true, cascade the deletion of the resources managed by this resource (e.g. Pods created by a ReplicationController). Default true.")

154
staging/src/k8s.io/kubectl/pkg/cmd/delete/delete_test.go
View File

@@ -21,6 +21,7 @@ import (
"io"
"io/ioutil"
"net/http"
"strconv"
"strings"
"testing"
@@ -250,32 +251,120 @@ func TestDeleteObject(t *testing.T) {
}
}
func TestDeleteObjectGraceZero(t *testing.T) {
cmdtesting.InitTestErrorHandler(t)
func TestGracePeriodScenarios(t *testing.T) {
pods, _, _ := cmdtesting.TestData()
count := 0
tf := cmdtesting.NewTestFactory().WithNamespace("test")
defer tf.Cleanup()
codec := scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...)
tc := []struct {
name string
cmdArgs []string
forceFlag bool
nowFlag bool
gracePeriodFlag string
expectedGracePeriod string
expectedOut string
expectedErrOut string
expectedDeleteRequestPath string
expectedExitCode int
}{
{
name: "Deleting an object with --force should use grace period = 0",
cmdArgs: []string{"pods/foo"},
forceFlag: true,
expectedGracePeriod: "0",
expectedOut: "pod/foo\n",
expectedErrOut: "warning: Immediate deletion does not wait for confirmation that the running resource has been terminated. The resource may continue to run on the cluster indefinitely.\n",
expectedDeleteRequestPath: "/namespaces/test/pods/foo",
},
{
name: "Deleting an object with --force and --grace-period 0 should use grade period = 0",
cmdArgs: []string{"pods/foo"},
forceFlag: true,
gracePeriodFlag: "0",
expectedGracePeriod: "0",
expectedOut: "pod/foo\n",
expectedErrOut: "warning: Immediate deletion does not wait for confirmation that the running resource has been terminated. The resource may continue to run on the cluster indefinitely.\n",
expectedDeleteRequestPath: "/namespaces/test/pods/foo",
},
{
name: "Deleting an object with --force and --grace-period > 0 should fail",
cmdArgs: []string{"pods/foo"},
forceFlag: true,
gracePeriodFlag: "10",
expectedErrOut: "error: --force and --grace-period greater than 0 cannot be specified together",
expectedExitCode: 1,
},
{
name: "Deleting an object with --grace-period 0 should use a grace period of 1",
cmdArgs: []string{"pods/foo"},
gracePeriodFlag: "0",
expectedGracePeriod: "1",
expectedOut: "pod/foo\n",
expectedDeleteRequestPath: "/namespaces/test/pods/foo",
},
{
name: "Deleting an object with --grace-period > 0 should use the specified grace period",
cmdArgs: []string{"pods/foo"},
gracePeriodFlag: "10",
expectedGracePeriod: "10",
expectedOut: "pod/foo\n",
expectedDeleteRequestPath: "/namespaces/test/pods/foo",
},
{
name: "Deleting an object with the --now flag should use grace period = 1",
cmdArgs: []string{"pods/foo"},
nowFlag: true,
expectedGracePeriod: "1",
expectedOut: "pod/foo\n",
expectedDeleteRequestPath: "/namespaces/test/pods/foo",
},
{
name: "Deleting an object with --now and --grace-period should fail",
cmdArgs: []string{"pods/foo"},
nowFlag: true,
gracePeriodFlag: "10",
expectedErrOut: "error: --now and --grace-period cannot be specified together",
expectedExitCode: 1,
},
}
for _, test := range tc {
t.Run(test.name, func(t *testing.T) {
// Use a custom fatal behavior with panic/recover so that we can test failure scenarios where
// os.Exit() would normally be called
cmdutil.BehaviorOnFatal(func(actualErrOut string, actualExitCode int) {
if test.expectedExitCode != actualExitCode {
t.Errorf("unexpected exit code:\n\tExpected: %d\n\tActual: %d\n", test.expectedExitCode, actualExitCode)
}
if test.expectedErrOut != actualErrOut {
t.Errorf("unexpected error:\n\tExpected: %s\n\tActual: %s\n", test.expectedErrOut, actualErrOut)
}
panic(nil)
})
defer func() {
if test.expectedExitCode != 0 {
recover()
}
}()
// Setup a fake HTTP Client to capture whether a delete request was made or not and if so,
// the actual grace period that was used.
actualGracePeriod := ""
deleteOccurred := false
tf.UnstructuredClient = &fake.RESTClient{
NegotiatedSerializer: resource.UnstructuredPlusDefaultContentConfig().NegotiatedSerializer,
Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
t.Logf("got request %s %s", req.Method, req.URL.Path)
switch p, m := req.URL.Path, req.Method; {
case p == "/namespaces/test/pods/nginx" && m == "GET":
count++
switch count {
case 1, 2, 3:
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, &pods.Items[0])}, nil
default:
return &http.Response{StatusCode: http.StatusNotFound, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, &metav1.Status{})}, nil
}
case p == "/api/v1/namespaces/test" && m == "GET":
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, &corev1.Namespace{})}, nil
case p == "/namespaces/test/pods/nginx" && m == "DELETE":
case m == "DELETE" && p == test.expectedDeleteRequestPath:
data := make(map[string]interface{})
_ = json.NewDecoder(req.Body).Decode(&data)
actualGracePeriod = strconv.FormatFloat(data["gracePeriodSeconds"].(float64), 'f', 0, 64)
deleteOccurred = true
return &http.Response{StatusCode: http.StatusOK, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, &pods.Items[0])}, nil
default:
t.Fatalf("unexpected request: %#v\n%#v", req.URL, req)
@@ -284,18 +373,35 @@ func TestDeleteObjectGraceZero(t *testing.T) {
}),
}
streams, _, buf, errBuf := genericclioptions.NewTestIOStreams()
// Test the command using the flags specified in the test case
streams, _, out, errOut := genericclioptions.NewTestIOStreams()
cmd := NewCmdDelete(tf, streams)
cmd.Flags().Set("output", "name")
cmd.Flags().Set("grace-period", "0")
cmd.Run(cmd, []string{"pods/nginx"})
// uses the name from the file, not the response
if buf.String() != "pod/nginx\n" {
t.Errorf("unexpected output: %s\n---\n%s", buf.String(), errBuf.String())
if test.forceFlag {
cmd.Flags().Set("force", "true")
}
if count != 0 {
t.Errorf("unexpected calls to GET: %d", count)
if test.nowFlag {
cmd.Flags().Set("now", "true")
}
if len(test.gracePeriodFlag) > 0 {
cmd.Flags().Set("grace-period", test.gracePeriodFlag)
}
cmd.Run(cmd, test.cmdArgs)
// Check actual vs expected conditions
if len(test.expectedDeleteRequestPath) > 0 && !deleteOccurred {
t.Errorf("expected http delete request to %s but it did not occur", test.expectedDeleteRequestPath)
}
if test.expectedGracePeriod != actualGracePeriod {
t.Errorf("unexpected grace period:\n\tExpected: %s\n\tActual: %s\n", test.expectedGracePeriod, actualGracePeriod)
}
if out.String() != test.expectedOut {
t.Errorf("unexpected output:\n\tExpected: %s\n\tActual: %s\n", test.expectedOut, out.String())
}
if errOut.String() != test.expectedErrOut {
t.Errorf("unexpected error output:\n\tExpected: %s\n\tActual: %s\n", test.expectedErrOut, errOut.String())
}
})
}
}