Kube-prometheus: Errors using `kube-prometheus-thanos-sidecar.libsonnet`

[sdurrheimer]

What happened?

Hello,

I was looking to update my setup with changes from master. I updated my jb dependencies and rebuild my setup, but now I'm encountering errors with the thanos-sidecar mixin.

The usage of (import 'kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet') + seems to be broken since #815 and #816.

Did you expect to see some different?

I expect the kube-prometheus-thanos-sidebar.libsonnet mixin import to work the same way as before without errors.

How to reproduce it (as minimally and precisely as possible):

With the default configuration from the README + the thanos-sidecar mixin un-commented:

local kp =
  (import 'kube-prometheus/kube-prometheus.libsonnet') +
  // Uncomment the following imports to enable its patches
  // (import 'kube-prometheus/kube-prometheus-anti-affinity.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-managed-cluster.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-node-ports.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-static-etcd.libsonnet') +
  (import 'kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-custom-metrics.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-external-metrics.libsonnet') +
  {
    _config+:: {
      namespace: 'monitoring',
    },
  };

{ ['setup/0namespace-' + name]: kp.kubePrometheus[name] for name in std.objectFields(kp.kubePrometheus) } +
{
  ['setup/prometheus-operator-' + name]: kp.prometheusOperator[name]
  for name in std.filter((function(name) name != 'serviceMonitor'), std.objectFields(kp.prometheusOperator))
} +
// serviceMonitor is separated so that it can be created after the CRDs are ready
{ 'prometheus-operator-serviceMonitor': kp.prometheusOperator.serviceMonitor } +
{ ['node-exporter-' + name]: kp.nodeExporter[name] for name in std.objectFields(kp.nodeExporter) } +
{ ['kube-state-metrics-' + name]: kp.kubeStateMetrics[name] for name in std.objectFields(kp.kubeStateMetrics) } +
{ ['alertmanager-' + name]: kp.alertmanager[name] for name in std.objectFields(kp.alertmanager) } +
{ ['prometheus-' + name]: kp.prometheus[name] for name in std.objectFields(kp.prometheus) } +
{ ['prometheus-adapter-' + name]: kp.prometheusAdapter[name] for name in std.objectFields(kp.prometheusAdapter) } +
{ ['grafana-' + name]: kp.grafana[name] for name in std.objectFields(kp.grafana) }

I get the following error:

RUNTIME ERROR: must provide selector for Thanos Sidecar alerts
    vendor/github.com/thanos-io/thanos/mixin/alerts/sidecar.libsonnet:4:15-70   object <obj>
    std.jsonnet:754:15-21   thunk <val>
    std.jsonnet:761:27-30   thunk <val>
    std.jsonnet:585:22-25   thunk <a>
    std.jsonnet:36:17   
    std.jsonnet:36:8-19 thunk <a>
    std.jsonnet:36:8-31 function <anonymous>
    std.jsonnet:36:8-31 function <anonymous>
    std.jsonnet:585:9-26    function <format_code>
    std.jsonnet:761:15-50   thunk <s>
    ...
    std.jsonnet:772:7-43    function <anonymous>
    std.jsonnet:237:7-23    function <anonymous>
    vendor/github.com/thanos-io/thanos/mixin/alerts/sidecar.libsonnet:(17:19)-(19:33)   object <rule>
    vendor/coredns-mixin/alerts/../lib/utils.libsonnet:6:11-25  thunk <array_element>
    vendor/coredns-mixin/alerts/../lib/utils.libsonnet:(5:16)-(8:10)    object <anonymous>
    vendor/kube-prometheus/kube-prometheus-managed-cluster.libsonnet:20:11  thunk <array_element>
    vendor/kube-prometheus/./prometheus/prometheus.libsonnet:67:17-41   object <anonymous>
    vendor/kube-prometheus/./prometheus/prometheus.libsonnet:(66:13)-(68:8) object <anonymous>
    kube-prometheus.jsonnet:502:1-92    object <anonymous>
    During manifestation

This seems to be linked to PR #816, importing sidecars alerts but without the selector config required by the thanos-mixin.

By adding the thanos-mixin/config.libsonnet file for debugging purpose as the following:

local kp =
  (import 'kube-prometheus/kube-prometheus.libsonnet') +
  // Uncomment the following imports to enable its patches
  // (import 'kube-prometheus/kube-prometheus-anti-affinity.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-managed-cluster.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-node-ports.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-static-etcd.libsonnet') +
  (import 'kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-custom-metrics.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-external-metrics.libsonnet') +
  (import 'github.com/thanos-io/thanos/mixin/config.libsonnet') +
  {
    _config+:: {
      namespace: 'monitoring',
    },
  };

