Add support to disable monitoring at the installation or cluster level #48
Conversation
QuentinBisson
force-pushed
the
add-support-to-disable-monitoring
branch
7 times, most recently
from
404b28f to
f5155cc
Compare
Signed-off-by: QuentinBisson <[email protected]>
QuentinBisson
force-pushed
the
add-support-to-disable-monitoring
branch
from
f5155cc to
e53ed84
Compare
|
@giantswarm/team-atlas This has been tested on Grizzly |
| @@ -78,3 +81,8 @@ require ( | |||
| sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect | |||
| sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect | |||
| ) | |||
|
|
|||
| replace ( | |||
There was a problem hiding this comment.
Fix a nancy issue because opsgenie sdk is quite old
Signed-off-by: QuentinBisson <[email protected]>
QuentinBisson
force-pushed
the
add-support-to-disable-monitoring
branch
from
dbc22b3 to
a9d0b79
Compare
| @@ -88,23 +91,27 @@ func (r *ClusterMonitoringReconciler) Reconcile(ctx context.Context, req ctrl.Re | |||
| ctx = log.IntoContext(ctx, logger) | |||
|
|
|||
| if !r.MonitoringConfig.Enabled { | |||
There was a problem hiding this comment.
Do we still need this condition since you add if !r.MonitoringConfig.IsMonitored(cluster) ?
There was a problem hiding this comment.
Yes because we need to create/delete things if the installation is monitored (mimir secrets for instance) and some we need to do for all clusters
There was a problem hiding this comment.
This condition is covered by this one https://github.com/giantswarm/observability-operator/pull/48/files#diff-1a9ac66aa1d96d275ecfebdf5b5b47a1659ffe91674fe1fea9892587c8801f13R97
No ?
There was a problem hiding this comment.
Yes but this is an info log only. The issue is that thé mc cluster Can have thé giantswarm.io/monitoring: false but not the installation
There was a problem hiding this comment.
I'd have preferred adding the log in the IsMonitored function and keep only one condition in the controller.
There was a problem hiding this comment.
But I don't want the code to log everytime I use this condition :D
There was a problem hiding this comment.
And as mentionned, we cannot keep only 1 condition because we create MC level resources (like the mimir ingress) and resources for each WCs and they do not depend on the same value
Co-authored-by: Marie Roque <[email protected]> Signed-off-by: QuentinBisson <[email protected]>
QuentinBisson
force-pushed
the
add-support-to-disable-monitoring
branch
from
ecd7183 to
0b7ef6f
Compare
What this PR does / why we need it
Allow support to disable prometheus agents and the monitoring stack when monitoring is disabled at the installation or cluster level giantswarm/roadmap#3513
This cannot be released as long as we do not have mimir everywhere (cf. giantswarm/roadmap#3335) but I would love your reviews
Checklist