From 6405b792875058555b64021fdc3ba7e466f804c6 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Wed, 16 Jul 2025 14:37:46 +0200 Subject: [PATCH 1/4] enable event recording for changed conditions --- pkg/conditions/updater.go | 199 +++++++++++++++++++++++++------ pkg/controller/status_updater.go | 17 ++- 2 files changed, 181 insertions(+), 35 deletions(-) diff --git a/pkg/conditions/updater.go b/pkg/conditions/updater.go index 3785570..e9c55f2 100644 --- a/pkg/conditions/updater.go +++ b/pkg/conditions/updater.go @@ -1,20 +1,46 @@ package conditions import ( + "reflect" "slices" "strings" + "github.com/openmcp-project/controller-utils/pkg/collections" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/record" +) + +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 + // 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. + // All changed conditions will be listed, but not their old or new status. + EventIfChanged EventVerbosity = "ifChanged" ) // conditionUpdater is a helper struct for updating a list of Conditions. // Use the ConditionUpdater constructor for initializing. type conditionUpdater struct { - Now metav1.Time - conditions map[string]metav1.Condition - updated sets.Set[string] - changed bool + Now metav1.Time + conditions map[string]metav1.Condition + original map[string]metav1.Condition + eventRecoder record.EventRecorder + eventVerbosity EventVerbosity + updates map[string]metav1.ConditionStatus + removeUntouched bool } // ConditionUpdater creates a builder-like helper struct for updating a list of Conditions. @@ -31,19 +57,29 @@ type conditionUpdater struct { // status.conditions = ConditionUpdater(status.conditions, true).UpdateCondition(...).UpdateCondition(...).Conditions() func ConditionUpdater(conditions []metav1.Condition, removeUntouched bool) *conditionUpdater { res := &conditionUpdater{ - Now: metav1.Now(), - conditions: make(map[string]metav1.Condition, len(conditions)), - changed: false, + Now: metav1.Now(), + conditions: make(map[string]metav1.Condition, len(conditions)), + updates: make(map[string]metav1.ConditionStatus), + removeUntouched: removeUntouched, + original: make(map[string]metav1.Condition, len(conditions)), } for _, con := range conditions { res.conditions[con.Type] = con - } - if removeUntouched { - res.updated = sets.New[string]() + res.original[con.Type] = con } return res } +// WithEventRecorder enables event recording for condition changes. +// Note that this method must be called before any UpdateCondition calls, otherwise the events for the conditions will not be recorded. +// The verbosity argument controls how many events are recorded and what information they contain. +// If the event recorder is nil, no events will be recorded. +func (c *conditionUpdater) WithEventRecorder(recorder record.EventRecorder, verbosity EventVerbosity) *conditionUpdater { + c.eventRecoder = recorder + c.eventVerbosity = verbosity + return c +} + // UpdateCondition updates or creates the condition with the specified type. // All fields of the condition are updated with the values given in the arguments, but the condition's LastTransitionTime is only updated (with the timestamp contained in the receiver struct) if the status changed. // Returns the receiver for easy chaining. @@ -61,17 +97,8 @@ func (c *conditionUpdater) UpdateCondition(conType string, status metav1.Conditi // update LastTransitionTime only if status changed con.LastTransitionTime = old.LastTransitionTime } - if !c.changed { - if ok { - c.changed = old.Status != con.Status || old.Reason != con.Reason || old.Message != con.Message - } else { - c.changed = true - } - } + c.updates[conType] = status c.conditions[conType] = con - if c.updated != nil { - c.updated.Insert(conType) - } return c } @@ -83,7 +110,8 @@ func (c *conditionUpdater) UpdateConditionFromTemplate(con metav1.Condition) *co // HasCondition returns true if a condition with the given type exists in the updated condition list. func (c *conditionUpdater) HasCondition(conType string) bool { _, ok := c.conditions[conType] - return ok && (c.updated == nil || c.updated.Has(conType)) + _, updated := c.updates[conType] + return ok && (!c.removeUntouched || updated) } // RemoveCondition removes the condition with the given type from the updated condition list. @@ -92,10 +120,7 @@ func (c *conditionUpdater) RemoveCondition(conType string) *conditionUpdater { return c } delete(c.conditions, conType) - if c.updated != nil { - c.updated.Delete(conType) - } - c.changed = true + delete(c.updates, conType) return c } @@ -105,20 +130,126 @@ func (c *conditionUpdater) RemoveCondition(conType string) *conditionUpdater { // The conditions are returned sorted by their type. // The second return value indicates whether the condition list has actually changed. func (c *conditionUpdater) Conditions() ([]metav1.Condition, bool) { + res := c.updatedConditions() + slices.SortStableFunc(res, func(a, b metav1.Condition) int { + return strings.Compare(a.Type, b.Type) + }) + return res, c.changed(res) +} + +func (c *conditionUpdater) updatedConditions() []metav1.Condition { res := make([]metav1.Condition, 0, len(c.conditions)) for _, con := range c.conditions { - if c.updated == nil { + if _, updated := c.updates[con.Type]; updated || !c.removeUntouched { res = append(res, con) - continue } - if c.updated.Has(con.Type) { - res = append(res, con) - } else { - c.changed = true + } + return res +} + +func (c *conditionUpdater) changed(newCons []metav1.Condition) bool { + if len(c.original) != len(newCons) { + return true + } + for _, newCon := range newCons { + oldCon, found := c.original[newCon.Type] + if !found || !reflect.DeepEqual(newCon, oldCon) { + return true } } - slices.SortStableFunc(res, func(a, b metav1.Condition) int { - return strings.Compare(a.Type, b.Type) + return false +} + +// Record records events for the updated conditions on the given object. +// Which events are recorded depends on the eventVerbosity setting. +// In any setting, events are only recorded for conditions that have somehow changed. +// This is a no-op if either the event recorder or the given object is nil. +// Note that events will be duplicated if this method is called multiple times. +// Returns the receiver for easy chaining. +func (c *conditionUpdater) Record(obj runtime.Object) *conditionUpdater { + if c.eventRecoder == nil || obj == nil { + return c + } + + updatedCons := c.updatedConditions() + if !c.changed(updatedCons) { + // nothing to do if there are no changes + return c + } + lostCons := collections.ProjectMapToMap(c.original, func(conType string, con metav1.Condition) (string, metav1.ConditionStatus) { + return conType, con.Status }) - return res, c.changed + for _, con := range updatedCons { + delete(lostCons, con.Type) + } + + switch c.eventVerbosity { + case EventPerChange: + 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)) + 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)) + continue + } + } + for conType, oldStatus := range lostCons { + c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "Condition '%s' with status '%s' removed", conType, string(oldStatus)) + } + + case EventPerNewStatus: + trueCons := sets.New[string]() + falseCons := sets.New[string]() + unknownCons := sets.New[string]() + + for _, con := range updatedCons { + // only add conditions that have changed + if oldCon, found := c.original[con.Type]; found && con.Status == oldCon.Status { + continue + } + switch con.Status { + case metav1.ConditionTrue: + trueCons.Insert(con.Type) + case metav1.ConditionFalse: + falseCons.Insert(con.Type) + case metav1.ConditionUnknown: + unknownCons.Insert(con.Type) + } + } + + if trueCons.Len() > 0 { + c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "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), ", ")) + } + if unknownCons.Len() > 0 { + c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "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)), ", ")) + } + + case EventIfChanged: + changedCons := sets.New[string]() + for _, con := range updatedCons { + if oldCon, found := c.original[con.Type]; !found || con.Status != oldCon.Status { + changedCons.Insert(con.Type) + } + } + for conType := range lostCons { + changedCons.Insert(conType) + } + if changedCons.Len() > 0 { + c.eventRecoder.Eventf(obj, corev1.EventTypeNormal, "The following conditions have changed: %s", strings.Join(sets.List(changedCons), ", ")) + } + } + + ns := &corev1.Namespace{} + ns.GetObjectKind() + + return c } diff --git a/pkg/controller/status_updater.go b/pkg/controller/status_updater.go index cb2d695..326cb63 100644 --- a/pkg/controller/status_updater.go +++ b/pkg/controller/status_updater.go @@ -10,6 +10,7 @@ import ( "github.com/openmcp-project/controller-utils/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -90,6 +91,15 @@ func (b *StatusUpdaterBuilder[Obj]) WithConditionUpdater(removeUntouchedConditio return b } +// WithConditionEvents sets the event recorder and the verbosity that is used for recording events for changed conditions. +// If the event recorder is nil, no events are recorded. +// Note that this has no effect if condition updates are enabled, see WithConditionUpdater(). +func (b *StatusUpdaterBuilder[Obj]) WithConditionEvents(eventRecorder record.EventRecorder, verbosity conditions.EventVerbosity) *StatusUpdaterBuilder[Obj] { + b.internal.eventRecorder = eventRecorder + b.internal.eventVerbosity = verbosity + return b +} + // WithPhaseUpdateFunc sets the function that determines the phase of the object. // It is strongly recommended to either disable the phase field or override this function, because the default will simply set the Phase to an empty string. // The function is called with a deep copy of the object, after all other status updates have been applied (except for the custom update). @@ -146,6 +156,8 @@ type statusUpdater[Obj client.Object] struct { phaseUpdateFunc func(obj Obj, rr ReconcileResult[Obj]) (string, error) customUpdateFunc func(obj Obj, rr ReconcileResult[Obj]) error removeUntouchedConditions bool + eventRecorder record.EventRecorder + eventVerbosity conditions.EventVerbosity } func newStatusUpdater[Obj client.Object]() *statusUpdater[Obj] { @@ -214,11 +226,14 @@ func (s *statusUpdater[Obj]) UpdateStatus(ctx context.Context, c client.Client, if s.fieldNames[STATUS_FIELD_CONDITIONS] != "" { oldCons := GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false).([]metav1.Condition) cu := conditions.ConditionUpdater(oldCons, s.removeUntouchedConditions) + if s.eventRecorder != nil { + cu.WithEventRecorder(s.eventRecorder, s.eventVerbosity) + } cu.Now = now for _, con := range rr.Conditions { cu.UpdateConditionFromTemplate(con) } - newCons, _ := cu.Conditions() + newCons, _ := cu.Record(rr.Object).Conditions() SetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], newCons) } if s.fieldNames[STATUS_FIELD_PHASE] != "" { From 245188c54ffbf135f693762627ba9cac1e3f5cac Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Thu, 17 Jul 2025 09:57:44 +0200 Subject: [PATCH 2/4] add tests for condition event recording --- pkg/conditions/updater.go | 27 +++-- pkg/conditions/updater_test.go | 203 +++++++++++++++++++++++++++++++++ 2 files changed, 218 insertions(+), 12 deletions(-) diff --git a/pkg/conditions/updater.go b/pkg/conditions/updater.go index e9c55f2..ea55668 100644 --- a/pkg/conditions/updater.go +++ b/pkg/conditions/updater.go @@ -13,6 +13,8 @@ import ( "k8s.io/client-go/tools/record" ) +const EventReasonConditionChanged = "ConditionChanged" + type EventVerbosity string const ( @@ -20,10 +22,11 @@ const ( // 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), ", ")) } } diff --git a/pkg/conditions/updater_test.go b/pkg/conditions/updater_test.go index 23e1afb..8deaf69 100644 --- a/pkg/conditions/updater_test.go +++ b/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 +} From ae8b70d0b23aedd5275bed8a6056f80fcf2c56f6 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Thu, 17 Jul 2025 11:32:39 +0200 Subject: [PATCH 3/4] adapt documentation --- docs/libs/status.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/libs/status.md b/docs/libs/status.md index dd9beaf..89732e0 100644 --- a/docs/libs/status.md +++ b/docs/libs/status.md @@ -29,6 +29,26 @@ For simplicity, all commands can be chained: updatedCons, changed := conditions.ConditionUpdater(oldCons, false).UpdateCondition("myCondition", conditions.FromBool(true), myObj.Generation, "newReason", "newMessage").Conditions() ``` +### Event Recording for Conditions + +The condition updater can optionally record events for changed conditions. To enable event recording, call first `WithEventRecorder` and later `Record` on the `ConditionUpdater`: +```go +updatedCons, changed := conditions.ConditionUpdater(...).WithEventRecorder(recorder, conditions.EventIfChanged).UpdateCondition(...).Record(myObj).Conditions() +``` +Note that `Record` records all changes (`UpdateCondition` and `RemoveCondition` calls) that happened between construction of the condition updater and the `Record` call. Changes that are done after the `Record` call will not result in events. It is therefore recommended to call this method only after all conditions have been updated. Calling `Record` multiple times will lead to duplicate events. + +`Record` is a no-op, if either the event recorder is `nil` (most likely because `WithEventRecorder` has not been called before) or the given object is `nil`. + +The `WithEventRecorder` method takes a verbosity as second argument. There are three known verbosity values, each of which is stored in a corresponding constant in the `conditions` package: +- `perChange` (constant: `EventPerChange`) + - This is the most verbose option which creates a single event for each condition that was added, removed, or changed its status. It also displays the new and/or previous status of the condition, if applicable. +- `perNewStatus` (constant: `EventPerNewStatus`) + - This verbosity bundles all changes to the same status in a single event. This means that it will at most record four events per conditions update: one for all conditions that became `True`, one for all that became `False`, one for all that became `Unknown`, and one for all conditions that were removed. The condition types are listed in the events, their respective previous status is not. +- `ifChanged` (constant: `EventIfChanged`) + - This is the least verbose option. It will always log only a single event that bundles all changes. The condition types are listed, but the event does not allow to differentiate between added, removed, or changed conditions and does not contain any information about any condition's previous or current status. + +Setting the verbosity to any other than these values results in no events being recorded. + ## Status Updater The status updater is based on the idea that many of our resources use a status similar to this: @@ -140,6 +160,7 @@ You can then `Build()` the status updater and run `UpdateStatus()` to do the act - The package contains constants with the field keys that are required by most of these methods. `STATUS_FIELD` refers to the `Status` field itself, the other field keys are prefixed with `STATUS_FIELD_`. - The `AllStatusFields()` function returns a list containing all status field keys, _except the one for the status field itself_, for convenience. - The `WithCustomUpdateFunc` method can be used to inject a function that performs custom logic on the resource's status. Note that while the function gets the complete object as an argument, only changes to its status will be updated by the status updater. +- `WithConditionEvents` can be used to enable event recording for changed conditions. The events are automatically connected to the resource from the `ReconcileResult`'s `Object` field, no events will be recorded if that field is `nil`. ### The ReconcileResult From e5e349ec6d9cd09ce09859a7373783ea6138aec4 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Thu, 17 Jul 2025 11:38:09 +0200 Subject: [PATCH 4/4] task generate --- pkg/conditions/updater.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/conditions/updater.go b/pkg/conditions/updater.go index ea55668..f53d52b 100644 --- a/pkg/conditions/updater.go +++ b/pkg/conditions/updater.go @@ -5,12 +5,13 @@ import ( "slices" "strings" - "github.com/openmcp-project/controller-utils/pkg/collections" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" + + "github.com/openmcp-project/controller-utils/pkg/collections" ) const EventReasonConditionChanged = "ConditionChanged"