{ ['setup/0namespace-' + name]: kp.kubePrometheus[name] for name in std.objectFields(kp.kubePrometheus) } +
{
  ['setup/prometheus-operator-' + name]: kp.prometheusOperator[name]
  for name in std.filter((function(name) name != 'serviceMonitor'), std.objectFields(kp.prometheusOperator))
} +
// serviceMonitor is separated so that it can be created after the CRDs are ready
{ 'prometheus-operator-serviceMonitor': kp.prometheusOperator.serviceMonitor } +
{ ['node-exporter-' + name]: kp.nodeExporter[name] for name in std.objectFields(kp.nodeExporter) } +
{ ['kube-state-metrics-' + name]: kp.kubeStateMetrics[name] for name in std.objectFields(kp.kubeStateMetrics) } +
{ ['alertmanager-' + name]: kp.alertmanager[name] for name in std.objectFields(kp.alertmanager) } +
{ ['prometheus-' + name]: kp.prometheus[name] for name in std.objectFields(kp.prometheus) } +
{ ['prometheus-adapter-' + name]: kp.prometheusAdapter[name] for name in std.objectFields(kp.prometheusAdapter) } +
{ ['grafana-' + name]: kp.grafana[name] for name in std.objectFields(kp.grafana) }

Now the error disappears.

In my full case, I then get further errors about p.name and p.namespace usage non-existing:

RUNTIME ERROR: field does not exist: namespace
    vendor/kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet:57:22-33    object <anonymous>
    vendor/kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet:(55:19)-(61:10) object <anonymous>
    kube-prometheus.jsonnet:500:1-92    object <anonymous>
    During manifestation

RUNTIME ERROR: field does not exist: name
    vendor/kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet:67:27-33    object <anonymous>
    vendor/kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet:(66:26)-(69:14) object <anonymous>
    vendor/kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet:(65:21)-(70:12) object <anonymous>
    vendor/kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet:(62:15)-(77:10) object <anonymous>
    kube-prometheus.jsonnet:500:1-92    object <anonymous>
    During manifestation

I'm still trying to figure out (starting from the basic README example) what is causing those does not exists errors I'm encountering will my full example (which is a bit too long to share here).
I suspect the combination of two optional mixins that are conflicting (currently trying the find the faulty combination).

[sdurrheimer]

Okay found the faulty combination, kube-prometheus-managed-cluster.libsonnet and kube-prometheus-thanos-sidecar.libsonnet doesn't seem to like each other:

local kp =
  (import 'kube-prometheus/kube-prometheus.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-all-namespaces.libsonnet') +
  // Uncomment the following imports to enable its patches
  // (import 'kube-prometheus/kube-prometheus-anti-affinity.libsonnet') +
  (import 'kube-prometheus/kube-prometheus-managed-cluster.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-node-ports.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-static-etcd.libsonnet') +
  (import 'kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-custom-metrics.libsonnet') +
  // (import 'kube-prometheus/kube-prometheus-external-metrics.libsonnet') +
  (import 'github.com/thanos-io/thanos/mixin/config.libsonnet') +
  {
    _config+:: {
      namespace: 'monitoring',
    },
  };

{ ['setup/0namespace-' + name]: kp.kubePrometheus[name] for name in std.objectFields(kp.kubePrometheus) } +
{
  ['setup/prometheus-operator-' + name]: kp.prometheusOperator[name]
  for name in std.filter((function(name) name != 'serviceMonitor'), std.objectFields(kp.prometheusOperator))
} +
// serviceMonitor is separated so that it can be created after the CRDs are ready
{ 'prometheus-operator-serviceMonitor': kp.prometheusOperator.serviceMonitor } +
{ ['node-exporter-' + name]: kp.nodeExporter[name] for name in std.objectFields(kp.nodeExporter) } +
{ ['kube-state-metrics-' + name]: kp.kubeStateMetrics[name] for name in std.objectFields(kp.kubeStateMetrics) } +
{ ['alertmanager-' + name]: kp.alertmanager[name] for name in std.objectFields(kp.alertmanager) } +
{ ['prometheus-' + name]: kp.prometheus[name] for name in std.objectFields(kp.prometheus) } +
{ ['prometheus-adapter-' + name]: kp.prometheusAdapter[name] for name in std.objectFields(kp.prometheusAdapter) } +
{ ['grafana-' + name]: kp.grafana[name] for name in std.objectFields(kp.grafana) }

