knative / build

A Kubernetes-native Build resource.
Apache License 2.0
575 stars 159 forks source link

Proposal: Allow events to initiate builds via new Cloud Event Listener CRD #591

Closed iancoffey closed 5 years ago

iancoffey commented 5 years ago

Fixes #577

Depends:

Proposed Changes

This is a functional proposal which introduces a means for knative eventing to directly trigger builds upon receipt of specific events. The build listener accepts cloud events and produces builds for them. Currently this proposal only accepts Github check_suite events, but any event can theoretically produce any build.

An overview is provided in the readme.

This PR is very much a proposal and a wip, created to generate feedback

Cloud Events Listener Flow

Release Note

knative-prow-robot commented 5 years ago

Hi @iancoffey. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
googlebot commented 5 years ago

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot commented 5 years ago

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot commented 5 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

iancoffey commented 5 years ago

/assign @shashwathi

imjasonh commented 5 years ago

Thanks for this PR! Sorry for the delay in responding. This looks great! There's just one problem :confused: :

At this time the future of Knative Build is a bit murky, and we're working through how it should interact with things like Eventing and Serving, and what kinds of usability benefits it provides. The main tension is that the "Build" concept as currently defined might be overly restrictive for real-world build-test-deploy use cases -- steps only execute in order within the same Pod, and are currently tied to Serving deployments, since that was their original use case.

However, the Tekton Pipelines (née Knative Pipelines) project is working toward a full CI/CD platform on Kubernetes, with support for Knative deployments, as well as deployments to anywhere, and non-deployment steps like tests and vulnerability scans. Pipelines aims to support complex workflows, retries, results and artifacts, everything.

Tekton is currently actively designing how it will integrate events like this to automatically (i.e., continuously) trigger pipeline executions. I think they'd be very interested to learn about this proposal, and if you feel like discussing/demoing this at the weekly working group meeting, it's every Tuesday at 9am PST -- just add yourself to the agenda.

Let me know if you have any questions about Tekton, or reach out #build-pipeline on Slack and we'd be happy to answer any questions.

Or, if you'd prefer to find a fit for this inside Knative Build, I'd love to chat about that too, we can do that here, or on Slack, or at the next Knative Build working group meeting, which is next week.

This is truly very exciting work and we'd love your input. :+1:

iancoffey commented 5 years ago

@ImJasonH Wow, that is a lot of great information! Thanks much.

I cant actually see any of the Tekton docs you linked yet or the calendar but I have requested access, so Im very exited to read through that discussion. Also, I will def attend the next working group meeting Tuesday.

Unfortunately, I cant seem to sign up for a knative slack account since I lack the appropriate company email (I work for Salesforce), which is a major bummer. Can I be invited to Slack perhaps?

As for where this logic lives, that question is one of the many that originally inspired this proposal - it started as a standalone project, but seemed a decent fit here given the context in https://github.com/knative/build/issues/577. That said, one nice thing about this approach is that it is portable and simple to move to another project, or become its own. The more I think over this approach however, the more I realize that I do really like this general approach. It is just so simple to configure and the isolation/security of data seems a strong point. Also, other things could be made to control the listeners for a truly large scale deployment scenario.

Thanks again and hopefully we can continue to chat on slack and those documents soon when I get access 😄

evankanderson commented 5 years ago

https://github.com/knative/docs/blob/master/contributing/SLACK-GUIDELINES.md has info on joining slack; the actual signup is at https://slack.knative.dev/

I'll take a look at this piece later today, but I think this may be a "missing link" between source repo changes and triggering CD, and it seems like a useful component that could be included / vendored in multiple locations as needed.

knative-prow-robot commented 5 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: iancoffey To fully approve this pull request, please assign additional approvers. We suggest the following additional approver: imjasonh

If they are not already assigned, you can assign the PR to them by writing /assign @imjasonh in a comment when ready.

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: - **[OWNERS](https://github.com/knative/build/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
iancoffey commented 5 years ago

At this point, I can actually end to end test this locally with minikube and example cloudevents loaded with an example github check_suite cloudevent payload. It actually works! Builds are made, everything seems configured as I expect it should be. That means that if/when I get this deployed somewhere other than locally, I should have a full end to end pipeline for simple eventing->build integration. And even though this change is humongous, the actual crd is small which is pretty neat.

Screen Shot 2019-04-09 at 12 42 34 PM

I added a bunch more documentation on this proposal here in the cmd dir.

So this experiment has succeeded I guess, it can work end to end.

What is the feeling for bringing this or something similar in and if its possible, what needs to happen before that can happen? I really am enjoying working with this crd, it opens a lot of neat possibilities for my work and perhaps others as well.

If we determine this PR should be closed in preference of adding similar logic elsewhere, Id love to chat about that too :)

@evankanderson @ImJasonH

iancoffey commented 5 years ago

Also, I found this tekton proposal and the crd in this PR might be a partial fit there, and could maybe be modified to just handle the event differently for whatever tekton expects.

iancoffey commented 5 years ago

I am going to close this in favor of my Tekton PR -> https://github.com/tektoncd/pipeline/pull/783 - but if someone finds this useful please do re-open.

Thanks @ImJasonH and @evankanderson for the help!