add skipReasons arg and extend tests (#275)

.github/workflows/e2e.yaml

@@ -33,7 +33,7 @@ jobs:
             --selector=app=event-logger-example-eventlogger \
             --timeout=90s
 
-      - name: Start test pod
+      - name: Start/Stop test pod
         run: |
           kubectl apply -f testdata/e2e/test-pod.yaml
           kubectl wait --namespace e2e-test --for=condition=ready pod nginx --timeout=90s

Makefile

@@ -18,7 +18,9 @@ test: tidy test-ci fmt test-ci
 
 # Run tests
 test-ci: generate mocks manifests
-	go test ./... -coverprofile cover.out
+	go test ./... -coverprofile cover.out.tmp
+	@cat cover.out.tmp | grep -v "zz_generated.deepcopy.go" > cover.out # filter coverage of generated code
+	@rm -f cover.out.tmp
 
 # Build manager binary
 manager: generate fmt

README.md

@@ -30,13 +30,15 @@ spec:
     - name: DeploymentConfig # the kind of the event source to be logged
       apiGroup: apps.openshift.io # optional
       eventTypes: # optional
-       - Normal
-       - Warning
+        - Normal
+        - Warning
       reasons: # optional
-       - DeploymentCreated
-       - ReplicationControllerScaled
+        - DeploymentCreated
+        - ReplicationControllerScaled
+      skipReasons: # optional
+        - Unhealthy
       matchingPatterns: # optional - regexp pattern to match event messages
-       - .*
+        - .*
       skipOnMatch: false # optional - skip events where messages match the pattern. Default false
 
 
@@ -58,12 +60,12 @@ spec:
     key: value
 
   serviceAccount: "sa" # optional - if a custom ServiceAccount should be used for the pod. Default ServiceAccount is automatically created
-  
+
   logFields: # optional - map if custom log field names. Key then log field name / Value: the reflection fields to the value within the struct corev1.Event https://github.com/kubernetes/api/blob/master/core/v1/types.go
     - name: name
       path:
         - InvolvedObject
-        - Name 
+        - Name
     - name: kind
       path:
         - InvolvedObject

api/v1/eventlogger_types.go

@@ -75,6 +75,10 @@ type Kind struct {
 	// +kubebuilder:validation:MinItems=0
 	Reasons []string `json:"reasons,omitempty"`
 
+	// SkipReasons event reasons to log to skip. If empty events with any reasons are logged.
+	// +kubebuilder:validation:MinItems=0
+	SkipReasons []string `json:"skipReasons,omitempty"`
+
 	// MatchingPatterns optional regex pattern that must be contained in the message to be logged
 	// +kubebuilder:validation:MinItems=0
 	MatchingPatterns []string `json:"matchingPatterns,omitempty"`

api/v1/eventlogger_types_test.go

@@ -2,10 +2,13 @@ package v1_test
 
 import (
 	"encoding/json"
+	"errors"
 
 	v1 "github.com/bakito/k8s-event-logger-operator/api/v1"
+	"github.com/bakito/k8s-event-logger-operator/version"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
 )
 
@@ -38,4 +41,23 @@ var _ = Describe("V1", func() {
 			Ω(string(b)).Should(Equal(`{"name":"a","apiGroup":"b"}`))
 		})
 	})
+	Context("Apply", func() {
+		var el *v1.EventLogger
+		BeforeEach(func() {
+			el = &v1.EventLogger{}
+		})
+		It("should not set an error message", func() {
+			el.Apply(nil)
+			Ω(el.Status.Error).Should(BeEmpty())
+			Ω(el.Status.OperatorVersion).Should(Equal(version.Version))
+			Ω(el.Status.LastProcessed).ShouldNot(Equal(Equal(metav1.Time{})))
+		})
+		It("should set an error message", func() {
+			err := errors.New("this is an error")
+			el.Apply(err)
+			Ω(el.Status.Error).Should(Equal(err.Error()))
+			Ω(el.Status.OperatorVersion).Should(Equal(version.Version))
+			Ω(el.Status.LastProcessed).ShouldNot(Equal(Equal(metav1.Time{})))
+		})
+	})
 })

api/v1/eventlogger_webhook.go

