Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions docs/cmd/tkn_taskrun_delete.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,15 @@ or
### Options

```
--all Delete all TaskRuns in a namespace (default: false)
--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
-i, --ignore-running ignore running TaskRun (default true)
--ignore-running-pipelinerun ignore deleting taskruns of a running PipelineRun (default true)
--keep int Keep n most recent number of TaskRuns
--keep-since int When deleting all TaskRuns keep the ones that has been completed since n minutes
-o, --output string Output format. One of: (json, yaml, name, go-template, go-template-file, template, templatefile, jsonpath, jsonpath-as-json, jsonpath-file).
--show-managed-fields If true, keep the managedFields when printing objects in JSON or YAML format.
-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].
--all Delete all TaskRuns in a namespace (default: false)
-f, --force Whether to force deletion (default: false)
-h, --help help for delete
-i, --ignore-running ignore running TaskRun (default true)
--ignore-running-pipelinerun ignore deleting taskruns of a running PipelineRun (default true)
--keep int Keep n most recent number of TaskRuns
--keep-since int When deleting all TaskRuns keep the ones that has been completed since n minutes
-o, --output string Output format. One of: json|yaml. Skips the confirmation prompt
-t, --task string The name of a Task whose TaskRuns should be deleted (does not delete the task)
```

### Options inherited from parent commands
Expand Down
15 changes: 1 addition & 14 deletions docs/man/man1/tkn-taskrun-delete.1
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,6 @@ Delete TaskRuns in a namespace
\fB\-\-all\fP[=false]
Delete all TaskRuns in a namespace (default: false)

.PP
\fB\-\-allow\-missing\-template\-keys\fP[=true]
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.

.PP
\fB\-f\fP, \fB\-\-force\fP[=false]
Whether to force deletion (default: false)
Expand All @@ -53,21 +49,12 @@ Delete TaskRuns in a namespace

.PP
\fB\-o\fP, \fB\-\-output\fP=""
Output format. One of: (json, yaml, name, go\-template, go\-template\-file, template, templatefile, jsonpath, jsonpath\-as\-json, jsonpath\-file).

.PP
\fB\-\-show\-managed\-fields\fP[=false]
If true, keep the managedFields when printing objects in JSON or YAML format.
Output format. One of: json|yaml. Skips the confirmation prompt

.PP
\fB\-t\fP, \fB\-\-task\fP=""
The name of a Task whose TaskRuns should be deleted (does not delete the task)

.PP
\fB\-\-template\fP=""
Template string or path to template file to use when \-o=go\-template, \-o=go\-template\-file. The template format is golang templates [
\[la]http://golang.org/pkg/text/template/#pkg-overview\[ra]].


.SH OPTIONS INHERITED FROM PARENT COMMANDS
.PP
Expand Down
62 changes: 47 additions & 15 deletions pkg/cmd/taskrun/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"go.uber.org/multierr"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
cliopts "k8s.io/cli-runtime/pkg/genericclioptions"
)

