From d6d82425a4a2e56f40df64be8bb4adfbf8193de7 Mon Sep 17 00:00:00 2001 From: AdoHe Date: Sun, 15 Jan 2017 15:30:06 +0800 Subject: [PATCH] refactor delete to remove cobra dependency --- pkg/kubectl/cmd/delete.go | 118 +++++++++++++++++++++---------- pkg/kubectl/cmd/delete_test.go | 72 ++++++++++++------- pkg/kubectl/cmd/util/helpers.go | 5 ++ pkg/kubectl/cmd/util/printing.go | 5 ++ 4 files changed, 136 insertions(+), 64 deletions(-) diff --git a/pkg/kubectl/cmd/delete.go b/pkg/kubectl/cmd/delete.go index 5706b6239db..749e7b68657 100644 --- a/pkg/kubectl/cmd/delete.go +++ b/pkg/kubectl/cmd/delete.go @@ -89,8 +89,33 @@ var ( kubectl delete pods --all`) ) +type DeleteOptions struct { + resource.FilenameOptions + + Selector string + DeleteAll bool + IgnoreNotFound bool + Cascade bool + DeleteNow bool + ForceDeletion bool + WaitForDeletion bool + + GracePeriod int + Timeout time.Duration + + Include3rdParty bool + Output string + + Mapper meta.RESTMapper + Result *resource.Result + + f cmdutil.Factory + Out io.Writer + ErrOut io.Writer +} + func NewCmdDelete(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command { - options := &resource.FilenameOptions{} + options := &DeleteOptions{} // retrieve a list of handled resources from printer as valid args validArgs, argAliases := []string{}, []string{} @@ -110,44 +135,53 @@ func NewCmdDelete(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command { Example: delete_example, Run: func(cmd *cobra.Command, args []string) { cmdutil.CheckErr(cmdutil.ValidateOutputArgs(cmd)) - err := RunDelete(f, out, errOut, cmd, args, options) - cmdutil.CheckErr(err) + if err := options.Complete(f, out, errOut, args); err != nil { + cmdutil.CheckErr(err) + } + if err := options.Validate(f, cmd); err != nil { + cmdutil.CheckErr(cmdutil.UsageError(cmd, err.Error())) + } + if err := options.RunDelete(); err != nil { + cmdutil.CheckErr(err) + } }, SuggestFor: []string{"rm"}, ValidArgs: validArgs, ArgAliases: argAliases, } usage := "containing the resource to delete." - cmdutil.AddFilenameOptionFlags(cmd, options, usage) - cmd.Flags().StringP("selector", "l", "", "Selector (label query) to filter on.") - 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 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 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) + cmdutil.AddFilenameOptionFlags(cmd, &options.FilenameOptions, usage) + cmd.Flags().StringVarP(&options.Selector, "selector", "l", "", "Selector (label query) to filter on.") + cmd.Flags().BoolVar(&options.DeleteAll, "all", false, "[-all] to select all the specified resources.") + cmd.Flags().BoolVar(&options.IgnoreNotFound, "ignore-not-found", false, "Treat \"resource not found\" as a successful delete. Defaults to \"true\" when --all is specified.") + cmd.Flags().BoolVar(&options.Cascade, "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().IntVar(&options.GracePeriod, "grace-period", -1, "Period of time in seconds given to the resource to terminate gracefully. Ignored if negative.") + cmd.Flags().BoolVar(&options.DeleteNow, "now", false, "If true, resources are signaled for immediate shutdown (same as --grace-period=1).") + cmd.Flags().BoolVar(&options.ForceDeletion, "force", false, "Immediate deletion of some resources may result in inconsistency or data loss and requires confirmation.") + cmd.Flags().DurationVar(&options.Timeout, "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.AddOutputVarFlagsForMutation(cmd, &options.Output) + cmdutil.AddInclude3rdPartyVarFlags(cmd, &options.Include3rdParty) return cmd } -func RunDelete(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, args []string, options *resource.FilenameOptions) error { +func (o *DeleteOptions) Complete(f cmdutil.Factory, out, errOut io.Writer, args []string) error { cmdNamespace, enforceNamespace, err := f.DefaultNamespace() if err != nil { return err } - deleteAll := cmdutil.GetFlagBool(cmd, "all") + + // Set up client based support. mapper, typer, err := f.UnstructuredObject() if err != nil { return err } + o.Mapper = mapper r := resource.NewBuilder(mapper, typer, resource.ClientMapperFunc(f.UnstructuredClientForMapping), unstructured.UnstructuredJSONScheme). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). - FilenameParam(enforceNamespace, options). - SelectorParam(cmdutil.GetFlagString(cmd, "selector")). - SelectAllParam(deleteAll). + FilenameParam(enforceNamespace, &o.FilenameOptions). + SelectorParam(o.Selector). + SelectAllParam(o.DeleteAll). ResourceTypeOrNameArgs(false, args...).RequireObject(false). Flatten(). Do() @@ -155,9 +189,18 @@ func RunDelete(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, arg if err != nil { return err } + o.Result = r - ignoreNotFound := cmdutil.GetFlagBool(cmd, "ignore-not-found") - if deleteAll { + o.f = f + // Set up writer + o.Out = out + o.ErrOut = errOut + + return nil +} + +func (o *DeleteOptions) Validate(f cmdutil.Factory, cmd *cobra.Command) error { + if o.DeleteAll { f := cmd.Flags().Lookup("ignore-not-found") // The flag should never be missing if f == nil { @@ -165,37 +208,36 @@ func RunDelete(f cmdutil.Factory, out, errOut io.Writer, cmd *cobra.Command, arg } // If the user didn't explicitly set the option, default to ignoring NotFound errors when used with --all if !f.Changed { - ignoreNotFound = true + o.IgnoreNotFound = true } } - - gracePeriod := cmdutil.GetFlagInt(cmd, "grace-period") - force := cmdutil.GetFlagBool(cmd, "force") - if cmdutil.GetFlagBool(cmd, "now") { - if gracePeriod != -1 { + if o.DeleteNow { + if o.GracePeriod != -1 { return fmt.Errorf("--now and --grace-period cannot be specified together") } - gracePeriod = 1 + o.GracePeriod = 1 } - wait := false - if gracePeriod == 0 { - if force { - fmt.Fprintf(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") + if o.GracePeriod == 0 { + if 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") } else { // To preserve backwards compatibility, but prevent accidental data loss, we convert --grace-period=0 // into --grace-period=1 and wait until the object is successfully deleted. Users may provide --force // to bypass this wait. - wait = true - gracePeriod = 1 + o.WaitForDeletion = true + o.GracePeriod = 1 } } + return nil +} - shortOutput := cmdutil.GetFlagString(cmd, "output") == "name" +func (o *DeleteOptions) RunDelete() error { + shortOutput := o.Output == "name" // 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.GetFlagDuration(cmd, "timeout"), gracePeriod, wait, shortOutput, mapper, false) + if o.Cascade { + return ReapResult(o.Result, o.f, o.Out, true, o.IgnoreNotFound, o.Timeout, o.GracePeriod, o.WaitForDeletion, shortOutput, o.Mapper, false) } - return DeleteResult(r, out, ignoreNotFound, shortOutput, mapper) + return DeleteResult(o.Result, o.Out, o.IgnoreNotFound, shortOutput, o.Mapper) } func ReapResult(r *resource.Result, f cmdutil.Factory, out io.Writer, isDefaultDelete, ignoreNotFound bool, timeout time.Duration, gracePeriod int, waitForDeletion, shortOutput bool, mapper meta.RESTMapper, quiet bool) error { diff --git a/pkg/kubectl/cmd/delete_test.go b/pkg/kubectl/cmd/delete_test.go index e04ba78fd8b..81fdfee6bec 100644 --- a/pkg/kubectl/cmd/delete_test.go +++ b/pkg/kubectl/cmd/delete_test.go @@ -227,12 +227,19 @@ func TestDeleteObjectNotFound(t *testing.T) { tf.Namespace = "test" buf, errBuf := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}) - cmd := NewCmdDelete(f, buf, errBuf) - options := &resource.FilenameOptions{} - options.Filenames = []string{"../../../examples/guestbook/legacy/redis-master-controller.yaml"} - cmd.Flags().Set("cascade", "false") - cmd.Flags().Set("output", "name") - err := RunDelete(f, buf, errBuf, cmd, []string{}, options) + options := &DeleteOptions{ + FilenameOptions: resource.FilenameOptions{ + Filenames: []string{"../../../examples/guestbook/legacy/redis-master-controller.yaml"}, + }, + GracePeriod: -1, + Cascade: false, + Output: "name", + } + err := options.Complete(f, buf, errBuf, []string{}) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + err = options.RunDelete() if err == nil || !errors.IsNotFound(err) { t.Errorf("unexpected error: expected NotFound, got %v", err) } @@ -296,14 +303,20 @@ func TestDeleteAllNotFound(t *testing.T) { tf.Namespace = "test" buf, errBuf := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}) - cmd := NewCmdDelete(f, buf, errBuf) - cmd.Flags().Set("all", "true") - cmd.Flags().Set("cascade", "false") // Make sure we can explicitly choose to fail on NotFound errors, even with --all - cmd.Flags().Set("ignore-not-found", "false") - cmd.Flags().Set("output", "name") - - err := RunDelete(f, buf, errBuf, cmd, []string{"services"}, &resource.FilenameOptions{}) + options := &DeleteOptions{ + FilenameOptions: resource.FilenameOptions{}, + GracePeriod: -1, + Cascade: false, + DeleteAll: true, + IgnoreNotFound: false, + Output: "name", + } + err := options.Complete(f, buf, errBuf, []string{"services"}) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + err = options.RunDelete() if err == nil || !errors.IsNotFound(err) { t.Errorf("unexpected error: expected NotFound, got %v", err) } @@ -405,12 +418,19 @@ func TestDeleteMultipleObjectContinueOnMissing(t *testing.T) { tf.Namespace = "test" buf, errBuf := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}) - cmd := NewCmdDelete(f, buf, errBuf) - options := &resource.FilenameOptions{} - options.Filenames = []string{"../../../examples/guestbook/legacy/redis-master-controller.yaml", "../../../examples/guestbook/frontend-service.yaml"} - cmd.Flags().Set("cascade", "false") - cmd.Flags().Set("output", "name") - err := RunDelete(f, buf, errBuf, cmd, []string{}, options) + options := &DeleteOptions{ + FilenameOptions: resource.FilenameOptions{ + Filenames: []string{"../../../examples/guestbook/legacy/redis-master-controller.yaml", "../../../examples/guestbook/frontend-service.yaml"}, + }, + GracePeriod: -1, + Cascade: false, + Output: "name", + } + err := options.Complete(f, buf, errBuf, []string{}) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + err = options.RunDelete() if err == nil || !errors.IsNotFound(err) { t.Errorf("unexpected error: expected NotFound, got %v", err) } @@ -533,7 +553,6 @@ func TestDeleteMultipleSelector(t *testing.T) { func TestResourceErrors(t *testing.T) { testCases := map[string]struct { args []string - flags map[string]string errFn func(error) bool }{ "no args": { @@ -562,17 +581,18 @@ func TestResourceErrors(t *testing.T) { buf, errBuf := bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}) - cmd := NewCmdDelete(f, buf, errBuf) - cmd.SetOutput(buf) - - for k, v := range testCase.flags { - cmd.Flags().Set(k, v) + options := &DeleteOptions{ + FilenameOptions: resource.FilenameOptions{}, + GracePeriod: -1, + Cascade: false, + Output: "name", } - err := RunDelete(f, buf, errBuf, cmd, testCase.args, &resource.FilenameOptions{}) + err := options.Complete(f, buf, errBuf, testCase.args) if !testCase.errFn(err) { t.Errorf("%s: unexpected error: %v", k, err) continue } + if tf.Printer.(*testPrinter).Objects != nil { t.Errorf("unexpected print to default printer") } diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index 210a8be0996..d07b8afdc2a 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/pkg/kubectl/cmd/util/helpers.go @@ -557,6 +557,11 @@ func ShouldRecord(cmd *cobra.Command, info *resource.Info) bool { return GetRecordFlag(cmd) || (ContainsChangeCause(info) && !cmd.Flags().Changed("record")) } +func AddInclude3rdPartyVarFlags(cmd *cobra.Command, include3rdParty *bool) { + cmd.Flags().BoolVar(include3rdParty, "include-extended-apis", true, "If true, include definitions of new APIs via calls to the API server. [default true]") + cmd.Flags().MarkDeprecated("include-extended-apis", "No longer required.") +} + func AddInclude3rdPartyFlags(cmd *cobra.Command) { cmd.Flags().Bool("include-extended-apis", true, "If true, include definitions of new APIs via calls to the API server. [default true]") cmd.Flags().MarkDeprecated("include-extended-apis", "No longer required.") diff --git a/pkg/kubectl/cmd/util/printing.go b/pkg/kubectl/cmd/util/printing.go index 9c2b31f6ed3..d9c51617b1d 100644 --- a/pkg/kubectl/cmd/util/printing.go +++ b/pkg/kubectl/cmd/util/printing.go @@ -46,6 +46,11 @@ func AddOutputFlagsForMutation(cmd *cobra.Command) { cmd.Flags().StringP("output", "o", "", "Output mode. Use \"-o name\" for shorter output (resource/name).") } +// AddOutputVarFlagsForMutation adds output related flags to a command. Used by mutations only. +func AddOutputVarFlagsForMutation(cmd *cobra.Command, output *string) { + cmd.Flags().StringVarP(output, "output", "o", "", "Output mode. Use \"-o name\" for shorter output (resource/name).") +} + // AddOutputFlags adds output related flags to a command. func AddOutputFlags(cmd *cobra.Command) { cmd.Flags().StringP("output", "o", "", "Output format. One of: json|yaml|wide|name|custom-columns=...|custom-columns-file=...|go-template=...|go-template-file=...|jsonpath=...|jsonpath-file=... See custom columns [http://kubernetes.io/docs/user-guide/kubectl-overview/#custom-columns], golang template [http://golang.org/pkg/text/template/#pkg-overview] and jsonpath template [http://kubernetes.io/docs/user-guide/jsonpath].")