Add minimal initial API spec document #3131
Conversation
tekton-robot
added
the
release-note
tekton-robot
added
the
size/L
|
/kind feature |
tekton-robot
added
the
kind/feature
There was a problem hiding this comment.
metaquestion: what do you see as the process to add more fields here going forward? e.g. how to add when expressions into here. "Modifying This Specification" starts to talk about it but i wonder if it'll make sense to create a process around this to guide contributors
overall looks good, just lots of minor feedback from me!
|
|
||
| The Tekton Pipelines platform provides common abstractions for describing and executing container-based, run-to-completion workflows, typipcally in service of CI/CD scenarios. This document describes the structure, lifecycle and management of Knative resources in the context of the [Kubernetes Resource Model](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/architecture/resource-management.md). An understanding of the Kubernetes API interface and the capabilities of [Kubernetes Custom Resources](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/) is assumed. | ||
|
|
||
| This document does not define the [runtime contract](https://tekton.dev/docs/pipelines/container-contract/) nor prescribe specific implementations of supporting services such as access control, observability, or resource management. |
There was a problem hiding this comment.
would you see us including that here eventually or do you feel like that's something separate? i could see including it here eventually so you have a "one stop shop" for conformance requirements?
There was a problem hiding this comment.
I don't mind it being in a separate document. It's not currently phrased in terms of "platform implementations must satisfy this contract" so much as "users should expect this contract", but maybe that's just fine since the vast majority of readers will be users, and very few will be prospective platform providers.
In the meantime it's probably enough to just say "this isn't the runtime contract, that's over there" and when/if we get any confusion we can resolve it. Launch and iterate 😄
docs/api-spec.md
Outdated
|
|
||
| The key words “MUST”, “MUST NOT”, “REQUIRED”, “SHALL”, “SHALL NOT”, “SHOULD”, “SHOULD NOT”, “RECOMMENDED”, “NOT RECOMMENDED”, “MAY”, and “OPTIONAL” are to be interpreted as described in [RFC 2119](https://tools.ietf.org/html/rfc2119). | ||
|
|
||
| There is no formal specification of the Kubernetes API and Resource Model. This document assumes Kubernetes 1.13 behavior; this behavior will typically be supported by many future Kubernetes versions. Additionally, this document may reference specific core Kubernetes resources; these references may be illustrative (i.e. an implementation on Kubernetes) or descriptive (i.e. this Kubernetes resource MUST be exposed). References to these core Kubernetes resources will be annotated as either illustrative or descriptive. |
There was a problem hiding this comment.
any particular reason why 1.13? just wondering cuz we now require 1.16 i think
There was a problem hiding this comment.
Oh probably just because the original draft was written when 1.13 was the minimum required. 👴
AFAIK no relevant fields have been removed/changed since 1.13, so it's technically still true. 🤷
There was a problem hiding this comment.
Any reason we should not be updating this to 1.16 before this PR is merged?
docs/api-spec.md
Outdated
| This document considers two users of a given Tekton Pipelines environment, and is particularly concerned with the expectations of developers (and language and tooling developers, by extension) deploying applications to the environment. | ||
|
|
||
| * **Developers** write code against which Tekton Pipelines executes workflows. Developers may also author these workflow configurations. | ||
| * **Operators** (also known as platform providers) provision the compute resources and manage the software configuration of Tekton Pipelines and the underlying abstractions (for example: Linux, Kubernetes, etc). |
There was a problem hiding this comment.
scope creep / side note: one day i hope we could get https://github.com/tektoncd/community/blob/master/user-profiles.md into a state where we could usefully link to it from here
There was a problem hiding this comment.
do the folks authoring Tasks come into play here at all? i think they represent a 3rd group kinda between these two
There was a problem hiding this comment.
Right now they are grouped under "Developers" (Developers may also author these workflow configurations.) but they might be called out as workflow authors.
We do not mention these groups anywhere in the document though. What is the purpose of having these groups here in the API spec?
| | `env` | `[]EnvVar` | REQUIRED | | ||
| | `script` | string | REQUIRED | | ||
|
|
||
| **NB:** All other fields inherited from the [core.v1/Container](https://godoc.org/k8s.io/api/core/v1#Container) type supported by the Kubernetes implementation are **OPTIONAL** for the purposes of this spec. |
There was a problem hiding this comment.
nice, thanks for whittling this down!
is StepState similar (i.e. based on pod status) or is this something we created ourselves?
There was a problem hiding this comment.
Yeah StepState has the same problem, but it's slightly better since there at least aren't tons of extra inherited fields. But K8s could add more any time they want, so we should spell it out defensively.
|
|
||
| | Field Name | Field Type | Requirement | | ||
| |--------------|------------|-------------| | ||
| | `name` | string | REQUIRED | |
There was a problem hiding this comment.
am i crazy or is it possible to have a step without a name
There was a problem hiding this comment.
This doc is saying it's "required" that platform implementations support naming steps, not that the user's request must specify one.
If there's a good clear way to describe that somewhere I'd love to clear that up for future readers.
| | `name` | string | REQUIRED | | ||
| | `emptyDir` | empty struct | REQUIRED | | ||
|
|
||
| **NB:** All other Workspace types supported by the Kubernetes implementation are **OPTIONAL** for the purposes of this spec. |
There was a problem hiding this comment.
do we want to say anything more about this? in order to support tasks in a pipeline properly, the system would need to support something besides emptyDir
There was a problem hiding this comment.
Yeah, this version of the doc is just to get a minimal "anything" spec out there, we can add more workspace types that platforms must support as we go.
It is conceivable that a platform would only support emptyDir, though that would be a pretty serious limitation. So emptyDir should probably be the only REQUIRED option.
|
Ping @vdemeester @afrittoli or anybody else who may have thoughts? |
There was a problem hiding this comment.
Thank you for starting this!
Having everything not mentioned defaulting to OPTIONAL makes sense, it makes it harder to review though as it is not obvious what we are excluding :P I think it should be fine to iterate on this and add things incrementally as we discover them?
docs/api-spec.md
Outdated
|
|
||
| The key words “MUST”, “MUST NOT”, “REQUIRED”, “SHALL”, “SHALL NOT”, “SHOULD”, “SHOULD NOT”, “RECOMMENDED”, “NOT RECOMMENDED”, “MAY”, and “OPTIONAL” are to be interpreted as described in [RFC 2119](https://tools.ietf.org/html/rfc2119). | ||
|
|
||
| There is no formal specification of the Kubernetes API and Resource Model. This document assumes Kubernetes 1.13 behavior; this behavior will typically be supported by many future Kubernetes versions. Additionally, this document may reference specific core Kubernetes resources; these references may be illustrative (i.e. an implementation on Kubernetes) or descriptive (i.e. this Kubernetes resource MUST be exposed). References to these core Kubernetes resources will be annotated as either illustrative or descriptive. |
There was a problem hiding this comment.
Any reason we should not be updating this to 1.16 before this PR is merged?
docs/api-spec.md
Outdated
| This document considers two users of a given Tekton Pipelines environment, and is particularly concerned with the expectations of developers (and language and tooling developers, by extension) deploying applications to the environment. | ||
|
|
||
| * **Developers** write code against which Tekton Pipelines executes workflows. Developers may also author these workflow configurations. | ||
| * **Operators** (also known as platform providers) provision the compute resources and manage the software configuration of Tekton Pipelines and the underlying abstractions (for example: Linux, Kubernetes, etc). |
There was a problem hiding this comment.
Right now they are grouped under "Developers" (Developers may also author these workflow configurations.) but they might be called out as workflow authors.
We do not mention these groups anywhere in the document though. What is the purpose of having these groups here in the API spec?
|
|
||
| ## Modifying This Specification | ||
|
|
||
| This spec is a living document, meaning new resources and fields may be added, and may transition from being OPTIONAL to RECOMMENDED to REQUIRED over time. In general a resource or field should not be added as REQUIRED directly, as this may cause unsuspecting previously-conformant implementations to suddenly no longer be conformant. These should be first OPTIONAL or RECOMMENDED, then change to be REQUIRED once a survey of conformant implementations indicates that doing so will not cause undue burden on any implementation. |
There was a problem hiding this comment.
Conformance could be associated to version numbers.
We do not have a Tekton wide version number as such; I think it's something that we might introduce in future, at least for the pipeline + triggers couple. That would allow us to have conformance associated to an overall Tekton version - which would also be helpful for operator.
In any case having to introduce new fields as OPTIONAL matches the model of adding features behind features flags that we should follow for the API.
There was a problem hiding this comment.
This is a really interesting concept I'd like to think on more. I do like the idea of an implementation being able to say it's "conformant as of Tekton 0.14", but that also adds a lot of complexity, for potential implementors, for conformance tests, for users just trying to build on Tekton.
How would you feel about punting on this until after this initial spec lands, and circling back?
|
|
||
| Its HTTP API endpoint is `/apis/tekton.dev/v1beta1/[parent]/taskruns`, where `[parent]` can refer to any string identifying a grouping of `TaskRun`s. | ||
|
|
||
| For example, in the Kubernetes implementation `[parent]` refers to a Kubernetes namespace. Other implementations could interpret the string differently, including enabling hierarchies of resources containing `TaskRun`s, for example the API endpoint `/apis/tekton.dev/v1beta1/folders/my-folder/project/my-project/taskruns` could represent a hierarchy of "projects" that own `TaskRun`s. |
There was a problem hiding this comment.
I'm a bit confused about the "Kubernetes implementation" terminology.
My understanding is that this documents abstracts the Tekton API with the aim to define what a "Tekton conform" API should look like.
The reference implementation is the Tekton implementation, and it's based on k8s. There may be other implementation in future, which will not be called Tekton, and they may or may not be k8s based.
One might decide to create another k8s version of Tekton written in Rust for instance (probably not).
There was a problem hiding this comment.
Yeah, I agree that's a confusing distinction. Words are hard 😅
I'd like to avoid calling anything "the Tekton implementation", since the rest of this doc tries to draw the distinction that Tekton is primarily an API, with potentially many implementations.
I'd love to find a good term for "the implementation at https://github.com/tektoncd/pipeline", I've been referring to this as "the Kubernetes implementation", but you're right, that's not necessarily a unique identifier. Suggestions welcome, and we can also punt on finding the best term and just merge this and update it later when we find it.
|
|
||
| Tekton implementations MUST NOT require `spec` fields outside this implementation; to do so would break interoperability between such implementations and implementations which implement validation of field names. | ||
|
|
||
| **NB:** All fields and resources not listed below are assumed to be **OPTIONAL**, not RECOMMENDED or REQUIRED. For example, at this time, support for `PipelineRun`s and for CRUD operations on `Task`s or `Pipeline`s is **OPTIONAL**. |
There was a problem hiding this comment.
Why are PipelineRuns not included? They are part of the beta API and I see for reason for them to be excluded.
I also have mixed feeling about having Tasks as OPTIONAL.
While it is possible to re-use Tasks (from the public or a private catalog) by injecting them on the fly in a TaskRun's taskSpec, doing so would require a non-trivial amount of work for a Tekton use to move some of their workflows to such Tekton conformant platform.
There was a problem hiding this comment.
PipelineRuns are excluded purely because this is the minimal set I thought we could get away with. It's definitely coming, but this PR is already huge :)
| | Field Name | Field Type | Requirement | | ||
| |-----------------------|----------------------|-------------| | ||
| | `params` | `[]Param` | REQUIRED | | ||
| | `taskSpec` | `TaskSpec` | REQUIRED | |
There was a problem hiding this comment.
This means that support for taskSpec is required for conformance, right?
There was a problem hiding this comment.
That's correct. Implementations can support task references (including someday Tekton Bundles!), but they must support inline-defined task specs.
| | `taskSpec` | `TaskSpec` | REQUIRED | | ||
| | `workspaces` | `[]WorkspaceBinding` | REQUIRED | | ||
| | `status` | Enum:<br>- `""` (default)<br>- `"TaskRunCancelled"` | RECOMMENDED | | ||
| | `timeout` | string (duration) | RECOMMENDED | |
There was a problem hiding this comment.
I have mixed feelings about this. Timeouts are key to the usability of the platform... if you want to keep this optional we could mention that the platform should at least provide some timeout mechanism (which would not be workflow specific).
There was a problem hiding this comment.
I agree! I wanted to have the initial spec be as minimally-scoped as possible, and timeout was on the bubble.
I could add a note that platforms should consider enforcing timeouts in some way, but honestly that kinda seems like their business. If they don't want to support timeout, I'm not sure we need to make them? I'm on the fence. 🤷♂️
|
In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vdemeester 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
|
/lgtm |
|
/meow |
|
In response to this:
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. |
First part of TEP-0012
Rendered
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.
/assign @bobcatfish
/assign @vdemeester