Fixed bad string formatting on events and made reasons machine unders…

…tandable #55 (#55)

* Fixed reasons in network policy reconciler

* Fixed reasons in kafka acls

* Fixed reasons in network policy

* Fixed reasons in pods

* Fixed reasons in kafka server config controller

src/go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/otterize/lox v0.0.0-20220525164329-9ca2bf91c3dd
 	github.com/samber/lo v1.25.0
 	github.com/sirupsen/logrus v1.8.1
+	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.7.1
 	github.com/vishalkuo/bimap v0.0.0-20220726225509-e0b4f20de28b
 	k8s.io/api v0.24.0
@@ -77,7 +78,6 @@ require (
 	github.com/prometheus/common v0.32.1 // indirect
 	github.com/prometheus/procfs v0.7.3 // indirect
 	github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect
-	github.com/spf13/pflag v1.0.5 // indirect
 	go.uber.org/atomic v1.7.0 // indirect
 	go.uber.org/multierr v1.6.0 // indirect
 	go.uber.org/zap v1.19.1 // indirect

src/operator/controllers/external_traffic/network_policy.go

@@ -19,6 +19,15 @@ import (
 	"strings"
 )
 
+const (
+	ReasonCreatingExternalTrafficPolicyFailed = "CreatingExternalTrafficPolicyFailed"
+	ReasonCreatedExternalTrafficPolicy        = "CreatedExternalTrafficPolicy"
+	ReasonGettingExternalTrafficPolicyFailed  = "GettingExternalTrafficPolicyFailed"
+	ReasonRemovingExternalTrafficPolicy       = "RemovingExternalTrafficPolicy"
+	ReasonRemovingExternalTrafficPolicyFailed = "RemovingExternalTrafficPolicyFailed"
+	ReasonRemovedExternalTrafficPolicy        = "RemovedExternalTrafficPolicy"
+)
+
 type NetworkPolicyCreator struct {
 	client client.Client
 	scheme *runtime.Scheme
@@ -48,25 +57,25 @@ func (r *NetworkPolicyCreator) handleNetworkPolicyCreationOrUpdate(
 				"Creating network policy to enable access from external traffic to load balancer service %s (ns %s)", endpoints.GetName(), endpoints.GetNamespace())
 			err := r.client.Create(ctx, newPolicy)
 			if err != nil {
-				r.RecordWarningEvent(eventsObject, "failed to create external traffic network policy", err.Error())
+				r.RecordWarningEventf(eventsObject, ReasonCreatingExternalTrafficPolicyFailed, "failed to create external traffic network policy: %s", err.Error())
 				return err
 			}
-			r.RecordNormalEventf(eventsObject, "created external traffic network policy", "service '%s' refers to pods protected by network policy '%s'", endpoints.GetName(), netpol.GetName())
+			r.RecordNormalEventf(eventsObject, ReasonCreatedExternalTrafficPolicy, "created external traffic network policy. service '%s' refers to pods protected by network policy '%s'", endpoints.GetName(), netpol.GetName())
 		}
 		return nil
 	} else if errGetExistingPolicy != nil {
-		r.RecordWarningEvent(eventsObject, "failed to get external traffic network policy", err.Error())
+		r.RecordWarningEventf(eventsObject, ReasonGettingExternalTrafficPolicyFailed, "failed to get external traffic network policy: %s", err.Error())
 		return errGetExistingPolicy
 	}
 
 	if !r.enabled {
-		r.RecordNormalEvent(eventsObject, "removing external traffic network policy", "reconciler was disabled")
+		r.RecordNormalEvent(eventsObject, ReasonRemovingExternalTrafficPolicy, "removing external traffic network policy, reconciler was disabled")
 		err := r.client.Delete(ctx, existingPolicy)
 		if err != nil {
-			r.RecordWarningEvent(eventsObject, "failed removing external traffic network policy", err.Error())
+			r.RecordWarningEventf(eventsObject, ReasonRemovingExternalTrafficPolicyFailed, "failed removing external traffic network policy: %s", err.Error())
 			return err
 		}
-		r.RecordNormalEvent(eventsObject, "removed external traffic network policy", "success")
+		r.RecordNormalEvent(eventsObject, ReasonRemovedExternalTrafficPolicy, "removed external traffic network policy, success")
 		return nil
 	}
 

src/operator/controllers/intents_reconcilers/kafka_acls.go

