Move Prometheus as an Add-On by Pothulapati · Pull Request #4362 · linkerd/linkerd2

Pothulapati · 2020-05-08T14:12:11Z

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

Signed-off-by: Tarun Pothulapati <[email protected]>

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 · 2020-05-11T22:42:25Z

charts/add-ons/prometheus/templates/prometheus.yaml

@@ -34,7 +29,6 @@ data:
      static_configs:
      - targets: ['localhost:9090']

-    {{ if .Values.grafana.enabled -}} 


Why the removal of this?

@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!

charts/add-ons/prometheus/templates/prometheus.yaml

cli/cmd/install_addon_test.go

Signed-off-by: Tarun Pothulapati <[email protected]>

Pothulapati · 2020-06-24T13:21:05Z

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 · 2020-06-24T13:25:11Z

charts/linkerd2/values.yaml


 # controller configuration
 controllerImage: gcr.io/linkerd-io/controller
-controllerLogLevel: &controller_log_level info


@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)

Signed-off-by: Tarun Pothulapati <[email protected]>

alpeb · 2020-06-25T22:38:57Z

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 · 2020-06-26T10:05:40Z

@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 · 2020-06-26T13:39:15Z

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

Signed-off-by: Tarun Pothulapati <[email protected]>

Pothulapati · 2020-06-29T12:02:09Z

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

Signed-off-by: Tarun Pothulapati <[email protected]>

alpeb · 2020-06-30T15:40:48Z

@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 👍

Signed-off-by: Tarun Pothulapati <[email protected]>

files accordingly to include more Signed-off-by: Tarun Pothulapati <[email protected]>

Signed-off-by: Tarun Pothulapati <[email protected]>

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.

Signed-off-by: Tarun Pothulapati <[email protected]>

