Repository navigation
feat: add JSON output to taskrun delete #3086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 { | ||
|
|
@@ -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': | ||
|
|
||
| tkn taskrun delete foo bar -n quux | ||
|
|
@@ -92,6 +90,16 @@ or | |
| Err: cmd.OutOrStderr(), | ||
| } | ||
|
|
||
| output, err := cmd.LocalFlags().GetString("output") | ||
|
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 | ||
|
|
@@ -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} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can we skip the confirmation explicitly instead For example, call |
||
| } | ||
| 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)") | ||
|
|
@@ -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 { | ||
|
|
@@ -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) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With Can we skip that |
||
| } | ||
|
|
||
| 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 | ||
|
|
@@ -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 { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,8 @@ | |
| package taskrun | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "encoding/json" | ||
| "io" | ||
| "strings" | ||
| "testing" | ||
|
|
@@ -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"}) | ||
|
|
@@ -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 { | ||
|
|
@@ -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", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Test improvements] One name fails, and one succeeds with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No test covers the bulk path with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a test that |
||
| 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"}, | ||
|
|
@@ -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", | ||
|
|
@@ -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", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test covers the |
||
| 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"}, | ||
|
|
@@ -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 { | ||
|
|
||
There was a problem hiding this comment.
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,
--alland--taskwith-o, plus a line about the running-PipelineRun behaviour? Then can we runmake generatedto refresh the docs and man page?