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

Remove alerting from Prometheus if mimir is enabled #1559

Merged

Conversation

QuentinBisson
Copy link
Contributor

Towards giantswarm/roadmap#3160

This PR removes alerting from prometheus to alertmanager if mimir is enabled to avoid duplicate alerts.

Alerting will keep working thanks to https://github.com/giantswarm/shared-configs/commit/bcd2f81fe326c06d7cd3f076006e3d642d866fa6

Checklist

I have:

  • Described why this change is being introduced
  • Separated out refactoring/reformatting in a dedicated PR
  • Updated changelog in CHANGELOG.md

@QuentinBisson QuentinBisson self-assigned this Mar 12, 2024
@QuentinBisson QuentinBisson requested a review from a team as a code owner March 12, 2024 13:11
@QuentinBisson QuentinBisson force-pushed the remove-alerting-from-prometheus-if-mimir-is-enabled branch from 2cb3023 to 1bfda39 Compare March 12, 2024 13:11
@@ -1,6 +1,6 @@
module github.com/giantswarm/prometheus-meta-operator/v2

go 1.22.1
go 1.22.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I cannot run this otherwise because fedora only has up to go 1.22.0

{
// This resource creates a static secret to connect Prometheus to Alertmanager. When using mimir, this is not needed anymore
if config.MimirEnabled {
alertmanagerWiringResource = &noop.Resource{}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a noop operation to not create useless secret in mimir backed installations

prometheus := &promv1.Prometheus{
ObjectMeta: objectMeta,
Spec: promv1.PrometheusSpec{
// We need to use this to connect each WC prometheus with the central alertmanager instead of the alerting section of the Prometheus CR
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved at the bottom only if mimir is not enabled

@QuentinBisson QuentinBisson merged commit 2d33166 into master Mar 12, 2024
5 checks passed
@QuentinBisson QuentinBisson deleted the remove-alerting-from-prometheus-if-mimir-is-enabled branch March 12, 2024 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants