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-0071: Custom Task SDK. #461

Merged
merged 1 commit into from
Jul 12, 2021
Merged

TEP-0071: Custom Task SDK. #461

merged 1 commit into from
Jul 12, 2021

Conversation

ScrapCodes
Copy link
Contributor

@ScrapCodes ScrapCodes commented Jun 15, 2021

Summary

A Custom Task SDK will make the work of custom task author easier and will offer standard way to implement custom task controllers. While adhering to current separation of concern between a custom task controller's role and tektoncd owned controller's role, it should make custom tasks easier to manage and reason by tektoncd.

/kind tep

Please vote: #459

@tekton-robot tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). labels Jun 15, 2021
@ScrapCodes ScrapCodes changed the title TEP-0071: Custom Task SDK. WIP TEP-0071: Custom Task SDK. Jun 15, 2021
@tekton-robot tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 15, 2021
@bobcatfish
Copy link
Contributor

/assign

@ScrapCodes ScrapCodes changed the title WIP TEP-0071: Custom Task SDK. TEP-0071: Custom Task SDK. Jun 21, 2021
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 21, 2021
@pritidesai
Copy link
Member

/assign @afrittoli
/assign @vdemeester

date example.
3. Presently, `tektoncd` does not manage the life cycle of a custom task at all.
A SDK can open up newer possibilities in this regard.
4. SDK available as a project template, can perform some of common tasks,
Copy link
Member

Choose a reason for hiding this comment

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

+1

Comment on lines +51 to +52
3. Presently, `tektoncd` does not manage the life cycle of a custom task at all.
A SDK can open up newer possibilities in this regard.
Copy link
Member

Choose a reason for hiding this comment

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

Indeed - we do create nightly builds, but that's about it

@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 21, 2021
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.

Thanks for opening this @ScrapCodes (and apologies for my slow time to review 😬 ). I added a couple of comments, mostly just brainstorming some stuff to flesh out the proposal but generally very in favor!!

/approve

## Motivation

1. Currently, a custom task author has limited ways to know what is a standard way
of building his custom task. His best bet is to explore one of the existing
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: possible to use gender neutral "they/them/their" instead of "he/him/his"? (same applies in other parts of this doc as well)

(im planning to put some recommendations around this into our standards docs eventually :))


### Non-Goals

TBD.
Copy link
Contributor

Choose a reason for hiding this comment

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

ideas:

  • providing SDKs in languages other than Go
  • providing a generic CRD controller lib

Question: would this project involve us creating an maintaining any libraries for this, or would the intention at this point be just boiler plate that folks copy and paste? (either makes sense to me - im thinking it would be simplest to start with boilerplate and think about adding libs later, since libs will require their own maintenance?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree. Starting with the boiler plate and then moving towards libs. Disadvantage with boiler plate is, every time we update it, the end users may not get the update. Or the update can be at times non trivial change.

Describe constraints on the solution that must be met. Examples might include
performance characteristics that must be met, specific edge cases that must
be handled, or user scenarios that will be affected and must be accomodated.
-->
Copy link
Contributor

Choose a reason for hiding this comment

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

ideas:

  • A user should be able to fork the SDK and and run it with no modifications required (e.g. it provides a working sample out of the box)
  • We version the SDK (i think? or maybe not?)
  • We could list everything the sample should include, e.g.: working controller, example configuration (built with ko?), docs, tests (unit and integration maybe? maybe system tests too)

It also might be worth describing in detail what the behavior of the example custom task would be (but np if we want to add that as we go)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bobcatfish these are great ideas, I was thinking, maybe I can add them in the proposal step.

Thank you very much for taking a look.

@tekton-robot tekton-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 29, 2021
Copy link
Member

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Thanks for this @ScrapCodes
/approve

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, bobcatfish, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jerop
Copy link
Member

jerop commented Jul 12, 2021

adding lgtm during the API WG, thank you everyone! 🥳

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 12, 2021
@tekton-robot tekton-robot merged commit 72e2088 into tektoncd:main Jul 12, 2021
@ScrapCodes
Copy link
Contributor Author

Thank you @afrittoli and @jerop 😃

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: Proposed
Development

Successfully merging this pull request may close these issues.

None yet

7 participants