Conversation
Signed-off-by: adamkenihan <adam.kenihan@est.tech>
Signed-off-by: adamkenihan <adam.kenihan@est.tech>
|
|
||
| ## Proposal | ||
|
|
||
| Add CDEvents to the list of available `Providers` in Flux Notification controller. The user should be able to create mappings within the configuration of the `Provider` that dictates the CDEvent payload to send depending on the Flux event that triggered the alert. These CDEvent payloads should have meaningful data from the source event. There will be an initial focus on HelmRelease and related events as the format within Flux for those events is much more consistent. |
There was a problem hiding this comment.
It would be easier to comment on the individual lines if the paragraph is wrapped, maybe at 80 or 120 characters, whatever you prefer.
These CDEvent payloads should have meaningful data from the source event.
"event source" may fit well here, instead of "source event", indicating that the event source, the controllers, are responsible for meaningful data.
There will be an initial focus on HelmRelease and related events as the format within Flux for those events is much more consistent.
I don't think this needs to be mentioned in proposal. The corresponding github issues can have such implementation details.
| mapping: | ||
| ReconciliationFinished: dev.cdevents.pipelinerun.finished | ||
| GitOperationFailed: dev.cdevents.incident.reported |
There was a problem hiding this comment.
I would like to discuss this further about adding a new mapping spec field for CDEvents specifically.
My initial understanding before this proposal was that the working of flux components are well defined which could be mapped to CDEvent event types in a deterministic way. Specific events from source-controller can be a kind of Source Code Control Event, events from kustomize-controller and helm-controller can be a kind of Continuous Deployment Event, etc.
I think the flux events can be categorized accordingly and statically configured to specific CDEvents event types. Any new event introduced in flux should also be categorized as a specific CDEvent. In notification-controller, the event handler can parse the events and forward them to message broker as per the fixed event categories.
I may be unaware of certain use cases of CDEvents that require such custom mapping, as I haven't used it myself.
There was a problem hiding this comment.
Hi sunny, apologies for the slow response, we have decided to in fact switch to an approach that focuses more on a mapping that is defined in the way you described rather than the custom mapping we had considered before. I will make the changes to the RFC proposal to reflect this.
Signed-off-by: adamkenihan <adam.kenihan@est.tech>
Signed-off-by: adamkenihan <adam.kenihan@est.tech>
Signed-off-by: adamkenihan <adam.kenihan@est.tech>
Signed-off-by: adamkenihan <adam.kenihan@est.tech>
adamkenihan
force-pushed
the
ProviderHelmRFC
branch
from
9e74282 to
bb8448b
Compare
Signed-off-by: adamkenihan <adam.kenihan@est.tech>
| |ReconciliationSucceeded | Environment.Modified | | ||
| |ReconciliationFailed | Incident.Detected | |
There was a problem hiding this comment.
In the latest HelmRelease v2 API, I can't find these being used. We had these in the old API.
| | Helm Event | CDEvent | | ||
| | ------------------------- | ------------------------------- | | ||
| | InstallSucceeded | Environment.Modified | | ||
| || Build.Finished (outcome not implemented yet) | | ||
| || PipeLineRun.Finished (with outcome success) | | ||
| | InstallFailed | Incident.Detected | | ||
| || Build.Finished (outcome not implemented yet) | | ||
| || PipeLineRun.Finished (with outcome failed) | | ||
| |UpgradeSucceeded | TaskRun.Finished (with outcome success) | | ||
| || Build.Finished (outcome not implemented yet) | | ||
| || PipeLineRun.Finished (with outcome success) | | ||
| |UpgradeFailed | TaskRun.Finished (with outcome failed) | | ||
| || Build.Finished (outcome not implemented yet) | | ||
| || PipeLineRun.Finished (with outcome failed) | | ||
| |TestSucceeded | TestCaseRun.finished (outcome pass) | | ||
| |TestFailed | TestCaseRun.finished (outcome fail) | | ||
| |RollbackSucceeded | TaskRun.Finished (with outcome success) | | ||
| || Build.Finished (outcome not implemented yet) | | ||
| || PipeLineRun.Finished (with outcome success) | | ||
| |RollbackFailed | TaskRun.Finished (with outcome failed) | | ||
| || Build.Finished (outcome not implemented yet) | | ||
| || PipeLineRun.Finished (with outcome failed) | | ||
| |UninstallSucceeded | Environment.Modified | | ||
| |UninstallFailed | Incident.Detected | | ||
| |ReconciliationSucceeded | Environment.Modified | |
There was a problem hiding this comment.
This mapping feels wrong to me, we should map the HelmRelease to CDEvent service for the deployed, upgraded, rolledback, removed predicates.
Co-authored-by: Sunny <github@darkowlzz.space> Signed-off-by: adamkenihan <adam.kenihan@est.tech>
Signed-off-by: adamkenihan <adam.kenihan@est.tech>
3 participants
This RFC proposes to add a
providerthat supports sendingCDEventsrelevant to the Flux events that trigger the alert. The initial focus of this RFC will be on implementing Helm Events.