Add event metadata field to Alert spec #519
Conversation
|
@stefanprodan Here's the PR for what we just chatted about. |
matheuscscp
force-pushed
the
spec-metadata
branch
3 times, most recently
from
e4e59da to
6ff10d4
Compare
Signed-off-by: Matheus Pimenta <[email protected]>
matheuscscp
force-pushed
the
spec-metadata
branch
from
6ff10d4 to
e9d1fb3
Compare
stefanprodan
added
enhancement
| // controller. Metadata fields added by the controller have priority over the fields | ||
| // added here, and the fields added here have priority over fields originally present | ||
| // in the event. |
There was a problem hiding this comment.
I need some help in clarifying my confusion about this statement.
Metadata fields added by the controller
Who is the controller being referred here? Is the controller notification-controller itself or the other controllers that sends event?
Giving this a try, by setting a metadata "revision": "20" in Alert .spec.eventMetadata and sending an event to the event server with metadata "source.toolkit.fluxcd.io/revision": "10", the outgoing notification seems to have "revision":"20".
So, .spec.eventMetadata has priority over everything else. Which leaves me confused about who is the controller mentioned here that has the highest priority?
There was a problem hiding this comment.
After going through the PR, I have the same confusion.
There was a problem hiding this comment.
you guys are right, i'm walking right now and will reply in a few minutes 👌
There was a problem hiding this comment.
the controller mentioned here which has priority over everything else is notification-controller. when the event arrives at notification-controller, first the metadata is cleaned up, then the eventMetadata field from the Alert spec is added, and then finally the summary field from the Alert spec is added (the only one at the moment). the priority is given by this order.
i will file a PR to improve this explanation
There was a problem hiding this comment.
I filed #528 for improving this doc. I hope it's clear now 👌 thanks for catching that! it was indeed not very well worded 😅
Fixes #511