type deleteOptions struct {
Expand Down Expand Up @@ -64,7 +63,6 @@ func trExists(args []string, p cli.Params) ([]string, error) {
func deleteCommand(p cli.Params) *cobra.Command {
opts := &options.DeleteOptions{Resource: "TaskRun", ForceDelete: false, DeleteAllNs: false}
deleteOpts := &deleteOptions{}
f := cliopts.NewPrintFlags("delete")
eg := `Delete TaskRuns with names 'foo' and 'bar' in namespace 'quux':

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add examples for a named delete,  --all and  --task  with  -o, plus a line about the running-PipelineRun behaviour? Then can we run  make generated  to refresh the docs and man page?


tkn taskrun delete foo bar -n quux
Expand Down Expand Up @@ -92,6 +90,16 @@ or
Err: cmd.OutOrStderr(),
}

output, err := cmd.LocalFlags().GetString("output")
Comment thread
Debashich marked this conversation as resolved.
Resolved
if err != nil {
return err
}

output = formatted.NormalizeOutput(output)
if output != "" && !formatted.IsStructured(output) {
return fmt.Errorf("invalid output format %q: must be json or yaml", output)
}

if deleteOpts.TaskName != "" {
opts.ParentResource = "Task"
opts.ParentResourceName = deleteOpts.TaskName
Expand Down Expand Up @@ -122,17 +130,22 @@ or
return errs
}

if err := opts.CheckOptions(s, availableTrs, p.Namespace()); err != nil {
checkStreams := s
if formatted.IsStructured(output) {
s.Err = cmd.ErrOrStderr()
checkStreams = &cli.Stream{In: strings.NewReader("y\n"), Out: &strings.Builder{}, Err: s.Err}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckOptions  gets a fake stream that auto-answers "y". This only works while the prompt logic stays exactly as it is. The other delete commands set  ForceDelete = true  for  -o and use the real stream.

Can we skip the confirmation explicitly instead For example, call  CheckOptions  with a copy of the options that has  ForceDelete  set. That keeps the validation, and the original ForceDelete  stays untouched for the running-PipelineRun check.

}
if err := opts.CheckOptions(checkStreams, availableTrs, p.Namespace()); err != nil {
return err
}

if err := deleteTaskRuns(s, p, availableTrs, opts); err != nil {
if err := deleteTaskRuns(s, p, availableTrs, opts, output); err != nil {
return err
}
return errs
},
}
f.AddFlags(c)
c.Flags().StringP("output", "o", "", formatted.DeleteOutputFlagUsage)
c.Flags().BoolVarP(&opts.ForceDelete, "force", "f", false, "Whether to force deletion (default: false)")
c.Flags().StringVarP(&deleteOpts.TaskName, "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)")
Expand All @@ -144,7 +157,7 @@ or
return c
}

func deleteTaskRuns(s *cli.Stream, p cli.Params, trNames []string, opts *options.DeleteOptions) error {
func deleteTaskRuns(s *cli.Stream, p cli.Params, trNames []string, opts *options.DeleteOptions, output string) error {
var numberOfDeletedTr, numberOfKeptTr int
cs, err := p.Clients()
if err != nil {
Expand Down Expand Up @@ -177,9 +190,17 @@ func deleteTaskRuns(s *cli.Stream, p cli.Params, trNames []string, opts *options
prFinished := ownerPrFinished(cs, *tr)

if !prFinished && opts.ForceDelete {
fmt.Fprintf(s.Out, "warning: Taskrun %s related pipelinerun still running.\n", tr.Name)
if formatted.IsStructured(output) {
fmt.Fprintf(s.Err, "warning: Taskrun %s related pipelinerun still running.\n", tr.Name)
} else {
fmt.Fprintf(s.Out, "warning: Taskrun %s related pipelinerun still running.\n", tr.Name)
}
}
if !prFinished && !opts.ForceDelete {
if formatted.IsStructured(output) {
return fmt.Errorf("taskrun %s is owned by a running PipelineRun; use --force to delete", tr.Name)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With  -o  and no  --force , one TaskRun owned by a running PipelineRun makes the function return before anything is deleted. Nothing is printed in the deleted list. Without  -o , declining the prompt only skips that TaskRun. The other delete commands also continue past a failing name.

Can we skip that TaskRun, record an error for it, and continue? The other names would still be deleted, the deleted list printed, and the recorded error returned at the end. Can we also document the  --force  requirement in the help text?

}

fmt.Fprintf(s.Out, "TaskRun(s): %s attached to PipelineRun is still running deleting will restart the completed taskrun. Proceed (y/n): ", tr.Name)
if err := opts.TakeInput(s, ""); err != nil {
continue
Expand Down Expand Up @@ -211,15 +232,26 @@ func deleteTaskRuns(s *cli.Stream, p cli.Params, trNames []string, opts *options
return actions.Delete(taskrunGroupResource, cs.Dynamic, cs.Tekton.Discovery(), taskRunName, p.Namespace(), metav1.DeleteOptions{})
})

if opts.Keep > 0 && opts.Keep == len(trToKeep) && len(trToDelete) == 0 {
fmt.Fprintf(s.Out, "Associated %s (%d) for Task:%s is/are equal to keep (%d) \n", opts.Resource, len(trToKeep), opts.ParentResourceName, opts.Keep)
return nil
}
if opts.Keep > len(trToKeep) {
fmt.Fprintf(s.Out, "There is/are only %d %s(s) associated for %s: %s \n", len(trToKeep), opts.Resource, opts.ParentResource, opts.ParentResourceName)
return nil
switch {
case opts.Keep > 0 && opts.Keep == len(trToKeep) && len(trToDelete) == 0:
if !formatted.IsStructured(output) {
fmt.Fprintf(s.Out, "Associated %s (%d) for Task:%s is/are equal to keep (%d) \n", opts.Resource, len(trToKeep), opts.ParentResourceName, opts.Keep)
return nil
}
case opts.Keep > len(trToKeep):
if !formatted.IsStructured(output) {
fmt.Fprintf(s.Out, "There is/are only %d %s(s) associated for %s: %s \n", len(trToKeep), opts.Resource, opts.ParentResource, opts.ParentResourceName)
return nil
}
default:
d.DeleteRelated([]string{opts.ParentResourceName})
}
d.DeleteRelated([]string{opts.ParentResourceName})
}

if formatted.IsStructured(output) {
deleted := append(d.SuccessfulRelatedDeletes(), d.SuccessfulDeletes()...)
printErr := formatted.PrintStructuredOutput(s.Out, output, formatted.NewDeleteResult(deleted))
return multierr.Append(printErr, d.Errors())
}

if !opts.DeleteAllNs {
Expand Down
151 changes: 136 additions & 15 deletions pkg/cmd/taskrun/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package taskrun

import (
"bytes"
"encoding/json"
"io"
"strings"
"testing"
Expand Down Expand Up @@ -228,7 +230,7 @@ func TestTaskRunDelete_v1beta1(t *testing.T) {
}

seeds := make([]clients, 0)
for i := 0; i < 17; i++ {
for i := 0; i < 18; i++ {
trs := trdata
cs, _ := test.SeedV1beta1TestData(t, test.Data{TaskRuns: trs, Tasks: tasks, Namespaces: ns})
cs.Pipeline.Resources = cb.APIResourceList(versionv1beta1, []string{"taskrun"})
Expand Down Expand Up @@ -572,6 +574,82 @@ func TestTaskRunDelete_v1beta1(t *testing.T) {
wantError: false,
want: "All 6 TaskRuns associated with Task \"random\" deleted in namespace \"ns\"\n",
},
{
name: "With JSON output",
command: []string{"rm", "tr0-1", "-n", "ns", "-o", "json"},
dynamic: seeds[14].dynamicClient,
input: seeds[14].pipelineClient,
inputStream: nil,
wantError: false,
want: `{
"deleted": [
"tr0-1"
]
}
`,
},
{
name: "With YAML output",
command: []string{"rm", "tr0-1", "-n", "ns", "-o", "yaml"},
dynamic: seeds[17].dynamicClient,
input: seeds[17].pipelineClient,
inputStream: nil,
wantError: false,
want: "deleted:\n- tr0-1\n",
},
{
name: "Unsupported output format",
command: []string{"rm", "tr0-1", "-n", "ns", "-o", "table"},
dynamic: seeds[0].dynamicClient,
input: seeds[0].pipelineClient,
inputStream: nil,
wantError: true,
want: "invalid output format \"table\": must be json or yaml",
},
{
name: "With JSON output for multiple TaskRuns",
command: []string{"rm", "tr0-1", "tr0-2", "-n", "ns", "-o", "json"},
dynamic: seeds[16].dynamicClient,
input: seeds[16].pipelineClient,
inputStream: strings.NewReader("y\n"),
wantError: false,
want: `{
"deleted": [
"tr0-1",
"tr0-2"
]
}
`,
},
{
name: "Delete with JSON output for --task but keep meets or exceeds existing (no-op)",
command: []string{"rm", "--task", "random", "-n", "ns", "--keep", "10", "-o", "json"},
dynamic: seeds[15].dynamicClient,
input: seeds[15].pipelineClient,
inputStream: nil,
wantError: false,
want: `{
"deleted": []
}
`,
},
{
name: "Delete with JSON output for --task",
command: []string{"rm", "--task", "random", "-n", "ns", "-o", "json"},
dynamic: seeds[15].dynamicClient,
input: seeds[15].pipelineClient,
inputStream: nil,
wantError: false,
want: `{
"deleted": [
"tr0-1",
"tr0-2",
"tr0-3",
"tr0-9"
]
}
`,
},
}

for _, tp := range testParams {
Expand Down Expand Up @@ -1318,14 +1396,24 @@ func Test_TaskRuns_Delete_With_Running_PipelineRun_v1beta1(t *testing.T) {
seeds = append(seeds, clients{cs, dc})
}
testParams := []struct {
name string
command []string
dynamic dynamic.Interface
input test.Clients
inputStream io.Reader
wantError bool
want string
name string
command []string
dynamic dynamic.Interface
input test.Clients
inputStream io.Reader
wantError bool
want string
assertStreams bool
}{
{
name: "Taskrun with running pipelinerun JSON without force",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Test improvements] One name fails, and one succeeds with  -o json . Can we add a test that checks the output lists the success and the command returns an error?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Test improvements] The current case uses one name. Can we add one with several names, with one running, and check that the others are deleted and listed?.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No test covers the bulk path with  -o. Can we add one that checks a non-empty deleted list?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test that  --all -o json  with nothing to delete gives deleted: []?

command: []string{"rm", "tr0-1", "-n", "ns", "-o", "json"},
dynamic: seeds[0].dynamicClient,
input: seeds[0].pipelineClient,
inputStream: nil,
wantError: true,
want: "taskrun tr0-1 is owned by a running PipelineRun; use --force to delete",
},
{
name: "Taskrun with running pipelinerun and answer y",
command: []string{"rm", "tr0-1", "-n", "ns"},
Expand Down Expand Up @@ -1542,13 +1630,14 @@ func Test_TaskRuns_Delete_With_Running_PipelineRun(t *testing.T) {
seeds = append(seeds, clients{cs, dc})
}
testParams := []struct {
name string
command []string
dynamic dynamic.Interface
input pipelinetest.Clients
inputStream io.Reader
wantError bool
want string
name string
command []string
dynamic dynamic.Interface
input pipelinetest.Clients
inputStream io.Reader
wantError bool
want string
assertStreams bool
}{
{
name: "Taskrun with running pipelinerun and answer y",
Expand All @@ -1568,6 +1657,14 @@ func Test_TaskRuns_Delete_With_Running_PipelineRun(t *testing.T) {
wantError: false,
want: "warning: Taskrun tr0-1 related pipelinerun still running.\nTaskRuns deleted: \"tr0-1\"\n",
},
{
name: "Taskrun with running pipelinerun force JSON streams",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test covers the --force case. Can we add the non-force error path to check that no prompt text reaches stdout?

command: []string{"rm", "tr0-1", "-n", "ns", "--force", "-o", "json"},
dynamic: seeds[1].dynamicClient,
input: seeds[1].pipelineClient,
wantError: false,
assertStreams: true,
},
{
name: "All taskruns with running pipelinerun ",
command: []string{"delete", "-n", "ns", "--all"},
Expand Down Expand Up @@ -1595,6 +1692,30 @@ func Test_TaskRuns_Delete_With_Running_PipelineRun(t *testing.T) {
if tp.inputStream != nil {
taskrun.SetIn(tp.inputStream)
}
if tp.assertStreams {
var stdout, stderr bytes.Buffer
taskrun.SetOut(&stdout)
taskrun.SetErr(&stderr)
taskrun.SetArgs(tp.command)

if err := taskrun.Execute(); err != nil {
t.Fatalf("unexpected error: %v", err)
}

if !json.Valid(stdout.Bytes()) {
t.Errorf("stdout is not valid JSON: %q", stdout.String())
}

if strings.Contains(stdout.String(), "warning:") {
t.Errorf("warning leaked into stdout: %q", stdout.String())
}

if !strings.Contains(stderr.String(), "warning:") {
t.Errorf("expected warning on stderr, got: %q", stderr.String())
}

return
}

out, err := test.ExecuteCommand(taskrun, tp.command...)
if tp.wantError {
Expand Down
Loading