From a06eb4c3c199cbe4709c22487c3db838d5bffd85 Mon Sep 17 00:00:00 2001 From: Daniel Helfand Date: Tue, 21 Apr 2020 10:15:06 -0400 Subject: [PATCH] fix keep description and remove need to use with --all flag --- docs/cmd/tkn_pipelinerun_delete.md | 2 +- docs/cmd/tkn_taskrun_delete.md | 2 +- docs/man/man1/tkn-pipelinerun-delete.1 | 2 +- docs/man/man1/tkn-taskrun-delete.1 | 2 +- pkg/cmd/pipelinerun/delete.go | 10 +++++++++- pkg/cmd/pipelinerun/delete_test.go | 13 +++++++++++-- pkg/cmd/taskrun/delete.go | 10 +++++++++- pkg/cmd/taskrun/delete_test.go | 13 +++++++++++-- pkg/options/delete.go | 8 -------- pkg/options/delete_test.go | 14 -------------- 10 files changed, 44 insertions(+), 32 deletions(-) diff --git a/docs/cmd/tkn_pipelinerun_delete.md b/docs/cmd/tkn_pipelinerun_delete.md index 98eaae02b..a715b4571 100644 --- a/docs/cmd/tkn_pipelinerun_delete.md +++ b/docs/cmd/tkn_pipelinerun_delete.md @@ -32,7 +32,7 @@ or --allow-missing-template-keys If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true) -f, --force Whether to force deletion (default: false) -h, --help help for delete - --keep int Keep n least recent number of pipelineruns when using --all with delete + --keep int Keep n most recent number of pipelineruns -o, --output string Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file. -p, --pipeline string The name of a pipeline whose pipelineruns should be deleted (does not delete the pipeline) --template string Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview]. diff --git a/docs/cmd/tkn_taskrun_delete.md b/docs/cmd/tkn_taskrun_delete.md index 8e88943f4..04741fd49 100644 --- a/docs/cmd/tkn_taskrun_delete.md +++ b/docs/cmd/tkn_taskrun_delete.md @@ -32,7 +32,7 @@ or --allow-missing-template-keys If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true) -f, --force Whether to force deletion (default: false) -h, --help help for delete - --keep int Keep n least recent number of taskruns when using --all with delete + --keep int Keep n most recent number of taskruns -o, --output string Output format. One of: json|yaml|name|go-template|go-template-file|template|templatefile|jsonpath|jsonpath-file. -t, --task string The name of a task whose taskruns should be deleted (does not delete the task) --template string Template string or path to template file to use when -o=go-template, -o=go-template-file. The template format is golang templates [http://golang.org/pkg/text/template/#pkg-overview]. diff --git a/docs/man/man1/tkn-pipelinerun-delete.1 b/docs/man/man1/tkn-pipelinerun-delete.1 index e92cbdd5d..a9eb6effe 100644 --- a/docs/man/man1/tkn-pipelinerun-delete.1 +++ b/docs/man/man1/tkn-pipelinerun-delete.1 @@ -37,7 +37,7 @@ Delete pipelineruns in a namespace .PP \fB\-\-keep\fP=0 - Keep n least recent number of pipelineruns when using \-\-all with delete + Keep n most recent number of pipelineruns .PP \fB\-o\fP, \fB\-\-output\fP="" diff --git a/docs/man/man1/tkn-taskrun-delete.1 b/docs/man/man1/tkn-taskrun-delete.1 index e4c157dc5..ad06e7f5d 100644 --- a/docs/man/man1/tkn-taskrun-delete.1 +++ b/docs/man/man1/tkn-taskrun-delete.1 @@ -37,7 +37,7 @@ Delete taskruns in a namespace .PP \fB\-\-keep\fP=0 - Keep n least recent number of taskruns when using \-\-all with delete + Keep n most recent number of taskruns .PP \fB\-o\fP, \fB\-\-output\fP="" diff --git a/pkg/cmd/pipelinerun/delete.go b/pkg/cmd/pipelinerun/delete.go index c64144ab7..5fda76d72 100644 --- a/pkg/cmd/pipelinerun/delete.go +++ b/pkg/cmd/pipelinerun/delete.go @@ -64,6 +64,14 @@ or return err } + if opts.Keep < 0 { + return fmt.Errorf("keep option should not be lower than 0") + } + + if opts.Keep > 0 { + opts.DeleteAllNs = true + } + if err := opts.CheckOptions(s, args, p.Namespace()); err != nil { return err } @@ -74,7 +82,7 @@ or f.AddFlags(c) c.Flags().BoolVarP(&opts.ForceDelete, "force", "f", false, "Whether to force deletion (default: false)") c.Flags().StringVarP(&opts.ParentResourceName, "pipeline", "p", "", "The name of a pipeline whose pipelineruns should be deleted (does not delete the pipeline)") - c.Flags().IntVarP(&opts.Keep, "keep", "", 0, "Keep n least recent number of pipelineruns when using --all with delete") + c.Flags().IntVarP(&opts.Keep, "keep", "", 0, "Keep n most recent number of pipelineruns") c.Flags().BoolVarP(&opts.DeleteAllNs, "all", "", false, "Delete all pipelineruns in a namespace (default: false)") _ = c.MarkZshCompPositionalArgumentCustom(1, "__tkn_get_pipelinerun") return c diff --git a/pkg/cmd/pipelinerun/delete_test.go b/pkg/cmd/pipelinerun/delete_test.go index 8073ab97c..357073734 100644 --- a/pkg/cmd/pipelinerun/delete_test.go +++ b/pkg/cmd/pipelinerun/delete_test.go @@ -236,7 +236,7 @@ func TestPipelineRunDelete(t *testing.T) { }, { name: "Delete all keeping 2", - command: []string{"delete", "--all", "-f", "--keep", "2", "-n", "ns"}, + command: []string{"delete", "-f", "--keep", "2", "-n", "ns"}, dynamic: seeds[4].dynamicClient, input: seeds[4].pipelineClient, inputStream: nil, @@ -245,13 +245,22 @@ func TestPipelineRunDelete(t *testing.T) { }, { name: "Keep -1 is a no go", - command: []string{"delete", "--all", "-f", "--keep", "-1", "-n", "ns"}, + command: []string{"delete", "-f", "--keep", "-1", "-n", "ns"}, dynamic: seeds[4].dynamicClient, input: seeds[4].pipelineClient, inputStream: nil, wantError: true, want: "keep option should not be lower than 0", }, + { + name: "Delete all keeping 1 with --all flag", + command: []string{"delete", "-f", "--all", "--keep", "1", "-n", "ns"}, + dynamic: seeds[4].dynamicClient, + input: seeds[4].pipelineClient, + inputStream: nil, + wantError: false, + want: "All but 1 PipelineRuns deleted in namespace \"ns\"\n", + }, { name: "Error from using pipelinerun name with --all", command: []string{"delete", "pipelinerun", "--all", "-n", "ns"}, diff --git a/pkg/cmd/taskrun/delete.go b/pkg/cmd/taskrun/delete.go index b6f7447aa..2fd60d058 100644 --- a/pkg/cmd/taskrun/delete.go +++ b/pkg/cmd/taskrun/delete.go @@ -65,6 +65,14 @@ or return err } + if opts.Keep < 0 { + return fmt.Errorf("keep option should not be lower than 0") + } + + if opts.Keep > 0 { + opts.DeleteAllNs = true + } + if err := opts.CheckOptions(s, args, p.Namespace()); err != nil { return err } @@ -76,7 +84,7 @@ or c.Flags().BoolVarP(&opts.ForceDelete, "force", "f", false, "Whether to force deletion (default: false)") c.Flags().StringVarP(&opts.ParentResourceName, "task", "t", "", "The name of a task whose taskruns should be deleted (does not delete the task)") c.Flags().BoolVarP(&opts.DeleteAllNs, "all", "", false, "Delete all taskruns in a namespace (default: false)") - c.Flags().IntVarP(&opts.Keep, "keep", "", 0, "Keep n least recent number of taskruns when using --all with delete") + c.Flags().IntVarP(&opts.Keep, "keep", "", 0, "Keep n most recent number of taskruns") _ = c.MarkZshCompPositionalArgumentCustom(1, "__tkn_get_taskrun") return c } diff --git a/pkg/cmd/taskrun/delete_test.go b/pkg/cmd/taskrun/delete_test.go index a934dce6e..218928265 100644 --- a/pkg/cmd/taskrun/delete_test.go +++ b/pkg/cmd/taskrun/delete_test.go @@ -206,16 +206,25 @@ func TestTaskRunDelete(t *testing.T) { }, { name: "Delete all keeping 2", - command: []string{"delete", "--all", "-f", "--keep", "2", "-n", "ns"}, + command: []string{"delete", "-f", "--keep", "2", "-n", "ns"}, dynamic: seeds[4].dynamicClient, input: seeds[4].pipelineClient, inputStream: nil, wantError: false, want: "All but 2 TaskRuns deleted in namespace \"ns\"\n", }, + { + name: "Delete all keeping 1 with --all flag", + command: []string{"delete", "-f", "--all", "--keep", "1", "-n", "ns"}, + dynamic: seeds[4].dynamicClient, + input: seeds[4].pipelineClient, + inputStream: nil, + wantError: false, + want: "All but 1 TaskRuns deleted in namespace \"ns\"\n", + }, { name: "Keep -1 is a no go", - command: []string{"delete", "--all", "-f", "--keep", "-1", "-n", "ns"}, + command: []string{"delete", "-f", "--keep", "-1", "-n", "ns"}, dynamic: seeds[4].dynamicClient, input: seeds[4].pipelineClient, inputStream: nil, diff --git a/pkg/options/delete.go b/pkg/options/delete.go index 7ea0cc262..3ed576633 100644 --- a/pkg/options/delete.go +++ b/pkg/options/delete.go @@ -35,10 +35,6 @@ type DeleteOptions struct { } func (o *DeleteOptions) CheckOptions(s *cli.Stream, resourceNames []string, ns string) error { - if o.Keep > 0 && !(o.DeleteAllNs || o.DeleteAll) { - return fmt.Errorf("must use --all flag with --keep") - } - // make sure no resource names are provided when using --all flag if len(resourceNames) > 0 && (o.DeleteAllNs || o.DeleteAll) || o.DeleteAllNs && o.DeleteRelated { return fmt.Errorf("--all flag should not have any arguments or flags specified with it") @@ -56,10 +52,6 @@ func (o *DeleteOptions) CheckOptions(s *cli.Stream, resourceNames []string, ns s return fmt.Errorf("must provide %s name(s) or use --all flag with delete", o.Resource) } - if o.Keep < 0 { - return fmt.Errorf("keep option should not be lower than 0") - } - if o.ForceDelete { return nil } diff --git a/pkg/options/delete_test.go b/pkg/options/delete_test.go index eb6ade631..8a4563c44 100644 --- a/pkg/options/delete_test.go +++ b/pkg/options/delete_test.go @@ -145,20 +145,6 @@ func TestDeleteOptions(t *testing.T) { wantError: true, want: "must provide Condition name(s) or use --all flag with delete", }, - { - name: "Error when using --keep without --all", - opt: &DeleteOptions{Keep: 7, DeleteAllNs: false, DeleteAll: false}, - resourcesNames: []string{}, - wantError: true, - want: "must use --all flag with --keep", - }, - { - name: "Error --keep < 0", - opt: &DeleteOptions{Keep: -1, DeleteAllNs: true}, - resourcesNames: []string{}, - wantError: true, - want: "keep option should not be lower than 0", - }, } for _, tp := range testParams {