diff --git a/VERSION b/VERSION index d92a93b..d609fec 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -v0.13.0-dev \ No newline at end of file +v0.13.1 \ No newline at end of file diff --git a/go.mod b/go.mod index 1b186a7..a927fd8 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/onsi/ginkgo v1.16.5 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.37.0 - github.com/openmcp-project/controller-utils/api v0.13.0 + github.com/openmcp-project/controller-utils/api v0.13.1 github.com/spf13/pflag v1.0.6 github.com/stretchr/testify v1.10.0 go.uber.org/zap v1.27.0 diff --git a/pkg/conditions/updater.go b/pkg/conditions/updater.go index f53d52b..fc47495 100644 --- a/pkg/conditions/updater.go +++ b/pkg/conditions/updater.go @@ -88,6 +88,10 @@ func (c *conditionUpdater) WithEventRecorder(recorder record.EventRecorder, verb // 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. func (c *conditionUpdater) UpdateCondition(conType string, status metav1.ConditionStatus, observedGeneration int64, reason, message string) *conditionUpdater { + if reason == "" { + // the metav1.Condition type requires a reason, so let's add a dummy if none is given + reason = conType + string(status) + } con := metav1.Condition{ Type: conType, Status: status, @@ -96,11 +100,6 @@ func (c *conditionUpdater) UpdateCondition(conType string, status metav1.Conditi ObservedGeneration: observedGeneration, LastTransitionTime: c.Now, } - old, ok := c.conditions[conType] - if ok && old.Status == con.Status { - // update LastTransitionTime only if status changed - con.LastTransitionTime = old.LastTransitionTime - } c.updates[conType] = status c.conditions[conType] = con return c @@ -134,7 +133,13 @@ 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() + res := collections.ProjectSlice(c.updatedConditions(), func(con metav1.Condition) metav1.Condition { + if c.original[con.Type].Status == con.Status { + // if the status has not changed, reset the LastTransitionTime to the original value + con.LastTransitionTime = c.original[con.Type].LastTransitionTime + } + return con + }) slices.SortStableFunc(res, func(a, b metav1.Condition) int { return strings.Compare(a.Type, b.Type) }) diff --git a/pkg/conditions/updater_test.go b/pkg/conditions/updater_test.go index 8deaf69..faa1b31 100644 --- a/pkg/conditions/updater_test.go +++ b/pkg/conditions/updater_test.go @@ -161,6 +161,16 @@ var _ = Describe("Conditions", func() { Expect(newCon.LastTransitionTime.After(oldCon.LastTransitionTime.Time)).To(BeTrue()) }) + It("should not change the last transition time if the same condition is updated multiple times with the last update setting it to the original value again", func() { + cons := testConditionSet() + oldCon := conditions.GetCondition(cons, "true") + updater := conditions.ConditionUpdater(cons, false) + updated, changed := updater.UpdateCondition(oldCon.Type, invert(oldCon.Status), oldCon.ObservedGeneration+1, "newReason", "newMessage"). + UpdateCondition(oldCon.Type, oldCon.Status, oldCon.ObservedGeneration, oldCon.Reason, oldCon.Message).Conditions() + Expect(changed).To(BeFalse()) + Expect(updated).To(ConsistOf(cons)) + }) + It("should sort the conditions by type", func() { cons := []metav1.Condition{ TestConditionFromValues("c", conditions.FromBool(true), 0, "reason", "message", metav1.Now()).ToCondition(), diff --git a/pkg/controller/status_updater.go b/pkg/controller/status_updater.go index 326cb63..f57488d 100644 --- a/pkg/controller/status_updater.go +++ b/pkg/controller/status_updater.go @@ -231,7 +231,11 @@ func (s *statusUpdater[Obj]) UpdateStatus(ctx context.Context, c client.Client, } cu.Now = now for _, con := range rr.Conditions { - cu.UpdateConditionFromTemplate(con) + gen := con.ObservedGeneration + if gen == 0 { + gen = rr.Object.GetGeneration() + } + cu.UpdateCondition(con.Type, con.Status, gen, con.Reason, con.Message) } newCons, _ := cu.Record(rr.Object).Conditions() SetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], newCons) @@ -366,3 +370,21 @@ type ReconcileResult[Obj client.Object] struct { // The lastTransition timestamp of the condition will be overwritten with the current time while updating. Conditions []metav1.Condition } + +// GenerateCreateConditionFunc returns a function that can be used to add a condition to the given ReconcileResult. +// If the ReconcileResult's Object is not nil, the condition's ObservedGeneration is set to the object's generation. +func GenerateCreateConditionFunc[Obj client.Object](rr *ReconcileResult[Obj]) func(conType string, status metav1.ConditionStatus, reason, message string) { + var gen int64 = 0 + if any(rr.Object) != nil { + gen = rr.Object.GetGeneration() + } + return func(conType string, status metav1.ConditionStatus, reason, message string) { + rr.Conditions = append(rr.Conditions, metav1.Condition{ + Type: conType, + Status: status, + ObservedGeneration: gen, + Reason: reason, + Message: message, + }) + } +} diff --git a/pkg/controller/status_updater_test.go b/pkg/controller/status_updater_test.go index 9fa042c..786af50 100644 --- a/pkg/controller/status_updater_test.go +++ b/pkg/controller/status_updater_test.go @@ -194,6 +194,47 @@ var _ = Describe("Status Updater", func() { } }) + Context("GenerateCreateConditionFunc", func() { + + It("should add the condition to the given ReconcileResult", func() { + rr := controller.ReconcileResult[*CustomObject]{ + Object: &CustomObject{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: 15, + }, + }, + Conditions: dummyConditions(), + } + createCon := controller.GenerateCreateConditionFunc(&rr) + createCon("TestConditionFoo", metav1.ConditionTrue, "TestReasonFoo", "TestMessageFoo") + Expect(rr.Conditions).To(ConsistOf( + MatchCondition(TestConditionFromCondition(dummyConditions()[0])), + MatchCondition(TestConditionFromCondition(dummyConditions()[1])), + MatchCondition(TestConditionFromValues("TestConditionFoo", metav1.ConditionTrue, 15, "TestReasonFoo", "TestMessageFoo", metav1.Time{})), + )) + }) + + It("should create the condition list, if it is nil", func() { + rr := controller.ReconcileResult[*CustomObject]{ + Object: &CustomObject{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "default", + Generation: 15, + }, + }, + } + createCon := controller.GenerateCreateConditionFunc(&rr) + createCon("TestConditionFoo", metav1.ConditionTrue, "TestReasonFoo", "TestMessageFoo") + Expect(rr.Conditions).To(ConsistOf( + MatchCondition(TestConditionFromValues("TestConditionFoo", metav1.ConditionTrue, 15, "TestReasonFoo", "TestMessageFoo", metav1.Time{})), + )) + }) + + }) + }) func preconfiguredStatusUpdaterBuilder() *controller.StatusUpdaterBuilder[*CustomObject] {