feat: TEP-0097 change in taskrun breakpoints api fields #6489

chengjoey · 2023-04-04T03:51:22Z

Changes

make TaskBreakpoints struct for taskrun debug.
breakpoint on failure of a step was moved to breakpoints.onFailure spec and onFailure breakpoint are now enabled by setting "taskRun.spec.debug.breakpoints.onFailure" to "enabled".

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

action required: syntax change for taskRun breakpoints (alpha feature). TaskRun breakpoints are now enabled by setting "taskRun.spec.debug.breakpoints.onFailure" to "enabled".

chengjoey · 2023-04-04T03:52:01Z

/kind feature

chengjoey · 2023-04-04T03:52:49Z

/assign @lbernick

tekton-robot · 2023-04-04T03:56:53Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	77.4%	82.4%	5.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.7%	95.0%	-2.8
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.7%	0.5
pkg/apis/pipeline/v1beta1/taskrun_types.go	78.6%	83.1%	4.5
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.2%	-0.6
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4

tekton-robot · 2023-04-04T04:46:00Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	77.4%	82.4%	5.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.7%	95.0%	-2.8
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.7%	0.5
pkg/apis/pipeline/v1beta1/taskrun_types.go	78.6%	83.1%	4.5
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.2%	-0.6
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4

chengjoey · 2023-04-04T06:10:01Z

this pr is splited from 5691, only the breakpoint api field has been adjusted

lbernick · 2023-04-04T17:23:07Z

pkg/apis/pipeline/v1/taskrun_types.go

+	BeforeSteps []string `json:"beforeSteps,omitempty"`
 	// +optional
 	// +listType=atomic
-	Breakpoint []string `json:"breakpoint,omitempty"`
+	AfterSteps []string `json:"afterSteps,omitempty"`


Let's scope this PR to just the change in onFailure, i.e. let's avoid introducing beforeSteps and afterSteps in this PR

thanks @lbernick , i have removed beforeSteps and afterSteps in this PR, and changed commit message and pr description

lbernick · 2023-04-04T17:24:38Z

pkg/apis/pipeline/v1/taskrun_types.go

+	// if enabled, pause TaskRun on failure of a step
+	// failed step will not exit
+	// +optional
+	OnFailure   bool     `json:"onFailure,omitempty"`


There was some discussion of this on the previous PR, but it looks like this is a string (not a bool) in the TEP. Let's make sure the API changes in this PR match the API changes described in the TEP.

The PR for the relevant type change is here,PTAL！

hi @lbernick , Onfailure uses the string type as described in TEP

tekton-robot · 2023-04-05T06:58:18Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	77.4%	79.3%	2.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.7%	97.6%	-0.1
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.3%	0.2
pkg/apis/pipeline/v1beta1/taskrun_types.go	78.6%	80.3%	1.8
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.7%	-0.1
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4

tekton-robot · 2023-04-05T06:59:40Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	77.4%	79.3%	2.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.7%	97.6%	-0.1
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.3%	0.2
pkg/apis/pipeline/v1beta1/taskrun_types.go	78.6%	80.3%	1.8
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.7%	-0.1
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4

tekton-robot · 2023-04-06T11:17:30Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	77.4%	79.3%	2.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.7%	96.9%	-0.8
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.3%	0.2
pkg/apis/pipeline/v1beta1/taskrun_types.go	78.6%	80.3%	1.8
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.0%	-0.8
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4

tekton-robot · 2023-04-06T11:19:26Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	77.4%	79.3%	2.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.7%	96.9%	-0.8
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.3%	0.2
pkg/apis/pipeline/v1beta1/taskrun_types.go	78.6%	80.3%	1.8
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.0%	-0.8
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4

lbernick · 2023-04-06T14:44:26Z

pkg/apis/pipeline/v1/taskrun_validation.go

@@ -212,16 +212,14 @@ func combineParamSpec(p ParamSpec, paramSpecForValidation map[string]ParamSpec)
 	return paramSpecForValidation, nil
 }

-// validateDebug
+// validateDebug validates the debug section of the TaskRun.
+// onFailure breakpoint is only allowed to be set as enabled.


Suggested change

// onFailure breakpoint is only allowed to be set as enabled.

// if set, onFailure breakpoint must be "enabled"

good suggestion! both v1 and v1beta1 have been modified according to this suggestion

tekton-robot · 2023-04-06T14:45:08Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [lbernick]

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

lbernick · 2023-04-06T14:47:29Z

Thanks @chengjoey, looks good! Suggested release note: "action required: syntax change for taskRun breakpoints (alpha feature). TaskRun breakpoints are now enabled by setting "taskRun.spec.debug.breakpoints.onFailure" to "enabled"."

