viz: make prom checks dynamic by using annotations#5680
Conversation
This commit adds new annotations that are added when a external prometheus or grafana is used. Based on these annotations, The CLI can get to know if a external instance is being used and if the annotation is absent, that the default instance is present. This logic can be used by the CLi to perform checks, etc This also cleans up some `grafanaUrl` stuff from the core control-plane chart. Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
This updates the viz Checks to skip some checkers if default prometheus or grafana instances are absent. Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
Pothulapati
changed the title
Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
| {{- if .Values.prometheusUrl }} | ||
| viz.linkerd.io/external-prometheus: {{.Values.prometheusUrl}} | ||
| {{- end }} | ||
| {{- if .Values.grafanaUrl }} |
There was a problem hiding this comment.
This logic i.e assuming linkerd-grafana is presently based on the absence of this annotation does not really work with grafana because both being absent case is allowed here. Should we make disabled an explicit annotation then?
dadjeibaah
left a comment
dadjeibaah
left a comment
There was a problem hiding this comment.
Changes look good. Tested with an external prometheus and works great. I left a couple of comments.
viz/pkg/healthcheck/healthcheck.go
Outdated
| Warning(). | ||
| WithCheck(func(ctx context.Context) error { | ||
| // TODO: Skip if prometheus is disabled | ||
| if _, ok := hc.vizNamespace.Annotations[labels.VizExternalPrometheus]; ok { |
There was a problem hiding this comment.
For my own understanding, is this a temporary fix. I'd imagine that we would at least want to confirm that the external URL provided is reachable i.e. returning a valid Prometheus response.
There was a problem hiding this comment.
We do validate if the URL is working or not but that test is part of viz extension self-check where the metrics-api checks if the Prometheus actually works and fails if not whose output got changed a bit slightly in #5665
Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
dadjeibaah
left a comment
dadjeibaah
left a comment
There was a problem hiding this comment.
Thanks for making that change! LGTM! 📦 🚢
viz/pkg/labels/labels.go
Outdated
| VizExternalPrometheus = VizAnnotationsPrefix + "/external-prometheus" | ||
|
|
||
| // VizExternalGrafana is only set on the namespace by the install | ||
| // when az external Grafana is being used. |
There was a problem hiding this comment.
| // when az external Grafana is being used. | |
| // when an external Grafana is being used. |
Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
|
After further offline discussions, We settled to not include grafana checks as they are not that useful and add unnecessary complexity especially that there are no other components being dependent on grafana, like they do for prometheus. |
Pothulapati
changed the title
Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com>
kleimkuhler
left a comment
kleimkuhler
left a comment
There was a problem hiding this comment.
I test this with an external Prometheus and works as expected. Left a nit about getting annotation values, but otherwise looks good.
Fixes linkerd#5652 This PR adds new annotation that is added when a external Prometheus is used. Based on that annotations, The CLI can get to know if an external instance is being used and if the annotation is absent, that the the default instance is present. This updates the viz Checks to skip some checkers if the default Prometheus instances are absent. This PR also removes the grafana checks as they are not useful and add unnecessary complexity. This also cleans up some `grafanaUrl` stuff from the core control-plane chart. Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com> Signed-off-by: Jijeesh <jijeesh.ka@gmail.com>
Fixes linkerd#5652 This PR adds new annotation that is added when a external Prometheus is used. Based on that annotations, The CLI can get to know if an external instance is being used and if the annotation is absent, that the the default instance is present. This updates the viz Checks to skip some checkers if the default Prometheus instances are absent. This PR also removes the grafana checks as they are not useful and add unnecessary complexity. This also cleans up some `grafanaUrl` stuff from the core control-plane chart. Signed-off-by: Tarun Pothulapati <tarunpothulapati@outlook.com> Signed-off-by: Jijeesh <jijeesh.ka@gmail.com>
4 participants
Fixes #5652
This PR adds new annotation that is added when a
external Prometheus is used. Based on that
annotations, The CLI can get to know if an external instance
is being used and if the annotation is absent, that the
the default instance is present.
This updates the viz Checks to skip some checkers if the default
Prometheus instances are absent.
This PR also removes the grafana checks as they are not useful
and add unnecessary complexity.
This also cleans up some
grafanaUrlstuff from the corecontrol-plane chart.
Signed-off-by: Tarun Pothulapati tarunpothulapati@outlook.com