WIP: TaskRun Debug

[waveywaves]

.spec.debug bool, is added to TaskRun API which enables
debug mode for the particular TaskRun.

Co-authored-by: Vibhav Bobade [email protected]
Co-authored-by: Nikhil Thomas [email protected]

Changes

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes tests (if functionality changed/added)
• Includes docs (if user facing)
• Commit messages follow commit message best practices

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

• If you are adding a new binary/image to the cmd dir, please update
  the release Task to build and release this image.

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

Release Notes

Describe any user facing changes here, or delete this block.

Examples of user facing changes:
- API changes
- Bug fixes
- Any changes in behavior

[googlebot]

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

[tekton-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign imjasonh
You can assign the PR to them by writing /assign @imjasonh 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]

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

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/taskrun_conversion.go	95.2%	95.4%	0.1

[imjasonh]

Can you add more details to the PR description and commit message to describe what debug mode does? Is there any supporting design doc with more info?

[waveywaves]

@imjasonh https://docs.google.com/document/d/1SWtGfkGOaeggqj3zY4pyJo1cJz0hBw9wStbIWkCPnL0/edit#

[imjasonh]

@imjasonh https://docs.google.com/document/d/1SWtGfkGOaeggqj3zY4pyJo1cJz0hBw9wStbIWkCPnL0/edit#

Thanks, that's helpful. Adding this to the PR description and commit message will make this design more discoverable to future spelunkers.

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to determine that you authored the commits in this PR. Maybe you used a different email address in the git commits than was used to sign the CLA? If someone else authored these commits, then please add them to this pull request and have them confirm that they're okay with them being contributed to Google. If there are co-authors, make sure they're formatted properly.

In order to pass this check, please resolve this problem and then comment@googlebot I fixed it... If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[tekton-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1alpha1/taskrun_conversion.go	95.2%	95.4%	0.1

[tekton-robot]

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

Test name	Commit	Details	Rerun command
pull-tekton-pipeline-unit-tests	daec24f	link	/test tekton-pipeline-unit-tests
pull-tekton-pipeline-build-tests	daec24f	link	/test pull-tekton-pipeline-build-tests
pull-tekton-pipeline-integration-tests	daec24f	link	/test pull-tekton-pipeline-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.

[imjasonh]

Some thoughts on this design, that I didn't see covered in the doc:

1. The term "debug" means quite a lot of things, and might be confusing since this feature doesn't, for instance, cover stepping through/in/out of operations in the container. I realize there's historical context around the field in Openshift, but I don't think we should necessarily carry it over without thinking it through. This might make more sense as breakpoint: true, and ties into the second point:
2. I think we should look toward a future where there might be more than two options for the field. That is, breakpoint: true and breakpoint: false might be overly limiting. What if we wanted to add a breakpoint: OnlyOnFailure or OnlyOnTimeout or something? Supporting an enum (string) today makes this an easier change in the future.
3. Are we anticipating tkn CLI surfaces to interact with this feature? I didn't see them discussed in the doc. Things like tkn unpause to nudge past the breakpoint, or tkn connect to streamline connecting to the paused container? In the absence of these it can be a bit hard to really gauge how useful this would be, and therefore whether it's worth adopting and supporting.

[bobcatfish]

@waveywaves I'm going to close this PR for now, I think the next step here would be to expand the design proposal to include @imjasonh's recommendations, which I strongly 👍 (I'd love to see more specifically about the API this will expose to debugging tools, e.g. CLI, vscode plugin).

Please reopen or let me know if you disagree!