-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Introduce (Cluster)EventSource CRD to subscribe to CloudEvents emitted by KEDA #3533
Comments
@kedacore/keda-maintainers I don't have a preference on the implementation - We can start doing things in our current operator and later on introduce a dedicated container (if we really have to) |
@zroubalik @JorTurFer We had some discussion on what the best model for
Make sense? |
Hi all, here's the high-level idea of how I'd like to implement CloudEvents in KEDA: To acheive this, we'll need to
OperationsTo help operate this at scale, we should offer new Prometheus & OTEL metrics:
Proposed Action plan Estimation:The proposal is to implement the whole scope in multiple phases to be more agile and merge changes faster. For the MVP, it feels best to implement the following features with one event example:
The following features can be implemented with follow-up PRs (in order):
|
LGTM |
LGTM but I have one question. How are we going to take the cluster name? I mean, IIRC we don't have that info inside the pod, we have to request it to the users (it's not a problem IMHO, but just to be sure) |
That's a valid question but no need to worry. This is configurable and when not specified "default" will be used (AFAIK, need to check design doc) |
Hi, sorry for the delay. I like the proposal and the proposed direction. Great job @SpiritZhou and @tomkerkhove! |
All the work was done by @SpiritZhou :) |
There may be a scenario where the user needs to send one CloudEvent to multiple destinations, and there are two possible solutions:
I think it would be convenient for users to create one CloudEvent resource, but I am not sure if there are any drawbacks or if the concept of CloudEvents would be better served by creating multiple CloudEvent resources. What do you think? Which one is better? @tomkerkhove @JorTurFer @zroubalik |
There is actually a 3rd option which is what i originally had in mind:
I personally think creating 1 resource per scenario (aka send all events when auth fails for my app) is what we should strive for and avoid having monolithical subscriptions. That's why I prefer to keep the model simple and have multiple event source resources. Does that mean I want to avoid using an array? No. But if that is at the cost of having a good schema which we can validate against then I'd say yes. I'd love to know what business scenario there is for creating 1 EventSource resource that pushes to 3 HTTP endpoints? |
I'm not an expert in CloudEvents (and neither a noob, I'm a real ignorant), so maybe I'm saying something stupid, but where is the problem of supporting multiple CloudEvent resources with multiple targets of the same type? I'm not saying that we have to support it now (we could just wait to get feedback before doing a lot of things), it's just a question. Maybe in the beginning we could start with a single CloudEvent with multiple targets or even with a single CloudEvent with a single target to get feedback from end users |
I have also reviewed the PR and I'm curious about how you will choose between HTTP and Event Grid. Will you include the extra CRD that @tomkerkhove proposed for it? Will it be part of the current CRD? |
Yes, and because it's owned by different departments they should create their own CRD instance and not bundle it in to a single "subscription" in my opinion. That's why for me 1 subscription is linked to 1 destination (or at least of the same type IMO). If you need multiple destinations, then it's because that's for a difference scenario and thus a separate resource, but that's just my opinion.
The current PR is not according to what we specced out and I believe @SpiritZhou is going to update the PR to align with it but he's waiting for our outcome on the single or multiple destination per type. The original proposal which we agreed on is this: apiVersion: events.keda.sh/v1alpha1
kind: EventSource # Or ClusterEventSource
metadata:
name: operations-cross-cluster-events
spec:
destination:
http:
uri: http://foo.bar/
authentication:
apiKey:
headerName: x-api-key
valueFrom:
secretKeyRef:
name: secrets-operations-events
key: webhook-api-key
azureEventgrid:
topicEndpoint: https://{resource-name}.{region}.eventgrid.azure.net/api/events # Mandatory
authentication: # End-users must use accessKey or activeDirectory
accessKey:
# Allow end-users to pull information from Kubernetes secret or from TriggerAuthentication resources
valueFrom:
secretKeyRef:
name: secrets-operations-events
key: webhook-api-key
triggerAuthenticationRef:
name: trigger-auth-sample
parameterName: eventGridAuth
activeDirectory:
tenantId: xyz
clientApplication:
id: ABC
secret:
valueFrom:
secretKeyRef:
name: secrets-operations-events
key: webhook-api-key
triggerAuthenticationRef:
name: trigger-auth-sample
parameterName: eventGridAuth
managedIdentity:
valueFrom:
triggerAuthenticationRef:
name: trigger-auth-sample
key: webhook-api-key
eventSubscription:
includedEventTypes:
- keda.example.Event
excludedEventTypes:
- keda.example.Event This allows end-users to use HTTP and/or Azure Event Grid, but only 1 destination and not an array. The thing that is now brought up is:
Personally I'd say no to both and they need separate But curious about your thoughts @zroubalik & @JorTurFer. |
I think that we can start with 1-1 and over it, iterate. I mean, currently we can support 1 destination per EventSource, and then based on feedback, we could update it to support multiple destinations or not. I have a question here. Does it make sense to have another CRD for destinations and link to it in the EventSource? I mean, could a situation like a cluster admin setting allowed destinations (and teams just using them inside their EventSources) be a real world scenario or it doesn't apply here? |
That's a valid question, but instead I'd introduce crd in the future then for cluster admiks to define what is (not) allowed and use the for event source validation going forward. I think that is nicer rather than adding more resources because of destinations? |
These are valid points, we should think about what is the main usecase though for mutliple destinations. Is it that admin would like to create a different types of But I agree that we can start with 1-1 relation, but don't block ourselves on adding more in the future (in this case we shouldn't probably change the CRD spec, ie going from a single field to array). |
Based on @JorTurFer's remarks above and a chat I've had with @zroubalik we've agreed to go with:
We'll start simple and will evaluate this if it comes up in the future |
This is done |
Re-opening to better list events supported in our docs |
@SpiritZhou did we add cluster-wide CRD already or should we open a separate issue for this? |
@SpiritZhou any update on this? |
Any ETA on this? I'd like to contribute to a few of the CloudEvents integrations, is this issue a blocker for them? |
What are you planning on contributing? The crds are already in |
I can start with this one: #3527 |
Sure, you should be able to get started - Thanks @neelanjan00! |
Use-Case
From the design proposal:
Goal
Allow end-users to subscribe to events emitted by KEDA allowing them to gain insights into what is going on and how their applications are being scaled.
These should serve several purposes such as:
Events that are being emitted must be CloudEvent-compliant and support pushing to destination endpoints inside/outside of the cluster.
With this proposal, our intention is to extend the event capabilities of KEDA with CloudEvents.
From a high-level perspective, KEDA will be doing three things in this area:
What are we doing
With the introduction of CloudEvents, we are targeting two audiences:
With the introduction of
EventSource
&ClusterEventSource
CRD, we cover both scenarios and give end-users the controls that they need. The goal is that they can define an endpoint, authentication, and optional filter to which KEDA will emit its events.Similar to
TriggerAuthentication
&ClusterTriggerAuthentication
CRDs, the idea is thatEventSource
is scoped to a single namespace whileClusterEventSource
is cluster-wide.End-users can optionally define event types they want to exclude so that the destination will never receive them.
Do we need a new CRD?
In order to build a scalable way of emitting events, across various teams and parties it is not an option to configure an endpoint directly on our KEDA control plane, but rather rely on a new CRD.
Otherwise, it would quickly become a bottleneck for end-users that have numerous namespaces and want to have more control over who is allowed to receive what events:
While every team could do the filtering on their own, this is a waste of compute.
Requirements
Introduce a new
(Cluster)EventSource
CRD that allows people to subscribe for CloudEvents:Once these are created by end-users; KEDA will automatically push the events to the configured sink(s).
Issues for events are created separately.
Anything else?
Relates to #479
The text was updated successfully, but these errors were encountered: