Skip to content

Commit

Permalink
fix keep description and remove need to use with --all flag
Browse files Browse the repository at this point in the history
  • Loading branch information
danielhelfand authored and tekton-robot committed Apr 21, 2020
1 parent 62365e4 commit a06eb4c
Show file tree
Hide file tree
Showing 10 changed files with 44 additions and 32 deletions.
2 changes: 1 addition & 1 deletion docs/cmd/tkn_pipelinerun_delete.md
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
2 changes: 1 addition & 1 deletion docs/cmd/tkn_taskrun_delete.md
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down
2 changes: 1 addition & 1 deletion docs/man/man1/tkn-pipelinerun-delete.1
Original file line number Diff line number Diff line change
Expand Up @@ -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=""
Expand Down
2 changes: 1 addition & 1 deletion docs/man/man1/tkn-taskrun-delete.1
Original file line number Diff line number Diff line change
Expand Up @@ -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=""
Expand Down
10 changes: 9 additions & 1 deletion pkg/cmd/pipelinerun/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down
13 changes: 11 additions & 2 deletions pkg/cmd/pipelinerun/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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"},
Expand Down
10 changes: 9 additions & 1 deletion pkg/cmd/taskrun/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
13 changes: 11 additions & 2 deletions pkg/cmd/taskrun/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 0 additions & 8 deletions pkg/options/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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
}
Expand Down
14 changes: 0 additions & 14 deletions pkg/options/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit a06eb4c

Please sign in to comment.