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.

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

TEP-0127: Larger Results via Sidecar Logs #887

Merged
merged 1 commit into from
Dec 1, 2022
Merged

Conversation

jerop
Copy link
Member

@jerop jerop commented Nov 30, 2022

Today, Results have a size limit of 4KB per Step and 12KB per TaskRun in the best case - see issue.

The goal of TEP-0086 is to support larger Results beyond the current size limits. TEP-0086 has many alternatives but no proposal. This TEP proposes experimenting with one of the alternatives - Sidecar logs. This allows us to support larger Results that are stored within TaskRun CRDs.

Huge shout out to @chitrangpatel for implementing the POC - see the demo 👏

(As discussed in WG meetings, moving over proposal from #745 into a separate TEP so we can mark it as implementable)

@tekton-robot tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 30, 2022
@jerop
Copy link
Member Author

jerop commented Nov 30, 2022

/kind tep
/assign @afrittoli

@jerop jerop force-pushed the main branch 2 times, most recently from 06e6856 to 10431fb Compare November 30, 2022 19:44
@skaegi
Copy link
Contributor

skaegi commented Nov 30, 2022

I've read the TEP and this looks very good to me! This is an ongoing problem that our users encounter. We're happy to sign up to test and use this approach. Thank-you @jerop and @chitrangpatel

@bobcatfish
Copy link
Contributor

/assign

Copy link
Contributor

@bobcatfish bobcatfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments from me but generally I'm very much in favor and excited to see this move forward! 🎉

/approve

teps/0127-larger-results-via-sidecar-logs.md Outdated Show resolved Hide resolved
teps/0127-larger-results-via-sidecar-logs.md Outdated Show resolved Hide resolved
teps/0127-larger-results-via-sidecar-logs.md Show resolved Hide resolved
teps/0127-larger-results-via-sidecar-logs.md Show resolved Hide resolved
teps/0127-larger-results-via-sidecar-logs.md Show resolved Hide resolved
teps/0127-larger-results-via-sidecar-logs.md Outdated Show resolved Hide resolved
@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 30, 2022
@jerop jerop force-pushed the main branch 4 times, most recently from 9c731cb to 5953ab7 Compare December 1, 2022 12:54
@afrittoli
Copy link
Member

A couple of comments, but nothing blocking.
Thank you for this, looking forward to this feature!
/approve

Today, `Results` have a size limit of 4KB per `Step` and 12KB per
`TaskRun` in the best case - see [issue][4012].

The goal of [TEP-0086][tep-0086] is to support larger `Results`
beyond the current size limits. TEP-0086 has many alternatives
but no proposal.

This TEP is proposes experimenting with one of the alternatives
- `Sidecar` logs. This allows us to support larger `Results` that
are stored within `TaskRun` CRDs.

[4012]: tektoncd/pipeline#4012
[tep-0086]: https://github.com/tektoncd/community/blob/main/teps/0086-changing-the-way-result-parameters-are-stored.md
@abayer
Copy link
Contributor

abayer commented Dec 1, 2022

/approve

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abayer, afrittoli, bobcatfish

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@abayer
Copy link
Contributor

abayer commented Dec 1, 2022

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 1, 2022
@tekton-robot tekton-robot merged commit 184b93a into tektoncd:main Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: UnAssigned
Development

Successfully merging this pull request may close these issues.

7 participants