Initial implementation for trigger groups #1052

jmcshane · 2021-04-14T17:30:23Z

Changes

Initial demo of using trigger groups to process eventlistener requests

Submitter Checklist

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

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices
Release notes block has been filled in or deleted (only if no user facing changes)

See the contribution guide for more details.

Release Notes

Implement triggerGroups and interceptor chains

tekton-robot · 2021-04-14T17:30:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign iancoffey
You can assign the PR to them by writing /assign @iancoffey in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

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

tekton-robot · 2021-04-14T20:59:40Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	90.3%	-7.4
pkg/sink/sink.go	79.0%	56.4%	-22.6
pkg/template/resource.go	100.0%	90.5%	-9.5

tekton-robot · 2021-04-15T20:44:47Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	93.5%	-4.2
pkg/sink/sink.go	79.0%	72.6%	-6.4

tekton-robot · 2021-04-16T01:28:42Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	93.5%	-4.2
pkg/sink/sink.go	79.0%	66.5%	-12.5

tekton-robot · 2021-04-16T01:55:08Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	97.8%	0.1
pkg/sink/sink.go	79.0%	66.5%	-12.5
test/builder/eventlistener.go	81.5%	79.8%	-1.7

tekton-robot · 2021-04-16T01:57:20Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	97.8%	0.1
pkg/sink/sink.go	79.0%	66.5%	-12.5
test/builder/eventlistener.go	81.5%	79.8%	-1.7

tekton-robot · 2021-04-16T02:05:19Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	97.8%	0.1
pkg/sink/sink.go	79.0%	66.5%	-12.5
test/builder/eventlistener.go	81.5%	81.9%	0.4

tekton-robot · 2021-04-16T02:07:17Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	97.8%	0.1
pkg/sink/sink.go	79.0%	66.5%	-12.5
test/builder/eventlistener.go	81.5%	81.9%	0.4

tekton-robot · 2021-04-16T14:17:28Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	97.8%	0.1
pkg/sink/sink.go	79.0%	66.5%	-12.5
test/builder/eventlistener.go	81.5%	81.9%	0.4

tekton-robot · 2021-04-16T14:43:52Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	97.8%	0.1
pkg/sink/sink.go	79.0%	74.2%	-4.8
test/builder/eventlistener.go	81.5%	81.9%	0.4

tekton-robot · 2021-04-16T14:55:18Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.7%	97.8%	0.1
pkg/sink/sink.go	79.0%	79.3%	0.2
test/builder/eventlistener.go	81.5%	81.9%	0.4

jmcshane · 2021-04-16T14:59:50Z

@dibyom @afrittoli @wlynch this implementation is now ready in line with our discussion for TEP-0053. I reduced the changes that are a part of this PR to the absolute simplest possible, including maintaining the response code validation. The key reason I kept the response codes in here for now is because of the huge test surface that depends on this response code and expects the HTTP function to block until all resources are created.

I think it makes sense to open a separate issue to track discussion on how we manage the HTTP status code. For now, this PR is ready from a code perspective. I think what we need to finish it are:

Documentation about triggerGroups in the eventlistener document
Example eventlistener configuration including triggergroups

Please let me know if there is anything else you would like to see in this PR.

dibyom · 2021-04-23T14:59:56Z

The key reason I kept the response codes in here for now is because of the huge test surface that depends on this response code and expects the HTTP function to block until all resources are created.

Sounds good. We can tackle that separately as part of #931

dibyom

Haven't gone through all of the code yet. But yes, docs and examples would be the two remaining things.
Also, we should update the release notes section of the PR to something a bit more descriptive -- (Adds a way to group common interceptors across multiple triggers ...)

dibyom · 2021-04-23T15:02:20Z

pkg/sink/sink.go

