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.

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

Reject invalid PrometheusRule objects beforehand rather than failing the reconciliation #5191

Closed
simonpasquier opened this issue Nov 25, 2022 · 3 comments · Fixed by #5221
Closed

Comments

@simonpasquier
Copy link
Contributor

What is missing?

When the Thanos/Prometheus controllers detect an invalid PrometheusRule object (invalid PromQL expression for instance), they fail the reconciliation (and for Prometheus, the Reconciled condition will be set to False). Instead the controllers should reject the invalid object and proceed (just like we do for service/pod monitors and probes).

Why do we need it?

In self-service environments, blocking the reconciliation means that one single user can block all other users if they push a bad rule.

Environment

  • Prometheus Operator version: N/A

Anything else we need to know?:

Probably a good time to refactor the code handling the rules which is duplicated between pkg/prometheus and pkg/thanos.

@slashpai slashpai self-assigned this Nov 25, 2022
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Dec 12, 2022
Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Dec 21, 2022
Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Dec 21, 2022
Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Dec 27, 2022
pkg/thanos: Update selectRules method to be robust

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
@1it
Copy link

1it commented Jan 17, 2023

Erm… Sorry, this is a four years old problem, already. And still not going to be solved?

@simonpasquier
Copy link
Contributor Author

@1it you missed #5221 which is going to fix it :)

@1it
Copy link

1it commented Jan 17, 2023

Ah, OK. Thanks! I was about to start fixing this myself. 😅

slashpai added a commit to slashpai/prometheus-operator that referenced this issue Jan 19, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Jan 20, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Jan 20, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Jan 24, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Jan 30, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Jan 30, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Mar 7, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Mar 9, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Mar 9, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Mar 9, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Mar 9, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure, controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Mar 10, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure, controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
slashpai added a commit to slashpai/prometheus-operator that referenced this issue Mar 10, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure, controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
ArthurSens pushed a commit to ArthurSens/prometheus-operator that referenced this issue Mar 10, 2023
reorganized rule selection code from prometheus
and thanos package to reduce duplicate code.

This change will also make sure
there is no reconcillation failure on controller
when a bad PrometheusRule object is created.
Instead of a reconcillation failure, controller
will just reject the bad PrometheusRule.

Fixes prometheus-operator#5191

Signed-off-by: Jayapriya Pai <slashpai9@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants