Helm-charts: [prometheus] Double merge of global configs with no clear override method

Created on 11 Nov 2020  路  4Comments  路  Source: prometheus-community/helm-charts

Describe the bug
I don't see a good way to set external_labels. This is specifically for Thanos configuration. The most likely route to success was passing ServerFiles.prometheus.yml, but these lines of code prevents that approach from working:
https://github.com/prometheus-community/helm-charts/blob/d79879d5001144d847c914da869c887e88c3960f/charts/prometheus/templates/server/cm.yaml#L14-L16

Note that this issue is very similar to https://github.com/prometheus-community/helm-charts/issues/230, but I am specifically trying to override global values (external_labels subfield). I also tried passing those values into extraScrapeConfigs as suggested in 230, although naturally that didn't work since that doesn't really make sense in context. But since it was doing a suffix append to the prometheus.yml file content I gave it a shot.

Version of Helm and Kubernetes:
Helm Version:
I believe v3. There are several abstractions between me and helm so I'm not 100% sure, but I can get this info if it's important. However, this is mostly an issue with the chart and I think the other info here should be adequate.

Kubernetes Version:

Client Version: version.Info{Major:"1", Minor:"16", GitVersion:"v1.16.0", GitCommit:"2bd9643cee5b3b3a5ecbd3af49d09018f0773c77", GitTreeState:"clean", BuildDate:"2019-09-18T14:36:53Z", GoVersion:"go1.12.9", Compiler:"gc", Platform:"windows/amd64"}
Server Version: version.Info{Major:"1", Minor:"17", GitVersion:"v1.17.11", GitCommit:"3a3612132641768edd7f7e73d07772225817f630", GitTreeState:"clean", BuildDate:"2020-09-02T06:46:33Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"linux/amd64"}

Which chart:
https://github.com/prometheus-community/helm-charts/tree/main/charts/prometheus

Which version of the chart:
prometheus-11.15.0

What happened:
I got a double merged / concatenated global config similar to issue 230, which I don't believe is valid:

global:
  evaluation_interval: 1m
  scrape_interval: 1m
  scrape_timeout: 10s
global:
  external_labels: 
    environment: dev
    region: cae
scrape_configs:

... <rest of config>

What you expected to happen:
Either merge the 'global' yaml block into one set with corresponding attributes when bringing in ServerFiles.prometheus.yml, or overwrite the entire prometheus.yml config file. Awkward concatenation of server.global.scrape_interval, server.global.scrape_timeout, server.global.evaluation_interval and my global overrides via serverFiles is awkward and generates illegal syntax when processed by the helm template.

How to reproduce it (as minimally and precisely as possible):
Pass in global settings such as external_labels via serverFiles.prometheus.yml, other than those fields explicitly defined in the helm chart

bug

All 4 comments

Also, I agree with this statement from issue #230: "If not, parser need to be fixed to apply only file if no parameters set."

I think it's a bit presumptuous to try to concatenate the configuration together without giving the user the option to override. Configuration concatenation in my view is hazarding a best guess as to what the user might want to do. Granted I understand in some ways that's the very nature of helm. Still, the conditional prometheus.yml key check, at least from my perspective, is asking for trouble. I'm not sure I would even bother with this notion of some global keys, some serverFile config, and stapling on extraScrapeConfigs at the end. Why not just have the user pass the entire file? Perhaps I could do something similar with an 'server.extraConfigmapMounts' but my expertise is wearing thin at that point.

Overall, I'm hopeful I'm missing some larger piece of context here. Perhaps server.podLabels would be of use? I strongly suspect that's a different field though for kubernetes use and not configuration of prometheus.

I created a PR and tested the fix here:
https://github.com/prometheus-community/helm-charts/pull/342

This is working in my limited example. The values showed up in the config as expected and Thanos picked them up. However, I'll relent that there may be a better way of doing this overall or something I have missed, so only approve accordingly. Also, this is my first PR to this repo so if I missed the mark anywhere my apologies.

I could use thanos with

  server:  
    global:
      external_labels:
        label: added

Thanks @SCLogo, I think I conflated some mistakes I made earlier with that pattern not working. Closing this issue and the PR as this works in the default chart. Hopefully this is useful to someone at any rate.

Was this page helpful?
0 / 5 - 0 ratings