NETOBSERV-927 Improve servicemonitor/prometheusrule checking changes … (

#302) * NETOBSERV-927 Improve servicemonitor/prometheusrule checking changes perf Only check for spec change, not meta NB: namespace or name changes follow a different process; it is safe to not check for name/namespace changes there; you cannot anyway update a resource for a namespace or name change * Check labels change, add labels to SM
netobserv · Mar 22, 2023 · 1b56f8e · 1b56f8e
1 parent 460d0b0
commit 1b56f8e
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 11 deletions.
diff --git a/controllers/flowlogspipeline/flp_common_objects.go b/controllers/flowlogspipeline/flp_common_objects.go
@@ -693,6 +693,7 @@ func (b *builder) serviceMonitor() *monitoringv1.ServiceMonitor {
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      b.serviceMonitorName(),
 			Namespace: b.namespace,
+			Labels:    b.labels,
 		},
 		Spec: monitoringv1.ServiceMonitorSpec{
 			Endpoints: []monitoringv1.Endpoint{

diff --git a/controllers/flowlogspipeline/flp_test.go b/controllers/flowlogspipeline/flp_test.go
@@ -43,6 +43,7 @@ var resources = corev1.ResourceRequirements{
 	},
 }
 var image = "quay.io/netobserv/flowlogs-pipeline:dev"
+var image2 = "quay.io/netobserv/flowlogs-pipeline:dev2"
 var pullPolicy = corev1.PullIfNotPresent
 var minReplicas = int32(1)
 var maxReplicas = int32(5)
@@ -494,13 +495,20 @@ func TestServiceMonitorChanged(t *testing.T) {
 	first := b.generic.serviceMonitor()
 
 	// Check namespace change
-	cfg.Processor.Metrics.Server.Port = 9999
 	b = newMonolithBuilder("namespace2", image, &cfg, true, &certWatcher)
 	second := b.generic.serviceMonitor()
 
 	report := helper.NewChangeReport("")
 	assert.True(helper.ServiceMonitorChanged(first, second, &report))
-	assert.Contains(report.String(), "ServiceMonitor meta changed")
+	assert.Contains(report.String(), "ServiceMonitor spec changed")
+
+	// Check labels change
+	b = newMonolithBuilder("namespace2", image2, &cfg, true, &certWatcher)
+	third := b.generic.serviceMonitor()
+
+	report = helper.NewChangeReport("")
+	assert.True(helper.ServiceMonitorChanged(second, third, &report))
+	assert.Contains(report.String(), "ServiceMonitor labels changed")
 }
 
 func TestPrometheusRuleNoChange(t *testing.T) {
@@ -524,19 +532,26 @@ func TestPrometheusRuleChanged(t *testing.T) {
 	assert := assert.New(t)
 
 	// Get first
-	ns := "namespace"
 	cfg := getConfig()
-	b := newMonolithBuilder(ns, image, &cfg, true, &certWatcher)
+	b := newMonolithBuilder("namespace", image, &cfg, true, &certWatcher)
 	first := b.generic.prometheusRule()
 
 	// Check namespace change
-	cfg.Processor.Metrics.Server.Port = 9999
-	b = newMonolithBuilder("namespace2", image, &cfg, true, &certWatcher)
+	cfg.Processor.Metrics.DisableAlerts = []flowslatest.FLPAlert{flowslatest.AlertNoFlows}
+	b = newMonolithBuilder("namespace", image, &cfg, true, &certWatcher)
 	second := b.generic.prometheusRule()
 
 	report := helper.NewChangeReport("")
 	assert.True(helper.PrometheusRuleChanged(first, second, &report))
-	assert.Contains(report.String(), "PrometheusRule meta changed")
+	assert.Contains(report.String(), "PrometheusRule spec changed")
+
+	// Check labels change
+	b = newMonolithBuilder("namespace2", image2, &cfg, true, &certWatcher)
+	third := b.generic.prometheusRule()
+
+	report = helper.NewChangeReport("")
+	assert.True(helper.PrometheusRuleChanged(second, third, &report))
+	assert.Contains(report.String(), "PrometheusRule labels changed")
 }
 
 func TestConfigMapShouldDeserializeAsJSON(t *testing.T) {

diff --git a/pkg/helper/comparators.go b/pkg/helper/comparators.go
@@ -80,13 +80,13 @@ func ServiceChanged(old, new *corev1.Service, report *ChangeReport) bool {
 }
 
 func ServiceMonitorChanged(old, new *monitoringv1.ServiceMonitor, report *ChangeReport) bool {
-	return report.Check("ServiceMonitor meta changed", !equality.Semantic.DeepDerivative(new.ObjectMeta, old.ObjectMeta)) ||
-		report.Check("ServiceMonitor spec changed", !equality.Semantic.DeepDerivative(new.Spec, old.Spec))
+	return report.Check("ServiceMonitor spec changed", !equality.Semantic.DeepDerivative(new.Spec, old.Spec)) ||
+		report.Check("ServiceMonitor labels changed", !IsSubSet(old.Labels, new.Labels))
 }
 
 func PrometheusRuleChanged(old, new *monitoringv1.PrometheusRule, report *ChangeReport) bool {
-	return report.Check("PrometheusRule meta changed", !equality.Semantic.DeepDerivative(new.ObjectMeta, old.ObjectMeta)) ||
-		report.Check("PrometheusRule spec changed", !equality.Semantic.DeepDerivative(new.Spec, old.Spec))
+	return report.Check("PrometheusRule spec changed", !equality.Semantic.DeepDerivative(new.Spec, old.Spec)) ||
+		report.Check("PrometheusRule labels changed", !IsSubSet(old.Labels, new.Labels))
 }
 
 // FindContainer searches in pod containers one that matches the provided name