Skip to content

feat: add JSON output to taskrun delete - #3086

Merged
tekton-robot merged 1 commit into
tektoncd:mainfrom
Debashich:feat/consistent-delete
Oct 21, 2026
Merged

tekton-robot merged 1 commit into
tektoncd:mainfrom
Debashich:feat/consistent-delete

Conversation

@Debashich

@Debashich Debashich commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

Closes #2850

Changes

Add --output json/yaml (-o) structured output support to tkn taskrun delete.

When specified, the command returns machine-readable output in JSON or YAML format. Using -o json or -o yaml skips the interactive confirmation prompt, allowing non-interactive usage.

Example:

$ tkn taskrun delete foo bar -n ns -o json
{
    "deleted": [
        "foo",
        "bar"
    ]
}
$ tkn taskrun delete foo -n ns -o yaml
deleted:
- foo
  • Supports both JSON and YAML output formats.
  • Uses the shared structured-output formatter for consistent output.
  • Preserves safeguards around TaskRuns associated with running PipelineRuns.
  • Includes tests for structured output and deletion behavior.

Submitter Checklist

See [the contribution guide](https://github.com/tektoncd/cli/blob/master/CONTRIBUTING.md) for more details.

Release Notes

Add --output json/yaml support to taskrun delete.
Using -o skips the confirmation prompt.

@tekton-robot tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Aug 3, 2026
@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 3, 2026
@Debashich
Debashich marked this pull request as ready for review August 3, 2026 22:22
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 3, 2026
@tekton-robot
tekton-robot requested a review from pratap0007 August 3, 2026 22:22
@divyansh42
divyansh42 requested a lite review from Copilot August 28, 2026 10:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds -o/--output json support to tkn taskrun delete, enabling machine-readable output of deleted TaskRuns and extending the shared pkg/deleter helper with accessors for successful deletions (plus unit tests).

Changes:

  • Added JSON output path for taskrun delete, and suppressed interactive confirmation when JSON is requested.
  • Extended pkg/deleter with accessors for successful deletes / related deletes.
  • Added unit tests for the new deleter accessors and JSON output for taskrun delete.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
pkg/deleter/deleter.go Adds accessors for successful deletes and successful related deletes.
pkg/deleter/deleter_test.go Adds unit tests validating the new deleter accessors.
pkg/cmd/taskrun/delete.go Implements JSON output mode and adjusts delete flow to suppress prompts in JSON mode.
pkg/cmd/taskrun/delete_test.go Adds tests asserting JSON output for taskrun delete.
Suppressed comments (1)

pkg/deleter/deleter.go:113

  • SuccessfulRelatedDeletes also returns the backing slice directly, which allows external mutation of Deleter state. Returning a copy prevents subtle bugs when callers append/modify the returned slice.
func (d *Deleter) SuccessfulRelatedDeletes() []string {
	return d.successfulRelatedDeletes
}

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pkg/deleter/deleter.go
Comment thread pkg/cmd/taskrun/delete.go Outdated
Comment thread pkg/cmd/taskrun/delete.go
Comment thread pkg/cmd/taskrun/delete.go Outdated
Comment thread pkg/cmd/taskrun/delete.go Outdated
Comment thread pkg/cmd/taskrun/delete.go Outdated
Comment thread pkg/cmd/taskrun/delete_test.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (5)

pkg/cmd/taskrun/delete.go:135

  • JSON mode skips CheckOptions entirely, but that method performs validation before prompting. As a result, -o json accepts invalid destructive combinations such as named TaskRuns with --all/--keep, and even an invocation with neither names nor --all succeeds with an empty result. Keep the validation and suppress only the confirmation prompt.
			if output != "json" {
				if err := opts.CheckOptions(s, availableTrs, p.Namespace()); err != nil {
					return err
				}
			}

pkg/cmd/taskrun/delete.go:190

  • With --force -o json, a TaskRun owned by a running PipelineRun writes this warning to stdout before the JSON document, making stdout invalid JSON. Send the warning to stderr so stdout remains machine-readable.
			if !prFinished && !opts.ForceDelete && output != "json" {

pkg/cmd/taskrun/delete.go:240

  • This returns success immediately after encoding and drops any deletion errors accumulated by Deleter. A delete that fails after lookup (for example, due to a race or API error) is therefore reported as a successful command with only the failed name omitted; encode the result, then return d.Errors().
		return json.NewEncoder(s.Out).Encode(result)

pkg/cmd/taskrun/delete.go:233

  • The JSON branch is unreachable for the two no-op --task --keep cases above because they return after printing human-readable status text. For example, when --keep equals the number of matching TaskRuns, -o json does not produce JSON at all. Route those early exits through the JSON renderer and return an empty deleted array.
	if output == "json" {

pkg/cmd/taskrun/delete.go:237

  • The --task deletion path records deleted TaskRun names in successfulRelatedDeletes, so this field becomes null even when TaskRuns were successfully deleted. Include both direct and related successes in the JSON result.
			Deleted: d.SuccessfulDeletes(),

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

pkg/cmd/taskrun/delete.go:243

  • In -o json mode this function currently returns after encoding JSON, so any accumulated deletion errors from d.Errors() are dropped (command can exit 0 even when deletes failed). Also, some early returns in the --task/--keep paths print human output and bypass JSON entirely, and when nothing is deleted the current slice construction can encode as null instead of []. Consider (1) emitting JSON for the early-return paths, (2) always encoding an empty array when nothing was deleted, and (3) returning d.Errors() after successfully writing JSON.
	if output == "json" {
		result := struct {
			Deleted []string `json:"deleted"`
		}{
			Deleted: append(append([]string(nil), d.SuccessfulRelatedDeletes()...), d.SuccessfulDeletes()...),

Comment thread pkg/deleter/deleter.go
@tekton-robot tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 28, 2026
@Debashich
Debashich requested a balanced review from Copilot August 28, 2026 18:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

pkg/cmd/taskrun/delete.go:246

  • When neither accessor contains an item, these nested appends preserve a nil slice, so encoding/json emits {"deleted":null} rather than the {"deleted":[]} contract asserted by the new no-op test. Initialize the accumulation with a non-nil empty slice so no-op deletions still produce an array.
			Deleted: append(append([]string(nil), d.SuccessfulRelatedDeletes()...), d.SuccessfulDeletes()...),

Comment thread pkg/cmd/taskrun/delete.go Outdated
Comment thread pkg/cmd/taskrun/delete.go Outdated
Comment thread pkg/cmd/taskrun/delete.go Outdated
Comment thread pkg/cmd/taskrun/delete.go Outdated
Comment thread pkg/cmd/taskrun/delete.go
@divyansh42

Copy link
Copy Markdown
Member

@Debashich do you have any updates on this PR?

@Debashich
Debashich force-pushed the feat/consistent-delete branch from 019d0a0 to c411e88 Compare September 17, 2026 11:05
@Debashich
Debashich requested a balanced review from Copilot September 17, 2026 12:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Debashich
Debashich force-pushed the feat/consistent-delete branch from 8e3b4f6 to 2774b93 Compare September 17, 2026 14:40
@Debashich

Copy link
Copy Markdown
Contributor Author

@divyansh42 Quick update on what happened after your requested changes:

I implemented all 3 review changes, added the release note + sample output, removed the generic output flags so only json is advertised, ran gofmt, regenerated the docs, and pushed the updated commit.

During testing, one existing TaskRun delete unit test was failing due to a fixture/expected-output mismatch. I reproduced the same failure on the base branch, so I confirmed it wasn't caused by this PR, then fixed the test fixture and amended the commit.

After the latest push, build, lint, unit tests, generated code, multi-arch, CodeQL and k8s-plus-one E2E are passing. The only remaining failure is k8s-oldest E2E.

I checked the full E2E logs, and the failure is actually TestPipelineInteractiveStartE2E, which times out after 10 minutes in the go-expect interactive test path. The PipelineRun itself starts successfully before the test hangs. The TaskRun/Pipeline deletion E2Es are passing.

So I haven't changed anything in the E2E tests/infrastructure since the remaining failure appears unrelated to this PR.

@pratap0007

Copy link
Copy Markdown
Contributor

/retest

@pratap0007

Copy link
Copy Markdown
Contributor

@Debashich Could you please add support for the -o yaml format as well? We are adding support for both -o yaml and -o json to the other commands for consistency.

@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 8, 2026
@divyansh42

Copy link
Copy Markdown
Member

@Debashich a recent PR #3318 has been merged, which covers other commands.
Could you please refer to the same and rebase your PR? Thanks!

@Debashich
Debashich force-pushed the feat/consistent-delete branch from 2774b93 to 7daab8a Compare October 11, 2026 00:19
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 11, 2026
@Debashich
Debashich force-pushed the feat/consistent-delete branch from 2ada9c9 to 7c8789b Compare October 11, 2026 08:20
@Debashich

Copy link
Copy Markdown
Contributor Author

@divyansh42 @pratap0007 I've made the requested changes, added validation for unsupported output formats, and updated the docs for JSON/YAML output. Everything is pushed now. Could you take another look when you get a chance? Thanks!

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Forced-deletion warnings can enter stdout and invalidate JSON when separate Cobra output writers are configured.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread pkg/cmd/taskrun/delete.go
@Debashich
Debashich requested a balanced review from Copilot October 20, 2026 12:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Signed-off-by: Debashich <debashishsinha555@gmail.com>
@Debashich
Debashich force-pushed the feat/consistent-delete branch from 908e3e6 to 443e662 Compare October 20, 2026 12:16
@Debashich

Copy link
Copy Markdown
Contributor Author

Hi @divyansh42 I've addressed the review feedback, rebased on the latest changes from #3318, added JSON/YAML support, and resolved the inline review threads.

I also fixed the structured-output stream routing issue flagged by the latest review and added a regression test. All unit tests, lint, generated-code checks, builds, and CodeQL are passing. The only CI failure is the k8s-plus-one E2E job timing out in TestPipelineInteractiveStartE2E, unrelated to TaskRun deletion.

Could you please take another look when you get a chance? Thanks!

@divyansh42 divyansh42 left a comment

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.

Thank you for the changes @Debashich
I am sorry for the delay in reviews, I hope this is going to be final round of the reviews.

Also please note that you might need to rebase this PR, once PR #3334 is merged.

Comment thread pkg/cmd/taskrun/delete.go
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.

Comment thread pkg/cmd/taskrun/delete.go
}
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?

Comment thread pkg/cmd/taskrun/delete.go
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?

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?

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] 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?.

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.

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

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.

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

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?

@divyansh42

Copy link
Copy Markdown
Member

/retest

@vdemeester vdemeester left a comment

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.

/lgtm

@tekton-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 21, 2026
@tekton-robot
tekton-robot merged commit fa82032 into tektoncd:main Oct 21, 2026
18 of 20 checks passed
@divyansh42

Copy link
Copy Markdown
Member

@Debashich provided PR is merged now. I would recommend to create a follow up PR with the fixes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add consistent --output json support to all tkn commands

6 participants