@@ -17,7 +17,14 @@ import (
 )
 
 const (
-	KafkaACLsFinalizerName = "intents.otterize.com/kafka-finalizer"
+	KafkaACLsFinalizerName                  = "intents.otterize.com/kafka-finalizer"
+	ReasonCouldNotConnectToKafkaServer      = "CouldNotConnectToKafkaServer"
+	ReasonCouldNotApplyIntentsOnKafkaServer = "CouldNotApplyIntentsOnKafkaServer"
+	ReasonKafkaACLCreationDisabled          = "KafkaACLCreationDisabled"
+	ReasonKafkaServerNotConfigured          = "KafkaServerNotConfigured"
+	ReasonRemovingKafkaACLsFailed           = "RemovingKafkaACLsFailed"
+	ReasonApplyingKafkaACLsFailed           = "ApplyingKafkaACLsFailed"
+	ReasonAppliedKafkaACLs                  = "AppliedKafkaACLs"
 )
 
 type KafkaACLReconciler struct {
@@ -57,14 +64,14 @@ func (r *KafkaACLReconciler) applyACLs(intents *otterizev1alpha1.ClientIntents)
 		kafkaIntentsAdmin, err := kafkaacls.NewKafkaIntentsAdmin(*config, r.enableKafkaACLCreation)
 		if err != nil {
 			err = fmt.Errorf("failed to connect to Kafka server %s: %w", serverName, err)
-			r.RecordWarningEventf(intents, "Kafka ACL reconcile failed", err.Error())
+			r.RecordWarningEventf(intents, ReasonCouldNotConnectToKafkaServer, "Kafka ACL reconcile failed: %s", err.Error())
 			return err
 		}
 		defer kafkaIntentsAdmin.Close()
 
 		intentsForServer := intentsByServer[serverName]
 		if err := kafkaIntentsAdmin.ApplyClientIntents(intents.Spec.Service.Name, intents.Namespace, intentsForServer); err != nil {
-			r.RecordWarningEventf(intents, "Kafka ACL reconcile failed", err.Error())
+			r.RecordWarningEventf(intents, ReasonCouldNotApplyIntentsOnKafkaServer, "Kafka ACL reconcile failed: %s", err.Error())
 			return fmt.Errorf("failed applying intents on kafka server %s: %w", serverName, err)
 		}
 		return nil
@@ -73,12 +80,12 @@ func (r *KafkaACLReconciler) applyACLs(intents *otterizev1alpha1.ClientIntents)
 	}
 
 	if !r.enableKafkaACLCreation {
-		r.RecordNormalEvent(intents, "KafkaACLCreationDisabled", "Kafka ACL creation is disabled, creation skipped")
+		r.RecordNormalEvent(intents, ReasonKafkaACLCreationDisabled, "Kafka ACL creation is disabled, creation skipped")
 	}
 
 	for serverName, _ := range intentsByServer {
 		if !r.KafkaServersStore.Exists(serverName.Name, serverName.Namespace) {
-			r.RecordWarningEventf(intents, "Kafka ACL reconcile failed", "broker %s not configured", serverName)
+			r.RecordWarningEventf(intents, ReasonKafkaServerNotConfigured, "broker %s not configured", serverName)
 			logrus.WithField("server", serverName).Warning("Did not apply intents to server - no server configuration was defined")
 		}
 	}
