[stable/prometheus-operator] Re-sync prometheus rules and grafana dashboards

[iuriaranda]

What this PR does / why we need it

Re-sync the prometheus rules and grafana dashboards.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

• DCO signed
• Chart Version bumped
• Variables are documented in the README.md
• Title of the PR starts with chart name (e.g. [stable/mychartname])

[k8s-ci-robot]

Hi @iuriaranda. Thanks for your PR.

I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[vsliouniaev]

/ok-to-test
/lgtm

[iuriaranda]

Yeah I still have to resolve that linting error somehow. The problem is that they've introduced a special notation in a prometheus rule upstream (here), and it's giving issues when translated to a Helm template.

I'm still figuring out how to fix it, we either change that notation upstream in the kube-prometheus repo or we figure out how to escape a backtick (`) in a helm command.

[iuriaranda]

@vsliouniaev Can you review again please? I've fixed the linting error and merged from master.

[vsliouniaev]

The sync scripts produce a different output than what is in your PR

[vsliouniaev]

/hold

[iuriaranda]

The sync scripts produce a different output than what is in your PR

Indeed, it's about the notation of the PrometheusRemoteWriteDesiredShards alert. The upstream alert description uses a backtick character that gets enclosed inside a {{`...`}} block in Helm by the sync script. So it ends up transformed to something like this:

{{`{{ printf `prometheus_remote_storage_shards_max...` }}`}}

Everything inside the outer {{`...`}} should be interpreted literal by Helm, so after Helm rendering should end up like {{ printf `prometheus_remote_storage_shards_max...` }}. But of course the inside backticks are breaking the Helm interpolation.

I've seen that this has been manually fixed in previous syncs like this, so that's what I did this time:

{{print `{{ printf ` "`" `prometheus_remote_storage_shards_max` "`" `}}`}}

I don't think this can easily be incorporated into the sync script so it automatically transforms such expressions. So IMO we either modify the expression upstream in the prometheus mixin or we accept a manual change.

Any other ideas?

[iuriaranda]

@vsliouniaev Have you seen my previous comment? How do you propose to proceed?

[vsliouniaev]

I've seen that this has been manually fixed in previous syncs like this

Sounds like the scripts need to be updated to take care of this

[iuriaranda]

Sounds like the scripts need to be updated to take care of this

@vsliouniaev that would be great. Do you have any ideas on how to do that? I can't think of any regex or method that can perform this kind of transformation.

[vsliouniaev]

/lgtm

[vsliouniaev]

/retest

[iuriaranda]

Apparently tests are failing again, same problem as the other day. @unguiculus could you check the test cluster again please?
Thanks!

[davidham]

There is a change in the current rules-1.14/k8s.rules.yaml from coreos/kube-prometheus that will fix a PrometheusRuleFailures issue in prometheus-operator, anything I can do to help get this merged?

[iuriaranda]

@davidham well, apparently the test environment is not fully cleaned up sometimes, and the tests fail systematically. We can't really proceed until somebody fixes the test cluster.

cc @vsliouniaev @unguiculus

@davidham the change in the rules you mention, is it already included in this PR? Or should I re-sync the rules again?

[iuriaranda]

/retest

[unguiculus]

/retest

[iuriaranda]

/retest

[iuriaranda]

Finally all tests are green and PR is up to date with the latest changes from master 🎉

Now it only needs the lgtm label again 😛

@vsliouniaev can you review again please?

[vsliouniaev]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: iuriaranda, vsliouniaev

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• stable/prometheus-operator/OWNERS [vsliouniaev]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment