Add debug with breakpoint configuration to PipelineRun

[jstrachan]

Based on TEP-0042, this commit will allow users to enable the debug.breakpoints property on any TaskRun resources generated by a PipelineRun

the reconciler will pass any debug.breakpoints properties from the PipelineRun's taskRunSpecs over to the generated resolved TaskRun instances so that per Task debugging can be enabled

Signed-off-by: James Strachan james.strachan@gmail.com

Changes

Submitter Checklist

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

• Docs included if any changes are user facing
• Tests included if any functionality added or changed
• Follows the commit message standard
• Meets the Tekton contributor standards (including
  functionality, content, code)
• Release notes block below has been filled in or deleted (only if no user facing changes)

Release Notes

[tekton-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign bobcatfish
You can assign the PR to them by writing /assign @bobcatfish in a comment when ready.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[tekton-robot]

Hi @jstrachan. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[pritidesai]

/ok-to-test

[tekton-robot]

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
internal/builder/v1beta1/task.go	81.0%	80.3%	-0.7
pkg/apis/pipeline/v1beta1/pipelinerun_types.go	74.3%	73.6%	-0.7

[pritidesai]

/kind feature

/cc @waveywaves @vdemeester

[vdemeester]

We need to mark this field as an alpha field just how we did it for on TaskRun

[pritidesai]

Trying to understand how this would work? debug every resource in a sequence or the order its created in? would this pause the execution of the entire pipeline? how would you resume? do you really need debugging every resource a pipeline creates? or just a step level debug is sufficient, for example, pausing a particular step irrespective of whether its part of a taskRun or a pipelineRun? Of course, a step level debug wouldn't work with pipeline resources.

[jstrachan]

@pritidesai this PR let’s you enable debugging on a PipelineRun for a named Task, so that when the controller creates the TaskRun, the debug setting is applied. From that point onwards the TaskRun the breakpoint works exactly the same as if you created a TaskRun with the same debug configuration.

If a TaskRun pauses on a breakpoint there is no effect on any other Task in the same pipeline.

Resume works the same as with a TaskRun - running scripts inside the pod.

Basically debug/breakpoints only affect a TaskRun. The reason for this PR is that the tekton controller creates the TaskRun resources if you use a PipelineRun

[jstrachan]

@vdemeester you mean in the validate code: https://github.com/tektoncd/pipeline/pull/3857/files#diff-1153e019c943846e52e599290b137d535b134d52a5d922e32ed42c5cdefe6f8aR81 or is there another way to mark the debug field as alpha?

[jstrachan]

@vdemeester I just added alpha validation to the new debug field on PipelineRun.spec.taskRunSpecs 322a861

[tekton-robot]

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
internal/builder/v1beta1/task.go	81.0%	80.3%	-0.7
pkg/apis/pipeline/v1beta1/pipelinerun_types.go	74.3%	73.6%	-0.7
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go	98.5%	91.5%	-6.9

[tekton-robot]

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
internal/builder/v1beta1/task.go	81.0%	80.3%	-0.7
pkg/apis/pipeline/v1beta1/pipelinerun_types.go	74.3%	73.6%	-0.7
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go	98.5%	98.6%	0.1

[jstrachan]

here's a little blog and demo of how to use this PR with Jenkins X and Lens to debug pipelines

[waveywaves]

I would first like to thank you for the PR on debug ! This is great !

(In my opinion) It would be preferrable to have debug as a first order specification alongside pipelineSpec, pipelineRef etc.
which would look like something below. The debug specification

spec:
  debug: ['onFailure']
  pipelineRef: "a-debuggable-pipeline"

I do like the idea of giving a choice to the user for making some of the taskRuns debuggable initially by mentioning it in the taskRunSpecs. The above example I mentioned is something we can introduce later as well.

[jstrachan]

@waveywaves how about we allow a global debug setting applied to all Tasks and a per task debug option & a way to merge the 2 structs? So that folks can use one or the other?

[waveywaves]

how about we allow a global debug setting applied to all Tasks and a per task debug option & a way to merge the 2 structs? So that folks can use one or the other?

@jstrachan
that sounds good 💯
and also I am guessing in this case the individual task debug spec get precedence over the global PipelineRun debug spec ?

@bobcatfish @vdemeester I am not sure, but do we need a discussion around this(maybe in the next working group call) and update to the existing TEP to hash out the details before proceeding ? This level of change where taskRun debug is being enabled through pipelineRuns should be alright initially to let users enable taskRun debug through pipelineRuns. wdyt ?

[jstrachan]

@waveywaves OK I've added a PipelineRun.Spec.Debug flag in addition to TaskRun specific structs; so you can debug all Tasks with a top level setting or add specific settings on a TaskRun instead

[tekton-robot]

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
internal/builder/v1beta1/pipeline.go	86.7%	85.8%	-0.9
internal/builder/v1beta1/task.go	81.0%	80.3%	-0.7
pkg/apis/pipeline/v1beta1/pipelinerun_types.go	74.3%	73.6%	-0.7
pkg/apis/pipeline/v1beta1/pipelinerun_validation.go	98.5%	98.6%	0.1
pkg/reconciler/pipelinerun/pipelinerun.go	82.8%	83.2%	0.5

[jstrachan]

@waveywaves the initial implementation of merge combines the breakpoint names from the PipelineRun.Spec.Debug and the TaskRunSpec[i].Debug. Would you refer we just use the TaskRunSpec[i].Debug if its specified otherwise use the PipelineRun.Spec.Debug?

[tekton-robot]

@jstrachan: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-tekton-pipeline-unit-tests	9fa2075	link	/test tekton-pipeline-unit-tests
pull-tekton-pipeline-alpha-integration-tests	9fa2075	link	/test pull-tekton-pipeline-alpha-integration-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[tekton-robot]

@jstrachan: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-tekton-pipeline-unit-tests	9fa2075	link	/test tekton-pipeline-unit-tests
pull-tekton-pipeline-alpha-integration-tests	9fa2075	link	/test pull-tekton-pipeline-alpha-integration-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[vdemeester]

@bobcatfish @vdemeester I am not sure, but do we need a discussion around this(maybe in the next working group call) and update to the existing TEP to hash out the details before proceeding ? This level of change where taskRun debug is being enabled through pipelineRuns should be alright initially to let users enable taskRun debug through pipelineRuns. wdyt ?

Indeed, I think we need to update the TEP or do a follow-up TEP on PipelineRun support.

[tekton-robot]

@jstrachan: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jerop]

A new TEP or an update to TEP-0042: TaskRun Breakpoint on Failure of Step is needed before making this change

So, closing this PR for now (@jstrachan feel free to open when the TEP is implementable)

[yelhouti]

@jerop as you might know, James is no longer working the project. Could anyone on the Teton team try to reimplemented this on top the TEP which now marked as implémentés ? This a great future to have.

[jerop]

@waveywaves opened a TEP for this - tektoncd/community#572

we are meeting tomorrow to discuss some details and will update the TEP soon after - in the meantime please share your feedback on the proposal in tektoncd/community#572

Hey team, is there any update #4145 (adding debug with breakpoint to pipelinerun). It's a really great feature to have. Is there any workaround at the moment. It's quite difficult debugging failures without it. I have a trigger template that spins off a pipelinerun and would like to be able to debug task runs in that pipeline run. ~ feature request in slack

cc @yelhouti @hokadiri

[yelhouti]

@jerop thanks :)

[wuhuizuo]

@jstrachan @jerop @waveywaves I think it can be reopened and then make some updates:

• The TEP is accepted.
• There is a developing issue: TEP 0097: Add breakpoints, breakpoints.onFailure to debug and extend support to PipelineRun #5184, but it's closed by bot without completion.

🌹 🌹 🌹