TEP-0021: Results API #217

wlynch · 2020-09-28T20:31:44Z

This TEP proposes a Results API to store long term Tekton results independent of
on-cluster runtime data stored in etcd.

/cc @imjasonh @afrittoli

This TEP proposes a Results API to store long term Tekton results independent of on-cluster runtime data stored in etcd.

dlorenc · 2020-10-05T14:16:41Z

/lgtm for proposed state

teps/0021-results-api.md

wlynch · 2020-10-05T16:56:30Z

@skaegi Sounded like you had use cases that align well here. Would love any feedback!

bobcatfish · 2020-10-13T14:29:58Z

teps/0021-results-api.md

+```
+
+`Execution`s contain Tekton execution types, namely TaskRuns or
+PipelineRuns. They may also contain opaque types, which can be useful for


what do you think about calling these Runs considering all the types we want to store are Runs? (TaskRuns, PipelineRuns, I'm guessing custom tasks aka Runs)

I don't feel too strongly about this, but something I like about executions is that it's intentionally not a Run and can have broader meaning beyond the existing Run types (e.g. Custom Tasks like you mentioned, but also DSLs, and other types we don't natively support), even if we expect Runs to be the primary type we handle for most Tekton users.

That said, I'm fine with either. Was there similar discussion when naming Task/PipelineRuns? If so, why was run chosen over execution?

Update on this, we decided to change this to Events to be a bit broader. The intent is still the same.

teps/0021-results-api.md

Add more alternatives: `to get the process started: what if the API was in a different format? (e.g. not gRPC) what if there was no results store? what if there was no generic API?`

There was some confusion around what data could be stored within an execution, particularly for meta-configs/DSLs that get mapped to TaskRuns (e.g. you could have multiple executions, even if only 1 thing was actually ran). This has the side effect of treating Tekton execution types just like any other user extensible data type, which simiplifies how users need to think about the different subtypes, and how API Server implementations need to handle this data / filter queries.

vdemeester

(oh.. I never sent my review 😓 )

teps/0021-results-api.md

vdemeester · 2020-10-06T12:53:53Z

teps/0021-results-api.md

+know that this has succeeded?
+-->
+
+- Define a Result API spec to store and query Tekton results.


We need to define what "results" are here. Is this limited to the results type in Pipeline/Task or the status of the execution or is it more ; like any artifacts generated by a pipeline (tests reports, archive, …)

Added some definition here, as well as some discussion in Alternatives for how this relates to Task output results.

This is intentionally vague so that we can store broad data about TaskRuns and PipelineRuns. This will generally include the entire TaskRun/PipelineRun object (e.g. both spec and status), but we also see this evolving to include other information (input events, post-run receipts of GitHub status updates / Cloud Event publishing).

This is intended to be extensible to allow for future types which could include the artifacts you mentioned, though we are not looking to introduce new types as part of this proposal.

teps/0021-results-api.md

vdemeester · 2020-10-06T13:01:03Z

teps/0021-results-api.md

+-->
+
+Our goal is to make results have minimal impact on core Pipeline execution. By
+running a separate controller, while this does add some more overhead, this


What overhead do we talk about here ? (I guess on the operation side)

Primarily wanted to acknowledge that at minimum this would run as a separate pod, which likely has some additional cost as opposed to bundling this in the Pipeline controller itself (but in exchange we gain component modularity).

I suspect the operational overhead would vary depending on the cluster environment (e.g. multi-tenant clusters may require per tenant controllers? - not 100% sure. I instinctively suspect that there would be a result controller for each Tekton Pipeline controller), but these details are likely too implementation/environment specific to detail in this doc.

Address comments on how this proposal relates to similiarly named Task output results, and alternative names considered. Also adds some minor changes for auto-delete future work, resource overhead, and removes mentions of already completed work being in-progress.

bobcatfish

🎉 🎉 🎉

bobcatfish · 2020-11-09T20:26:17Z

teps/0021-results-api.md

+  - "@wlynch"
+creation-date: 2020-09-23
+last-updated: 2020-10-26
+status: proposed


after the discussion in today's API working group, i want to float the idea that we merge this as proposed very quickly (without everything from "proposal" down)

(np if that doesn't help and want to continue the discussion as is)

teps/0021-results-api.md

bobcatfish · 2020-11-09T20:33:21Z

teps/0021-results-api.md

+- Service providers can choose to provide their own controller and server
+  implementations to upload Results with additional service specific metadata or
+  implementation specific validation / field formats, so long as it conforms to
+  the Results API.


great requirements 👍 i think these can be good examples to point to if folks are trying to distinguish b/w goals + requirements in other TEPs

bobcatfish · 2020-11-09T20:42:07Z

teps/0021-results-api.md

+Results are intended to be this abstraction over execution types (i.e.
+PipelineRun, TaskRun) so that we can group this data for users and provide a
+mechanism to include additional metadata that does not fit neatly into TaskRuns
+or PipelineRuns.


i would think we'd still store the individual TaskRuns along with the PipelineRun tho right?

Yup! Exactly. Rephrased to make this a bit clearer.

bobcatfish · 2020-11-09T20:43:42Z

teps/0021-results-api.md

+We may choose to provide a facade of the Task/Pipeline APIs in the future for
+convenience, but this would be a layer in addition to the Results API.
+
+### REST/Open API


👍 👍 👍 Thanks for exploring this alternative, seems reasonable to me.

bobcatfish · 2020-11-09T20:46:04Z

teps/0021-results-api.md

+[initial mention](https://github.com/tektoncd/pipeline/issues/1273#issuecomment-546494832)
+of the Task results field (named results to not conflict with PipelineResource
+outputs) was suggested the same week as the original Results API design doc.
+Both of these predated the current TEP process.


🤣 🤭

tektoncd/pipeline#2706

fwiw THIS particular feature has been called "result" since at least tektoncd/pipeline#454 (jan 2019)

bobcatfish · 2020-11-09T20:46:48Z

teps/0021-results-api.md

+  the project from Task output results.
+- Event API : While event is general enough to fill a similar role, this takes
+  away our usage of event as a field of the current Result resource, which then
+  leaves us with a different naming problem.


"stuff that happened"

The only other name that seems appealing to me is something like "history"? "Tekton History"?

afrittoli

Thanks for this, great work!

I really like the idea of abstracting away from Tekton specific types, and be open to store other "results", like events, manifests from DSLs or else.

The main question (concern?) I have is about how the various Event are attached to a Result. Due to the distributed nature of tekton workflows (even a relatively simple one like git hosting -> triggers -> pipeline -> notifications), I feel it should be possible to add Events to a Result incrementally. Is that the way it is planned to work?

AFAIU, the controller would be responsible to create the initial Result and attach extra Events to it as they are eventually discovered. Would the controller be the only one responsible for talking to the API to write objects, or would it be possible for other parties to push events directly to the API?
Since not everything is a CRD, it may be difficult otherwise for the controller to discover all relevant events.

Since a result is a collection of events, does it have to be statically defined as a result, or could it be dynamically collected from available events based on a set of selection criteria?
Results could be basically views on set of events, that can be stored and queried via the API.

Anyways, I'm totally in favour of the overall idea; I hope we can be flexible still in the definition of the API, so it may change from what is in the doc today.

/approve

afrittoli · 2020-11-13T15:36:28Z

teps/0021-results-api.md

+
+  // The etag for this result.
+  // If this is provided on update, it must match the server's etag.
+  string etag = 6


What happened with 5 ?

Various edits/reordering. 😅 I'll reorder the numbers when we create the API for real.

tekton-robot · 2020-11-13T16:29:59Z

[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:

~~teps/OWNERS~~ [afrittoli]

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

wlynch · 2020-11-16T15:42:48Z

The main question (concern?) I have is about how the various Event are attached to a Result. Due to the distributed nature of tekton workflows (even a relatively simple one like git hosting -> triggers -> pipeline -> notifications), I feel it should be possible to add Events to a Result incrementally. Is that the way it is planned to work?

Yes! We've been working on some modifications to the proposal to change events to a more explicit subresource, which would make it easy to do just that. I've been hesitant to add this addition to this proposal, so that we can get this in and make progress. Even in the current API, you could do this by updating a current result, using the etag field to detect any concurrent updates.

AFAIU, the controller would be responsible to create the initial Result and attach extra Events to it as they are eventually discovered. Would the controller be the only one responsible for talking to the API to write objects, or would it be possible for other parties to push events directly to the API?
Since not everything is a CRD, it may be difficult otherwise for the controller to discover all relevant events.

I think we'll want to be agnostic to the source of the initial event creation. Even within Tekton, there might be different controllers creating results (e.g. it seems reasonable for the trigger controller to also create results to record the input event before handing off to pipelines for execution). As long as the client is authorized, the API server shouldn't care.

Since a result is a collection of events, does it have to be statically defined as a result, or could it be dynamically collected from available events based on a set of selection criteria?
Results could be basically views on set of events, that can be stored and queried via the API.

We will probably want an explicit Result resource. While I agree with you that the most relevant bits of data are in the Events, having a container for logical groupings of CI runs is a large benefit we get by having a Result resource.

That said, I think it would be reasonable to be able to query across events, where the Events then link back to the Results they are contained in. This is another thing that is enabled with the subresource change mentioned above! ^_^

Let me know if you're okay with defering to another PR for that change, or if you'd like to see it reflected here.

afrittoli · 2020-11-16T17:08:41Z

/cc @sbwsg

ghost · 2020-11-16T18:54:12Z

/lgtm

TEP-0021: Results API

f8b4f9d

This TEP proposes a Results API to store long term Tekton results independent of on-cluster runtime data stored in etcd.

tekton-robot requested review from afrittoli and imjasonh September 28, 2020 20:31

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 28, 2020

vdemeester added the kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). label Sep 29, 2020

R2wenD2 suggested changes Oct 5, 2020

View reviewed changes

teps/0021-results-api.md Outdated Show resolved Hide resolved

TEP-0021 Results API: Fix whitespace.

7f79cc8

bobcatfish reviewed Oct 13, 2020

View reviewed changes

teps/0021-results-api.md Outdated Show resolved Hide resolved

wlynch force-pushed the results-tep branch from 1b6edfd to a8cb93b Compare October 14, 2020 19:51

TEP-0021 Results API: Add more alternatives.

b6540ad

Add more alternatives: `to get the process started: what if the API was in a different format? (e.g. not gRPC) what if there was no results store? what if there was no generic API?`

wlynch force-pushed the results-tep branch from a8cb93b to b6540ad Compare October 14, 2020 19:58

wlynch force-pushed the results-tep branch from a890889 to bcea7fb Compare October 26, 2020 22:27

vdemeester reviewed Oct 27, 2020

View reviewed changes

bobcatfish reviewed Nov 9, 2020

View reviewed changes

bobcatfish mentioned this pull request Nov 12, 2020

Design result reporting to to Result Store. tektoncd/pipeline#454

Closed

TEP-0021 Results API: Minor clarifying updates based on TEP comments.

adce4e5

afrittoli reviewed Nov 13, 2020

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 13, 2020

tekton-robot requested a review from a user November 16, 2020 17:08

tekton-robot assigned ghost Nov 16, 2020

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2020

tekton-robot merged commit 2139595 into tektoncd:master Nov 16, 2020

LOZORD mentioned this pull request Nov 23, 2020

TEP-0032 Tekton notifications #275

Merged

TEP-0021: Results API #217

TEP-0021: Results API #217

Uh oh!

Conversation

wlynch commented Sep 28, 2020

Uh oh!

dlorenc commented Oct 5, 2020

Uh oh!

Uh oh!

wlynch commented Oct 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobcatfish left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

afrittoli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tekton-robot commented Nov 13, 2020

Uh oh!

wlynch commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afrittoli commented Nov 16, 2020

Uh oh!

ghost commented Nov 16, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

wlynch commented Oct 5, 2020 •

edited

Loading

wlynch commented Nov 16, 2020 •

edited

Loading