From ab0c9b8e1e007017cbcacf351c89c2b3ea29e2a7 Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Fri, 29 May 2015 16:16:30 -0700 Subject: [PATCH] Add a custom timeout flag for stop/delete. Also try to be smarter about setting the timeout. --- contrib/completions/bash/kubectl | 2 ++ docs/kubectl_delete.md | 3 ++- docs/kubectl_stop.md | 3 ++- docs/man/man1/kubectl-delete.1 | 4 ++++ docs/man/man1/kubectl-stop.1 | 4 ++++ pkg/kubectl/cmd/delete.go | 8 +++++--- pkg/kubectl/cmd/stop.go | 3 ++- pkg/kubectl/stop.go | 21 +++++++++++++++------ pkg/kubectl/stop_test.go | 10 +++++----- test/e2e/rc.go | 2 +- test/e2e/util.go | 2 +- test/integration/framework/master_utils.go | 2 +- 12 files changed, 44 insertions(+), 20 deletions(-) diff --git a/contrib/completions/bash/kubectl b/contrib/completions/bash/kubectl index 130d349b5be..1d7f600e8b4 100644 --- a/contrib/completions/bash/kubectl +++ b/contrib/completions/bash/kubectl @@ -365,6 +365,7 @@ _kubectl_delete() flags+=("--ignore-not-found") flags+=("--selector=") two_word_flags+=("-l") + flags+=("--timeout=") must_have_one_flag=() must_have_one_noun=() @@ -589,6 +590,7 @@ _kubectl_stop() flags+=("--ignore-not-found") flags+=("--selector=") two_word_flags+=("-l") + flags+=("--timeout=") must_have_one_flag=() must_have_one_noun=() diff --git a/docs/kubectl_delete.md b/docs/kubectl_delete.md index c5e094de147..9df402f78ba 100644 --- a/docs/kubectl_delete.md +++ b/docs/kubectl_delete.md @@ -49,6 +49,7 @@ $ kubectl delete pods --all -h, --help=false: help for delete --ignore-not-found=false: Treat "resource not found" as a successful delete. -l, --selector="": Selector (label query) to filter on. + --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 ``` ### Options inherited from parent commands @@ -83,6 +84,6 @@ $ kubectl delete pods --all ### SEE ALSO * [kubectl](kubectl.md) - kubectl controls the Kubernetes cluster manager -###### Auto generated by spf13/cobra at 2015-05-21 18:30:45.437003409 +0000 UTC +###### Auto generated by spf13/cobra at 2015-06-03 18:21:01.053120485 +0000 UTC [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/kubectl_delete.md?pixel)]() diff --git a/docs/kubectl_stop.md b/docs/kubectl_stop.md index e64cffd3f19..316c79c4129 100644 --- a/docs/kubectl_stop.md +++ b/docs/kubectl_stop.md @@ -39,6 +39,7 @@ $ kubectl stop -f path/to/resources -h, --help=false: help for stop --ignore-not-found=false: Treat "resource not found" as a successful stop. -l, --selector="": Selector (label query) to filter on. + --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 ``` ### Options inherited from parent commands @@ -73,6 +74,6 @@ $ kubectl stop -f path/to/resources ### SEE ALSO * [kubectl](kubectl.md) - kubectl controls the Kubernetes cluster manager -###### Auto generated by spf13/cobra at 2015-05-21 18:30:45.439945376 +0000 UTC +###### Auto generated by spf13/cobra at 2015-05-29 23:14:50.709764383 +0000 UTC [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/kubectl_stop.md?pixel)]() diff --git a/docs/man/man1/kubectl-delete.1 b/docs/man/man1/kubectl-delete.1 index d3d9e282566..c1c74f614fd 100644 --- a/docs/man/man1/kubectl-delete.1 +++ b/docs/man/man1/kubectl-delete.1 @@ -57,6 +57,10 @@ will be lost along with the rest of the resource. \fB\-l\fP, \fB\-\-selector\fP="" Selector (label query) to filter on. +.PP +\fB\-\-timeout\fP=0 + The length of time to wait before giving up on a delete, zero means determine a timeout from the size of the object + .SH OPTIONS INHERITED FROM PARENT COMMANDS .PP diff --git a/docs/man/man1/kubectl-stop.1 b/docs/man/man1/kubectl-stop.1 index b09739cac69..a00bcce9382 100644 --- a/docs/man/man1/kubectl-stop.1 +++ b/docs/man/man1/kubectl-stop.1 @@ -45,6 +45,10 @@ If the resource is scalable it will be scaled to 0 before deletion. \fB\-l\fP, \fB\-\-selector\fP="" Selector (label query) to filter on. +.PP +\fB\-\-timeout\fP=0 + The length of time to wait before giving up on a delete, zero means determine a timeout from the size of the object + .SH OPTIONS INHERITED FROM PARENT COMMANDS .PP diff --git a/pkg/kubectl/cmd/delete.go b/pkg/kubectl/cmd/delete.go index b7ab12a3717..55b0e1b8270 100644 --- a/pkg/kubectl/cmd/delete.go +++ b/pkg/kubectl/cmd/delete.go @@ -19,6 +19,7 @@ package cmd import ( "fmt" "io" + "time" "github.com/spf13/cobra" @@ -76,6 +77,7 @@ 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.") cmd.Flags().Bool("cascade", true, "If true, cascade the delete 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().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") return cmd } @@ -102,12 +104,12 @@ func RunDelete(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, args []str ignoreNotFound := cmdutil.GetFlagBool(cmd, "ignore-not-found") // By default use a reaper to delete all related resources. if cmdutil.GetFlagBool(cmd, "cascade") { - return ReapResult(r, f, out, cmdutil.GetFlagBool(cmd, "cascade"), ignoreNotFound, cmdutil.GetFlagInt(cmd, "grace-period")) + return ReapResult(r, f, out, cmdutil.GetFlagBool(cmd, "cascade"), ignoreNotFound, cmdutil.GetFlagDuration(cmd, "timeout"), cmdutil.GetFlagInt(cmd, "grace-period")) } return DeleteResult(r, out, ignoreNotFound) } -func ReapResult(r *resource.Result, f *cmdutil.Factory, out io.Writer, isDefaultDelete, ignoreNotFound bool, gracePeriod int) error { +func ReapResult(r *resource.Result, f *cmdutil.Factory, out io.Writer, isDefaultDelete, ignoreNotFound bool, timeout time.Duration, gracePeriod int) error { found := 0 if ignoreNotFound { r = r.IgnoreErrors(errors.IsNotFound) @@ -126,7 +128,7 @@ func ReapResult(r *resource.Result, f *cmdutil.Factory, out io.Writer, isDefault if gracePeriod >= 0 { options = api.NewDeleteOptions(int64(gracePeriod)) } - if _, err := reaper.Stop(info.Namespace, info.Name, options); err != nil { + if _, err := reaper.Stop(info.Namespace, info.Name, timeout, options); err != nil { return err } fmt.Fprintf(out, "%s/%s\n", info.Mapping.Resource, info.Name) diff --git a/pkg/kubectl/cmd/stop.go b/pkg/kubectl/cmd/stop.go index a821fde854d..ac8b443cf40 100644 --- a/pkg/kubectl/cmd/stop.go +++ b/pkg/kubectl/cmd/stop.go @@ -63,6 +63,7 @@ func NewCmdStop(f *cmdutil.Factory, out io.Writer) *cobra.Command { cmd.Flags().Bool("all", false, "[-all] to select all the specified resources.") cmd.Flags().Bool("ignore-not-found", false, "Treat \"resource not found\" as a successful stop.") cmd.Flags().Int("grace-period", -1, "Period of time in seconds given to the resource to terminate gracefully. Ignored if negative.") + 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") return cmd } @@ -84,5 +85,5 @@ func RunStop(f *cmdutil.Factory, cmd *cobra.Command, args []string, filenames ut if r.Err() != nil { return r.Err() } - return ReapResult(r, f, out, false, cmdutil.GetFlagBool(cmd, "ignore-not-found"), cmdutil.GetFlagInt(cmd, "grace-period")) + return ReapResult(r, f, out, false, cmdutil.GetFlagBool(cmd, "ignore-not-found"), cmdutil.GetFlagDuration(cmd, "timeout"), cmdutil.GetFlagInt(cmd, "grace-period")) } diff --git a/pkg/kubectl/stop.go b/pkg/kubectl/stop.go index d9525909cd6..0e25b0d754a 100644 --- a/pkg/kubectl/stop.go +++ b/pkg/kubectl/stop.go @@ -26,13 +26,15 @@ import ( ) const ( - Interval = time.Millisecond * 100 + Interval = time.Second * 1 Timeout = time.Minute * 5 ) // A Reaper handles terminating an object as gracefully as possible. +// timeout is how long we'll wait for the termination to be successful +// gracePeriod is time given to an API object for it to delete itself cleanly (e.g. pod shutdown) type Reaper interface { - Stop(namespace, name string, gracePeriod *api.DeleteOptions) (string, error) + Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) } type NoSuchReaperError struct { @@ -80,14 +82,21 @@ type objInterface interface { Get(name string) (meta.Interface, error) } -func (reaper *ReplicationControllerReaper) Stop(namespace, name string, gracePeriod *api.DeleteOptions) (string, error) { +func (reaper *ReplicationControllerReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) { rc := reaper.ReplicationControllers(namespace) scaler, err := ScalerFor("ReplicationController", NewScalerClient(*reaper)) if err != nil { return "", err } + if timeout == 0 { + rc, err := rc.Get(name) + if err != nil { + return "", err + } + timeout = Timeout + time.Duration(10*rc.Spec.Replicas)*time.Second + } retry := NewRetryParams(reaper.pollInterval, reaper.timeout) - waitForReplicas := NewRetryParams(reaper.pollInterval, reaper.timeout) + waitForReplicas := NewRetryParams(reaper.pollInterval, timeout) if err = scaler.Scale(namespace, name, 0, nil, retry, waitForReplicas); err != nil { return "", err } @@ -97,7 +106,7 @@ func (reaper *ReplicationControllerReaper) Stop(namespace, name string, gracePer return fmt.Sprintf("%s stopped", name), nil } -func (reaper *PodReaper) Stop(namespace, name string, gracePeriod *api.DeleteOptions) (string, error) { +func (reaper *PodReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) { pods := reaper.Pods(namespace) _, err := pods.Get(name) if err != nil { @@ -110,7 +119,7 @@ func (reaper *PodReaper) Stop(namespace, name string, gracePeriod *api.DeleteOpt return fmt.Sprintf("%s stopped", name), nil } -func (reaper *ServiceReaper) Stop(namespace, name string, gracePeriod *api.DeleteOptions) (string, error) { +func (reaper *ServiceReaper) Stop(namespace, name string, timeout time.Duration, gracePeriod *api.DeleteOptions) (string, error) { services := reaper.Services(namespace) _, err := services.Get(name) if err != nil { diff --git a/pkg/kubectl/stop_test.go b/pkg/kubectl/stop_test.go index 248c9dc539e..bf04294c9c4 100644 --- a/pkg/kubectl/stop_test.go +++ b/pkg/kubectl/stop_test.go @@ -34,7 +34,7 @@ func TestReplicationControllerStop(t *testing.T) { }) reaper := ReplicationControllerReaper{fake, time.Millisecond, time.Millisecond} name := "foo" - s, err := reaper.Stop("default", name, nil) + s, err := reaper.Stop("default", name, 0, nil) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -42,10 +42,10 @@ func TestReplicationControllerStop(t *testing.T) { if s != expected { t.Errorf("expected %s, got %s", expected, s) } - if len(fake.Actions) != 4 { - t.Errorf("unexpected actions: %v, expected 4 actions (get, update, get, delete)", fake.Actions) + if len(fake.Actions) != 5 { + t.Errorf("unexpected actions: %v, expected 4 actions (get, get, update, get, delete)", fake.Actions) } - for i, action := range []string{"get", "update", "get", "delete"} { + for i, action := range []string{"get", "get", "update", "get", "delete"} { if fake.Actions[i].Action != action+"-replicationController" { t.Errorf("unexpected action: %v, expected %s-replicationController", fake.Actions[i], action) } @@ -142,7 +142,7 @@ func TestSimpleStop(t *testing.T) { if err != nil { t.Errorf("unexpected error: %v (%s)", err, test.test) } - s, err := reaper.Stop("default", "foo", nil) + s, err := reaper.Stop("default", "foo", 0, nil) if err != nil && !test.expectError { t.Errorf("unexpected error: %v (%s)", err, test.test) } diff --git a/test/e2e/rc.go b/test/e2e/rc.go index 489be59e670..4940a8ef86c 100644 --- a/test/e2e/rc.go +++ b/test/e2e/rc.go @@ -103,7 +103,7 @@ func ServeImageOrFail(c *client.Client, test string, image string) { if err != nil { Logf("Failed to cleanup replication controller %v: %v.", controller.Name, err) } - if _, err = rcReaper.Stop(ns, controller.Name, nil); err != nil { + if _, err = rcReaper.Stop(ns, controller.Name, 0, nil); err != nil { Logf("Failed to stop replication controller %v: %v.", controller.Name, err) } }() diff --git a/test/e2e/util.go b/test/e2e/util.go index 38b940f08c5..8089f69dc75 100644 --- a/test/e2e/util.go +++ b/test/e2e/util.go @@ -898,7 +898,7 @@ func DeleteRC(c *client.Client, ns, name string) error { return err } startTime := time.Now() - _, err = reaper.Stop(ns, name, api.NewDeleteOptions(0)) + _, err = reaper.Stop(ns, name, 0, api.NewDeleteOptions(0)) deleteRCTime := time.Now().Sub(startTime) Logf("Deleting RC took: %v", deleteRCTime) return err diff --git a/test/integration/framework/master_utils.go b/test/integration/framework/master_utils.go index 3ff952e3547..df97ec34fe9 100644 --- a/test/integration/framework/master_utils.go +++ b/test/integration/framework/master_utils.go @@ -184,7 +184,7 @@ func StopRC(rc *api.ReplicationController, restClient *client.Client) error { if err != nil || reaper == nil { return err } - _, err = reaper.Stop(rc.Namespace, rc.Name, nil) + _, err = reaper.Stop(rc.Namespace, rc.Name, 0, nil) if err != nil { return err }