Move Prometheus as an Add-On

[Pothulapati]

Fixes #3590, #3406
Implements first part of linkerd/rfc#16, #4375

This PR moves Prometheus as a add-on, thus making it optional but enabled by default. The main benefit of this work is to make linkerd-prometheus more configurable, and allow it to have its own life-cycle for upgrades, configuration, etc.

This work will be followed by documentation that help users configure existing Prometheus to work with Linkerd.

Changes Include:

• moving prometheus manifests into a separate chart at charts/add-ons/prometheus, and adding it as a dependency to linkerd2
• implement the addOn interface to support the same with CLI.
• include configuration in linkerd-config-addons

[alpeb]

Looking good 👍

Can I ask you to make a parent issue, besides the couple of issues already referenced, with a list of checkboxes for the specific things that will be addressed in separate PRs? So that we can keep track of other things like the health checks, the extra validations the helm chart will require (discussed in the RFC), documentation, test plan issue, etc. And then that issue can be the main one that the RFC will link to. Makes sense?

I'll be thoroughly testing this tomorrow 😉

[alpeb]

Why the removal of this?

[Pothulapati]

@alpeb As grafana is a separate add-on, and those fields are only propagated into their sub-chart. We can't get this value in Prometheus add-on.

I don't think prometheus would have any problem regarding the scrape-config even when there is no grafana, but I will test this out!

For other fields that would need to be accessed across multiple add-ons, we can use the global and have that in linkerd-config-addons!

[Pothulapati]

This PR also has the following additional changes:

• prometheusImage and prometheusLogLevel flags have been removed as when these flags are set we would essentially want to set prometheus. fields which in the default case are not present as those fields are commented out in values.yaml and defaults are in prometheus/values.yaml and would cause additional complexity to create an append. This is not worth in my opinion as users can now set these values in prometheus.image and promtehus.args.log.level through the addon-config file.

[Pothulapati]

@alpeb Though we moved this flag to be global like we should, but we don't actually have a way to use this directly in prometheus/values.yaml as we can't template values there. and putting that in the templates directly means that we don't have a way to merge them and appending might cause prometheus to complain just like it did with other values in configmap (I will try this out today)

[alpeb]

Thanks @Pothulapati for the extended explanations 🙂

According to my understanding on how this should work, I suggest we remove the commented properties in charts/linkerd2/values.yaml under the prometheus section, since that's not the place to override them (also avoiding the extra work for us to keep them in sync with charts/add-ons/prometheus/values.yaml). And instead put a comment like this one before that section:

# This add-on's default property values are declared in `charts/add-ons/prometheus/values.yaml`.
# If installing/upgrading with Helm, you can override them through the usual `--set` or `-f` flags.
# Do not override them in this file!
# If installing/upgrading with linkerd's CLI, use the `--addon-config` flag.

And a comment like this in the header of charts/add-ons/prometheus/values.yaml:

# These are the default properties for this add-on.
# If installing/upgrading with Helm, you can override them through the usual `--set` or `-f` flags.
# Do not override them in this file!
# If installing/upgrading with linkerd's CLI, use the `--addon-config` flag.

Does that makes sense?

[Pothulapati]

@alpeb I was thinking that with Helm, when users want to overwrite they would use linkerd2/values.yaml and not add-on specific values.yaml. As the configuration into linkerd-config-addons is taken from linkerd2//values.yaml

This way they still get

• Checks, etc that depend on the configuration present in linkerd-config-addons
• Upgrades, etc would still work as its still values.yaml and helm would re-use that and hence provide the same helm upgrade semantics with that of

[alpeb]

Ok cool. So the only thing we have to say is to not touch the charts/add-ons/prometheus/values.yaml file?

[Pothulapati]

@alpeb Yep, Updated /prometheus/values.yaml accordingly.

[alpeb]

@Pothulapati would you check the behavior of linkerd upgrade --addon-overwrite? I installed using a custom prom config passed with --addon-config but then linkerd upgrade --addon-overwrite isn't resetting to the default values...

[alpeb]

It seems I had done a linkerd upgrade --addon-overwrite while at the same time changing something inside the main values.yaml, so that's the change I was seeing in linkerd-config-addons. So it's working as expected 👍
I did a round of tests with CLI and Helm for installs and upgrade, and it all seems correct to me 👍

[alpeb]

The addon-related changes still look good to me 👍

#4724 addresses the linkerd check changes required when the addon is disabled. There are still lots of other places that fail without it (heartbeat, dashboard (when accessed, the controller pod dies), smi-metrics, CLI commands).

What do you think about addressing the non-prometheus scenario in all those places before tackling the customized prom url case and the "addonification" of the other components? That way we don't leave that failed case open in the upcoming edges for too long. Definitely in a separate PR, so we can merge this one and move on.