@@ -131,7 +138,7 @@ func (r *KafkaACLReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 		if controllerutil.ContainsFinalizer(intents, KafkaACLsFinalizerName) {
 			logger.Infof("Removing associated Acls")
 			if err := r.RemoveACLs(intents); err != nil {
-				r.RecordWarningEvent(intents, "could not remove Kafka ACLs", err.Error())
+				r.RecordWarningEventf(intents, ReasonRemovingKafkaACLsFailed, "Could not remove Kafka ACLs: %s", err.Error())
 				return ctrl.Result{}, err
 			}
 
@@ -150,12 +157,12 @@ func (r *KafkaACLReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	logger.Info("Applying new ACLs")
 	serverCount, err := r.applyACLs(intents)
 	if err != nil {
-		r.RecordWarningEvent(intents, "could not apply Kafka ACLs", err.Error())
+		r.RecordWarningEventf(intents, ReasonApplyingKafkaACLsFailed, "could not apply Kafka ACLs: %s", err.Error())
 		return ctrl.Result{}, err
 	}
 
 	if serverCount > 0 {
-		r.RecordNormalEventf(intents, "Kafka ACL reconcile complete", "Reconciled %d Kafka brokers", serverCount)
+		r.RecordNormalEventf(intents, ReasonAppliedKafkaACLs, "Kafka ACL reconcile complete, reconciled %d Kafka brokers", serverCount)
 	}
 	return ctrl.Result{}, nil
 

src/operator/controllers/intents_reconcilers/network_policy.go

@@ -21,6 +21,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 )
 
+const (
+	ReasonNetworkPolicyCreationDisabled = "NetworkPolicyCreationDisabled"
+	ReasonGettingNetworkPolicyFailed    = "GettingNetworkPolicyFailed"
+	ReasonRemovingNetworkPolicyFailed   = "RemovingNetworkPolicyFailed"
+	ReasonNamespaceNotAllowed           = "NamespaceNotAllowed"
+	ReasonCreatingNetworkPoliciesFailed = "CreatingNetworkPoliciesFailed"
+	ReasonCreatedNetworkPolicies        = "CreatedNetworkPolicies"
+)
+
 type NetworkPolicyReconciler struct {
 	client.Client
 	Scheme                      *runtime.Scheme
@@ -70,7 +79,7 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 			if k8serrors.IsConflict(err) {
 				return ctrl.Result{Requeue: true}, nil
 			}
-			r.RecordWarningEvent(intents, "could not remove network policies", err.Error())
+			r.RecordWarningEventf(intents, ReasonRemovingNetworkPolicyFailed, "could not remove network policies: %s", err.Error())
 			return ctrl.Result{}, err
 		}
 		return ctrl.Result{}, nil
@@ -90,26 +99,26 @@ func (r *NetworkPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		}
 		if len(r.RestrictToNamespaces) != 0 && !lo.Contains(r.RestrictToNamespaces, intent.Namespace) {
 			// Namespace is not in list of namespaces we're allowed to act in, so drop it.
-			r.RecordWarningEventf(intents, "namespace not allowed", "namespace %s was specified in intent, but is not allowed by configuration", intent.Namespace)
+			r.RecordWarningEventf(intents, ReasonNamespaceNotAllowed, "namespace %s was specified in intent, but is not allowed by configuration", intent.Namespace)
 			continue
 		}
 		err := r.handleNetworkPolicyCreation(ctx, intents, intent, req.Namespace)
 		if err != nil {
-			r.RecordWarningEvent(intents, "could not create network policies", err.Error())
+			r.RecordWarningEventf(intents, ReasonCreatingNetworkPoliciesFailed, "could not create network policies: %s", err.Error())
 			return ctrl.Result{}, err
 		}
 	}
 
 	if len(intents.GetCallsList()) > 0 {
-		r.RecordNormalEventf(intents, "NetworkPolicy reconcile complete", "Reconciled %d servers", len(intents.GetCallsList()))
+		r.RecordNormalEventf(intents, ReasonCreatedNetworkPolicies, "NetworkPolicy reconcile complete, reconciled %d servers", len(intents.GetCallsList()))
 	}
 	return ctrl.Result{}, nil
 }
 
 func (r *NetworkPolicyReconciler) handleNetworkPolicyCreation(
 	ctx context.Context, intentsObj *otterizev1alpha1.ClientIntents, intent otterizev1alpha1.Intent, intentsObjNamespace string) error {
 	if !r.enableNetworkPolicyCreation {
-		r.RecordNormalEvent(intentsObj, "NetworkPolicyCreationDisabled", "Network policy creation is disabled, creation skipped")
+		r.RecordNormalEvent(intentsObj, ReasonNetworkPolicyCreationDisabled, "Network policy creation is disabled, creation skipped")
 		return nil
 	}
 
@@ -169,7 +178,7 @@ func (r *NetworkPolicyReconciler) handleNetworkPolicyCreation(
 		return nil
 
 	} else if err != nil {
-		r.RecordWarningEvent(existingPolicy, "failed to get network policy", err.Error())
+		r.RecordWarningEventf(existingPolicy, ReasonGettingNetworkPolicyFailed, "failed to get network policy: %s", err.Error())
 		return err
 	}
 

src/operator/controllers/intents_reconcilers/pods.go

@@ -16,6 +16,12 @@ import (
 
 const PodLabelFinalizerName = "intents.otterize.com/pods-finalizer"
 
+const (
+	ReasonRemovingPodLabelsFailed = "RemovingPodLabelsFailed"
+	ReasonUpdatePodFailed         = "UpdatePodFailed"
+	ReasonListPodsFailed          = "ListPodsFailed"
+)
+
 type PodLabelReconciler struct {
 	client.Client
 	Scheme *runtime.Scheme
@@ -49,7 +55,7 @@ func (r *PodLabelReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 			if k8serrors.IsConflict(err) {
 				return ctrl.Result{Requeue: true}, nil
 			}
-			r.RecordWarningEvent(intents, "could not remove pod labels", err.Error())
+			r.RecordWarningEventf(intents, ReasonRemovingPodLabelsFailed, "could not remove pod labels: %s", err.Error())
 			return ctrl.Result{}, err
 		}
 		return ctrl.Result{}, nil
@@ -69,7 +75,7 @@ func (r *PodLabelReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	// List the pods in the namespace and update labels if required
 	labelSelector, err := intents.BuildPodLabelSelector()
 	if err != nil {
-		r.RecordWarningEvent(intents, "could not list pods", err.Error())
+		r.RecordWarningEventf(intents, ReasonListPodsFailed, "could not list pods: %s", err.Error())
 		return ctrl.Result{}, err
 	}
 
@@ -87,7 +93,7 @@ func (r *PodLabelReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 			updatedPod := otterizev1alpha1.UpdateOtterizeAccessLabels(pod.DeepCopy(), intentLabels)
 			err := r.Patch(ctx, updatedPod, client.MergeFrom(&pod))
 			if err != nil {
-				r.RecordWarningEvent(intents, "could not update pod", err.Error())
+				r.RecordWarningEventf(intents, ReasonUpdatePodFailed, "could not update pod: %s", err.Error())
 				return ctrl.Result{}, err
 			}
 		}

src/operator/controllers/kafkaserverconfig_controller.go

@@ -36,7 +36,10 @@ import (
 )
 
 const (
-	finalizerName = "intents.otterize.com/kafkaserverconfig-finalizer"
+	finalizerName                              = "intents.otterize.com/kafkaserverconfig-finalizer"
+	ReasonIntentsOperatorIdentityResolveFailed = "IntentsOperatorIdentityResolveFailed"
+	ReasonApplyingKafkaServerConfigFailed      = "ApplyingKafkaServerConfigFailed"
+	ReasonAppliedKafkaServerConfigFailed       = "AppliedKafkaServerConfigFailed"
 )
 
 // KafkaServerConfigReconciler reconciles a KafkaServerConfig object
@@ -167,7 +170,7 @@ func (r *KafkaServerConfigReconciler) createIntentsFromOperatorToKafkaServer(ctx
 
 	annotatedServiceName, ok := serviceidresolver.ResolvePodToServiceIdentityUsingAnnotationOnly(operatorPod)
 	if !ok {
-		r.RecordWarningEventf(config, "IntentsOperatorIdentityResolveFailed", "failed resolving intents operator identity - service name annotation required")
+		r.RecordWarningEventf(config, ReasonIntentsOperatorIdentityResolveFailed, "failed resolving intents operator identity - service name annotation required")
 		return fmt.Errorf("failed resolving intents operator identity - service name annotation required")
 	}
 
@@ -251,11 +254,11 @@ func (r *KafkaServerConfigReconciler) Reconcile(ctx context.Context, req ctrl.Re
 	defer kafkaIntentsAdmin.Close()
 
 	if err := kafkaIntentsAdmin.ApplyServerTopicsConf(kafkaServerConfig.Spec.Topics); err != nil {
-		r.RecordWarningEvent(kafkaServerConfig, "failed to apply server config to Kafka broker", err.Error())
+		r.RecordWarningEventf(kafkaServerConfig, ReasonApplyingKafkaServerConfigFailed, "failed to apply server config to Kafka broker: %s", err.Error())
 		return ctrl.Result{}, err
 	}
 
-	r.RecordNormalEvent(kafkaServerConfig, "successfully applied server config", "")
+	r.RecordNormalEvent(kafkaServerConfig, ReasonAppliedKafkaServerConfigFailed, "successfully applied server config")
 	return ctrl.Result{}, nil
 }