add tests for condition event recording

Author: Diaphteiros

pkg/conditions/updater.go

@@ -13,17 +13,20 @@ import (
 	"k8s.io/client-go/tools/record"
 )
 
+const EventReasonConditionChanged = "ConditionChanged"
+
 type EventVerbosity string
 
 const (
 	// EventPerChange causes one event to be recorded for each condition that has changed.
 	// This is the most verbose setting. The old and new status of each changed condition will be visible in the event message.
 	EventPerChange EventVerbosity = "perChange"
 	// EventPerNewStatus causes one event to be recorded for each new status that any condition has reached.
-	// This means that at max three events will be recorded:
-	// - the following conditions changed to True
-	// - the following conditions changed to False
-	// - the following conditions changed to Unknown
+	// This means that at max four events will be recorded:
+	// - the following conditions changed to True (including newly added conditions)
+	// - the following conditions changed to False (including newly added conditions)
+	// - the following conditions changed to Unknown (including newly added conditions)
+	// - the following conditions were removed
 	// The old status of the conditions will not be part of the event message.
 	EventPerNewStatus EventVerbosity = "perNewStatus"
 	// EventIfChanged causes a single event to be recorded if any condition's status has changed.
@@ -188,16 +191,16 @@ func (c *conditionUpdater) Record(obj runtime.Object) *conditionUpdater {
 		for _, con := range updatedCons {
 			oldCon, found := c.original[con.Type]
 			if !found {
-				c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "Condition '%s' added with status '%s'", con.Type, string(con.Status))
+				c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, EventReasonConditionChanged, "Condition '%s' added with status '%s'", con.Type, con.Status)
 				continue
 			}
 			if con.Status != oldCon.Status {
-				c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "Condition '%s' changed from '%s' to '%s'", con.Type, string(oldCon.Status), string(con.Status))
+				c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, EventReasonConditionChanged, "Condition '%s' changed from '%s' to '%s'", con.Type, oldCon.Status, con.Status)
 				continue
 			}
 		}
 		for conType, oldStatus := range lostCons {
-			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "Condition '%s' with status '%s' removed", conType, string(oldStatus))
+			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, EventReasonConditionChanged, "Condition '%s' with status '%s' removed", conType, oldStatus)
 		}
 
 	case EventPerNewStatus:
@@ -221,16 +224,16 @@ func (c *conditionUpdater) Record(obj runtime.Object) *conditionUpdater {
 		}
 
 		if trueCons.Len() > 0 {
-			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "The following conditions changed to 'True': %s", strings.Join(sets.List(trueCons), ", "))
+			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, EventReasonConditionChanged, "The following conditions changed to 'True': %s", strings.Join(sets.List(trueCons), ", "))
 		}
 		if falseCons.Len() > 0 {
-			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "The following conditions changed to 'False': %s", strings.Join(sets.List(falseCons), ", "))
+			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, EventReasonConditionChanged, "The following conditions changed to 'False': %s", strings.Join(sets.List(falseCons), ", "))
 		}
 		if unknownCons.Len() > 0 {
-			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "The following conditions changed to 'Unknown': %s", strings.Join(sets.List(unknownCons), ", "))
+			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, EventReasonConditionChanged, "The following conditions changed to 'Unknown': %s", strings.Join(sets.List(unknownCons), ", "))
 		}
 		if len(lostCons) > 0 {
-			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "The following conditions were removed: %s", strings.Join(sets.List(sets.KeySet(lostCons)), ", "))
+			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, EventReasonConditionChanged, "The following conditions were removed: %s", strings.Join(sets.List(sets.KeySet(lostCons)), ", "))
 		}
 
 	case EventIfChanged:
@@ -244,7 +247,7 @@ func (c *conditionUpdater) Record(obj runtime.Object) *conditionUpdater {
 			changedCons.Insert(conType)
 		}
 		if changedCons.Len() > 0 {
-			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "The following conditions have changed: %s", strings.Join(sets.List(changedCons), ", "))
+			c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, EventReasonConditionChanged, "The following conditions have changed: %s", strings.Join(sets.List(changedCons), ", "))
 		}
 	}
 

pkg/conditions/updater_test.go

@@ -11,6 +11,8 @@ import (
 	. "github.com/openmcp-project/controller-utils/pkg/testing/matchers"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/client-go/tools/record"
 
 	"github.com/openmcp-project/controller-utils/pkg/conditions"
 )
@@ -213,4 +215,205 @@ var _ = Describe("Conditions", func() {
 
 	})
 