@@ -39,7 +39,7 @@ func (in *EventLogger) ValidateCreate() error {
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
-func (in *EventLogger) ValidateUpdate(old runtime.Object) error {
+func (in *EventLogger) ValidateUpdate(_ runtime.Object) error {
 	return in.Spec.Validate()
 }
 

api/v1/eventlogger_webhook_test.go

@@ -0,0 +1,53 @@
+package v1_test
+
+import (
+	v1 "github.com/bakito/k8s-event-logger-operator/api/v1"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("V1", func() {
+	var el *v1.EventLogger
+	BeforeEach(func() {
+		el = &v1.EventLogger{
+			Spec: v1.EventLoggerSpec{},
+		}
+	})
+	Context("Valid", func() {
+		Context("ValidateCreate", func() {
+			It("should be valid", func() {
+				Ω(el.ValidateCreate()).ShouldNot(HaveOccurred())
+			})
+		})
+		Context("ValidateUpdate", func() {
+			It("should be valid", func() {
+				Ω(el.ValidateUpdate(nil)).ShouldNot(HaveOccurred())
+			})
+		})
+		Context("ValidateUpdate", func() {
+			It("should be nil", func() {
+				Ω(el.ValidateDelete()).Should(BeNil())
+			})
+		})
+	})
+	Context("Invalid", func() {
+		BeforeEach(func() {
+			el.Spec.Labels = map[string]string{"in valid": "valid"}
+		})
+		Context("ValidateCreate", func() {
+			It("should be invalid", func() {
+				Ω(el.ValidateCreate()).Should(HaveOccurred())
+			})
+		})
+		Context("ValidateUpdate", func() {
+			It("should be invalid", func() {
+				Ω(el.ValidateUpdate(nil)).Should(HaveOccurred())
+			})
+		})
+		Context("ValidateUpdate", func() {
+			It("should be nil", func() {
+				Ω(el.ValidateDelete()).Should(BeNil())
+			})
+		})
+	})
+})

controllers/logging/event_controller.go

@@ -31,7 +31,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
@@ -186,19 +185,14 @@ func (p *loggingPredicate) logEvent(e runtime.Object) bool {
 	return false
 }
 
-func getLatestRevision(ctx context.Context, mgr manager.Manager, namespace string) (string, error) {
-	cl, err := client.New(mgr.GetConfig(), client.Options{})
-	if err != nil {
-		return "", err
-	}
-
+func getLatestRevision(ctx context.Context, cl client.Client, namespace string) (string, error) {
 	eventList := &corev1.EventList{}
 	opts := []client.ListOption{
 		client.Limit(0),
 		client.InNamespace(namespace),
 	}
 
-	err = cl.List(ctx, eventList, opts...)
+	err := cl.List(ctx, eventList, opts...)
 	if err != nil {
 		return "", err
 	}
@@ -207,7 +201,12 @@ func getLatestRevision(ctx context.Context, mgr manager.Manager, namespace strin
 
 // SetupWithManager setup with manager
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager, namespace string) error {
-	lv, err := getLatestRevision(context.Background(), mgr, namespace)
+	cl, err := client.New(mgr.GetConfig(), client.Options{})
+	if err != nil {
+		return err
+	}
+
+	lv, err := getLatestRevision(context.Background(), cl, namespace)
 	if err != nil {
 		return err
 	}

controllers/logging/event_controller_test.go

@@ -21,6 +21,7 @@ import (
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
@@ -34,10 +35,12 @@ var _ = Describe("Logging", func() {
 	var (
 		mockCtrl *gm.Controller
 		ctx      context.Context
+		cl       *mc.MockClient
 	)
 
 	BeforeEach(func() {
 		mockCtrl = gm.NewController(GinkgoT())
+		cl = mc.NewMockClient(mockCtrl)
 		ctx = context.Background()
 	})
 	AfterEach(func() {
@@ -48,14 +51,12 @@ var _ = Describe("Logging", func() {
 		var (
 			s   *runtime.Scheme
 			r   *Reconciler
-			cl  *mc.MockClient
 			req reconcile.Request
 		)
 
 		BeforeEach(func() {
 			s = scheme.Scheme
 			Ω(v1.SchemeBuilder.AddToScheme(s)).ShouldNot(HaveOccurred())
-			cl = mc.NewMockClient(mockCtrl)
 			r = &Reconciler{
 				Client:     cl,
 				Log:        ctrl.Log.WithName("controllers").WithName("Event"),
@@ -112,11 +113,22 @@ var _ = Describe("Logging", func() {
 			lp := &loggingPredicate{}
 			lp.logEvent(&corev1.Event{})
 		})
+		It("should log nothing if object is not an event", func() {
+			mockSink.EXPECT().WithValues().Times(0)
+
+			lp := &loggingPredicate{
+				lastVersion: "2",
+				Config:      &Config{filter: filter.Always},
+			}
+
+			lp.logEvent(&corev1.Pod{})
+		})
 		It("should log nothing if resource version does not match", func() {
 			mockSink.EXPECT().WithValues().Times(0)
 
 			lp := &loggingPredicate{
 				lastVersion: "2",
+				Config:      &Config{filter: filter.Always},
 			}
 
 			lp.logEvent(&corev1.Event{
@@ -331,6 +343,31 @@ var _ = Describe("Logging", func() {
 				true,
 				"( ( ( Kind == 'Pod' AND ( true XOR ( Message matches /.*Message.*/ ) ) ) ) )",
 			),
+			Entry("23",
+
+				v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", EventTypes: []string{}, SkipReasons: []string{"Created", "Started"}}}},
+				corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Reason: "Created"},
+				false,
+				"( ( ( Kind == 'Pod' AND Reason NOT in [Created, Started] ) ) )",
+			),
+			Entry("24",
+
+				v1.EventLoggerSpec{Kinds: []v1.Kind{{Name: "Pod", EventTypes: []string{}, SkipReasons: []string{"Created", "Started"}}}},
+				corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Reason: "Killing"},
+				true,
+				"( ( ( Kind == 'Pod' AND Reason NOT in [Created, Started] ) ) )",
+			),
+			Entry("25",
+
+				v1.EventLoggerSpec{Kinds: []v1.Kind{{
+					Name: "Pod", EventTypes: []string{},
+					SkipReasons: []string{"Created"},
+					Reasons:     []string{"Created"},
+				}}},
+				corev1.Event{InvolvedObject: corev1.ObjectReference{Kind: "Pod"}, Reason: "Created"},
+				false,
+				"( ( ( Kind == 'Pod' AND Reason NOT in [Created] AND Reason in [Created] ) ) )",
+			),
 		)
 	})
 
@@ -437,6 +474,16 @@ var _ = Describe("Logging", func() {
 			})
 		})
 	})
+	Context("getLatestRevision", func() {
+		It("should find the last revision", func() {
+			cl.EXPECT().List(gm.Any(), gm.Any(), gm.Any()).Do(func(_ context.Context, el *corev1.EventList, _ ...client.ListOption) {
+				el.ResourceVersion = "3"
+			})
+			rev, err := getLatestRevision(ctx, cl, "")
+			Ω(err).ShouldNot(HaveOccurred())
+			Ω(rev).Should(Equal("3"))
+		})
+	})
 })
 
 type sld struct {

controllers/logging/types.go

@@ -60,6 +60,12 @@ func newFilterForKind(k eventloggerv1.Kind) filter.Filter {
 		}, fmt.Sprintf("EventType in [%s]", strings.Join(k.EventTypes, ", "))))
 	}
 
+	if len(k.SkipReasons) > 0 {
+		filters = append(filters, filter.New(func(e *corev1.Event) bool {
+			return !contains(k.SkipReasons, e.Reason)
+		}, fmt.Sprintf("Reason NOT in [%s]", strings.Join(k.SkipReasons, ", "))))
+	}
+
 	if len(k.Reasons) > 0 {
 		filters = append(filters, filter.New(func(e *corev1.Event) bool {
 			return contains(k.Reasons, e.Reason)

controllers/logging/types_test.go

@@ -0,0 +1,21 @@
+package logging
+
+import (
+	"github.com/google/uuid"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Logging", func() {
+	Context("ConfigFor", func() {
+		It("should create a correct config", func() {
+			name := uuid.NewString()
+			podNs := uuid.NewString()
+			watchNs := uuid.NewString()
+			cfg := ConfigFor(name, podNs, watchNs)
+			Ω(cfg.name).Should(Equal(name))
+			Ω(cfg.podNamespace).Should(Equal(podNs))
+			Ω(cfg.watchNamespace).Should(Equal(watchNs))
+		})
+	})
+})

helm/Chart.yaml

@@ -8,7 +8,5 @@ sources:
   - https://github.com/bakito/k8s-event-logger-operator
 annotations:
   artifacthub.io/changes: |
-    - Allow setting securityContext for controller and logger pods
-    - Logger Pod container is configured via a ConfigMap
-    - Allow overwriting image.tag
+    - add skipReasons property for EventLogger CRD
 icon: https://raw.githubusercontent.com/bakito/k8s-event-logger-operator/main/docs/chart-logo.png

helm/crds/eventlogger.bakito.ch_eventloggers.yaml

@@ -82,6 +82,13 @@ spec:
                     skipOnMatch:
                       description: SkipOnMatch skip the entry if matched
                       type: boolean
+                    skipReasons:
+                      description: SkipReasons event reasons to log to skip. If empty
+                        events with any reasons are logged.
+                      items:
+                        type: string
+                      minItems: 0
+                      type: array
                   required:
                   - name
                   type: object

testdata/e2e/buildImage.sh

@@ -1,4 +1,4 @@
 #!/bin/bash
 set -e
 docker build -f Dockerfile --build-arg VERSION=e2e-tests -t localhost:5001/k8s-event-logger:e2e .
-docker push  localhost:5001/k8s-event-logger:e2e
+docker push localhost:5001/k8s-event-logger:e2e