EDIT: That being said, moving the kube-prometheus-thanos-sidecar.libsonnet before kube-prometheus-managed-cluster.libsonnet doesn't provoque the error.

[brancz]

That being said, moving the kube-prometheus-thanos-sidecar.libsonnet before kube-prometheus-managed-cluster.libsonnet doesn't provoque the error.

This should be the workaround for now, but this shouldn't stay this way.

@simonpasquier @lilic @paulfantom I think this is going to need a refactoring so these ordering issues don't occur. We've had it other places a couple of times now.

[sdurrheimer]

The github.com/thanos-io/thanos/mixin/alerts/sidecar.libsonnet import is also a bit annoying actually.

1. The base config for it to work is missing as shown in the first error.
2. It prevents the user from including other prometheusAlerts and prometheusRules from the thanos-mixin/mixin.libsonnet entirely as Prometheus will complain that the thanos-sidecar.rules group name is declared twice:

... groupname: \"thanos-sidecar.rules\" is repeated in the same file"

You will have to include individual groups or filter out the one for thanos-sidecar, which is not very user-friendly.

[paulfantom]

It prevents the user from including other prometheusAlerts and prometheusRules from the thanos-mixin/mixin.libsonnet entirely as Prometheus will complain that the thanos-sidecar.rules group name is declared twice

This should be fixed upstream in https://github.com/thanos-io/thanos/pull/3542

[sdurrheimer]

It prevents the user from including other prometheusAlerts and prometheusRules from the thanos-mixin/mixin.libsonnet entirely as Prometheus will complain that the thanos-sidecar.rules group name is declared twice

This should be fixed upstream in thanos-io/thanos#3542

Not really, the PR you mentioned will fix duplicated groupname between thanos rules and thanos alerts mixins (which I reported).

What I was referring to is that, by importing github.com/thanos-io/thanos/mixin/alerts/sidecar.libsonnet natively in kube-prometheus, a user won't be able to import easily the other rules and alerts for other thanos components, like the following:

local thanosMixin = import 'thanos-mixin/mixin.libsonnet';

local kp =
  (import 'kube-prometheus/kube-prometheus.libsonnet') +
  (import 'kube-prometheus/kube-prometheus-thanos-sidecar.libsonnet') +
  (import 'thanos-mixin/config.libsonnet') +
  // prometheusAlertsFilter + prometheusAlertsUpdate +
  {
    _config+:: {
      namespace: 'monitoring',
    },
    prometheusRules+::
      thanosMixin.prometheusRules +
      {
        // groups+: [],
      },
    prometheusAlerts+:: 
      thanosMixin.prometheusAlerts +
      {
        // cat existingrule.yaml | gojsontoyaml -yamltojson > existingrule.json
        // groups+: (import 'existingrule.json').groups,
      },
  }
  ...

You will end up with:

groupname: \"thanos-sidecar\" is repeated in the same file"

Because (import 'thanos-mixin/mixin.libsonnet').prometheusAlerts also includes alerts/sidecar.libsonnet.
Tested with the updated version of the thanos-mixin including the PR mentionned.

[dimm0]

Any progress? Still can't build monitoring..

[paulfantom]

We are working on refactoring the codebase. After this effort is finished, importing mixins should be done per component and without using global variables. As a direct effect, conflicts like one described here shouldn't happen.

The current progress of this initiative can be tracked in #566

A workaround would be to import thanos mixin and filter out sidecar alerts and rules during import.

[dimm0]

How can I filter those?

[dimm0]

Could you please post the workaround here? Thanks!

[paulfantom]

After last refactoring, this should not be an issue anymore. Thanos sidecar alerts should be imported when setting $.prometheus.thanos and other alerts should be imported from thanos mixin by the user. To fix conflict with alerts and groups you should import Thanos mixin with the option sidecar:: null as this doesn't add thanos-sidecar group.

Thanos mixin import should look similar to:

import 'github.com/thanos-io/thanos/mixin/mixin.libsonnet' + {
  sidecar:: null,
}

[dimm0]

I'm getting:

STATIC ERROR: rules.jsonnet:(32:15)-(34:8): computed imports are not allowed.

How's refactoring going? Is there going to be a completely new version soon?

[paulfantom]

I'm getting:
STATIC ERROR: rules.jsonnet:(32:15)-(34:8): computed imports are not allowed.

