notification-controller icon indicating copy to clipboard operation
notification-controller copied to clipboard

Adding cluster name (or something else) to Github commit status context

Open gmaiztegi opened this issue 4 years ago • 6 comments

We run a few clusters with Flux and are moving towards Flux 2 now, and so far the experience is being awesome.

We have seen an issue in this notification controller, and it's that when having multiple clusters posting the commit status for the same resource names (such as kustomization/flux-system), the statuses get overwritten, as they use the <resource type>/<name> as context.

Therefore, it would be awesome if we could add some kind of prefix or suffix to this context in order to distinguish the status posted by the different clusters.

And once again, Flux 2 is great. Thanks for your hard work.

gmaiztegi avatar May 06 '21 08:05 gmaiztegi

I agree that we should have a second look at this. When we first implemented the commit status feature a lot was unknown. Currently I am solving this by adding the environment or cluster name to the Kustomization resource. This will cause the statuses to be unique between different clusters.

One solution may be to expose some sort of format configuration for the provider. A risk is that users would hit the character limit for a status message. Most of the providers have one but they are not documented so they are found by trial and error. Do you have any suggestion for how this configuration should be implemented in the provider or alert resource @gmaiztegi?

phillebaba avatar May 17 '21 20:05 phillebaba

I'm not sure on wether it should be on the provide or the alert. It seems that this is a Github specific issue regarding the commit status, although the question might be wether it might be useful to have different alerts with different identifiers, all of them pointing to the Github provider. It's not our use case, nor I can think of any right now, but this seems more flexible.

gmaiztegi avatar May 25 '21 14:05 gmaiztegi

On a related note, adding some cluster metadata to a notification would be extremely valuable. I am also running flux on 20+ clusters now, and we are unable to use the notification controller effectively since we cannot tell where the notification originated without having a separate chat room per notification-controller instance (we are going to have 50 or more clusters so not very reasonable).

gdoctor avatar May 27 '21 20:05 gdoctor

@gdoctor you can add the cluster name to the alert message with spec.summary, docs here: https://fluxcd.io/docs/components/notification/alert/

stefanprodan avatar May 27 '21 20:05 stefanprodan

Hi, I've also run into the issue described here, and adding the "summary" to an "alert" unfortunately does not seem to do anything in the case of github status messages; github status seem to only contain the kind/name of the referenced object and the event reason as defined in formatNameAndDescription here: https://github.com/fluxcd/notification-controller/blob/c4377ceb51f0ed969aa3fac1c151189bfa18d7fc/internal/notifier/util.go#L47

Just mentioning this here in case anyone else stumbles on the same thing... if I understand correctly, there's already other solutions being considered, like the one described here: https://github.com/fluxcd/flux2/discussions/1529?

bforchhammer avatar Sep 23 '21 13:09 bforchhammer

@stefanprodan Is there any progress regarding the dedicated CommitStatus CRD? This would extend the room of enhancements by far (e.g. https://github.com/fluxcd/notification-controller/issues/225)

jrauschenbusch avatar Apr 01 '22 08:04 jrauschenbusch