+	trItems, err := r.selectTriggers(el.Spec.NamespaceSelector, el.Spec.LabelSelector)
+	if err != nil {
+		r.Logger.Errorf("Unable to select configured triggers: %s", err)
+		response.WriteHeader(http.StatusInternalServerError)


Should we return some info in the HTTP response as well?

What info are you considering here to return besides the downstream error message?

I was thinking something along the lines of a generic error message (like the unable to select configured triggers) along with the usual debugging pointers (eventID, eventListenerUID) so that its easy to look for in the logs. What do you think?

pkg/apis/triggers/v1alpha1/event_listener_validation_test.go

jmcshane · 2021-05-04T18:27:27Z

/test pull-tekton-triggers-go-coverage

tekton-robot · 2021-05-04T18:31:41Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.8%	97.9%	0.1
pkg/interceptors/server/server.go	82.1%	82.5%	0.4
pkg/sink/sink.go	79.0%	79.3%	0.2

This feature allows an operator to specify a set of interceptors that will be executed before a group of triggers are selected and executed. This allows common data to be passed from interceptor execution down to multiple triggers to solve a set of common use cases across multiple Triggers. This feature is enabled for now inline in the EventListener spec, but in the future may be enabled only in alpha once the feature gates proposal is implemented within this project.

tekton-robot · 2021-05-06T03:05:07Z

The following is the coverage report on the affected files.
Say /test pull-tekton-triggers-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/triggers/v1alpha1/event_listener_validation.go	97.8%	97.9%	0.1
pkg/interceptors/server/server.go	82.1%	82.5%	0.4
pkg/sink/sink.go	79.0%	79.3%	0.2

jmcshane · 2021-05-06T12:11:35Z

/test pull-tekton-triggers-integration-tests

tekton-robot · 2021-05-08T20:07:18Z

@jmcshane: PR needs rebase.

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.

savitaashture · 2021-10-12T06:54:35Z

PR handled as part of this #1232

tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 14, 2021

tekton-robot requested review from iancoffey and ncskier April 14, 2021 17:30

tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 14, 2021

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 15, 2021

tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 16, 2021

jmcshane force-pushed the feature/trigger-groups-implementation branch from 5232a70 to 0d14397 Compare April 16, 2021 01:56

jmcshane force-pushed the feature/trigger-groups-implementation branch from 0d14397 to 786b15b Compare April 16, 2021 02:03

jmcshane force-pushed the feature/trigger-groups-implementation branch from 786b15b to c317287 Compare April 16, 2021 02:06

jmcshane force-pushed the feature/trigger-groups-implementation branch from c317287 to 47a1463 Compare April 16, 2021 14:15

jmcshane force-pushed the feature/trigger-groups-implementation branch from 47a1463 to a418ffc Compare April 16, 2021 14:41

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 16, 2021

jmcshane force-pushed the feature/trigger-groups-implementation branch from a418ffc to f886d40 Compare April 16, 2021 14:53

jmcshane marked this pull request as ready for review April 16, 2021 14:56

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 Apr 16, 2021

dibyom reviewed Apr 23, 2021

View reviewed changes

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 4, 2021

jmcshane force-pushed the feature/trigger-groups-implementation branch from f886d40 to bebe3a2 Compare May 4, 2021 17:30

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 4, 2021

jmcshane force-pushed the feature/trigger-groups-implementation branch 2 times, most recently from 1d36627 to bde9e40 Compare May 4, 2021 18:21

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 4, 2021

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 5, 2021

jmcshane force-pushed the feature/trigger-groups-implementation branch from bde9e40 to 18f81b9 Compare May 6, 2021 03:03

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2021

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 8, 2021

dibyom added this to the Triggers v0.17 milestone Sep 29, 2021

dibyom mentioned this pull request Oct 1, 2021

Feature: TriggerGroups #1232

Merged

4 tasks

savitaashture closed this Oct 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial implementation for trigger groups #1052

Initial implementation for trigger groups #1052

jmcshane commented Apr 14, 2021 •

edited

Loading

tekton-robot commented Apr 14, 2021

tekton-robot commented Apr 14, 2021

tekton-robot commented Apr 15, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

jmcshane commented Apr 16, 2021 •

edited

Loading

dibyom commented Apr 23, 2021

dibyom left a comment

dibyom Apr 23, 2021

jmcshane May 4, 2021

dibyom May 11, 2021

jmcshane commented May 4, 2021

tekton-robot commented May 4, 2021

tekton-robot commented May 6, 2021

jmcshane commented May 6, 2021

tekton-robot commented May 8, 2021

savitaashture commented Oct 12, 2021

Initial implementation for trigger groups #1052

Initial implementation for trigger groups #1052

Conversation

jmcshane commented Apr 14, 2021 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Apr 14, 2021

tekton-robot commented Apr 14, 2021

tekton-robot commented Apr 15, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

tekton-robot commented Apr 16, 2021

jmcshane commented Apr 16, 2021 • edited Loading

dibyom commented Apr 23, 2021

dibyom left a comment

Choose a reason for hiding this comment

dibyom Apr 23, 2021

Choose a reason for hiding this comment

jmcshane May 4, 2021

Choose a reason for hiding this comment

dibyom May 11, 2021

Choose a reason for hiding this comment

jmcshane commented May 4, 2021

tekton-robot commented May 4, 2021

tekton-robot commented May 6, 2021

jmcshane commented May 6, 2021

tekton-robot commented May 8, 2021

savitaashture commented Oct 12, 2021

jmcshane commented Apr 14, 2021 •

edited

Loading

jmcshane commented Apr 16, 2021 •

edited

Loading