Ah, imported thing should be surrounded by ( and ) or done in local variable. It should work with both:

local sidecar = (import 'github.com/thanos-io/thanos/mixin/mixin.libsonnet');

sidecar + {
  sidecar:: null,
}

or

(import 'github.com/thanos-io/thanos/mixin/mixin.libsonnet') + {
  sidecar:: null,
}

How's refactoring going?

It's done.

Is there going to be a completely new version soon?

Yes, but we encourage to use main branch :)

[dimm0]

Yup, all works now! Thanks

[sdurrheimer]

It won't work with the new kube-prometheus/lib/mixin.libsonnet lib tho:

local addMixin = (import 'kube-prometheus/lib/mixin.libsonnet');
local thanosMixin = addMixin({
  name: 'thanos',
  namespace: 'monitoring', // optional
  dashboardFolder: 'Infra', // optional
  mixin: (import 'thanos-mixin/mixin.libsonnet') + {
    sidecar:: null
  },
});
...
      grafana+: {
        folderDashboards+:: {
          Infra: {
            // https://github.com/kubernetes/ingress-nginx/tree/master/deploy/grafana/dashboards
            'nginx.json': (import 'dashboards/ingress/nginx.json'),
            'request-handling-performance.json': (import 'dashboards/ingress/request-handling-performance.json'),
          },
        } + thanosMixin.grafanaDashboards,
      },
...

The error:

RUNTIME ERROR: can only index objects, strings, and arrays, got null.
        vendor/thanos-mixin/dashboards/sidecar.libsonnet:93:28-48       thunk <name>
        vendor/thanos-mixin/dashboards/../lib/thanos-grafana-builder/builder.libsonnet:16:20-24 object <v>
        std.jsonnet:947:43-47   thunk <v>
        std.jsonnet:919:10      thunk <a>
        std.jsonnet:919:10-19   function <anonymous>
        std.jsonnet:919:10-19   function <aux>
        std.jsonnet:947:39-78   thunk <array_element>
        std.jsonnet:951:9-28    function <aux>
        std.jsonnet:937:50-83   thunk <array_element>
        std.jsonnet:941:9-28    function <aux>
        ...
        std.jsonnet:941:9-28    function <aux>
        std.jsonnet:947:39-78   thunk <array_element>
        std.jsonnet:951:9-28    function <aux>
        std.jsonnet:952:5-23    function <anonymous>
        vendor/github.com/brancz/kubernetes-grafana/grafana/grafana.libsonnet:91:27-103 object <anonymous>
        vendor/github.com/brancz/kubernetes-grafana/grafana/grafana.libsonnet:91:17-105 object <anonymous>
        vendor/github.com/brancz/kubernetes-grafana/grafana/grafana.libsonnet:(82:9)-(92:10)    thunk <array_element>
        vendor/kube-prometheus/./components/grafana.libsonnet:81:12-45  object <anonymous>
        kube-prometheus.jsonnet:1126:1-83       object <anonymous>
        During manifestation

[paulfantom]

Thanos mixin is added automatically if you enable thanos config in $.values.prometheus.thanos. There is no need to add it manually. Take a look at https://github.com/prometheus-operator/kube-prometheus/blob/main/jsonnet/kube-prometheus/components/prometheus.libsonnet#L52-L63

[sdurrheimer]

Thanos mixin is added automatically if you enable thanos config in $.values.prometheus.thanos. There is no need to add it manually. Take a look at https://github.com/prometheus-operator/kube-prometheus/blob/main/jsonnet/kube-prometheus/components/prometheus.libsonnet#L52-L63

The whole point of this issue is to able to import every component from the mixin, not only the sidecar part.

[paulfantom]

Sorry, I haven't had my morning coffee before writing that comment :smile:

It looks like there is still a bug in thanos mixin. Setting sidecar:: null should exclude all sidecar-related objects, which isn't the case. The problem is that https://github.com/thanos-io/thanos/blob/main/mixin/dashboards/sidecar.libsonnet#L93 is still included. This should be written as:

[if thanos.sidecar != null then '__overviewRows__']+:: [

To follow the pattern from https://github.com/thanos-io/thanos/blob/main/mixin/dashboards/sidecar.libsonnet#L15

@sdurrheimer do you want to contribute that to thanos project?

/cc @kakkoyun

[kakkoyun]

Yes, contributions are most welcome 🙏 At the very least if you could open an issue, it'd be amazing. Thanks.