Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Add release label to prometheusRule example #3346

Merged
merged 1 commit into from
May 3, 2021
Merged

docs: Add release label to prometheusRule example #3346

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/operation/cluster_and_service_configuration.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ In order to add Alert rules, a new ``PrometheusRule`` manifest must be created.
labels:
app: prometheus-operator
app.kubernetes.io/name: prometheus-operator
release: prometheus-operator
name: <prometheus-rule-name>
namespace: <namespace-name>
spec:
Expand All @@ -394,6 +395,11 @@ In order to add Alert rules, a new ``PrometheusRule`` manifest must be created.
labels:
severity: warning

.. note::

All the labels in the example above are mandatory for Prometheus
to take the new rules into account.
Comment on lines +400 to +401
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems like an issue, no? The app deploys the rule, so app.kubernetes.io/name and app must be related to the app creating the rule, totally unrelated to prometheus-operator. Similarly, release should not be part of any (required) set of labels, and should not be prometheus-release.

In essence, we need to change the set of labels our prometheus-operator uses to filter out rules (and other objects) it should take into account, e.g., turn the selector into metalk8s.scality.com/monitor: "true" or something along those lines.

Copy link
Contributor Author

@alexandre-allard alexandre-allard May 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can easily be overridden using prometheus.prometheusSpec.ruleSelector in the charts/kube-prometheus-stack.yaml file:

prometheus:
  prometheusSpec:
    ruleSelector:
      matchLabels:
        metalk8s.scality.com/monitor: "true"

And we should probably do the same for ServiceMonitor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, that's what we recommend in #2199
This PR was more about making sure our docs are accurate about the current state of the product (although it's debatable whether it's a useful change 😜)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oneline fix, does not cost much, at least now it's accurate even if it's ugly (which was already the case).


Then this manifest must be applied.

.. code-block:: shell
Expand Down