+	Context("EventRecorder", func() {
+
+		var recorder *record.FakeRecorder
+
+		dummy := &dummyObject{
+			TypeMeta: metav1.TypeMeta{
+				Kind:       "Dummy",
+				APIVersion: "dummy/v1",
+			},
+		}
+
+		BeforeEach(func() {
+			recorder = record.NewFakeRecorder(100)
+		})
+
+		AfterEach(func() {
+			close(recorder.Events)
+		})
+
+		Context("Verbosity: EventPerChange", func() {
+
+			It("should record an event for each changed condition status", func() {
+				cons := testConditionSet()
+				updater := conditions.ConditionUpdater(cons, false).WithEventRecorder(recorder, conditions.EventPerChange)
+				trueCon1 := conditions.GetCondition(cons, "true")
+				trueCon2 := conditions.GetCondition(cons, "alsoTrue")
+				_, changed := updater.
+					UpdateCondition(trueCon1.Type, invert(trueCon1.Status), trueCon1.ObservedGeneration+1, "newReason", "newMessage").
+					UpdateCondition(trueCon2.Type, invert(trueCon2.Status), trueCon2.ObservedGeneration+1, "newReason", "newMessage").
+					Record(dummy).Conditions()
+				Expect(changed).To(BeTrue())
+
+				events := flush(recorder.Events)
+				Expect(events).To(ConsistOf(
+					ContainSubstring("Condition '%s' changed from '%s' to '%s'", trueCon1.Type, trueCon1.Status, invert(trueCon1.Status)),
+					ContainSubstring("Condition '%s' changed from '%s' to '%s'", trueCon2.Type, trueCon2.Status, invert(trueCon2.Status)),
+				))
+			})
+
+			It("should not record any events if no condition status changed, even if other fields changed", func() {
+				cons := testConditionSet()
+				updater := conditions.ConditionUpdater(cons, false).WithEventRecorder(recorder, conditions.EventPerChange)
+				trueCon1 := conditions.GetCondition(cons, "true")
+				trueCon2 := conditions.GetCondition(cons, "alsoTrue")
+				_, changed := updater.
+					UpdateCondition(trueCon1.Type, trueCon1.Status, trueCon1.ObservedGeneration+1, "newReason", "newMessage").
+					UpdateCondition(trueCon2.Type, trueCon2.Status, trueCon2.ObservedGeneration+1, "newReason", "newMessage").
+					Record(dummy).Conditions()
+				Expect(changed).To(BeTrue())
+
+				events := flush(recorder.Events)
+				Expect(events).To(BeEmpty())
+			})
+
+			It("should record added and lost conditions", func() {
+				cons := testConditionSet()
+				updater := conditions.ConditionUpdater(cons, false).WithEventRecorder(recorder, conditions.EventPerChange)
+				_, changed := updater.
+					UpdateCondition("new", metav1.ConditionUnknown, 1, "newReason", "newMessage").
+					RemoveCondition("true").
+					Record(dummy).Conditions()
+				Expect(changed).To(BeTrue())
+
+				events := flush(recorder.Events)
+				Expect(events).To(ConsistOf(
+					ContainSubstring("Condition 'new' added with status '%s'", metav1.ConditionUnknown),
+					ContainSubstring("Condition 'true' with status '%s' removed", metav1.ConditionTrue),
+				))
+			})
+
+		})
+
+		Context("Verbosity: EventPerNewStatus", func() {
+
+			It("should record an event for each new status that any condition has reached", func() {
+				cons := testConditionSet()
+				updater := conditions.ConditionUpdater(cons, false).WithEventRecorder(recorder, conditions.EventPerNewStatus)
+				trueCon1 := conditions.GetCondition(cons, "true")
+				trueCon2 := conditions.GetCondition(cons, "alsoTrue")
+				falseCon := conditions.GetCondition(cons, "false")
+				_, changed := updater.
+					UpdateCondition(trueCon1.Type, metav1.ConditionUnknown, trueCon1.ObservedGeneration+1, "newReason", "newMessage").
+					UpdateCondition(trueCon2.Type, metav1.ConditionUnknown, trueCon2.ObservedGeneration+1, "newReason", "newMessage").
+					UpdateCondition("anotherOne", metav1.ConditionUnknown, 1, "newReason", "newMessage").
+					UpdateCondition(falseCon.Type, invert(falseCon.Status), falseCon.ObservedGeneration+1, "newReason", "newMessage").
+					Record(dummy).Conditions()
+				Expect(changed).To(BeTrue())
+
+				events := flush(recorder.Events)
+				Expect(events).To(ConsistOf(
+					And(
+						ContainSubstring("The following conditions changed to '%s'", metav1.ConditionUnknown),
+						ContainSubstring(trueCon1.Type),
+						ContainSubstring(trueCon2.Type),
+						ContainSubstring("anotherOne"),
+					),
+					And(
+						ContainSubstring("The following conditions changed to '%s'", invert(falseCon.Status)),
+						ContainSubstring(falseCon.Type),
+					),
+				))
+			})
+
+			It("should not record any events if no condition status changed, even if other fields changed", func() {
+				cons := testConditionSet()
+				updater := conditions.ConditionUpdater(cons, false).WithEventRecorder(recorder, conditions.EventPerNewStatus)
+				trueCon1 := conditions.GetCondition(cons, "true")
+				trueCon2 := conditions.GetCondition(cons, "alsoTrue")
+				_, changed := updater.
+					UpdateCondition(trueCon1.Type, trueCon1.Status, trueCon1.ObservedGeneration+1, "newReason", "newMessage").
+					UpdateCondition(trueCon2.Type, trueCon2.Status, trueCon2.ObservedGeneration+1, "newReason", "newMessage").
+					Record(dummy).Conditions()
+				Expect(changed).To(BeTrue())
+
+				events := flush(recorder.Events)
+				Expect(events).To(BeEmpty())
+			})
+
+			It("should record lost conditions", func() {
+				cons := testConditionSet()
+				updater := conditions.ConditionUpdater(cons, true).WithEventRecorder(recorder, conditions.EventPerNewStatus)
+				_, changed := updater.Record(dummy).Conditions()
+				Expect(changed).To(BeTrue())
+
+				events := flush(recorder.Events)
+				Expect(events).To(ConsistOf(
+					And(
+						ContainSubstring("The following conditions were removed"),
+						ContainSubstring("true"),
+						ContainSubstring("false"),
+						ContainSubstring("alsoTrue"),
+					),
+				))
+			})
+
+		})
+
+		Context("Verbosity: EventIfChanged", func() {
+
+			It("should only record a single event, no matter how many conditions changed", func() {
+				cons := testConditionSet()
+				updater := conditions.ConditionUpdater(cons, false).WithEventRecorder(recorder, conditions.EventIfChanged)
+				trueCon := conditions.GetCondition(cons, "true")
+				falseCon := conditions.GetCondition(cons, "false")
+				_, changed := updater.
+					UpdateCondition(trueCon.Type, invert(trueCon.Status), trueCon.ObservedGeneration+1, "newReason", "newMessage").
+					UpdateCondition(falseCon.Type, invert(falseCon.Status), falseCon.ObservedGeneration+1, "newReason", "newMessage").
+					UpdateCondition("new", metav1.ConditionUnknown, 1, "newReason", "newMessage").
+					RemoveCondition("alsoTrue").
+					Record(dummy).Conditions()
+				Expect(changed).To(BeTrue())
+
+				events := flush(recorder.Events)
+				Expect(events).To(ConsistOf(
+					And(
+						ContainSubstring("The following conditions have changed"),
+						ContainSubstring("true"),
+						ContainSubstring("false"),
+						ContainSubstring("alsoTrue"),
+						ContainSubstring("new"),
+					),
+				))
+			})
+
+			It("should not record any events if no condition status changed, even if other fields changed", func() {
+				cons := testConditionSet()
+				updater := conditions.ConditionUpdater(cons, false).WithEventRecorder(recorder, conditions.EventIfChanged)
+				trueCon1 := conditions.GetCondition(cons, "true")
+				trueCon2 := conditions.GetCondition(cons, "alsoTrue")
+				_, changed := updater.
+					UpdateCondition(trueCon1.Type, trueCon1.Status, trueCon1.ObservedGeneration+1, "newReason", "newMessage").
+					UpdateCondition(trueCon2.Type, trueCon2.Status, trueCon2.ObservedGeneration+1, "newReason", "newMessage").
+					Record(dummy).Conditions()
+				Expect(changed).To(BeTrue())
+
+				events := flush(recorder.Events)
+				Expect(events).To(BeEmpty())
+			})
+
+		})
+
+	})
+
 })
+
+type dummyObject struct {
+	metav1.TypeMeta `json:",inline"`
+}
+
+func (d *dummyObject) DeepCopyObject() runtime.Object {
+	return &dummyObject{
+		TypeMeta: d.TypeMeta,
+	}
+}
+
+func flush(c chan string) []string {
+	res := []string{}
+	for len(c) > 0 {
+		res = append(res, <-c)
+	}
+	return res
+}