Define v1alpha1 Run type and generated scaffolding #2871
Conversation
tekton-robot
added
the
do-not-merge/work-in-progress
tekton-robot
added
the
size/L
|
This PR cannot be merged: expecting exactly one kind/ label Available
|
2 similar comments
|
This PR cannot be merged: expecting exactly one kind/ label Available
|
|
This PR cannot be merged: expecting exactly one kind/ label Available
|
imjasonh
added
the
kind/feature
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
imjasonh
force-pushed
the
run-types
branch
2 times, most recently
from
e7a2be7
to
8e9f758
Compare
imjasonh
changed the title
tekton-robot
added
do-not-merge/release-note-label-needed
imjasonh
added
release-note
tekton-robot
added
do-not-merge/release-note-label-needed
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
tekton-robot
added
size/XL
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
/retest |
|
The following is the coverage report on the affected files.
|
imjasonh
removed
the
needs-rebase
tekton-robot
added
the
needs-rebase
|
The following is the coverage report on the affected files.
|
tekton-robot
removed
the
needs-rebase
|
/retest 🙄 |
This defines the basic Run type, which will be used to specify executions of Custom Tasks and report status of those executions. This change doesn't wire the new type in to any controllers or validating webhooks, so that this time nothing will happen when a Run is created. It also doesn't wire in to PipelineRun execution to enable Pipelines to specify Custom Tasks.
|
The following is the coverage report on the affected files.
|
imjasonh
requested review from
vdemeester and
afrittoli
and removed request for
vdemeester
| // - workspaces ? | ||
| } | ||
|
|
||
| // TODO(jasonhall): Move this to a Params type so other code can use it? |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afrittoli 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:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
|
@afrittoli mentioned this in slack but before merging I would have liked to include docs explaining these new fields - with our goal to be always in a releasable state, we'd want to make sure that if for some reason we cut a release tomorrow, there'd be an adequate explanation for users of why these types were added and do nothing ( @pritidesai did a great job of this with #2650) Also kinda related to the 2 reviewer conversation in slack, our policy for additive API changes is that they be approved by at least 2 OWNERS (https://github.com/tektoncd/pipeline/blob/master/api_compatibility_policy.md#additive-changes). Maybe jason + andrea counts, but I'd really like to get more eyes on API changes in general (selfishly: my own :D) |
i think the docs should block the PR (in the spirit of being always releasable) |
| TaskRunControllerName = "TaskRun" | ||
|
|
||
| // TaskRunControllerName holds the name of the PipelineRun controller |
There was a problem hiding this comment.
i think the comment is a copy pasta error
| TaskRunControllerName = "TaskRun" | ||
|
|
||
| // TaskRunControllerName holds the name of the PipelineRun controller | ||
| RunControllerName = "Run" |
There was a problem hiding this comment.
what does run controller name mean here?
| } | ||
| ) | ||
|
|
||
| // RunSpec defines the desired state of Run |
There was a problem hiding this comment.
can this docstring and the others in this file elaborate on what a Run is?
| return nil | ||
| } | ||
|
|
||
| type RunStatus struct { |
There was a problem hiding this comment.
can has docstrings for all exported types plz
| // separately and inlined so that other types can readily consume these fields | ||
| // via duck typing. | ||
| type RunStatusFields struct { | ||
| // StartTime is the time the build is actually started. |
There was a problem hiding this comment.
some copy pasta in these i think (our old friend build)
|
|
||
| var _ apis.Validatable = (*Run)(nil) | ||
|
|
||
| // Validate taskrun |
This defines the basic Run type, which will be used to specify
executions of Custom Tasks and report status of those executions.
This change doesn't wire the new type in to any controllers or
validating webhooks, so that this time nothing will happen when a Run is
created. It also doesn't wire in to PipelineRun execution to enable
Pipelines to specify Custom Tasks.
This is groundwork to enable TEP 0002 and enable further experimentation. This type may change in breaking ways as a result of that experimentation and further iteration on the design.
Next steps: moar samples!!, webhook-validate Runs, enforce initial update timeout in controller, create Runs from PipelineRuns, omgsomanytests
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