This moves Prometheus as a add-on, thus making it optional but enabled by default. The also 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` **User Facing Changes:** The default install experience does not change much but for users who have already configured Prometheus differently, would need to apply the same using the new configuration fields present in chart README Signed-off-by: Eric Solomon <[email protected]>

Pothulapati added 7 commits May 6, 2020 12:48

move prometheus templates as a add-on

913ca77

Signed-off-by: Tarun Pothulapati <[email protected]>

remove prom flags and fix fiields

0d675af

Signed-off-by: Tarun Pothulapati <[email protected]>

prometheus charts linting

751d236

Signed-off-by: Tarun Pothulapati <[email protected]>

use common fix

04bbcae

Signed-off-by: Tarun Pothulapati <[email protected]>

add prometheus sub-chart dep to linkerd2

6947cd0

Signed-off-by: Tarun Pothulapati <[email protected]>

update golden files

48cc206

Signed-off-by: Tarun Pothulapati <[email protected]>

update install metaValues test

79e15b3

Signed-off-by: Tarun Pothulapati <[email protected]>

Pothulapati requested review from adleong, alpeb and zaharidichev as code owners May 8, 2020 14:12

Pothulapati added 2 commits May 8, 2020 20:01

Merge branch 'master' into tarun/prom-addon

3fe342e

Signed-off-by: Tarun Pothulapati <[email protected]>

fix template correctly

be3d3ea

Signed-off-by: Tarun Pothulapati <[email protected]>

Pothulapati force-pushed the tarun/prom-addon branch from 7a031d6 to 443f7fa Compare May 10, 2020 16:43

remove linkerd-prometheus healthchecks

ab854df

Signed-off-by: Tarun Pothulapati <[email protected]>

Pothulapati force-pushed the tarun/prom-addon branch from 443f7fa to ab854df Compare May 10, 2020 17:07

Pothulapati added 3 commits May 11, 2020 16:07

add more fields for configuration

d54ad31

Signed-off-by: Tarun Pothulapati <[email protected]>

add extra prometheus config tests

92a72cb

Signed-off-by: Tarun Pothulapati <[email protected]>

fix prom example config

f8152de

Signed-off-by: Tarun Pothulapati <[email protected]>

Pothulapati requested a review from kleimkuhler as a code owner May 11, 2020 19:35

Pothulapati force-pushed the tarun/prom-addon branch from 9d348b9 to f8152de Compare May 11, 2020 19:36

Pothulapati added 3 commits May 12, 2020 01:14

Merge branch 'master' into tarun/prom-addon

2ee0ead

Signed-off-by: Tarun Pothulapati <[email protected]>

update golden files

4fb9c38

Signed-off-by: Tarun Pothulapati <[email protected]>

remove grafana.enabled check in prometheus

d801768

Signed-off-by: Tarun Pothulapati <[email protected]>

grampelberg assigned alpeb May 11, 2020

alpeb reviewed May 11, 2020

View reviewed changes

Pothulapati added 5 commits May 12, 2020 19:29

rename to prometheus test

bfd5b24

Signed-off-by: Tarun Pothulapati <[email protected]>

full config with examples

ec245e1

Signed-off-by: Tarun Pothulapati <[email protected]>

add args documentation in charts/README.md

e3d58c6

Signed-off-by: Tarun Pothulapati <[email protected]>

fix name in docs

e5dbbd4

Signed-off-by: Tarun Pothulapati <[email protected]>

fix ruleConfigMapMounts

c8ba283

Signed-off-by: Tarun Pothulapati <[email protected]>

Pothulapati commented Jun 24, 2020

View reviewed changes

olix0r changed the base branch from master to main June 24, 2020 18:39

Pothulapati added 2 commits June 25, 2020 05:46

Merge branch 'main' into tarun/prom-addon

c4792cd

Signed-off-by: Tarun Pothulapati <[email protected]>

update prometheus add-on to include proxy resources along with tests

f663373

Signed-off-by: Tarun Pothulapati <[email protected]>

add instructions in prometheus/values.yaml

725dc2e

Signed-off-by: Tarun Pothulapati <[email protected]>

Merge branch 'main' into tarun/prom-addon

1952bbf

Signed-off-by: Tarun Pothulapati <[email protected]>

alpeb approved these changes Jun 30, 2020

View reviewed changes

surajssd mentioned this pull request Jul 2, 2020

component: Add linkerd kinvolk/lokomotive#690

Merged

5 tasks

Pothulapati added 3 commits July 4, 2020 19:29

Merge branch 'main' into tarun/prom-addon

b6c3aa9

Signed-off-by: Tarun Pothulapati <[email protected]>

update golden files

a1650ee

Signed-off-by: Tarun Pothulapati <[email protected]>

make prometheus urls optional and based on add-on

97ecefa

Signed-off-by: Tarun Pothulapati <[email protected]>

Pothulapati mentioned this pull request Jul 7, 2020

Remove/Relax prometheus related checks #4724

Merged

Pothulapati added 6 commits July 7, 2020 16:00

Merge branch 'main' into tarun/prom-addon

2c65d13

update golden files

05057cd

Signed-off-by: Tarun Pothulapati <[email protected]>

Merge branch 'main' into tarun/prom-addon

5ad5cad

Signed-off-by: Tarun Pothulapati <[email protected]>

Merge branch 'main' into tarun/prom-addon

81030d1

updates golden files now that add-on level values

2ab6472

files accordingly to include more Signed-off-by: Tarun Pothulapati <[email protected]>

update prom template to not have default value

3769341

Signed-off-by: Tarun Pothulapati <[email protected]>

alpeb approved these changes Jul 8, 2020

View reviewed changes

Pothulapati added 2 commits July 9, 2020 22:57

Merge branch 'main' into tarun/prom-addon

4232b61

Signed-off-by: Tarun Pothulapati <[email protected]>

update golden files

af63998

Signed-off-by: Tarun Pothulapati <[email protected]>

Pothulapati merged commit 2a099cb into main Jul 9, 2020

Pothulapati deleted the tarun/prom-addon branch July 9, 2020 17:59

Conversation

Pothulapati commented May 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alpeb left a comment

Choose a reason for hiding this comment

Uh oh!

alpeb May 11, 2020

Choose a reason for hiding this comment

Uh oh!

Pothulapati May 12, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Pothulapati commented Jun 24, 2020

Uh oh!

Pothulapati Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

alpeb commented Jun 25, 2020

Uh oh!

Pothulapati commented Jun 26, 2020

Uh oh!

alpeb commented Jun 26, 2020

Uh oh!

Pothulapati commented Jun 29, 2020

Uh oh!

alpeb commented Jun 30, 2020

Uh oh!

alpeb left a comment

Choose a reason for hiding this comment

Uh oh!

alpeb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Pothulapati commented May 8, 2020 •

edited

Loading

alpeb left a comment •

edited

Loading