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

WIP Add a CatalogTask Custom Task Type #723

Closed
wants to merge 1 commit into from
Closed

WIP Add a CatalogTask Custom Task Type #723

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 10, 2021

Changes

This Custom Task type knows how to fetch a Task YAML from
a catalog and directly run it without the user needing to
kubectl apply -f the task first.

Looking at the following PipelineTask, it uses entries from the
catalog without having those tasks installed in the cluster.

  tasks:
    - name: unit-test
      taskRef:
        apiVersion: catalogtask.tekton.dev/v1alpha1
        kind: CatalogTask
        name: golang-test--0.1
      params:
        - name: package
          value: github.com/tektoncd/pipeline
        - name: context
          value: ""
      workspaces:
        - name: source
          workspace: shared

I've written this as a proof of concept initially. The code is very junky. We could optimize by only performing sparse checkouts of the remote task yaml. There's no support yet for credentials when fetching the remote tasks. And so on.

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

  • Includes docs (if user facing)
  • Commit messages follow commit message best practices
  • Commit messages includes a project tag in the subject - e.g. [OCI], [hub], [results], [task-loops]

See the contribution guide
for more details.

/hold

@tekton-robot tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Mar 10, 2021
@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 10, 2021
@imjasonh
Copy link
Member

First of all, I'm very very excited to see this, both as an example of playing with Custom Tasks, and as an example of prototyping behavior that's been proposed for core Tekton. I'll approve it into experimental just based on that alone, junky code or not. 😄

I'm curious whether you plan to improve this over time, with the goal of eventually bringing it into core Tekton, or if this is more of a proof-of-concept that we don't expect to end up as core behavior. Not that it changes whether I'll approve it or whether I'm excited about it (yes and yes!), just to get an idea of where this is expected to end up.

@ghost
Copy link
Author

ghost commented Mar 10, 2021

I'm curious whether you plan to improve this over time, with the goal of eventually bringing it into core Tekton, or if this is more of a proof-of-concept that we don't expect to end up as core behavior.

I'm not sure! Lots of directions I could see this going. If it solely existed as an artifact to spur discussion when we're looking at Step and Task Reuse I think it would have done its job. I wanted to test the hypothesis that we could do at least some of what we're talking about with the existing extension mechanisms we have today. But it could be taken further:

  • Improve it to the point where it's usable in production, remain a Custom Task to limit feature creep in Pipeline core
  • Improve it to the point it's usable in production, absorb in to Pipeline core
  • Shape it into a template for users who want to implement their own (SVNTask, GCSTask, etc...)
  • Expand it to be a "RemoteTaskResolver" that can support git, bundles, svn, gcs, s3, and so on and so on. Again keeping it a Custom Task to limit feature creep in Pipelines core
  • Use it to explore the idea of a TaskResolver Interface that any Custom Task could implement (on any conforming platform)

@imjasonh
Copy link
Member

imjasonh commented Mar 10, 2021

I love it.

/lgtm

edit: The "close with comment" button is too close to the "comment" button. 😊

@imjasonh imjasonh closed this Mar 10, 2021
@imjasonh imjasonh reopened this Mar 10, 2021
@ghost ghost changed the title WIP Add a GitTask Custom Task controller WIP Add a GitRef Custom Task controller Mar 11, 2021
@ghost ghost changed the title WIP Add a GitRef Custom Task controller WIP Add a CatalogTask Custom Task Type Mar 12, 2021
This Custom Task type knows how to fetch a Task YAML from
a catalog and directly run it without the user needing to
`kubectl apply -f` the task first.

Looking at the following PipelineTask, it uses entries from the
catalog without having those tasks installed in the cluster.

```yaml
  tasks:
    - name: unit-test
      taskRef:
        apiVersion: catalogtask.tekton.dev/v1alpha1
        kind: Task
        name: golang-test--0.1
      params:
        - name: package
          value: github.com/tektoncd/pipeline
        - name: context
          value: ""
      workspaces:
        - name: source
          workspace: shared
```
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

Couple of questions.

  • This is tied to one catalog instance/repository right ? and this is only configurable one the catalogtask controller ? Wouldn't it make sense to have a mecahinsms closer to
  • Assuming we have OCI bundles widely adopted, is there still a need for this custom task ? 🙃 I think it would still be useful for a some use cases.

@ghost
Copy link
Author

ghost commented Mar 17, 2021

This is tied to one catalog instance/repository right ? and this is only configurable one the catalogtask controller ? Wouldn't it make sense to have a mecahinsms closer to

I think the end here got chopped off?

I want to add support for multiple catalogs in a single resolver something like this:

# resolver's ConfigMap
- name: tekton
  bundle: https://github.com/tektoncd/catalog.git
- name: internal
  bundle: [email protected]:cicd/catalog.git

@ghost
Copy link
Author

ghost commented Mar 17, 2021

Assuming we have OCI bundles widely adopted, is there still a need for this custom task ? 🙃 I think it would still be useful for a some use cases.

I think there are lots of advantages in keeping pipelines flexible on integrating possible sources of tasks and pipelines. "Tekton Bundle" OCI image is one possible source but so is git repo, bucket, etc etc.

Edit to add: "integrating" here is doing a lot of heavy lifting but I'm not proposing tekton pipelines should bake support for these other sources into its codebase. I think this is an interesting exploration of what could be supported if tekton instead offered a generic interface that a custom task (or any controller) responds to with a resource YAML.

@vdemeester
Copy link
Member

I think the end here got chopped off?

Indeed. I think I thought of closer to "oci bundle reference"/image, where you can refer to which catalog you are looking into.

I want to add support for multiple catalogs in a single resolver something like this […]

Would we support only git then, or more VCS ? (sorry for asking naive and probably "roadmap" questions 😛 )

I think there are lots of advantages in keeping pipelines flexible on integrating possible sources of tasks and pipelines. "Tekton Bundle" OCI image is one possible source but so is git repo, bucket, etc etc.

Edit to add: "integrating" here is doing a lot of heavy lifting but I'm not proposing tekton pipelines should bake support for these other sources into its codebase. I think this is an interesting exploration of what could be supported if tekton instead offered a generic interface that a custom task (or any controller) responds to with a resource YAML.

Yes 👼🏼 I tend to agree with this statement (especially "not proposing tekton pipelines should bake support for these …" 😛). I mainly was asking to understand more the approach.

/approve

@tekton-robot
Copy link

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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 19, 2021
@vdemeester
Copy link
Member

/retest

@ghost
Copy link
Author

ghost commented Mar 19, 2021

Would we support only git then, or more VCS ? (sorry for asking naive and probably "roadmap" questions 😛 )

This is very helpful to discuss and an interesting question! Up to now I've been thinking about "catalog", "oci", "git", "gcs" as all different kinds of storage which should each get their own Resolver. But maybe "catalog" is a concept independent of storage type?

Thinking about it this way, a catalog could be described by the tuple (storage, cursor). storage is oci/git/svn/etc... cursor maps from a task name ("golang-test") to its YAML/JSON in that storage.

So our open source catalog is git storage plus a cursor that goes from {task-name} to the contents of /tasks/{task-name}/{version}/{task-name}.yaml.

Edit to add: thinking about catalogs this way is fun. Pipelines' current "catalog" uses the cluster as its storage and a cursor that knows how to map from {task-name} to the Task in that cluster.

(I'm using "Task" here but intend the meaning to also be "Pipeline" or any other types we want to support).

@ghost
Copy link
Author

ghost commented May 10, 2021

/close

@tekton-robot
Copy link

@sbwsg: Closed this PR.

In response to this:

/close

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.

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants