Add additional context to HelmRelease events

[stefansedich]

Today the events for HelmReleases do not really contain sufficient information for our monitoring needs as the event only contains the chart version, this has come up in a number of issues over the past few years and users have asked for a way to expand the metadata that these events contain.

fluxcd/helm-controller#256
fluxcd/helm-controller#412
fluxcd/helm-controller#798

And this issue has been tackled by a number of abandoned PRs.

fluxcd/helm-controller#514
fluxcd/helm-controller#682

In our case even if we were to push appVersion into the event due to the way we use a shared chart for multiple application releases we really need the value of image.tag in the alert to make it useful for our needs.

For tools like DataDog we are able to configure them to extract release values as tags and add them to events, for example the following configuration for the Helm check will add the image.tag release value as a tag called helm_image_tag on events.

helmCheck:
  ...
  valuesAsTags:
    image.tag: helm_image_tag

Below are a few initial ideas I had on how to solve this problem based on our needs and previous attempts to solve this.

1. Add support for configuring the helm-controller to add additional metadata to all events based on the DataDog example above (7/10 would work but a little 🤮)
2. Provide the ability to template release values directly to alert metadata 9/10 would be nice but would require the helm-controller to add all release values to the event which I am not sure of feasibility at this stage)
3. Add the release values JSON diff to the event and then render this in the alert (8/10 would be a good start for our need :hattip: to @stefanprodan for this idea)

Would love to hear additional thoughts/ideas from maintainers so that I can work on a PR to get this issue closed off and improve the HelmRelease alert story.

[ebachle]

I don't have answers on this yet, but I'm working towards getting some at the moment.

I had set up the fluxcd cluster check integration a few days ago, but we were still running v2.1.x and then realized upon updating to v2.2.x that the gotk_reconcile_condition that I was planning on using for monitoring the HelmRelease object was deprecated and removed. So events might be one way I go at this. For the moment, I'm probably going to attempt to get metrics working again with kube-state-metrics and seeing if I can use that to analyze the state of my cluster.

One reason I'm leaning in that direction as opposed to events currently is that on periodic refresh, I've noticed that I'd get alerted more heavily for flakey communication with HelmRepositories, GitRepositories, etc. That's probably just a consequence of the internet being a series of tubes, but I can probably do more to smooth that long-term using metrics based alerting.

But overall at this point, I just wanted to say that I hear you on the desire for more observability into the HelmRelease lifecycle. I'm going to work on it and report back what I find works for me.

[stefanprodan]

Flakey communication with external sources are not reflected in events emitted by helm-controller for HelmRelease upgrade operations. But a flakey internet connection can certainly result in events not showing up on Slack or any of the Flux alert providers that are hosted outside the cluster.

[ebachle]

Gotcha, with v2.1 I was seeing it emerge as HelmChart not ready, but it sounds like the v2.2 rewrite takes care of that? Either that or the events emitted are just simply not affected by that?

[stefanprodan]

HelmChart not ready error events will not contain any chart metadata, so they are not relevant to the current discussion. If you don't care to know when charts fail to download you can drop these events. See the docs here: https://fluxcd.io/flux/components/notification/alerts/#event-exclusion

[stefanprodan]

If the Flux metrics exposed by kube-state-metrics are not sufficient, you could use https://github.com/sstarcher/helm-exporter

[stefanprodan]

[proposal] Diff reporting for user-defined values

When helm-controller detects a change in a HelmRelease object, it could calculate a diff between the values stored in the Helm storage and .spec.values.

The diff output must be filtered by jsondiff.DiffTypeUpdate to include only the values fields specified in .spec.values that differ from the Helm storage, thus avoiding leaking sensitive informations passed with .spec.valuesFrom.

The resulting diff report would be appended to the event message issued for Helm upgrades. For example, when image automation patches an image the output would be:

Helm upgrade succeeded for release default/podinfo.v2 with chart podinfo@6.6.1+0cc9a8446c95

Release values diff:

.spec.values.image.tag
-6.6.0
+6.6.1

.spec.values.image.repository
-ghcr.io/stefanprodan/podinfo
+docker.io/stefanprodan/podinfo

This feature would be opt-in and enabled with a feature gate e.g. --feature-gates=EmitValuesDiffEvents=true .

Important

This proposal is for reporting the diff between user-defined values specified in the HelmRelease object. Changes in the default values embedded in the chart are out of scope.

Drawbacks

• The diff report includes only changes in .spec.values to avoid leaking secrets passed as values with .spec.valuesFrom. We could include values mapped from ConfigMaps and exclude only Secrets, but then the diff could be huge.
• The diff could leak sensitive informations even if we only include .spec.values, for example the AWS account ID that's part of the image.repository string.

[stefanprodan]

Kubernetes events are stored in etcd

[gecube]

@stefanprodan thanks, I meant "events" not in terms of k8s event, but like emitting something via webhook or similar approach to the external system. Anyway, does not matter.

[stefanprodan]

Flux events are Kubernetes events that happen to also be forwarded to external systems if you configure an alert.

[stefanprodan]

UX wise for my team the diff is not going to be as simple to grok as a single "version" metadata field

For this I'm considering adding the appVersion from Chart.yaml to the helm-controller events. But this is a different proposal unrelated to the current one.

[stefansedich]

@stefanprodan can we perhaps provide a way to overrideappVersion with image.tag?

Couple reasons here:

1. it fulfills our needs where we use a shared chart that does not have appVersion set.
2. I see many charts in the wild that we are running today within our clusters that do not even set an appVersion, as mentioned we have solved this for the DataDog Helm check by mapping values.image.tag to an helm_image_tag tag within DataDog which covers the cases where appVersion is never set.

[stefanprodan]

In Flux v2.3, the chart app version was added to the .status.history snapshot and is now set as the app-version annotation to all the issued events. Also the app version is exposed under the chart_app_version label on the gotk_resource_info metric.

xref: fluxcd/helm-controller#968

[benoittgt]

Following this discussion. We would love to have more info about the source trigger of the reconciliation. For example a git sha when
it's because of a git change. Something similar to the annotations added. When we can get it, display it.

We have many release with Flux and sometimes it's hard to know the origin. Before Flux we were tracking who trigger the helm release command, and we had custom reporting tool to inform about the progress of deployment. This is sadly something we are missing with Flux.

Meanwhile, thanks a lot for the annotations @stefanprodan . 🙌