chengjoey · 2023-04-06T15:39:27Z

Thanks @chengjoey, looks good! Suggested release note: "action required: syntax change for taskRun breakpoints (alpha feature). TaskRun breakpoints are now enabled by setting "taskRun.spec.debug.breakpoints.onFailure" to "enabled"."

thanks @lbernick , i have modified release-note by your suggestion

tekton-robot · 2023-04-06T15:40:34Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	77.4%	79.3%	2.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.7%	96.9%	-0.8
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.3%	0.2
pkg/apis/pipeline/v1beta1/taskrun_types.go	78.6%	80.3%	1.8
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.0%	-0.8
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4

tekton-robot · 2023-04-06T15:41:14Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	77.4%	79.3%	2.0
pkg/apis/pipeline/v1/taskrun_validation.go	97.7%	96.9%	-0.8
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.3%	0.2
pkg/apis/pipeline/v1beta1/taskrun_types.go	78.6%	80.3%	1.8
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.0%	-0.8
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4

chengjoey · 2023-04-06T15:44:19Z

hi @afrittoli , you reviewed the previeous PR, could you please review this PR again?🙏

tekton-robot · 2023-09-18T15:17:25Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

jerop · 2023-09-21T14:15:36Z

@chengjoey please rebase this PR and I'll review it 🙏🏾

/assign

make TaskBreakpoints struct for taskrun debug. breakpoint on failure of a step was moved to `breakpoints.onFailure` spec and onFailure breakpoint are now enabled by setting `taskRun.spec.debug.breakpoints.onFailure` to `enabled`. Signed-off-by: chengjoey <zchengjoey@gmail.com>

chengjoey · 2023-10-07T03:53:50Z

@chengjoey please rebase this PR and I'll review it 🙏🏾

/assign

thanks @jerop , I've rebased this pr

tekton-robot · 2023-10-07T03:59:09Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	79.6%	81.4%	1.7
pkg/apis/pipeline/v1/taskrun_validation.go	97.8%	97.0%	-0.8
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.3%	0.2
pkg/apis/pipeline/v1beta1/taskrun_types.go	80.7%	82.3%	1.6
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.7%	96.9%	-0.8
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4
pkg/pod/script.go	99.0%	99.0%	-0.0

tekton-robot · 2023-10-07T04:01:57Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1/taskrun_types.go	79.6%	81.4%	1.7
pkg/apis/pipeline/v1/taskrun_validation.go	97.8%	97.0%	-0.8
pkg/apis/pipeline/v1beta1/taskrun_conversion.go	93.1%	93.3%	0.2
pkg/apis/pipeline/v1beta1/taskrun_types.go	80.7%	82.3%	1.6
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.7%	96.9%	-0.8
pkg/pod/entrypoint.go	89.8%	89.4%	-0.4
pkg/pod/script.go	99.0%	99.0%	-0.0

chitrangpatel · 2023-10-09T18:46:18Z

Thanks @chengjoey! Can you link other PRs in the description? It will help me understand what's been implemented and whats coming next. Here it seems like all the plumbing is good and feeding input to the entrypointer. Is the entrypointer logic implemented or upcoming? Thanks!

Otherwise lgtm.

chengjoey · 2023-10-10T03:56:36Z

Thanks @chengjoey! Can you link other PRs in the description? It will help me understand what's been implemented and whats coming next. Here it seems like all the plumbing is good and feeding input to the entrypointer. Is the entrypointer logic implemented or upcoming? Thanks!

Otherwise lgtm.

Thanks @chitrangpatel, this PR is split from 5691 and make changes to the onFaileure field as Tep-0097 described, entrypointer logic is upcoming~

chitrangpatel · 2023-10-10T14:13:03Z

Thanks @chengjoey! Can you link other PRs in the description? It will help me understand what's been implemented and whats coming next. Here it seems like all the plumbing is good and feeding input to the entrypointer. Is the entrypointer logic implemented or upcoming? Thanks!
Otherwise lgtm.

Thanks @chitrangpatel, this PR is split from 5691 and make changes to the onFaileure field as Tep-0097 described, entrypointer logic is upcoming~

Sounds good! Thanks!

chitrangpatel · 2023-10-10T14:13:08Z

/lgtm

tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Apr 4, 2023

tekton-robot requested review from bobcatfish and wlynch April 4, 2023 03:51

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 4, 2023

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 4, 2023

tekton-robot assigned lbernick Apr 4, 2023

lbernick reviewed Apr 4, 2023

View reviewed changes

