Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add the command and arguments run in each step to the TaskRun.Status #2399

Closed
wants to merge 1 commit into from
Closed

Add the command and arguments run in each step to the TaskRun.Status #2399

wants to merge 1 commit into from

Conversation

dlorenc
Copy link
Contributor

@dlorenc dlorenc commented Apr 15, 2020

Changes

The TaskRun.Status currently serves as the best log of what happened in a TaskRun.
It contains the container digests for each step, as well as their exit code and
termination message. However, it is currently missing the Command and Arguments,
making it impossible to verify what was run inside of the container after the
pod completes.

This commit adds those fields to the API, and copies them over from the Pod
when the rest of the Status is populated.

Submitter Checklist

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

See the contribution guide for more details.

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

TaskRun.Status.Steps now includes the Command and Args run inside each container.

@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 15, 2020
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@googlebot googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Apr 15, 2020
@tekton-robot
Copy link
Collaborator

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/v1beta1/taskrun_types.go 71.4% 69.7% -1.7

@bobcatfish
Copy link
Collaborator

seems reasonable to me!

@dlorenc what do you think about inlining hte whole task spec so you get everything? i know @chhsia0 has run into etcd size limits, maybe we could make a config flag to switch it off

if you're into that id rather go that route but also happy to approve this

@dlorenc
Add the command and arguments run in each step to the TaskRun.Status
1987b26 
The TaskRun.Status currently serves as the best log of what happened in a TaskRun.
It contains the container digests for each step, as well as their exit code and
termination message. However, it is currently missing the Command and Arguments,
making it impossible to verify what was run inside of the container after the
pod completes.

This commit adds those fields to the API, and copies them over from the Pod
when the rest of the Status is populated.
@tekton-robot
Copy link
Collaborator

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/v1beta1/taskrun_types.go 71.4% 69.7% -1.7

@tekton-robot
Copy link
Collaborator

tekton-robot commented Apr 15, 2020 •
edited
Loading

@dlorenc: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-tekton-pipeline-integration-tests 1987b26 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.

@dlorenc
Copy link
Contributor Author

dlorenc commented Apr 15, 2020

/test pull-tekton-pipeline-build-tests

dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 20, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
ecb579f 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 20, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
1bbdbe8 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 20, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
907ffaa 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
@dlorenc dlorenc mentioned this pull request Apr 20, 2020
Merged
3 tasks
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 20, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
8de5547 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 21, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
4360852 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 21, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
1f9c107 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 21, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
12304b5 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 22, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
9ff346b 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 22, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
6f17ebd 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 22, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
5cff4f6 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request Apr 23, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
ea2a4dd 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
tekton-robot pushed a commit that referenced this pull request Apr 23, 2020
@dlorenc @tekton-robot
Dereference the TaskSpec into TaskRun.Status.
0c2798e 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for #2399
@dlorenc dlorenc closed this Apr 24, 2020
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request May 1, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
6675e3f 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
dlorenc added a commit to dlorenc/build-pipeline that referenced this pull request May 13, 2020
@dlorenc
Dereference the TaskSpec into TaskRun.Status.
9676c73 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
font pushed a commit to font/pipeline that referenced this pull request May 19, 2020
@dlorenc @font
Dereference the TaskSpec into TaskRun.Status.
ba0e5a8 
The Task definition used for a TaskRun can change after the TaskRun
has started. This poses problems for auditability post-run. Rather
than chase down every part of a Task that we might like to audit later,
let's just add the entire thing here.

This is a replacement for tektoncd#2399
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afrittoli afrittoli Awaiting requested review from afrittoli

@vdemeester vdemeester Awaiting requested review from vdemeester

Assignees
No one assigned
Labels
cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dlorenc @tekton-robot @bobcatfish @googlebot