NETOBSERV-638: avoid infinite update-retrigger loop

controllers/ebpf/agent_controller.go

@@ -161,6 +161,7 @@ func (c *AgentController) desired(coll *flowsv1alpha1.FlowCollector) *v1.DaemonS
 		// This operation must currently be performed manually (run "make fix-ebpf-kafka-tls"). It could be automated here.
 		volumes, volumeMounts = helper.AppendCertVolumes(volumes, volumeMounts, &coll.Spec.Kafka.TLS, kafkaCerts)
 	}
+
 	return &v1.DaemonSet{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      constants.EBPFAgentName,
@@ -241,7 +242,10 @@ func (c *AgentController) envConfig(coll *flowsv1alpha1.FlowCollector) []corev1.
 	}
 	dedup := dedupeDefault
 	dedupJustMark := dedupeJustMarkDefault
-	for k, v := range coll.Spec.Agent.EBPF.Env {
+	// we need to sort env map to keep idempotency,
+	// as equal maps could be iterated in different order
+	for _, pair := range helper.KeySorted(coll.Spec.Agent.EBPF.Env) {
+		k, v := pair[0], pair[1]
 		if k == envDedupe {
 			dedup = v
 		} else if k == envDedupeJustMark {
@@ -279,7 +283,8 @@ func (c *AgentController) envConfig(coll *flowsv1alpha1.FlowCollector) []corev1.
 			Name: envFlowsTargetHost,
 			ValueFrom: &corev1.EnvVarSource{
 				FieldRef: &corev1.ObjectFieldSelector{
-					FieldPath: "status.hostIP",
+					APIVersion: "v1",
+					FieldPath:  "status.hostIP",
 				},
 			},
 		}, corev1.EnvVar{
@@ -297,10 +302,16 @@ func (c *AgentController) requiredAction(current, desired *v1.DaemonSet) reconci
 	if current == nil && desired != nil {
 		return actionCreate
 	}
-	if equality.Semantic.DeepDerivative(&desired.Spec, current.Spec) {
-		return actionNone
+	cSpec, dSpec := current.Spec, desired.Spec
+	eq := equality.Semantic.DeepDerivative
+	if !helper.IsSubSet(current.ObjectMeta.Labels, desired.ObjectMeta.Labels) ||
+		!eq(dSpec.Selector, cSpec.Selector) ||
+		!eq(dSpec.Template, cSpec.Template) {
+
+		return actionUpdate
 	}
-	return actionUpdate
+
+	return actionNone
 }
 
 func (c *AgentController) securityContext(coll *flowsv1alpha1.FlowCollector) *corev1.SecurityContext {

controllers/ebpf/internal/permissions/permissions.go

@@ -76,20 +76,21 @@ func (c *Reconciler) reconcileNamespace(ctx context.Context) error {
 				"pod-security.kubernetes.io/enforce": "privileged",
 				"pod-security.kubernetes.io/audit":   "privileged",
 			},
-			Annotations: map[string]string{
-				// Means that only userID 0 is allowed in the eBPF pods
-				"openshift.io/sa.scc.uid-range": "0/1",
-				// unclassified Multi-Category Security (MCS) level of SELinux
-				"openshift.io/sa.scc.mcs": "s0",
-			},
 		},
 	}
 	if actual == nil && desired != nil {
 		rlog.Info("creating namespace")
 		return c.client.CreateOwned(ctx, desired)
 	}
 	if actual != nil && desired != nil {
-		if !helper.IsSubSet(actual.ObjectMeta.Labels, desired.ObjectMeta.Labels) {
+		// We noticed that audit labels are automatically removed
+		// in some configurations of K8s, so to avoid an infinite update loop, we just ignore
+		// it (if the user removes it manually, it's at their own risk)
+		if !helper.IsSubSet(actual.ObjectMeta.Labels,
+			map[string]string{
+				"app":                                constants.OperatorName,
+				"pod-security.kubernetes.io/enforce": "privileged",
+			}) {
 			rlog.Info("updating namespace")
 			return c.client.UpdateOwned(ctx, actual, desired)
 		}
@@ -169,7 +170,13 @@ func (c *Reconciler) reconcileOpenshiftPermissions(
 		rlog.Info("creating SecurityContextConstraints")
 		return c.client.CreateOwned(ctx, scc)
 	}
-	if !equality.Semantic.DeepDerivative(&scc, &actual) {
+	if scc.AllowHostNetwork != actual.AllowHostNetwork ||
+		!equality.Semantic.DeepDerivative(&scc.RunAsUser, &actual.RunAsUser) ||
+		!equality.Semantic.DeepDerivative(&scc.SELinuxContext, &actual.SELinuxContext) ||
+		!equality.Semantic.DeepDerivative(&scc.Users, &actual.Users) ||
+		scc.AllowPrivilegedContainer != actual.AllowPrivilegedContainer ||
+		!equality.Semantic.DeepDerivative(&scc.AllowedCapabilities, &actual.AllowedCapabilities) {
+
 		rlog.Info("updating SecurityContextConstraints")
 		return c.client.UpdateOwned(ctx, actual, scc)
 	}

controllers/flowcollector_controller_ebpf_test.go

@@ -62,6 +62,8 @@ func flowCollectorEBPFSpecs() {
 							ExcludeInterfaces:  []string{"br-3", "lo"},
 							LogLevel:           "trace",
 							Env: map[string]string{
+								// we'll test that multiple variables are reordered
+								"GOGC":           "400",
 								"BUFFERS_LENGTH": "100",
 							},
 						},
@@ -96,6 +98,7 @@ func flowCollectorEBPFSpecs() {
 				v1.EnvVar{Name: "INTERFACES", Value: "veth0,/^br-/"},
 				v1.EnvVar{Name: "EXCLUDE_INTERFACES", Value: "br-3,lo"},
 				v1.EnvVar{Name: "BUFFERS_LENGTH", Value: "100"},
+				v1.EnvVar{Name: "GOGC", Value: "400"},
 				v1.EnvVar{Name: "SAMPLING", Value: "123"},
 				v1.EnvVar{Name: "FLOWS_TARGET_PORT", Value: "9999"},
 			))

controllers/flowcollector_controller_test.go

@@ -191,7 +191,9 @@ func flowCollectorControllerSpecs() {
 					LogLevel:        "error",
 					Image:           "testimg:latest",
 					Env: map[string]string{
-						"GOGC": "400",
+						// we'll test that env vars are sorted, to keep idempotency
+						"GOMAXPROCS": "33",
+						"GOGC":       "400",
 					},
 				}
 				fc.Spec.Loki = flowsv1alpha1.FlowCollectorLoki{}
@@ -251,7 +253,9 @@ func flowCollectorControllerSpecs() {
 					ContainerPort: 7891,
 					Protocol:      "UDP",
 				}))
-				Expect(cnt.Env).To(ContainElement(v1.EnvVar{Name: "GOGC", Value: "400"}))
+				Expect(cnt.Env).To(Equal([]v1.EnvVar{
+					{Name: "GOGC", Value: "400"}, {Name: "GOMAXPROCS", Value: "33"},
+				}))
 			})
 
 			By("Allocating the proper toleration to allow its placement in the master nodes", func() {

controllers/flowlogspipeline/flp_common_objects.go

@@ -184,8 +184,10 @@ func (b *builder) podTemplate(hasHostPort, hasLokiInterface, hostNetwork bool, c
 	}
 
 	var envs []corev1.EnvVar
-	for k, v := range b.desired.Processor.Env {
-		envs = append(envs, corev1.EnvVar{Name: k, Value: v})
+	// we need to sort env map to keep idempotency,
+	// as equal maps could be iterated in different order
+	for _, pair := range helper.KeySorted(b.desired.Processor.Env) {
+		envs = append(envs, corev1.EnvVar{Name: pair[0], Value: pair[1]})
 	}
 
 	container := corev1.Container{
@@ -482,6 +484,10 @@ func (b *builder) fillPromService(svc *corev1.Service) {
 		Name:     prometheusServiceName,
 		Port:     b.desired.Processor.Metrics.Server.Port,
 		Protocol: corev1.ProtocolTCP,
+		// Some Kubernetes versions might automatically set TargetPort to Port. We need to
+		// explicitly set it here so the reconcile loop verifies that the owned service
+		// is equal as the desired service
+		TargetPort: intstr.FromInt(int(b.desired.Processor.Metrics.Server.Port)),
 	}}
 	if b.desired.Processor.Metrics.Server.TLS.Type == flowsv1alpha1.ServerTLSAuto {
 		if svc.ObjectMeta.Annotations == nil {

controllers/flowlogspipeline/flp_ingest_reconciler.go

@@ -2,11 +2,11 @@ package flowlogspipeline
 
 import (
 	"context"
-	"reflect"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	flowsv1alpha1 "github.com/netobserv/network-observability-operator/api/v1alpha1"
@@ -98,7 +98,7 @@ func (r *flpIngesterReconciler) reconcile(ctx context.Context, desired *flowsv1a
 		if err := r.CreateOwned(ctx, newCM); err != nil {
 			return err
 		}
-	} else if !reflect.DeepEqual(newCM.Data, r.owned.configMap.Data) {
+	} else if !equality.Semantic.DeepDerivative(newCM.Data, r.owned.configMap.Data) {
 		if err := r.UpdateOwned(ctx, r.owned.configMap, newCM); err != nil {
 			return err
 		}

controllers/flowlogspipeline/flp_monolith_reconciler.go

@@ -2,11 +2,11 @@ package flowlogspipeline
 
 import (
 	"context"
-	"reflect"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	flowsv1alpha1 "github.com/netobserv/network-observability-operator/api/v1alpha1"
@@ -100,7 +100,7 @@ func (r *flpMonolithReconciler) reconcile(ctx context.Context, desired *flowsv1a
 		if err := r.CreateOwned(ctx, newCM); err != nil {
 			return err
 		}
-	} else if !reflect.DeepEqual(newCM.Data, r.owned.configMap.Data) {
+	} else if !equality.Semantic.DeepDerivative(newCM.Data, r.owned.configMap.Data) {
 		if err := r.UpdateOwned(ctx, r.owned.configMap, newCM); err != nil {
 			return err
 		}

controllers/flowlogspipeline/flp_reconciler.go

@@ -3,10 +3,10 @@ package flowlogspipeline
 import (
 	"context"
 	"fmt"
-	"reflect"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 
 	flowsv1alpha1 "github.com/netobserv/network-observability-operator/api/v1alpha1"
 	"github.com/netobserv/network-observability-operator/controllers/constants"
@@ -93,8 +93,8 @@ func configChanged(tmpl *corev1.PodTemplateSpec, configDigest string) bool {
 }
 
 func serviceNeedsUpdate(actual *corev1.Service, desired *corev1.Service) bool {
-	return !reflect.DeepEqual(actual.ObjectMeta, desired.ObjectMeta) ||
-		!reflect.DeepEqual(actual.Spec, desired.Spec)
+	return !equality.Semantic.DeepDerivative(desired.ObjectMeta, actual.ObjectMeta) ||
+		!equality.Semantic.DeepDerivative(desired.Spec, actual.Spec)
 }
 
 func containerNeedsUpdate(podSpec *corev1.PodSpec, desired *flpSpec, expectHostPort bool) bool {
@@ -105,7 +105,7 @@ func containerNeedsUpdate(podSpec *corev1.PodSpec, desired *flpSpec, expectHostP
 		desired.Image != container.Image ||
 		desired.ImagePullPolicy != string(container.ImagePullPolicy) ||
 		probesNeedUpdate(container, desired.EnableKubeProbes) ||
-		!reflect.DeepEqual(desired.Resources, container.Resources)
+		!equality.Semantic.DeepDerivative(desired.Resources, container.Resources)
 }
 
 func probesNeedUpdate(container *corev1.Container, enabled bool) bool {

controllers/flowlogspipeline/flp_transfo_reconciler.go

@@ -2,12 +2,12 @@ package flowlogspipeline
 
 import (
 	"context"
-	"reflect"
 
 	appsv1 "k8s.io/api/apps/v1"
 	ascv2 "k8s.io/api/autoscaling/v2beta2"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	flowsv1alpha1 "github.com/netobserv/network-observability-operator/api/v1alpha1"
@@ -102,7 +102,7 @@ func (r *flpTransformerReconciler) reconcile(ctx context.Context, desired *flows
 		if err := r.CreateOwned(ctx, newCM); err != nil {
 			return err
 		}
-	} else if !reflect.DeepEqual(newCM.Data, r.owned.configMap.Data) {
+	} else if !equality.Semantic.DeepDerivative(newCM.Data, r.owned.configMap.Data) {
 		if err := r.UpdateOwned(ctx, r.owned.configMap, newCM); err != nil {
 			return err
 		}
@@ -196,7 +196,7 @@ func autoScalerNeedsUpdate(asc *ascv2.HorizontalPodAutoscaler, desired flowsv1al
 		differentPointerValues(asc.Spec.MinReplicas, desired.MinReplicas) {
 		return true
 	}
-	if !reflect.DeepEqual(asc.Spec.Metrics, desired.Metrics) {
+	if !equality.Semantic.DeepDerivative(desired.Metrics, asc.Spec.Metrics) {
 		return true
 	}
 	return false

go.mod

@@ -16,7 +16,7 @@ require (
 	k8s.io/client-go v0.24.0
 	k8s.io/kube-aggregator v0.23.5
 	k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9
-	sigs.k8s.io/controller-runtime v0.11.0
+	sigs.k8s.io/controller-runtime v0.11.2
 )
 
 replace github.com/prometheus/common v0.32.1 => github.com/netobserv/prometheus-common v0.31.2-0.20220720134304-43e74fd22881

go.sum

@@ -1741,8 +1741,9 @@ k8s.io/api v0.23.0/go.mod h1:8wmDdLBHBNxtOIytwLstXt5E9PddnZb0GaMcqsvDBpg=
 k8s.io/api v0.23.5/go.mod h1:Na4XuKng8PXJ2JsploYYrivXrINeTaycCGcYgF91Xm8=
 k8s.io/api v0.24.0 h1:J0hann2hfxWr1hinZIDefw7Q96wmCBx6SSB8IY0MdDg=
 k8s.io/api v0.24.0/go.mod h1:5Jl90IUrJHUJYEMANRURMiVvJ0g7Ax7r3R1bqO8zx8I=
-k8s.io/apiextensions-apiserver v0.23.0 h1:uii8BYmHYiT2ZTAJxmvc3X8UhNYMxl2A0z0Xq3Pm+WY=
 k8s.io/apiextensions-apiserver v0.23.0/go.mod h1:xIFAEEDlAZgpVBl/1VSjGDmLoXAWRG40+GsWhKhAxY4=
+k8s.io/apiextensions-apiserver v0.23.5 h1:5SKzdXyvIJKu+zbfPc3kCbWpbxi+O+zdmAJBm26UJqI=
+k8s.io/apiextensions-apiserver v0.23.5/go.mod h1:ntcPWNXS8ZPKN+zTXuzYMeg731CP0heCTl6gYBxLcuQ=
 k8s.io/apimachinery v0.19.2/go.mod h1:DnPGDnARWFvYa3pMHgSxtbZb7gpzzAZ1pTfaUNDVlmA=
 k8s.io/apimachinery v0.21.0/go.mod h1:jbreFvJo3ov9rj7eWT7+sYiRx+qZuCYXwWT1bcDswPY=
 k8s.io/apimachinery v0.23.0/go.mod h1:fFCTTBKvKcwTPFzjlcxp91uPFZr+JA0FubU4fLzzFYc=
@@ -1794,8 +1795,9 @@ rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
 rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.25/go.mod h1:Mlj9PNLmG9bZ6BHFwFKDo5afkpWyUISkb9Me0GnK66I=
 sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.30/go.mod h1:fEO7lRTdivWO2qYVCVG7dEADOMo/MLDCVr8So2g88Uw=
-sigs.k8s.io/controller-runtime v0.11.0 h1:DqO+c8mywcZLFJWILq4iktoECTyn30Bkj0CwgqMpZWQ=
 sigs.k8s.io/controller-runtime v0.11.0/go.mod h1:KKwLiTooNGu+JmLZGn9Sl3Gjmfj66eMbCQznLP5zcqA=
+sigs.k8s.io/controller-runtime v0.11.2 h1:H5GTxQl0Mc9UjRJhORusqfJCIjBO8UtUxGggCwL1rLA=
+sigs.k8s.io/controller-runtime v0.11.2/go.mod h1:P6QCzrEjLaZGqHsfd+os7JQ+WFZhvB8MRFsn4dWF7O4=
 sigs.k8s.io/e2e-framework v0.0.6/go.mod h1:XSknNb1ovbtOyNNjV8DKuY9Nr4rta4wwtnZq3IRGMl0=
 sigs.k8s.io/json v0.0.0-20211020170558-c049b76a60c6/go.mod h1:p4QtZmO4uMYipTQNzagwnNoseA6OxSUutVw05NhYDRs=
 sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 h1:kDi4JBNAsJWfz1aEXhO8Jg87JJaPNLh5tIzYHgStQ9Y=

pkg/helper/helpers.go

@@ -2,7 +2,10 @@
 // to perform some basic computational operations
 package helper
 
-import "strings"
+import (
+	"sort"
+	"strings"
+)
 
 func ContainsString(slice []string, s string) bool {
 	for _, item := range slice {
@@ -41,3 +44,15 @@ func IsSubSet(set, subset map[string]string) bool {
 	}
 	return true
 }
+
+// KeySorted returns the map key-value pairs sorted by Key
+func KeySorted(set map[string]string) [][2]string {
+	vals := make([][2]string, 0, len(set))
+	for k, v := range set {
+		vals = append(vals, [2]string{k, v})
+	}
+	sort.Slice(vals, func(i, j int) bool {
+		return vals[i][0] < vals[j][0]
+	})
+	return vals
+}

pkg/helper/helpers_test.go

@@ -47,3 +47,15 @@ func TestRemoveAllStrings(t *testing.T) {
 	s = RemoveAllStrings(s, "five")
 	assert.Equal([]string{"one", "two", "four"}, s)
 }
+
+func TestKeySorted(t *testing.T) {
+	set := map[string]string{
+		"b": "1",
+		"c": "2",
+		"a": "3",
+		"d": "4",
+	}
+	assert.Equal(t,
+		[][2]string{{"a", "3"}, {"b", "1"}, {"c", "2"}, {"d", "4"}},
+		KeySorted(set))
+}