chengjoey force-pushed the feat/taskrun-breakpoint-apis branch from 7ac0b39 to 9790600 Compare April 5, 2023 06:53

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 5, 2023

chengjoey force-pushed the feat/taskrun-breakpoint-apis branch from 9790600 to 32fb2b6 Compare April 6, 2023 11:12

chengjoey requested a review from lbernick April 6, 2023 11:40

lbernick approved these changes Apr 6, 2023

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 6, 2023

chengjoey force-pushed the feat/taskrun-breakpoint-apis branch from 32fb2b6 to 4070b89 Compare April 6, 2023 15:34

tekton-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 6, 2023

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2023

wlynch removed their request for review June 20, 2023 14:50

tekton-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 18, 2023

tekton-robot assigned jerop Sep 21, 2023

chengjoey force-pushed the feat/taskrun-breakpoint-apis branch from 4070b89 to b054778 Compare October 7, 2023 03:52

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 7, 2023

tekton-robot assigned chitrangpatel Oct 10, 2023

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 10, 2023

tekton-robot merged commit b507fa5 into tektoncd:main Oct 10, 2023
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: TEP-0097 change in taskrun breakpoints api fields #6489

feat: TEP-0097 change in taskrun breakpoints api fields #6489

chengjoey commented Apr 4, 2023 •

edited

Loading

chengjoey commented Apr 4, 2023

chengjoey commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

chengjoey commented Apr 4, 2023

lbernick Apr 4, 2023

chengjoey Apr 5, 2023

lbernick Apr 4, 2023

chengjoey Apr 5, 2023

chengjoey Apr 6, 2023

tekton-robot commented Apr 5, 2023

tekton-robot commented Apr 5, 2023

tekton-robot commented Apr 6, 2023

tekton-robot commented Apr 6, 2023

lbernick Apr 6, 2023

chengjoey Apr 6, 2023

tekton-robot commented Apr 6, 2023

lbernick commented Apr 6, 2023

chengjoey commented Apr 6, 2023

tekton-robot commented Apr 6, 2023

tekton-robot commented Apr 6, 2023

chengjoey commented Apr 6, 2023 •

edited

Loading

tekton-robot commented Sep 18, 2023

jerop commented Sep 21, 2023

chengjoey commented Oct 7, 2023

tekton-robot commented Oct 7, 2023

tekton-robot commented Oct 7, 2023

chitrangpatel commented Oct 9, 2023 •

edited

Loading

chengjoey commented Oct 10, 2023

chitrangpatel commented Oct 10, 2023

chitrangpatel commented Oct 10, 2023

	// onFailure breakpoint is only allowed to be set as enabled.
	// if set, onFailure breakpoint must be "enabled"

feat: TEP-0097 change in taskrun breakpoints api fields #6489

feat: TEP-0097 change in taskrun breakpoints api fields #6489

Conversation

chengjoey commented Apr 4, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

chengjoey commented Apr 4, 2023

chengjoey commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

tekton-robot commented Apr 4, 2023

chengjoey commented Apr 4, 2023

lbernick Apr 4, 2023

Choose a reason for hiding this comment

chengjoey Apr 5, 2023

Choose a reason for hiding this comment

lbernick Apr 4, 2023

Choose a reason for hiding this comment

chengjoey Apr 5, 2023

Choose a reason for hiding this comment

chengjoey Apr 6, 2023

Choose a reason for hiding this comment

tekton-robot commented Apr 5, 2023

tekton-robot commented Apr 5, 2023

tekton-robot commented Apr 6, 2023

tekton-robot commented Apr 6, 2023

lbernick Apr 6, 2023

Choose a reason for hiding this comment

chengjoey Apr 6, 2023

Choose a reason for hiding this comment

tekton-robot commented Apr 6, 2023

lbernick commented Apr 6, 2023

chengjoey commented Apr 6, 2023

tekton-robot commented Apr 6, 2023

tekton-robot commented Apr 6, 2023

chengjoey commented Apr 6, 2023 • edited Loading

tekton-robot commented Sep 18, 2023

jerop commented Sep 21, 2023

chengjoey commented Oct 7, 2023

tekton-robot commented Oct 7, 2023

tekton-robot commented Oct 7, 2023

chitrangpatel commented Oct 9, 2023 • edited Loading

chengjoey commented Oct 10, 2023

chitrangpatel commented Oct 10, 2023

chitrangpatel commented Oct 10, 2023

chengjoey commented Apr 4, 2023 •

edited

Loading

chengjoey commented Apr 6, 2023 •

edited

Loading

chitrangpatel commented Oct 9, 2023 •

edited

Loading