-
Notifications
You must be signed in to change notification settings - Fork 416
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
Add EventListener Selector For TriggerCRD #773
Conversation
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
// list of namespaces. | ||
// +k8s:openapi-gen=true | ||
type NamespaceSelector struct { | ||
// Boolean describing whether all namespaces are selected in contrast to a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious about this Any field. Wondering if we need a separate field or we could pass something to MatchNames to indicate we should select all namespaces (e.g. matchNames: ["*"]
or matchNames:[""]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AdmissionWebhooks also have NameSpaceSelector (and ObjectSelector). We could take some pointers from there: https://github.com/kubernetes/api/blob/master/admissionregistration/v1beta1/types.go#L410
(might be worth seeing if there are any reusable bits in the implementation that we could use too)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, any
here could be dispense with but AdmissionWebhook implementation is for more complex scenarios. https://github.com/kubernetes/api/blob/8519c5ea46199d57724725d5b969c5e8e0533692/admissionregistration/v1beta1/types.go#L372
pkg/sink/sink.go
Outdated
|
||
var trItems []triggersv1.Trigger | ||
if el.Spec.NamespaceSelector.Any { | ||
trList, err := r.TriggersClient.TriggersV1alpha1().Triggers("").List(metav1.ListOptions{}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might want to use an informer here to reduce calls to the API server. ( can be a follow up though)
@khrm was there additional work you were planning on doing before unmarking this as WIP? (One thing we should add are some release notes and some docs/examples). |
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
The following is the coverage report on the affected files.
|
Code looks good to me 👍 |
005adb7
to
837e2f5
Compare
The following is the coverage report on the affected files.
|
@savitaashture I would create another pr for e2e test after this is merge. |
EventListener have NamespaceSelector field which gives us namespaces from where EventListener fetches Trigger object to process events.
The following is the coverage report on the affected files.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Changes
EventListener have NamespaceSelector field which gives us namespaces from where EventListener fetches Trigger object to process events.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Release Notes