From 0b247a0da013a7ac2bf170c66eafcbb9814e440a Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Fri, 11 Apr 2025 09:25:24 +0200 Subject: [PATCH 01/10] add ReasonableError type --- pkg/errors/reasonable_error.go | 131 +++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 pkg/errors/reasonable_error.go diff --git a/pkg/errors/reasonable_error.go b/pkg/errors/reasonable_error.go new file mode 100644 index 0000000..0e53b80 --- /dev/null +++ b/pkg/errors/reasonable_error.go @@ -0,0 +1,131 @@ +package errors + +import ( + "errors" + "fmt" + "strings" +) + +var _ ReasonableError = &ErrorWithReason{} + +// ReasonableError enhances an error with a reason. +// The reason is meant to be a CamelCased, machine-readable, enum-like string. +// Use WithReason(err, reason) to wrap a normal error into an ReasonableError. +type ReasonableError interface { + error + Reason() string +} + +// ErrorWithReason wraps an error and adds a reason to it. +// The reason is meant to be a CamelCased, machine-readable, enum-like string. +// Use WithReason(err, reason) to wrap a normal error into an *ErrorWithReason. +type ErrorWithReason struct { + error + reason string +} + +// Reason returns the reason for this error. +func (e *ErrorWithReason) Reason() string { + return e.reason +} + +// WithReason wraps an error together with a reason into ErrorWithReason. +// The reason is meant to be a CamelCased, machine-readable, enum-like string. +// If the given error is nil, nil is returned. +func WithReason(err error, reason string) ReasonableError { + if err == nil { + return nil + } + return &ErrorWithReason{ + error: err, + reason: reason, + } +} + +// Errorf works similarly to fmt.Errorf, with the exception that it requires an ErrorWithReason as second argument and returns nil if that one is nil. +// Otherwise, it calls fmt.Errorf to construct an error and wraps it in an ErrorWithReason, using the reason from the given error. +// This is useful for expanding the error message without losing the reason. +func Errorf(format string, err ReasonableError, a ...any) ReasonableError { + if err == nil { + return nil + } + return WithReason(fmt.Errorf(format, a...), err.Reason()) +} + +// Join joins multiple errors into a single one. +// Returns nil if all given errors are nil. +// This is equivalent to NewErrorList(errs...).Aggregate(). +func Join(errs ...error) ReasonableError { + return NewReasonableErrorList(errs...).Aggregate() +} + +// ReasonableErrorList is a helper struct for situations in which multiple errors (with or without reasons) should be returned as a single one. +type ReasonableErrorList struct { + Errs []error + Reasons []string +} + +// NewReasonableErrorList creates a new *ErrorListWithReasons containing the provided errors. +func NewReasonableErrorList(errs ...error) *ReasonableErrorList { + res := &ReasonableErrorList{ + Errs: []error{}, + Reasons: []string{}, + } + return res.Append(errs...) +} + +// Aggregate aggregates all errors in the list into a single ErrorWithReason. +// Returns nil if the list is either nil or empty. +// If the list contains a single error, that error is returned. +// Otherwise, a new error is constructed by appending all contained errors' messages. +// The reason in the returned error is the first reason that was added to the list, +// or the empty string if none of the contained errors was an ErrorWithReason. +func (el *ReasonableErrorList) Aggregate() ReasonableError { + if el == nil || len(el.Errs) == 0 { + return nil + } + reason := "" + if len(el.Reasons) > 0 { + reason = el.Reasons[0] + } + if len(el.Errs) == 1 { + if ewr, ok := el.Errs[0].(ReasonableError); ok { + return ewr + } + return WithReason(el.Errs[0], reason) + } + sb := strings.Builder{} + sb.WriteString("multiple errors occurred:") + for _, e := range el.Errs { + sb.WriteString("\n") + sb.WriteString(e.Error()) + } + return WithReason(errors.New(sb.String()), reason) +} + +// Append appends all given errors to the ErrorListWithReasons. +// This modifies the receiver object. +// If a given error is of type ErrorWithReason, its reason is added to the list of reasons. +// nil pointers in the arguments are ignored. +// Returns the receiver for chaining. +func (el *ReasonableErrorList) Append(errs ...error) *ReasonableErrorList { + for _, e := range errs { + if e != nil { + el.Errs = append(el.Errs, e) + if ewr, ok := e.(ReasonableError); ok { + el.Reasons = append(el.Reasons, ewr.Reason()) + } + } + } + return el +} + +// Reason returns the first reason from the list of reasons contained in this error list. +// If the list is nil or no reasons are contained, the empty string is returned. +// This is equivalent to el.Aggregate().Reason(), except that it also works for an empty error list. +func (el *ReasonableErrorList) Reason() string { + if el == nil || len(el.Reasons) == 0 { + return "" + } + return el.Reasons[0] +} From 00a2f69e125ce9004f963826cf7980cc17cf3edc Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Tue, 15 Apr 2025 13:06:40 +0200 Subject: [PATCH 02/10] add StatusUpdater --- pkg/conditions/updater.go | 23 +- pkg/conditions/updater_test.go | 95 +---- pkg/controller/predicates.go | 17 +- pkg/controller/status_updater.go | 359 ++++++++++++++++ pkg/controller/status_updater_test.go | 402 ++++++++++++++++++ .../testdata/test-02/co_nostatus.yaml | 6 + pkg/testing/matchers/conditions.go | 178 ++++++++ pkg/testing/utils.go | 14 + 8 files changed, 991 insertions(+), 103 deletions(-) create mode 100644 pkg/controller/status_updater.go create mode 100644 pkg/controller/status_updater_test.go create mode 100644 pkg/controller/testdata/test-02/co_nostatus.yaml create mode 100644 pkg/testing/matchers/conditions.go diff --git a/pkg/conditions/updater.go b/pkg/conditions/updater.go index 9a32a02..af0fafc 100644 --- a/pkg/conditions/updater.go +++ b/pkg/conditions/updater.go @@ -11,11 +11,11 @@ import ( // conditionUpdater is a helper struct for updating a list of Conditions. // Use the ConditionUpdater constructor for initializing. type conditionUpdater[T comparable] struct { - Now time.Time - conditions map[string]Condition[T] - updated sets.Set[string] - constructor func() Condition[T] - changed bool + Now time.Time + conditions map[string]Condition[T] + updated sets.Set[string] + construct func() Condition[T] + changed bool } // ConditionUpdater creates a builder-like helper struct for updating a list of Conditions. @@ -30,12 +30,12 @@ type conditionUpdater[T comparable] struct { // // Usage example: // status.conditions = ConditionUpdater(status.conditions, true).UpdateCondition(...).UpdateCondition(...).Conditions() -func ConditionUpdater[T comparable](constructor func() Condition[T], conditions []Condition[T], removeUntouched bool) *conditionUpdater[T] { +func ConditionUpdater[T comparable](construct func() Condition[T], conditions []Condition[T], removeUntouched bool) *conditionUpdater[T] { res := &conditionUpdater[T]{ - Now: time.Now(), - conditions: make(map[string]Condition[T], len(conditions)), - constructor: constructor, - changed: false, + Now: time.Now(), + conditions: make(map[string]Condition[T], len(conditions)), + construct: construct, + changed: false, } for _, con := range conditions { res.conditions[con.GetType()] = con @@ -50,7 +50,7 @@ func ConditionUpdater[T comparable](constructor func() Condition[T], conditions // 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[T]) UpdateCondition(conType string, status T, reason, message string) *conditionUpdater[T] { - con := c.constructor() + con := c.construct() con.SetType(conType) con.SetStatus(status) con.SetReason(reason) @@ -103,6 +103,7 @@ func (c *conditionUpdater[T]) RemoveCondition(conType string) *conditionUpdater[ // If the condition updater was initialized with removeUntouched=true, this list will only contain the conditions which have been updated // in between the condition updater creation and this method call. Otherwise, it will potentially also contain old conditions. // The conditions are returned sorted by their type. +// The second return value indicates whether the condition list has actually changed. func (c *conditionUpdater[T]) Conditions() ([]Condition[T], bool) { res := make([]Condition[T], 0, len(c.conditions)) for _, con := range c.conditions { diff --git a/pkg/conditions/updater_test.go b/pkg/conditions/updater_test.go index b3e033f..70a2f31 100644 --- a/pkg/conditions/updater_test.go +++ b/pkg/conditions/updater_test.go @@ -7,76 +7,17 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + . "github.com/openmcp-project/controller-utils/pkg/testing/matchers" "github.com/openmcp-project/controller-utils/pkg/conditions" ) -type conImpl struct { - status bool - conType string - reason string - message string - lastTransitionTime time.Time -} - -func newConImplWithValues(conType string, status bool, reason, message string, lastTransitionTime time.Time) *conImpl { - return &conImpl{ - conType: conType, - status: status, - reason: reason, - message: message, - lastTransitionTime: lastTransitionTime, - } -} - -var _ conditions.Condition[bool] = &conImpl{} - -func (c *conImpl) GetLastTransitionTime() time.Time { - return c.lastTransitionTime -} - -func (c *conImpl) GetType() string { - return c.conType -} - -func (c *conImpl) GetStatus() bool { - return c.status -} - -func (c *conImpl) GetReason() string { - return c.reason -} - -func (c *conImpl) GetMessage() string { - return c.message -} - -func (c *conImpl) SetStatus(status bool) { - c.status = status -} - -func (c *conImpl) SetType(conType string) { - c.conType = conType -} - -func (c *conImpl) SetLastTransitionTime(timestamp time.Time) { - c.lastTransitionTime = timestamp -} - -func (c *conImpl) SetReason(reason string) { - c.reason = reason -} - -func (c *conImpl) SetMessage(message string) { - c.message = message -} - func testConditionSet() []conditions.Condition[bool] { now := time.Now().Add((-24) * time.Hour) return []conditions.Condition[bool]{ - newConImplWithValues("true", true, "reason", "message", now), - newConImplWithValues("false", false, "reason", "message", now), - newConImplWithValues("alsoTrue", true, "alsoReason", "alsoMessage", now), + NewConditionImplFromValues("true", true, "reason", "message", now), + NewConditionImplFromValues("false", false, "reason", "message", now), + NewConditionImplFromValues("alsoTrue", true, "alsoReason", "alsoMessage", now), } } @@ -134,7 +75,7 @@ var _ = Describe("Conditions", func() { It("should update the condition (same value, keep other cons)", func() { cons := testConditionSet() oldCon := conditions.GetCondition(cons, "true") - updated, changed := conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, false).UpdateCondition(oldCon.GetType(), oldCon.GetStatus(), "newReason", "newMessage").Conditions() + updated, changed := conditions.ConditionUpdater(NewCondition[bool], cons, false).UpdateCondition(oldCon.GetType(), oldCon.GetStatus(), "newReason", "newMessage").Conditions() Expect(changed).To(BeTrue()) newCon := conditions.GetCondition(updated, "true") Expect(updated).To(HaveLen(len(cons))) @@ -151,7 +92,7 @@ var _ = Describe("Conditions", func() { It("should update the condition (different value, keep other cons)", func() { cons := testConditionSet() oldCon := conditions.GetCondition(cons, "true") - updated, changed := conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, false).UpdateCondition(oldCon.GetType(), !oldCon.GetStatus(), "newReason", "newMessage").Conditions() + updated, changed := conditions.ConditionUpdater(NewCondition[bool], cons, false).UpdateCondition(oldCon.GetType(), !oldCon.GetStatus(), "newReason", "newMessage").Conditions() Expect(changed).To(BeTrue()) newCon := conditions.GetCondition(updated, "true") Expect(updated).To(HaveLen(len(cons))) @@ -169,7 +110,7 @@ var _ = Describe("Conditions", func() { It("should update the condition (same value, discard other cons)", func() { cons := testConditionSet() oldCon := conditions.GetCondition(cons, "true") - updated, changed := conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, true).UpdateCondition(oldCon.GetType(), oldCon.GetStatus(), "newReason", "newMessage").Conditions() + updated, changed := conditions.ConditionUpdater(NewCondition[bool], cons, true).UpdateCondition(oldCon.GetType(), oldCon.GetStatus(), "newReason", "newMessage").Conditions() Expect(changed).To(BeTrue()) newCon := conditions.GetCondition(updated, "true") Expect(updated).To(HaveLen(1)) @@ -186,7 +127,7 @@ var _ = Describe("Conditions", func() { It("should update the condition (different value, discard other cons)", func() { cons := testConditionSet() oldCon := conditions.GetCondition(cons, "true") - updated, changed := conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, true).UpdateCondition(oldCon.GetType(), !oldCon.GetStatus(), "newReason", "newMessage").Conditions() + updated, changed := conditions.ConditionUpdater(NewCondition[bool], cons, true).UpdateCondition(oldCon.GetType(), !oldCon.GetStatus(), "newReason", "newMessage").Conditions() Expect(changed).To(BeTrue()) newCon := conditions.GetCondition(updated, "true") Expect(updated).To(HaveLen(1)) @@ -203,16 +144,16 @@ var _ = Describe("Conditions", func() { It("should sort the conditions by type", func() { cons := []conditions.Condition[bool]{ - newConImplWithValues("c", true, "reason", "message", time.Now()), - newConImplWithValues("d", true, "reason", "message", time.Now()), - newConImplWithValues("a", true, "reason", "message", time.Now()), - newConImplWithValues("b", true, "reason", "message", time.Now()), + NewConditionImplFromValues("c", true, "reason", "message", time.Now()), + NewConditionImplFromValues("d", true, "reason", "message", time.Now()), + NewConditionImplFromValues("a", true, "reason", "message", time.Now()), + NewConditionImplFromValues("b", true, "reason", "message", time.Now()), } compareConditions := func(a, b conditions.Condition[bool]) int { return strings.Compare(a.GetType(), b.GetType()) } Expect(slices.IsSortedFunc(cons, compareConditions)).To(BeFalse(), "conditions in the test object are already sorted, unable to test sorting") - updated, changed := conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, false).Conditions() + updated, changed := conditions.ConditionUpdater(NewCondition[bool], cons, false).Conditions() Expect(changed).To(BeFalse()) Expect(len(updated)).To(BeNumerically(">", 1), "test object does not contain enough conditions to test sorting") Expect(len(updated)).To(Equal(len(cons))) @@ -221,14 +162,14 @@ var _ = Describe("Conditions", func() { It("should remove a condition", func() { cons := testConditionSet() - updated, changed := conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, false).RemoveCondition("true").Conditions() + updated, changed := conditions.ConditionUpdater(NewCondition[bool], cons, false).RemoveCondition("true").Conditions() Expect(changed).To(BeTrue()) Expect(updated).To(HaveLen(len(cons) - 1)) con := conditions.GetCondition(updated, "true") Expect(con).To(BeNil()) // removing a condition that does not exist should not change anything - updated, changed = conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, false).RemoveCondition("doesNotExist").Conditions() + updated, changed = conditions.ConditionUpdater(NewCondition[bool], cons, false).RemoveCondition("doesNotExist").Conditions() Expect(changed).To(BeFalse()) Expect(updated).To(HaveLen(len(cons))) }) @@ -236,17 +177,17 @@ var _ = Describe("Conditions", func() { It("should not mark a condition as changed if it has the same values as before", func() { cons := testConditionSet() con := conditions.GetCondition(cons, "true") - updated, changed := conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, false).UpdateCondition(con.GetType(), con.GetStatus(), con.GetReason(), con.GetMessage()).Conditions() + updated, changed := conditions.ConditionUpdater(NewCondition[bool], cons, false).UpdateCondition(con.GetType(), con.GetStatus(), con.GetReason(), con.GetMessage()).Conditions() Expect(changed).To(BeFalse()) Expect(updated).To(HaveLen(len(cons))) }) It("should return that a condition exists only if it will be contained in the returned list", func() { cons := testConditionSet() - updater := conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, false) + updater := conditions.ConditionUpdater(NewCondition[bool], cons, false) Expect(updater.HasCondition("true")).To(BeTrue()) Expect(updater.HasCondition("doesNotExist")).To(BeFalse()) - updater = conditions.ConditionUpdater(func() conditions.Condition[bool] { return &conImpl{} }, cons, true) + updater = conditions.ConditionUpdater(NewCondition[bool], cons, true) Expect(updater.HasCondition("true")).To(BeFalse()) Expect(updater.HasCondition("doesNotExist")).To(BeFalse()) updater.UpdateCondition("true", true, "reason", "message") diff --git a/pkg/controller/predicates.go b/pkg/controller/predicates.go index b5549e1..f17ee0b 100644 --- a/pkg/controller/predicates.go +++ b/pkg/controller/predicates.go @@ -152,23 +152,10 @@ type StatusChangedPredicate struct { } func (p StatusChangedPredicate) Update(e event.UpdateEvent) bool { - oldStatus := getStatus(e.ObjectOld) - newStatus := getStatus(e.ObjectNew) + oldStatus := GetField(e.ObjectOld, "Status", false) + newStatus := GetField(e.ObjectNew, "Status", false) if oldStatus == nil || newStatus == nil { return true } return !reflect.DeepEqual(oldStatus, newStatus) } - -func getStatus(obj any) any { - if obj == nil { - return nil - } - val := reflect.ValueOf(obj).Elem() - for i := 0; i < val.NumField(); i++ { - if val.Type().Field(i).Name == "Status" { - return val.Field(i).Interface() - } - } - return nil -} diff --git a/pkg/controller/status_updater.go b/pkg/controller/status_updater.go new file mode 100644 index 0000000..8bbf6f9 --- /dev/null +++ b/pkg/controller/status_updater.go @@ -0,0 +1,359 @@ +package controller + +import ( + "context" + "fmt" + "reflect" + "strings" + "time" + + "github.com/openmcp-project/controller-utils/pkg/conditions" + "github.com/openmcp-project/controller-utils/pkg/errors" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// NewStatusUpdaterBuilder initializes a new StatusUpdaterBuilder. +func NewStatusUpdaterBuilder[Obj client.Object, PhType ~string, ConType comparable]() *StatusUpdaterBuilder[Obj, PhType, ConType] { + return &StatusUpdaterBuilder[Obj, PhType, ConType]{ + internal: newStatusUpdater[Obj, PhType, ConType](), + } +} + +// StatusUpdaterBuilder is a builder for creating a status updater. +// Do not use this directly, use NewStatusUpdaterBuilder() instead. +type StatusUpdaterBuilder[Obj client.Object, PhType ~string, ConType comparable] struct { + internal *statusUpdater[Obj, PhType, ConType] +} + +// WithFieldOverride overwrites the name of the field. +// Use STATUS_FIELD to override the name of the field that holds the status itself. +// All other fields are expected to be within the status struct. +// Set to an empty string to disable the field. Doing this for STATUS_FIELD disables the complete status update. +// The default names are: +// - STATUS_FIELD: "Status" +// - STATUS_FIELD_OBSERVED_GENERATION: "ObservedGeneration" +// - STATUS_FIELD_LAST_RECONCILE_TIME: "LastReconcileTime" +// - STATUS_FIELD_CONDITIONS: "Conditions" +// - STATUS_FIELD_REASON: "Reason" +// - STATUS_FIELD_MESSAGE: "Message" +// - STATUS_FIELD_PHASE: "Phase" +func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithFieldOverride(field StatusField, name string) *StatusUpdaterBuilder[Obj, PhType, ConType] { + if name == "" { + delete(b.internal.fieldNames, field) + } else { + b.internal.fieldNames[field] = name + } + return b +} + +// WithFieldOverrides is a wrapper around WithFieldOverride that allows to apply multiple overrides at once. +func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithFieldOverrides(overrides map[StatusField]string) *StatusUpdaterBuilder[Obj, PhType, ConType] { + for field, name := range overrides { + b.WithFieldOverride(field, name) + } + return b +} + +// WithNestedStruct is a helper for easily updating the field names if some or all of the fields are wrapped in a nested struct within the status. +// Basically, the field names for all specified fields are prefixed with '.', unless the field is empty (which disables the field). +// If appliesTo is empty, all fields are assumed to be nested (except for the status itself). +func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithNestedStruct(name string, appliesTo ...StatusField) *StatusUpdaterBuilder[Obj, PhType, ConType] { + if len(appliesTo) == 0 { + appliesTo = AllStatusFields() + } + for _, field := range appliesTo { + oldName := b.internal.fieldNames[field] + if oldName == "" { + continue + } + b.WithFieldOverride(field, fmt.Sprintf("%s.%s", name, oldName)) + } + return b +} + +// WithoutField removes the field from the status update. +// This is an alias for WithFieldOverride(field, ""). +func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithoutField(field StatusField) *StatusUpdaterBuilder[Obj, PhType, ConType] { + return b.WithFieldOverride(field, "") +} + +// WithConditionUpdater must be called if the conditions should be updated, because this requires some additional information. +// Note that the conditions will only be updated if this method has been called (with a non-nil condition constructor) AND the conditions field has not been disabled. +func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithConditionUpdater(construct func() conditions.Condition[ConType], removeUntouchedConditions bool) *StatusUpdaterBuilder[Obj, PhType, ConType] { + b.internal.conConstruct = construct + b.internal.removeUntouchedConditions = removeUntouchedConditions + 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 the zero value of PhType. +// The function is called with a deep copy of the object, after all other status updates have been applied (except for the custom update). +// If the returned error is nil, the object's phase is then set to the returned value. +// Setting this to nil causes the default phase update function to be used. To disable the phase update altogether, use WithoutField(STATUS_FIELD_PHASE). +func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithPhaseUpdateFunc(f func(obj Obj, rr ReconcileResult[Obj, ConType]) (PhType, error)) *StatusUpdaterBuilder[Obj, PhType, ConType] { + b.internal.phaseUpdateFunc = f + return b +} + +// WithCustomUpdateFunc allows to pass in a function that performs a custom update on the object. +// This function is called after all other status updates have been applied. +// It gets the original object passed in and can modify it directly. +// Note that only changes to the status field are sent to the cluster. +// Set this to nil to disable the custom update (it is nil by default). +func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithCustomUpdateFunc(f func(obj Obj, rr ReconcileResult[Obj, ConType]) error) *StatusUpdaterBuilder[Obj, PhType, ConType] { + b.internal.customUpdateFunc = f + return b +} + +// Build returns the status updater. +func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) Build() *statusUpdater[Obj, PhType, ConType] { + return b.internal +} + +type StatusField string + +const ( + // This is kind of a meta field, it holds the name of the field that stores the status itself. + STATUS_FIELD StatusField = "Status" + STATUS_FIELD_OBSERVED_GENERATION StatusField = "ObservedGeneration" + STATUS_FIELD_LAST_RECONCILE_TIME StatusField = "LastReconcileTime" + STATUS_FIELD_CONDITIONS StatusField = "Conditions" + STATUS_FIELD_REASON StatusField = "Reason" + STATUS_FIELD_MESSAGE StatusField = "Message" + STATUS_FIELD_PHASE StatusField = "Phase" +) + +// AllStatusFields returns all status fields that are used by the status updater. +// The meta field STATUS_FIELD is not included. +func AllStatusFields() []StatusField { + return []StatusField{ + STATUS_FIELD_OBSERVED_GENERATION, + STATUS_FIELD_LAST_RECONCILE_TIME, + STATUS_FIELD_CONDITIONS, + STATUS_FIELD_REASON, + STATUS_FIELD_MESSAGE, + STATUS_FIELD_PHASE, + } +} + +type statusUpdater[Obj client.Object, PhType ~string, ConType comparable] struct { + fieldNames map[StatusField]string + phaseUpdateFunc func(obj Obj, rr ReconcileResult[Obj, ConType]) (PhType, error) + customUpdateFunc func(obj Obj, rr ReconcileResult[Obj, ConType]) error + conConstruct func() conditions.Condition[ConType] + removeUntouchedConditions bool +} + +func newStatusUpdater[Obj client.Object, PhType ~string, ConType comparable]() *statusUpdater[Obj, PhType, ConType] { + return &statusUpdater[Obj, PhType, ConType]{ + fieldNames: map[StatusField]string{ + STATUS_FIELD: string(STATUS_FIELD), + STATUS_FIELD_OBSERVED_GENERATION: string(STATUS_FIELD_OBSERVED_GENERATION), + STATUS_FIELD_LAST_RECONCILE_TIME: string(STATUS_FIELD_LAST_RECONCILE_TIME), + STATUS_FIELD_CONDITIONS: string(STATUS_FIELD_CONDITIONS), + STATUS_FIELD_REASON: string(STATUS_FIELD_REASON), + STATUS_FIELD_MESSAGE: string(STATUS_FIELD_MESSAGE), + STATUS_FIELD_PHASE: string(STATUS_FIELD_PHASE), + }, + phaseUpdateFunc: defaultPhaseUpdateFunc[Obj, PhType, ConType], + } +} + +func defaultPhaseUpdateFunc[Obj client.Object, PhType ~string, ConType comparable](obj Obj, _ ReconcileResult[Obj, ConType]) (PhType, error) { + // Default phase update function does nothing. + // This should be overridden by the caller. + var zero PhType + return zero, nil +} + +// UpdateStatus updates the status of the object in the given ReconcileResult, using the previously set field names and functions. +// The object is expected to be a pointer to a struct with the status field. +// If the 'Object' field in the ReconcileResult is nil, the status update becomes a no-op. +func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context, c client.Client, rr ReconcileResult[Obj, ConType]) error { + if IsNil(rr.Object) { + return nil + } + if s.fieldNames[STATUS_FIELD] == "" { + return nil + } + if IsNil(rr.OldObject) || IsSameObject(rr.OldObject, rr.Object) { + // create old object based on given one + rr.OldObject = rr.Object.DeepCopyObject().(Obj) + } + status := GetField(rr.Object, s.fieldNames[STATUS_FIELD], true) + if IsNil(status) { + return fmt.Errorf("unable to get pointer to status field '%s' of object %T", s.fieldNames[STATUS_FIELD], rr.Object) + } + + now := time.Now() + if s.fieldNames[STATUS_FIELD_LAST_RECONCILE_TIME] != "" { + SetField(status, s.fieldNames[STATUS_FIELD_LAST_RECONCILE_TIME], metav1.NewTime(now)) + } + if s.fieldNames[STATUS_FIELD_OBSERVED_GENERATION] != "" { + SetField(status, s.fieldNames[STATUS_FIELD_OBSERVED_GENERATION], rr.Object.GetGeneration()) + } + if s.fieldNames[STATUS_FIELD_MESSAGE] != "" { + message := rr.Message + if message == "" && rr.ReconcileError != nil { + message = rr.ReconcileError.Error() + } + SetField(status, s.fieldNames[STATUS_FIELD_MESSAGE], message) + } + if s.fieldNames[STATUS_FIELD_REASON] != "" { + reason := rr.Reason + if reason == "" && rr.ReconcileError != nil { + reason = rr.ReconcileError.Reason() + } + SetField(status, s.fieldNames[STATUS_FIELD_REASON], reason) + } + if s.fieldNames[STATUS_FIELD_CONDITIONS] != "" && s.conConstruct != nil { + oldConsRaw := GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false) + var oldCons []conditions.Condition[ConType] + if !IsNil(oldConsRaw) { + oldCons = oldConsRaw.([]conditions.Condition[ConType]) + } + cu := conditions.ConditionUpdater(s.conConstruct, oldCons, s.removeUntouchedConditions) + cu.Now = now + for _, con := range rr.Conditions { + cu.UpdateConditionFromTemplate(con) + } + newConsRaw, _ := cu.Conditions() + conType := reflect.TypeOf(s.conConstruct()) + newCons := reflect.MakeSlice(reflect.SliceOf(conType), len(newConsRaw), len(newConsRaw)).Interface() + for i := range newConsRaw { + val := reflect.ValueOf(newConsRaw[i]).Convert(conType) + reflect.ValueOf(newCons).Index(i).Set(val) + } + SetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], newCons) + } + if s.fieldNames[STATUS_FIELD_PHASE] != "" { + phase, err := s.phaseUpdateFunc(rr.Object, rr) + if err != nil { + return fmt.Errorf("error computing phase: %w", err) + } + SetField(status, s.fieldNames[STATUS_FIELD_PHASE], phase) + } + if s.customUpdateFunc != nil { + if err := s.customUpdateFunc(rr.Object, rr); err != nil { + return fmt.Errorf("error performing custom status update: %w", err) + } + } + + // update status in cluster + if err := c.Status().Patch(ctx, rr.Object, client.MergeFrom(rr.OldObject)); err != nil { + return fmt.Errorf("error patching status: %w", err) + } + return nil +} + +// GetField returns the value of the field with the given name from the given object. +// Nested fields can be accessed by separating them with '.' (e.g. "Foo.Bar"). +// If pointer is true, it returns a pointer to the field value instead. +// WARNING: This function will panic if pointer is true but obj is not a pointer itself! +// Panics if the object is nil or the field is not found. +func GetField(obj any, field string, pointer bool) any { + if obj == nil { + panic("object is nil") + } + current, next, _ := strings.Cut(field, ".") + val, ok := obj.(reflect.Value) + if !ok { + val = reflect.ValueOf(obj) + } + for val.Kind() == reflect.Ptr || val.Kind() == reflect.Interface { + val = val.Elem() + } + for i := range val.NumField() { + if val.Type().Field(i).Name == current { + res := val.Field(i) + if next != "" { + return GetField(res, next, pointer) + } + if pointer { + res = res.Addr() + } + return res.Interface() + } + } + panic(fmt.Sprintf("field '%s' not found in object %T", current, obj)) +} + +// SetField sets the field in the given object to the given value. +// Nested fields can be accessed by separating them with '.' (e.g. "Foo.Bar"). +// Panics if the object is nil or the field is not found. +// WARNING: This function will panic if the specified field is not settable (e.g. because obj is not a pointer). +func SetField(obj any, field string, value any) { + if obj == nil { + panic("object is nil") + } + current, next, _ := strings.Cut(field, ".") + val, ok := obj.(reflect.Value) + if !ok { + val = reflect.ValueOf(obj) + } + for val.Kind() == reflect.Ptr || val.Kind() == reflect.Interface { + val = val.Elem() + } + for i := range val.NumField() { + if val.Type().Field(i).Name == current { + res := val.Field(i) + if next != "" { + SetField(res, next, value) + return + } + res.Set(reflect.ValueOf(value)) + return + } + } + panic(fmt.Sprintf("field '%s' not found in object %T", current, obj)) +} + +// IsSameObject takes two interfaces of the same type and returns true if they both are pointers to the same underlying object. +func IsSameObject[T any](a, b T) bool { + aVal := reflect.ValueOf(a) + bVal := reflect.ValueOf(b) + if aVal.Kind() != reflect.Ptr || bVal.Kind() != reflect.Ptr { + return false + } + if aVal.IsNil() && bVal.IsNil() { + return true + } + if aVal.IsNil() || bVal.IsNil() { + return false + } + if aVal.Type() != bVal.Type() { + return false + } + return aVal.Interface() == bVal.Interface() +} + +// The result of a reconciliation. +// Obj is the k8s resource that is reconciled. +// ConType is the type of the "Status" field of the condition, usually a string alias. Simply use string if your object's status does not have conditions. +type ReconcileResult[Obj client.Object, ConType comparable] struct { + // The old object, before it was modified. + // Basically, if OldObject.Status differs from Object.Status, the status will be patched during status updating. + // May be nil, in this case only the status metadata (observedGeneration etc.) is updated. + // Changes to anything except the status are ignored. + OldObject Obj + // The current objectclient.Object // If nil, the status update becomes a no-op. + Object Obj + // The result of the reconciliation. + // Is simply passed through. + Result ctrl.Result + // The error that occurred during reconciliation, if any. + ReconcileError errors.ReasonableError + // The reason to display in the object's status. + // If empty, it is taken from the error, if any. + Reason string + // The message to display in the object's status. + // Potential error messages from the reconciliation error are appended. + Message string + // Conditions contains a list of conditions that should be updated on the object. + // Also note that names of conditions are globally unique, so take care to avoid conflicts with other objects. + // The lastTransition timestamp of the condition will be overwritten with the current time while updating. + Conditions []conditions.Condition[ConType] +} diff --git a/pkg/controller/status_updater_test.go b/pkg/controller/status_updater_test.go new file mode 100644 index 0000000..2fbee58 --- /dev/null +++ b/pkg/controller/status_updater_test.go @@ -0,0 +1,402 @@ +package controller_test + +import ( + "fmt" + "slices" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + . "github.com/openmcp-project/controller-utils/pkg/testing/matchers" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/scheme" + + "github.com/openmcp-project/controller-utils/pkg/conditions" + "github.com/openmcp-project/controller-utils/pkg/controller" + "github.com/openmcp-project/controller-utils/pkg/errors" + testutils "github.com/openmcp-project/controller-utils/pkg/testing" +) + +var coScheme *runtime.Scheme + +var _ = Describe("Status Updater", func() { + + It("should update an empty status", func() { + env := testutils.NewEnvironmentBuilder().WithFakeClient(coScheme).WithInitObjectPath("testdata", "test-02").WithDynamicObjectsWithStatus(&CustomObject{}).Build() + obj := &CustomObject{} + Expect(env.Client().Get(env.Ctx, testutils.ObjectKey("nostatus", "default"), obj)).To(Succeed()) + rr := controller.ReconcileResult[*CustomObject, ConditionStatus]{ + Object: obj, + ReconcileError: errors.WithReason(fmt.Errorf("test error"), "TestError"), + Conditions: dummyConditions(), + } + su := preconfiguredStatusUpdaterBuilder().Build() + now := time.Now() + Expect(su.UpdateStatus(env.Ctx, env.Client(), rr)).To(Succeed()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(obj), obj)).To(Succeed()) + + Expect(obj.Status.Phase).To(Equal(PhaseFailed)) + Expect(obj.Status.ObservedGeneration).To(Equal(obj.GetGeneration())) + Expect(obj.Status.Reason).To(Equal("TestError")) + Expect(obj.Status.Message).To(ContainSubstring("test error")) + Expect(obj.Status.LastReconcileTime.Time).To(BeTemporally("~", now, 1*time.Second)) + Expect(obj.Status.Conditions).To(ConsistOf( + MatchCondition(NewConditionImpl[ConditionStatus](). + WithType("TestConditionTrue"). + WithStatus(ConditionStatusTrue). + WithReason("TestReasonTrue"). + WithMessage("TestMessageTrue"). + WithLastTransitionTime(obj.Status.LastReconcileTime.Time)), + MatchCondition(NewConditionImpl[ConditionStatus](). + WithType("TestConditionFalse"). + WithStatus(ConditionStatusFalse). + WithReason("TestReasonFalse"). + WithMessage("TestMessageFalse"). + WithLastTransitionTime(obj.Status.LastReconcileTime.Time)), + )) + }) + +}) + +func preconfiguredStatusUpdaterBuilder() *controller.StatusUpdaterBuilder[*CustomObject, CustomObjectPhase, ConditionStatus] { + nestedFields := controller.AllStatusFields() + phaseIdx := slices.Index(nestedFields, controller.STATUS_FIELD_PHASE) + if phaseIdx >= 0 { + nestedFields = slices.Delete(nestedFields, phaseIdx, phaseIdx+1) + } + return controller.NewStatusUpdaterBuilder[*CustomObject, CustomObjectPhase, ConditionStatus]().WithNestedStruct("CommonStatus", nestedFields...).WithPhaseUpdateFunc(dummyPhaseUpdateFunc).WithConditionUpdater(func() conditions.Condition[ConditionStatus] { return &Condition{} }, true) +} + +func dummyConditions() []conditions.Condition[ConditionStatus] { + return []conditions.Condition[ConditionStatus]{ + &Condition{ + Type: "TestConditionTrue", + Status: ConditionStatusTrue, + Reason: "TestReasonTrue", + Message: "TestMessageTrue", + }, + &Condition{ + Type: "TestConditionFalse", + Status: ConditionStatusFalse, + Reason: "TestReasonFalse", + Message: "TestMessageFalse", + }, + } +} + +func dummyPhaseUpdateFunc(obj *CustomObject, rr controller.ReconcileResult[*CustomObject, ConditionStatus]) (CustomObjectPhase, error) { + if rr.ReconcileError != nil { + return PhaseFailed, nil + } + if len(obj.Status.Conditions) > 0 { + for _, con := range obj.Status.Conditions { + if con.GetStatus() != ConditionStatusTrue { + return PhaseFailed, nil + } + } + } + return PhaseSucceeded, nil +} + +///////////////////////////////// +// DUMMY OBJECT IMPLEMENTATION // +///////////////////////////////// + +var _ client.Object = &CustomObject{} + +// This is a dummy k8s object implementation to test the status updater on. +type CustomObject struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec CustomObjectSpec `json:"spec,omitempty"` + Status CustomObjectStatus `json:"status,omitempty"` +} + +type CustomObjectSpec struct { +} + +type CustomObjectStatus struct { + CommonStatus `json:",inline"` + + // Phase is the current phase of the cluster. + Phase CustomObjectPhase `json:"phase"` +} + +type CustomObjectPhase string + +const ( + PhaseSucceeded CustomObjectPhase = "Succeeded" + PhaseFailed CustomObjectPhase = "Failed" +) + +type ConditionStatus string + +const ( + ConditionStatusTrue ConditionStatus = "True" + ConditionStatusFalse ConditionStatus = "False" + ConditionStatusUnknown ConditionStatus = "Unknown" +) + +type Condition struct { + // Type is the type of the condition. + // Must be unique within the resource. + Type string `json:"type"` + + // Status is the status of the condition. + Status ConditionStatus `json:"status"` + + // Reason is expected to contain a CamelCased string that provides further information regarding the condition. + // It should have a fixed value set (like an enum) to be machine-readable. The value set depends on the condition type. + // It is optional, but should be filled at least when Status is not "True". + // +optional + Reason string `json:"reason,omitempty"` + + // Message contains further details regarding the condition. + // It is meant for human users, Reason should be used for programmatic evaluation instead. + // It is optional, but should be filled at least when Status is not "True". + // +optional + Message string `json:"message,omitempty"` + + // LastTransitionTime specifies the time when this condition's status last changed. + LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"` +} + +// Implement the Condition interface from our controller-utils library +func (c *Condition) GetType() string { + return c.Type +} +func (c *Condition) SetType(t string) { + c.Type = t +} +func (c *Condition) GetStatus() ConditionStatus { + return c.Status +} +func (c *Condition) SetStatus(s ConditionStatus) { + c.Status = s +} +func (c *Condition) GetReason() string { + return c.Reason +} +func (c *Condition) SetReason(r string) { + c.Reason = r +} +func (c *Condition) GetMessage() string { + return c.Message +} +func (c *Condition) SetMessage(m string) { + c.Message = m +} +func (c *Condition) GetLastTransitionTime() time.Time { + return c.LastTransitionTime.Time +} +func (c *Condition) SetLastTransitionTime(t time.Time) { + c.LastTransitionTime = metav1.NewTime(t) +} + +// ConditionList is a list of Conditions. +type ConditionList []*Condition + +type CommonStatus struct { + ObservedGeneration int64 `json:"observedGeneration"` + + // LastReconcileTime is the time when the resource was last reconciled by the controller. + LastReconcileTime metav1.Time `json:"lastReconcileTime"` + + // Reason is expected to contain a CamelCased string that provides further information in a machine-readable format. + // +optional + Reason string `json:"reason,omitempty"` + + // Message contains further details in a human-readable format. + // +optional + Message string `json:"message,omitempty"` + + // Conditions contains the conditions. + // +optional + Conditions ConditionList `json:"conditions,omitempty"` +} + +type CustomObjectList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitempty"` + Items []CustomObject `json:"items"` +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CommonStatus) DeepCopyInto(out *CommonStatus) { + *out = *in + in.LastReconcileTime.DeepCopyInto(&out.LastReconcileTime) + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make(ConditionList, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Condition) + (*in).DeepCopyInto(*out) + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CommonStatus. +func (in *CommonStatus) DeepCopy() *CommonStatus { + if in == nil { + return nil + } + out := new(CommonStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Condition) DeepCopyInto(out *Condition) { + *out = *in + in.LastTransitionTime.DeepCopyInto(&out.LastTransitionTime) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Condition. +func (in *Condition) DeepCopy() *Condition { + if in == nil { + return nil + } + out := new(Condition) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in ConditionList) DeepCopyInto(out *ConditionList) { + { + in := &in + *out = make(ConditionList, len(*in)) + for i := range *in { + if (*in)[i] != nil { + in, out := &(*in)[i], &(*out)[i] + *out = new(Condition) + (*in).DeepCopyInto(*out) + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConditionList. +func (in ConditionList) DeepCopy() ConditionList { + if in == nil { + return nil + } + out := new(ConditionList) + in.DeepCopyInto(out) + return *out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CustomObject) DeepCopyInto(out *CustomObject) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CustomObject. +func (in *CustomObject) DeepCopy() *CustomObject { + if in == nil { + return nil + } + out := new(CustomObject) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *CustomObject) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CustomObjectSpec) DeepCopyInto(out *CustomObjectSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CustomObjectSpec. +func (in *CustomObjectSpec) DeepCopy() *CustomObjectSpec { + if in == nil { + return nil + } + out := new(CustomObjectSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CustomObjectStatus) DeepCopyInto(out *CustomObjectStatus) { + *out = *in + in.CommonStatus.DeepCopyInto(&out.CommonStatus) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CustomObjectStatus. +func (in *CustomObjectStatus) DeepCopy() *CustomObjectStatus { + if in == nil { + return nil + } + out := new(CustomObjectStatus) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CustomObjectList) DeepCopyInto(out *CustomObjectList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]CustomObject, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CustomObjectList. +func (in *CustomObjectList) DeepCopy() *CustomObjectList { + if in == nil { + return nil + } + out := new(CustomObjectList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *CustomObjectList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +func init() { + SchemeBuilder.Register(&CustomObject{}, &CustomObjectList{}) + coScheme = runtime.NewScheme() + err := SchemeBuilder.AddToScheme(coScheme) + if err != nil { + panic(err) + } +} + +var ( + // GroupVersion is group version used to register these objects + GroupVersion = schema.GroupVersion{Group: "testing.openmcp.cloud", Version: "v1alpha1"} + + // SchemeBuilder is used to add go types to the GroupVersionKind scheme + SchemeBuilder = &scheme.Builder{GroupVersion: GroupVersion} +) diff --git a/pkg/controller/testdata/test-02/co_nostatus.yaml b/pkg/controller/testdata/test-02/co_nostatus.yaml new file mode 100644 index 0000000..04c0ebf --- /dev/null +++ b/pkg/controller/testdata/test-02/co_nostatus.yaml @@ -0,0 +1,6 @@ +apiVersion: testing.openmcp.cloud/v1alpha1 +kind: CustomObject +metadata: + name: nostatus + namespace: default +spec: {} diff --git a/pkg/testing/matchers/conditions.go b/pkg/testing/matchers/conditions.go new file mode 100644 index 0000000..1d20db4 --- /dev/null +++ b/pkg/testing/matchers/conditions.go @@ -0,0 +1,178 @@ +package matchers + +import ( + "fmt" + "reflect" + "time" + + "github.com/onsi/gomega/types" + "github.com/openmcp-project/controller-utils/pkg/conditions" +) + +// MatchCondition returns a Gomega matcher that checks if a Condition is equal to the expected one. +// If the passed in 'actual' is not a Condition, the matcher will fail. +// All fields which are set to their zero value in the expected condition will be ignored. +func MatchCondition[T comparable](con *ConditionImpl[T]) types.GomegaMatcher { + return &conditionMatcher[T]{expected: con} +} + +type conditionMatcher[T comparable] struct { + expected *ConditionImpl[T] +} + +var _ types.GomegaMatcher = &conditionMatcher[bool]{} + +// Match implements types.GomegaMatcher. +func (c *conditionMatcher[T]) Match(actualRaw interface{}) (success bool, err error) { + actual, ok := actualRaw.(conditions.Condition[T]) + if !ok { + return false, fmt.Errorf("expected actual to be of type Condition[%s], got %T", reflect.TypeFor[T]().Name(), actualRaw) + } + if c.expected.HasType() && c.expected.GetType() != actual.GetType() { + return false, nil + } + if c.expected.HasStatus() && c.expected.GetStatus() != actual.GetStatus() { + return false, nil + } + if c.expected.HasReason() && c.expected.GetReason() != actual.GetReason() { + return false, nil + } + if c.expected.HasMessage() && c.expected.GetMessage() != actual.GetMessage() { + return false, nil + } + if c.expected.HasLastTransitionTime() && !c.expected.GetLastTransitionTime().Equal(actual.GetLastTransitionTime()) { + return false, nil + } + return true, nil +} + +// FailureMessage implements types.GomegaMatcher. +func (c *conditionMatcher[T]) FailureMessage(actual interface{}) (message string) { + return fmt.Sprintf("Expected\n\t%#v\nto equal \n\t%#v", actual, c.expected) +} + +// NegatedFailureMessage implements types.GomegaMatcher. +func (c *conditionMatcher[T]) NegatedFailureMessage(actual interface{}) (message string) { + return fmt.Sprintf("Expected\n\t%#v\nto not equal \n\t%#v", actual, c.expected) +} + +func NewCondition[T comparable]() conditions.Condition[T] { + return NewConditionImpl[T]() +} + +func NewConditionImpl[T comparable]() *ConditionImpl[T] { + return &ConditionImpl[T]{} +} + +func NewConditionImplFromValues[T comparable](conType string, status T, reason, message string, now time.Time) *ConditionImpl[T] { + return &ConditionImpl[T]{ + status: &status, + conType: &conType, + reason: &reason, + message: &message, + lastTransitionTime: &now, + } +} + +func NewConditionImplFromCondition[T comparable](con conditions.Condition[T]) *ConditionImpl[T] { + return NewConditionImplFromValues(con.GetType(), con.GetStatus(), con.GetReason(), con.GetMessage(), con.GetLastTransitionTime()) +} + +type ConditionImpl[T comparable] struct { + status *T + conType *string + reason *string + message *string + lastTransitionTime *time.Time +} + +var _ conditions.Condition[bool] = &ConditionImpl[bool]{} + +func (c *ConditionImpl[T]) GetLastTransitionTime() time.Time { + return *c.lastTransitionTime +} + +func (c *ConditionImpl[T]) GetType() string { + return *c.conType +} + +func (c *ConditionImpl[T]) GetStatus() T { + return *c.status +} + +func (c *ConditionImpl[T]) GetReason() string { + return *c.reason +} + +func (c *ConditionImpl[T]) GetMessage() string { + return *c.message +} + +func (c *ConditionImpl[T]) SetStatus(status T) { + c.status = &status +} + +func (c *ConditionImpl[T]) SetType(conType string) { + c.conType = &conType +} + +func (c *ConditionImpl[T]) SetLastTransitionTime(timestamp time.Time) { + c.lastTransitionTime = ×tamp +} + +func (c *ConditionImpl[T]) SetReason(reason string) { + c.reason = &reason +} + +func (c *ConditionImpl[T]) SetMessage(message string) { + c.message = &message +} + +func (c *ConditionImpl[T]) HasLastTransitionTime() bool { + return c.lastTransitionTime != nil +} + +func (c *ConditionImpl[T]) HasType() bool { + return c.conType != nil +} + +func (c *ConditionImpl[T]) HasStatus() bool { + return c.status != nil +} + +func (c *ConditionImpl[T]) HasReason() bool { + return c.reason != nil +} + +func (c *ConditionImpl[T]) HasMessage() bool { + return c.message != nil +} + +func (c *ConditionImpl[T]) WithLastTransitionTime(timestamp time.Time) *ConditionImpl[T] { + c.SetLastTransitionTime(timestamp) + return c +} + +func (c *ConditionImpl[T]) WithType(conType string) *ConditionImpl[T] { + c.SetType(conType) + return c +} + +func (c *ConditionImpl[T]) WithStatus(status T) *ConditionImpl[T] { + c.SetStatus(status) + return c +} + +func (c *ConditionImpl[T]) WithReason(reason string) *ConditionImpl[T] { + c.SetReason(reason) + return c +} + +func (c *ConditionImpl[T]) WithMessage(message string) *ConditionImpl[T] { + c.SetMessage(message) + return c +} + +func Ptr[T any](v T) *T { + return &v +} diff --git a/pkg/testing/utils.go b/pkg/testing/utils.go index 8c9720d..acbddc2 100644 --- a/pkg/testing/utils.go +++ b/pkg/testing/utils.go @@ -71,6 +71,20 @@ func RequestFromStrings(name string, maybeNamespace ...string) reconcile.Request } } +// ObjectKey returns a client.ObjectKey for the given name and optionally namespace. +// The first argument is the name of the object. +// An optional second argument contains the namespace. All further arguments are ignored. +func ObjectKey(name string, maybeNamespace ...string) client.ObjectKey { + namespace := "" + if len(maybeNamespace) > 0 { + namespace = maybeNamespace[0] + } + return client.ObjectKey{ + Namespace: namespace, + Name: name, + } +} + // LoadObject reads a file and unmarshals it into the given object. // obj must be a non-nil pointer. func LoadObject(obj any, paths ...string) error { From b85ed7e1476f412ae1769a812d7f57ce6b83fa03 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Tue, 15 Apr 2025 13:25:12 +0200 Subject: [PATCH 03/10] change UpdateStatus signature to return reconcile result --- pkg/controller/status_updater.go | 19 +++++++++++-------- pkg/controller/status_updater_test.go | 5 ++++- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/pkg/controller/status_updater.go b/pkg/controller/status_updater.go index 8bbf6f9..2aa2ce9 100644 --- a/pkg/controller/status_updater.go +++ b/pkg/controller/status_updater.go @@ -172,12 +172,12 @@ func defaultPhaseUpdateFunc[Obj client.Object, PhType ~string, ConType comparabl // UpdateStatus updates the status of the object in the given ReconcileResult, using the previously set field names and functions. // The object is expected to be a pointer to a struct with the status field. // If the 'Object' field in the ReconcileResult is nil, the status update becomes a no-op. -func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context, c client.Client, rr ReconcileResult[Obj, ConType]) error { +func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context, c client.Client, rr ReconcileResult[Obj, ConType]) (ctrl.Result, error) { if IsNil(rr.Object) { - return nil + return rr.Result, nil } if s.fieldNames[STATUS_FIELD] == "" { - return nil + return rr.Result, nil } if IsNil(rr.OldObject) || IsSameObject(rr.OldObject, rr.Object) { // create old object based on given one @@ -185,9 +185,10 @@ func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context, } status := GetField(rr.Object, s.fieldNames[STATUS_FIELD], true) if IsNil(status) { - return fmt.Errorf("unable to get pointer to status field '%s' of object %T", s.fieldNames[STATUS_FIELD], rr.Object) + return rr.Result, fmt.Errorf("unable to get pointer to status field '%s' of object %T", s.fieldNames[STATUS_FIELD], rr.Object) } + errs := errors.NewReasonableErrorList(rr.ReconcileError) now := time.Now() if s.fieldNames[STATUS_FIELD_LAST_RECONCILE_TIME] != "" { SetField(status, s.fieldNames[STATUS_FIELD_LAST_RECONCILE_TIME], metav1.NewTime(now)) @@ -232,21 +233,23 @@ func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context, if s.fieldNames[STATUS_FIELD_PHASE] != "" { phase, err := s.phaseUpdateFunc(rr.Object, rr) if err != nil { - return fmt.Errorf("error computing phase: %w", err) + phase, _ = defaultPhaseUpdateFunc[Obj, PhType, ConType](rr.Object, rr) + errs.Append(fmt.Errorf("error computing phase: %w", err)) } SetField(status, s.fieldNames[STATUS_FIELD_PHASE], phase) } if s.customUpdateFunc != nil { if err := s.customUpdateFunc(rr.Object, rr); err != nil { - return fmt.Errorf("error performing custom status update: %w", err) + errs.Append(fmt.Errorf("error performing custom status update: %w", err)) } } // update status in cluster if err := c.Status().Patch(ctx, rr.Object, client.MergeFrom(rr.OldObject)); err != nil { - return fmt.Errorf("error patching status: %w", err) + errs.Append(fmt.Errorf("error patching status: %w", err)) } - return nil + + return rr.Result, errs.Aggregate() } // GetField returns the value of the field with the given name from the given object. diff --git a/pkg/controller/status_updater_test.go b/pkg/controller/status_updater_test.go index 2fbee58..b6d632c 100644 --- a/pkg/controller/status_updater_test.go +++ b/pkg/controller/status_updater_test.go @@ -36,7 +36,10 @@ var _ = Describe("Status Updater", func() { } su := preconfiguredStatusUpdaterBuilder().Build() now := time.Now() - Expect(su.UpdateStatus(env.Ctx, env.Client(), rr)).To(Succeed()) + res, err := su.UpdateStatus(env.Ctx, env.Client(), rr) + Expect(res).To(Equal(rr.Result)) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(rr.ReconcileError)) Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(obj), obj)).To(Succeed()) Expect(obj.Status.Phase).To(Equal(PhaseFailed)) From d5c80ca757c28638a1f1dc0bb3f1e00de081eeee Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Tue, 15 Apr 2025 13:33:56 +0200 Subject: [PATCH 04/10] change WithoutField to WithoutFields --- pkg/controller/status_updater.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/controller/status_updater.go b/pkg/controller/status_updater.go index 2aa2ce9..750ec5e 100644 --- a/pkg/controller/status_updater.go +++ b/pkg/controller/status_updater.go @@ -74,10 +74,14 @@ func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithNestedStruct(name strin return b } -// WithoutField removes the field from the status update. -// This is an alias for WithFieldOverride(field, ""). -func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithoutField(field StatusField) *StatusUpdaterBuilder[Obj, PhType, ConType] { - return b.WithFieldOverride(field, "") +// WithoutFields removes the specified fields from the status update. +// It basically calls WithFieldOverride(field, "") for each field. +// This can be used in combination with AllStatusFields() to disable all fields. +func (b *StatusUpdaterBuilder[Obj, PhType, ConType]) WithoutFields(fields ...StatusField) *StatusUpdaterBuilder[Obj, PhType, ConType] { + for _, field := range fields { + b.WithFieldOverride(field, "") + } + return b } // WithConditionUpdater must be called if the conditions should be updated, because this requires some additional information. From f8e98b96dd31725e84d9ac32a10f0472bb7e5db7 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Wed, 16 Apr 2025 09:07:53 +0200 Subject: [PATCH 05/10] make StatusUpdater work with condition implementations that are not pointers --- pkg/controller/status_updater.go | 17 +++++++- pkg/controller/status_updater_test.go | 14 ++----- pkg/testing/matchers/conditions.go | 60 +++++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 17 deletions(-) diff --git a/pkg/controller/status_updater.go b/pkg/controller/status_updater.go index 750ec5e..4fe10a8 100644 --- a/pkg/controller/status_updater.go +++ b/pkg/controller/status_updater.go @@ -227,10 +227,23 @@ func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context, } newConsRaw, _ := cu.Conditions() conType := reflect.TypeOf(s.conConstruct()) + targetType := reflect.TypeOf(GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false)) + depointerize := false + if !reflect.SliceOf(conType).AssignableTo(targetType) { + // this can happen if the constructor returns a *ConditionImplementation, which creates a []*ConditionImplementation below, + // but the condition list field is a []ConditionImplementation + if conType.Kind() == reflect.Ptr { + depointerize = true + conType = conType.Elem() + } + } newCons := reflect.MakeSlice(reflect.SliceOf(conType), len(newConsRaw), len(newConsRaw)).Interface() for i := range newConsRaw { - val := reflect.ValueOf(newConsRaw[i]).Convert(conType) - reflect.ValueOf(newCons).Index(i).Set(val) + val := reflect.ValueOf(newConsRaw[i]) + if depointerize { + val = val.Elem() + } + reflect.ValueOf(newCons).Index(i).Set(val.Convert(conType)) } SetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], newCons) } diff --git a/pkg/controller/status_updater_test.go b/pkg/controller/status_updater_test.go index b6d632c..50d64bb 100644 --- a/pkg/controller/status_updater_test.go +++ b/pkg/controller/status_updater_test.go @@ -202,7 +202,7 @@ func (c *Condition) SetLastTransitionTime(t time.Time) { } // ConditionList is a list of Conditions. -type ConditionList []*Condition +type ConditionList []Condition type CommonStatus struct { ObservedGeneration int64 `json:"observedGeneration"` @@ -237,11 +237,7 @@ func (in *CommonStatus) DeepCopyInto(out *CommonStatus) { in, out := &in.Conditions, &out.Conditions *out = make(ConditionList, len(*in)) for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(Condition) - (*in).DeepCopyInto(*out) - } + (*in)[i].DeepCopyInto(&(*out)[i]) } } } @@ -278,11 +274,7 @@ func (in ConditionList) DeepCopyInto(out *ConditionList) { in := &in *out = make(ConditionList, len(*in)) for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(Condition) - (*in).DeepCopyInto(*out) - } + (*in)[i].DeepCopyInto(&(*out)[i]) } } } diff --git a/pkg/testing/matchers/conditions.go b/pkg/testing/matchers/conditions.go index 1d20db4..ba384f6 100644 --- a/pkg/testing/matchers/conditions.go +++ b/pkg/testing/matchers/conditions.go @@ -20,13 +20,32 @@ type conditionMatcher[T comparable] struct { expected *ConditionImpl[T] } +func (c *conditionMatcher[T]) GomegaString() string { + if c == nil || c.expected == nil { + return "" + } + return c.expected.String() +} + var _ types.GomegaMatcher = &conditionMatcher[bool]{} // Match implements types.GomegaMatcher. -func (c *conditionMatcher[T]) Match(actualRaw interface{}) (success bool, err error) { - actual, ok := actualRaw.(conditions.Condition[T]) - if !ok { - return false, fmt.Errorf("expected actual to be of type Condition[%s], got %T", reflect.TypeFor[T]().Name(), actualRaw) +func (c *conditionMatcher[T]) Match(actualRaw any) (success bool, err error) { + actual, converted := actualRaw.(conditions.Condition[T]) + if !converted { + // actualRaw doesn't implement conditions.Condition[T], check if a pointer to it does + ptrValue := reflect.New(reflect.TypeOf(actualRaw)) + reflect.Indirect(ptrValue).Set(reflect.ValueOf(actualRaw)) + actual, converted = ptrValue.Interface().(conditions.Condition[T]) + } + if !converted { + return false, fmt.Errorf("expected actual (or &actual) to be of type Condition[%s], got %T", reflect.TypeFor[T]().Name(), actualRaw) + } + if actual == nil && c.expected == nil { + return true, nil + } + if actual == nil || c.expected == nil { + return false, nil } if c.expected.HasType() && c.expected.GetType() != actual.GetType() { return false, nil @@ -86,6 +105,39 @@ type ConditionImpl[T comparable] struct { lastTransitionTime *time.Time } +func (c *ConditionImpl[T]) String() string { + if c == nil { + return "" + } + var status, conType, reason, message, lastTransitionTime string + if c.status == nil { + status = "" + } else { + status = fmt.Sprintf("%v", *c.status) + } + if c.conType == nil { + conType = "" + } else { + conType = *c.conType + } + if c.reason == nil { + reason = "" + } else { + reason = *c.reason + } + if c.message == nil { + message = "" + } else { + message = *c.message + } + if c.lastTransitionTime == nil { + lastTransitionTime = "" + } else { + lastTransitionTime = c.lastTransitionTime.Format(time.RFC3339) + } + return fmt.Sprintf("Condition[%s]{\n\tType: %q,\n\tStatus: %s,\n\tReason: %q,\n\tMessage: %q,\n\tLastTransitionTime: %v,\n}", reflect.TypeFor[T]().Name(), conType, status, reason, message, lastTransitionTime) +} + var _ conditions.Condition[bool] = &ConditionImpl[bool]{} func (c *ConditionImpl[T]) GetLastTransitionTime() time.Time { From 24b8bb6c0f161746da8ba720a84d033673b26ef7 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Thu, 17 Apr 2025 18:02:22 +0200 Subject: [PATCH 06/10] add more tests for status updater and fix bugs --- pkg/controller/status_updater.go | 36 ++++-- pkg/controller/status_updater_test.go | 116 +++++++++++++++++- .../testdata/test-02/co_status.yaml | 23 ++++ pkg/testing/matchers/conditions.go | 9 +- 4 files changed, 169 insertions(+), 15 deletions(-) create mode 100644 pkg/controller/testdata/test-02/co_status.yaml diff --git a/pkg/controller/status_updater.go b/pkg/controller/status_updater.go index 4fe10a8..6f6d131 100644 --- a/pkg/controller/status_updater.go +++ b/pkg/controller/status_updater.go @@ -215,10 +215,31 @@ func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context, SetField(status, s.fieldNames[STATUS_FIELD_REASON], reason) } if s.fieldNames[STATUS_FIELD_CONDITIONS] != "" && s.conConstruct != nil { + conType := reflect.TypeOf(s.conConstruct()) + targetType := reflect.TypeOf(GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false)) + pointerMagic := false + if !reflect.SliceOf(conType).AssignableTo(targetType) { + // this can happen if the constructor returns a *ConditionImplementation, + // but the condition list field is a []ConditionImplementation + if conType.Kind() == reflect.Ptr { + pointerMagic = true + conType = conType.Elem() + } + } oldConsRaw := GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false) var oldCons []conditions.Condition[ConType] if !IsNil(oldConsRaw) { - oldCons = oldConsRaw.([]conditions.Condition[ConType]) + val := reflect.ValueOf(oldConsRaw) + oldCons := make([]conditions.Condition[ConType], val.Len()) + for i := 0; i < val.Len(); i++ { + eVal := val.Index(i) + if pointerMagic { + ptrValue := reflect.New(conType) + reflect.Indirect(ptrValue).Set(eVal) + eVal = ptrValue + } + oldCons[i] = eVal.Interface().(conditions.Condition[ConType]) + } } cu := conditions.ConditionUpdater(s.conConstruct, oldCons, s.removeUntouchedConditions) cu.Now = now @@ -226,21 +247,10 @@ func (s *statusUpdater[Obj, PhType, ConType]) UpdateStatus(ctx context.Context, cu.UpdateConditionFromTemplate(con) } newConsRaw, _ := cu.Conditions() - conType := reflect.TypeOf(s.conConstruct()) - targetType := reflect.TypeOf(GetField(status, s.fieldNames[STATUS_FIELD_CONDITIONS], false)) - depointerize := false - if !reflect.SliceOf(conType).AssignableTo(targetType) { - // this can happen if the constructor returns a *ConditionImplementation, which creates a []*ConditionImplementation below, - // but the condition list field is a []ConditionImplementation - if conType.Kind() == reflect.Ptr { - depointerize = true - conType = conType.Elem() - } - } newCons := reflect.MakeSlice(reflect.SliceOf(conType), len(newConsRaw), len(newConsRaw)).Interface() for i := range newConsRaw { val := reflect.ValueOf(newConsRaw[i]) - if depointerize { + if pointerMagic { val = val.Elem() } reflect.ValueOf(newCons).Index(i).Set(val.Convert(conType)) diff --git a/pkg/controller/status_updater_test.go b/pkg/controller/status_updater_test.go index 50d64bb..2ff6e0b 100644 --- a/pkg/controller/status_updater_test.go +++ b/pkg/controller/status_updater_test.go @@ -7,6 +7,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + . "github.com/openmcp-project/controller-utils/pkg/testing/matchers" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -28,7 +29,7 @@ var _ = Describe("Status Updater", func() { It("should update an empty status", func() { env := testutils.NewEnvironmentBuilder().WithFakeClient(coScheme).WithInitObjectPath("testdata", "test-02").WithDynamicObjectsWithStatus(&CustomObject{}).Build() obj := &CustomObject{} - Expect(env.Client().Get(env.Ctx, testutils.ObjectKey("nostatus", "default"), obj)).To(Succeed()) + Expect(env.Client().Get(env.Ctx, controller.ObjectKey("nostatus", "default"), obj)).To(Succeed()) rr := controller.ReconcileResult[*CustomObject, ConditionStatus]{ Object: obj, ReconcileError: errors.WithReason(fmt.Errorf("test error"), "TestError"), @@ -63,6 +64,119 @@ var _ = Describe("Status Updater", func() { )) }) + It("should update an existing status", func() { + env := testutils.NewEnvironmentBuilder().WithFakeClient(coScheme).WithInitObjectPath("testdata", "test-02").WithDynamicObjectsWithStatus(&CustomObject{}).Build() + obj := &CustomObject{} + Expect(env.Client().Get(env.Ctx, controller.ObjectKey("status", "default"), obj)).To(Succeed()) + rr := controller.ReconcileResult[*CustomObject, ConditionStatus]{ + Object: obj, + Conditions: dummyConditions(), + } + su := preconfiguredStatusUpdaterBuilder().WithPhaseUpdateFunc(func(obj *CustomObject, rr controller.ReconcileResult[*CustomObject, ConditionStatus]) (CustomObjectPhase, error) { + return PhaseSucceeded, nil + }).Build() + now := time.Now() + res, err := su.UpdateStatus(env.Ctx, env.Client(), rr) + Expect(res).To(Equal(rr.Result)) + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(obj), obj)).To(Succeed()) + + Expect(obj.Status.Phase).To(Equal(PhaseSucceeded)) + Expect(obj.Status.ObservedGeneration).To(Equal(obj.GetGeneration())) + Expect(obj.Status.Reason).To(BeEmpty()) + Expect(obj.Status.Message).To(BeEmpty()) + Expect(obj.Status.LastReconcileTime.Time).To(BeTemporally("~", now, 1*time.Second)) + Expect(obj.Status.Conditions).To(ConsistOf( + MatchCondition(NewConditionImpl[ConditionStatus](). + WithType("TestConditionTrue"). + WithStatus(ConditionStatusTrue). + WithReason("TestReasonTrue"). + WithMessage("TestMessageTrue"). + WithLastTransitionTime(obj.Status.LastReconcileTime.Time)), + MatchCondition(NewConditionImpl[ConditionStatus](). + WithType("TestConditionFalse"). + WithStatus(ConditionStatusFalse). + WithReason("TestReasonFalse"). + WithMessage("TestMessageFalse"). + WithLastTransitionTime(obj.Status.LastReconcileTime.Time)), + )) + }) + + It("should not update disabled fields", func() { + env := testutils.NewEnvironmentBuilder().WithFakeClient(coScheme).WithInitObjectPath("testdata", "test-02").WithDynamicObjectsWithStatus(&CustomObject{}).Build() + obj := &CustomObject{} + Expect(env.Client().Get(env.Ctx, controller.ObjectKey("status", "default"), obj)).To(Succeed()) + before := obj.DeepCopy() + for _, disabledField := range controller.AllStatusFields() { + By(fmt.Sprintf("Testing disabled field %s", disabledField)) + // reset object to remove changes from previous loop executions + modified := obj.DeepCopy() + obj.Status = before.Status + Expect(env.Client().Status().Patch(env.Ctx, obj, client.MergeFrom(modified))).To(Succeed()) + rr := controller.ReconcileResult[*CustomObject, ConditionStatus]{ + Object: obj, + Conditions: dummyConditions(), + Reason: "TestReason", + Message: "TestMessage", + } + su := preconfiguredStatusUpdaterBuilder().WithPhaseUpdateFunc(func(obj *CustomObject, rr controller.ReconcileResult[*CustomObject, ConditionStatus]) (CustomObjectPhase, error) { + return PhaseSucceeded, nil + }).WithoutFields(disabledField).Build() + now := time.Now() + res, err := su.UpdateStatus(env.Ctx, env.Client(), rr) + Expect(res).To(Equal(rr.Result)) + + Expect(err).ToNot(HaveOccurred()) + Expect(env.Client().Get(env.Ctx, client.ObjectKeyFromObject(obj), obj)).To(Succeed()) + + if disabledField == controller.STATUS_FIELD_PHASE { + Expect(obj.Status.Phase).To(Equal(before.Status.Phase)) + } else { + Expect(obj.Status.Phase).To(Equal(PhaseSucceeded)) + } + if disabledField == controller.STATUS_FIELD_OBSERVED_GENERATION { + Expect(obj.Status.ObservedGeneration).To(Equal(before.Status.ObservedGeneration)) + } else { + Expect(obj.Status.ObservedGeneration).To(Equal(obj.GetGeneration())) + } + if disabledField == controller.STATUS_FIELD_REASON { + Expect(obj.Status.Reason).To(Equal(before.Status.Reason)) + } else { + Expect(obj.Status.Reason).To(Equal(rr.Reason)) + } + if disabledField == controller.STATUS_FIELD_MESSAGE { + Expect(obj.Status.Message).To(Equal(before.Status.Message)) + } else { + Expect(obj.Status.Message).To(Equal(rr.Message)) + } + if disabledField == controller.STATUS_FIELD_LAST_RECONCILE_TIME { + Expect(obj.Status.LastReconcileTime.Time).To(Equal(before.Status.LastReconcileTime.Time)) + } else { + Expect(obj.Status.LastReconcileTime.Time).To(BeTemporally("~", now, 1*time.Second)) + } + if disabledField == controller.STATUS_FIELD_CONDITIONS { + Expect(obj.Status.Conditions).To(Equal(before.Status.Conditions)) + } else { + Expect(obj.Status.Conditions).To(ConsistOf( + MatchCondition(NewConditionImpl[ConditionStatus](). + WithType("TestConditionTrue"). + WithStatus(ConditionStatusTrue). + WithReason("TestReasonTrue"). + WithMessage("TestMessageTrue"). + WithLastTransitionTime(now). + WithTimestampTolerance(1*time.Second)), + MatchCondition(NewConditionImpl[ConditionStatus](). + WithType("TestConditionFalse"). + WithStatus(ConditionStatusFalse). + WithReason("TestReasonFalse"). + WithMessage("TestMessageFalse"). + WithLastTransitionTime(now). + WithTimestampTolerance(1*time.Second)), + )) + } + } + }) + }) func preconfiguredStatusUpdaterBuilder() *controller.StatusUpdaterBuilder[*CustomObject, CustomObjectPhase, ConditionStatus] { diff --git a/pkg/controller/testdata/test-02/co_status.yaml b/pkg/controller/testdata/test-02/co_status.yaml new file mode 100644 index 0000000..3cc7c4a --- /dev/null +++ b/pkg/controller/testdata/test-02/co_status.yaml @@ -0,0 +1,23 @@ +apiVersion: testing.openmcp.cloud/v1alpha1 +kind: CustomObject +metadata: + name: status + namespace: default +spec: {} +status: + conditions: + - type: "TestConditionTrue" + status: "True" + lastTransitionTime: "2023-10-01T00:00:00Z" + - type: "TestConditionFalse" + status: "Unknown" + lastTransitionTime: "2023-10-01T00:00:00Z" + - type: "AdditionalCondition" + status: "True" + lastTransitionTime: "2023-10-01T00:00:00Z" + observedGeneration: 0 + phase: "Failed" + reason: "OldReason" + message: "This is the old message" + lastReconcileTime: "2023-10-01T00:00:00Z" + diff --git a/pkg/testing/matchers/conditions.go b/pkg/testing/matchers/conditions.go index ba384f6..3dd8c44 100644 --- a/pkg/testing/matchers/conditions.go +++ b/pkg/testing/matchers/conditions.go @@ -6,6 +6,7 @@ import ( "time" "github.com/onsi/gomega/types" + "github.com/openmcp-project/controller-utils/pkg/conditions" ) @@ -59,7 +60,7 @@ func (c *conditionMatcher[T]) Match(actualRaw any) (success bool, err error) { if c.expected.HasMessage() && c.expected.GetMessage() != actual.GetMessage() { return false, nil } - if c.expected.HasLastTransitionTime() && !c.expected.GetLastTransitionTime().Equal(actual.GetLastTransitionTime()) { + if c.expected.HasLastTransitionTime() && c.expected.GetLastTransitionTime().Sub(actual.GetLastTransitionTime()) > c.expected.timestampTolerance { return false, nil } return true, nil @@ -103,6 +104,7 @@ type ConditionImpl[T comparable] struct { reason *string message *string lastTransitionTime *time.Time + timestampTolerance time.Duration } func (c *ConditionImpl[T]) String() string { @@ -225,6 +227,11 @@ func (c *ConditionImpl[T]) WithMessage(message string) *ConditionImpl[T] { return c } +func (c *ConditionImpl[T]) WithTimestampTolerance(t time.Duration) *ConditionImpl[T] { + c.timestampTolerance = t + return c +} + func Ptr[T any](v T) *T { return &v } From 80394ab49bf097e27ff81215db70511e11783736 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Thu, 17 Apr 2025 18:03:16 +0200 Subject: [PATCH 07/10] library improvements (from implementing Landscape controller of Gardener ClusterProvider) --- pkg/clusters/cluster.go | 9 ++++++- pkg/conditions/updater_test.go | 1 + pkg/controller/annotation_label.go | 1 - pkg/controller/predicates.go | 41 ++++++++++++++++++++++++++++++ pkg/controller/utils.go | 16 ++++++++++++ pkg/testing/utils.go | 14 ---------- 6 files changed, 66 insertions(+), 16 deletions(-) diff --git a/pkg/clusters/cluster.go b/pkg/clusters/cluster.go index d0c571a..e60d611 100644 --- a/pkg/clusters/cluster.go +++ b/pkg/clusters/cluster.go @@ -40,6 +40,13 @@ func (c *Cluster) WithConfigPath(cfgPath string) *Cluster { return c } +// WithRestConfig allows to set the REST config manually. +// Returns the cluster for chaining. +func (c *Cluster) WithRESTConfig(cfg *rest.Config) *Cluster { + c.restCfg = cfg + return c +} + // RegisterConfigPathFlag adds a flag '---cluster' for the cluster's config path to the given flag set. // Panics if the cluster's id is not set. func (c *Cluster) RegisterConfigPathFlag(flags *flag.FlagSet) { @@ -117,7 +124,7 @@ func (c *Cluster) InitializeClient(scheme *runtime.Scheme) error { if err != nil { return fmt.Errorf("failed to create '%s' cluster client: %w", c.ID(), err) } - clu, err := cluster.New(c.restCfg, func(o *cluster.Options) { o.Scheme = scheme }) + clu, err := cluster.New(c.restCfg, func(o *cluster.Options) { o.Scheme = scheme; o.Cache.Scheme = scheme }) if err != nil { return fmt.Errorf("failed to create '%s' cluster Cluster representation: %w", c.ID(), err) } diff --git a/pkg/conditions/updater_test.go b/pkg/conditions/updater_test.go index 70a2f31..5d79513 100644 --- a/pkg/conditions/updater_test.go +++ b/pkg/conditions/updater_test.go @@ -7,6 +7,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + . "github.com/openmcp-project/controller-utils/pkg/testing/matchers" "github.com/openmcp-project/controller-utils/pkg/conditions" diff --git a/pkg/controller/annotation_label.go b/pkg/controller/annotation_label.go index f08b5e3..c197077 100644 --- a/pkg/controller/annotation_label.go +++ b/pkg/controller/annotation_label.go @@ -103,7 +103,6 @@ func EnsureLabel(ctx context.Context, c client.Client, obj client.Object, labelK } // ensureMetadataEntry is the common base method for EnsureAnnotation and EnsureLabel. -// This is mainly exposed for testing purposes, usually it is statically known whether an annotation or label is to be modified and the respective method should be used. func ensureMetadataEntry(mType metadataEntryType, ctx context.Context, c client.Client, obj client.Object, key, value string, patch bool, mode ...ModifyMetadataEntryMode) error { modeDelete := false modeOverwrite := false diff --git a/pkg/controller/predicates.go b/pkg/controller/predicates.go index f17ee0b..bbacfd1 100644 --- a/pkg/controller/predicates.go +++ b/pkg/controller/predicates.go @@ -10,6 +10,47 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" ) +////////////////// +/// CONVERSION /// +////////////////// + +// ToTypedPredicate wraps a predicate.Predicate (which is an alias for predicate.TypedPredicate[client.Object]) into a predicate.TypedPredicate[Obj]. +func ToTypedPredicate[Obj client.Object](p predicate.Predicate) predicate.TypedPredicate[Obj] { + return &typedPredicateConverter[Obj]{internal: p} +} + +type typedPredicateConverter[Obj client.Object] struct { + internal predicate.Predicate +} + +var _ predicate.TypedPredicate[client.Object] = &typedPredicateConverter[client.Object]{} + +func (t *typedPredicateConverter[Obj]) Create(e event.TypedCreateEvent[Obj]) bool { + return t.internal.Create(event.TypedCreateEvent[client.Object]{ + Object: e.Object, + }) +} + +func (t *typedPredicateConverter[Obj]) Delete(e event.TypedDeleteEvent[Obj]) bool { + return t.internal.Delete(event.TypedDeleteEvent[client.Object]{ + Object: e.Object, + DeleteStateUnknown: e.DeleteStateUnknown, + }) +} + +func (t *typedPredicateConverter[Obj]) Generic(e event.TypedGenericEvent[Obj]) bool { + return t.internal.Generic(event.TypedGenericEvent[client.Object]{ + Object: e.Object, + }) +} + +func (t *typedPredicateConverter[Obj]) Update(e event.TypedUpdateEvent[Obj]) bool { + return t.internal.Update(event.TypedUpdateEvent[client.Object]{ + ObjectOld: e.ObjectOld, + ObjectNew: e.ObjectNew, + }) +} + ///////////////////////////////////// /// DELETION TIMESTAMP PREDICATES /// ///////////////////////////////////// diff --git a/pkg/controller/utils.go b/pkg/controller/utils.go index 7bfb774..f9c1f45 100644 --- a/pkg/controller/utils.go +++ b/pkg/controller/utils.go @@ -5,6 +5,8 @@ import ( "encoding/base32" "reflect" "strings" + + "sigs.k8s.io/controller-runtime/pkg/client" ) const ( @@ -35,3 +37,17 @@ func IsNil(i any) bool { } return false } + +// ObjectKey returns a client.ObjectKey for the given name and optionally namespace. +// The first argument is the name of the object. +// An optional second argument contains the namespace. All further arguments are ignored. +func ObjectKey(name string, maybeNamespace ...string) client.ObjectKey { + namespace := "" + if len(maybeNamespace) > 0 { + namespace = maybeNamespace[0] + } + return client.ObjectKey{ + Namespace: namespace, + Name: name, + } +} diff --git a/pkg/testing/utils.go b/pkg/testing/utils.go index acbddc2..8c9720d 100644 --- a/pkg/testing/utils.go +++ b/pkg/testing/utils.go @@ -71,20 +71,6 @@ func RequestFromStrings(name string, maybeNamespace ...string) reconcile.Request } } -// ObjectKey returns a client.ObjectKey for the given name and optionally namespace. -// The first argument is the name of the object. -// An optional second argument contains the namespace. All further arguments are ignored. -func ObjectKey(name string, maybeNamespace ...string) client.ObjectKey { - namespace := "" - if len(maybeNamespace) > 0 { - namespace = maybeNamespace[0] - } - return client.ObjectKey{ - Namespace: namespace, - Name: name, - } -} - // LoadObject reads a file and unmarshals it into the given object. // obj must be a non-nil pointer. func LoadObject(obj any, paths ...string) error { From c34632f8dbbf8f2b0ea4a13900949b0c656bbe2b Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Tue, 22 Apr 2025 10:28:18 +0200 Subject: [PATCH 08/10] add documentation for status updater --- README.md | 141 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/README.md b/README.md index abc558d..275ec10 100644 --- a/README.md +++ b/README.md @@ -103,10 +103,151 @@ The `pkg/controller` package contains useful functions for setting up and runnin #### Noteworthy Functions - `LoadKubeconfig` creates a REST config for accessing a k8s cluster. It can be used with a path to a kubeconfig file, or a directory containing files for a trust relationship. When called with an empty path, it returns the in-cluster configuration. + - See also the [`clusters`](#clusters) package, which uses this function internally, but provides some further tooling around it. - There are some functions useful for working with annotations and labels, e.g. `HasAnnotationWithValue` or `EnsureLabel`. - There are multiple predefined predicates to help with filtering reconciliation triggers in controllers, e.g. `HasAnnotationPredicate` or `DeletionTimestampChangedPredicate`. - The `K8sNameHash` function can be used to create a hash that can be used as a name for k8s resources. +#### Status Updater + +The status updater gets its own section, because it requires a slightly longer explanation. The idea of it is that many of our resources use a status similar to this: +```go +type MyStatus struct { + // ObservedGeneration is the generation of this resource that was last reconciled by the controller. + ObservedGeneration int64 `json:"observedGeneration"` + + // LastReconcileTime is the time when the resource was last reconciled by the controller. + LastReconcileTime metav1.Time `json:"lastReconcileTime"` + + // Phase is the overall phase of the resource. + Phase string + + // Reason is expected to contain a CamelCased string that provides further information in a machine-readable format. + // +optional + Reason string `json:"reason,omitempty"` + + // Message contains further details in a human-readable format. + // +optional + Message string `json:"message,omitempty"` + + // Conditions contains the conditions. + // +optional + Conditions []MyCondition `json:"conditions,omitempty"` +} +``` + +The logic for most of these fields is very similar across all of our controllers: `ObservedGeneration` and `LastReconcileTime` should always be updated, `Phase` is usually computed based on the conditions or on whether an error occurred, `Reason`, `Message` and `Conditions` are generated during reconciliation. + +To reduce redundant coding and ensure a similar behavior in all controllers, the _status updater_ can be used to update the status. A full example could look something like this: +```go +import ( + ctrlutils "github.com/openmcp-project/controller-utils/pkg/controller" + v1alpha1 // resource API package +) + +// optional, using a type alias removes the need to specify the type arguments every time +type ReconcileResult = ctrlutils.ReconcileResult[*v1alpha1.MyResource, v1alpha1.ConditionStatus] + +// this is the method called by the controller-runtime +func (r *GardenerMyResourceReconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + rr := r.reconcile(ctx, req) + // status update + return ctrlutils.NewStatusUpdaterBuilder[*v1alpha1.MyResource, v1alpha1.MyResourcePhase, v1alpha1.ConditionStatus](). + WithPhaseUpdateFunc(func(obj *v1alpha1.MyResource, rr ctrlutils.ReconcileResult[*v1alpha1.MyResource, v1alpha1.ConditionStatus]) (v1alpha1.MyResourcePhase, error) { + if rr.ReconcileError != nil { + return v1alpha1.PROVIDER_CONFIG_PHASE_FAILED, nil + } + if len(rr.Conditions) > 0 { + for _, con := range rr.Conditions { + if con.GetStatus() != v1alpha1.CONDITION_TRUE { + return v1alpha1.PROVIDER_CONFIG_PHASE_FAILED, nil + } + } + } + return v1alpha1.PROVIDER_CONFIG_PHASE_SUCCEEDED, nil + }). + WithConditionUpdater(func() conditions.Condition[v1alpha1.ConditionStatus] { + return &v1alpha1.Condition{} + }, true). + Build(). + UpdateStatus(ctx, r.PlatformCluster.Client(), rr) +} + +func (r *GardenerProviderConfigReconciler) reconcile(ctx context.Context, req reconcile.Request) ReconcileResult { + // actual reconcile logic here +} +``` + +Some information regarding the example: +- `v1alpha1.MyResource` is the resource type being reconciled in this example. +- `v1alpha1.MyResourcePhase` is the type of the `Phase` field used in the status of `MyResource`. + - It must be a string-like type, e.g. `type MyResourcePhase string`. + - If the resource status doesn't have a `Phase` or updating it is not desired, simply set this type argument to `string`. +- `v1alpha1.ConditionStatus` is the type of the `Status` field within the conditions. It must be `comparable`. + - Usually, this will either be a boolean or a string-like type. + - If the resource status doesn't have conditions or updating them is not desired, simply set this type argument to `bool`. +- The conditions must be a list of a type `T`, where either `T` or `*T` implements the `conditions.Condition[ConType]` interface. + - `ConType` is `v1alpha1.ConditionStatus` in this example. + + +**How to use the status updater** + +It is recommended to move the actual reconciliation logic into a helper function (`reconcile` in the example). This makes it easier to ensure that the status updater is always called, no matter where the reconciliation exits, e.g. due to an error. This helper function should then return the `ReconcileResult` required by the status updater. + +First, initialize a new `StatusUpdaterBuilder`: +```go +ctrlutils.NewStatusUpdaterBuilder[*v1alpha1.MyResource, v1alpha1.MyResourcePhase, v1alpha1.ConditionStatus]() +``` +It takes the type of the reconciled resource, the type of its `Phase` attribute and the type of the `Status` attribute of its conditions as type arguments. + +If you want to update the phase, you have to pass in a function that computes the new phase based on the the current state of the object and the returned reconcile result. Note that the function just has to return the phase, not to set it in the object. Failing to provide this function causes the updater to use a dummy implementation that sets the phase to the empty string. +```go +WithPhaseUpdateFunc(func(obj *v1alpha1.MyResource, rr ctrlutils.ReconcileResult[*v1alpha1.MyResource, v1alpha1.ConditionStatus]) (v1alpha1.MyResourcePhase, error) { + if rr.ReconcileError != nil { + return v1alpha1.PROVIDER_CONFIG_PHASE_FAILED, nil + } + if len(rr.Conditions) > 0 { + for _, con := range rr.Conditions { + if con.GetStatus() != v1alpha1.CONDITION_TRUE { + return v1alpha1.PROVIDER_CONFIG_PHASE_FAILED, nil + } + } + } + return v1alpha1.PROVIDER_CONFIG_PHASE_SUCCEEDED, nil +}) +``` + +If the conditions should be updated, the `WithConditionUpdater` method must be called. Similarly to the condition updater from the `conditions` package - which is used internally - it requires a constructor function that returns a new, empty instance of the controller-specific `conditions.Condition` implementation. The second argument specifies whether existing conditions that are not part of the updated conditions in the `ReconcileResult` should be removed or kept. + +You can then `Build()` the status updater and run `UpdateStatus()` to do the actual status update. The return values of this method are meant to be returned by the `Reconcile` function. + +**Some more details** + +- The status updater uses reflection to modifiy the status' fields. This requires it to know the field names (the ones in go, not the ones in the YAML representation). By default, it expects them to be `Status` for the status itself and `Phase`, `ObservedGeneration`, `LastReconcileTime`, `Reason`, `Message`, and `Conditions` for the respective fields within the status. + - To use a different field name, overwrite it by using either `WithFieldOverride` or `WithFieldOverrides`. + - If any of the fields is not contained top-level in the status but within a nested struct, the names of these fields must be prefixed with the names of the corresponding structs, separated by a `.`. The `WithNestedStruct` method can be used to set such a prefix quickly for one or more fields. + - To disable the update of a specific field altogether, set its name to the empty string. This can be done via the aforementioned `WithFieldOverride`/`WithFieldOverrides` methods, or simpler via `WithoutFields`. + - Doing this for the status field itself turns the status update into a no-op. + - 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. + +**The ReconcileResult** + +The `ReconcileResult` that is passed into the status updater is expected to contain a representation of what happened during the reconciliation. Its fields influence what the updated status will look like. + +- `Result` contains the `reconcile.Result` that is expected as a return value from the `reconcile.Reconciler` interface's `Reconcile` method. It is not modified in any way and simply passed through. It does not affect any of the status' fields. +- `ReconcileError` contains any error(s) that occurred during the actual reconciliation. It must be of type `errors.ReasonableError`. This will also be the second return argument from the `UpdateStatus()` method. +- `Reason` and `Message` can be set to set the status' corresponding fields. + - If either one is nil, but `ReconcileError` is not, it will be filled with a value derived from the error. +- `Conditions` contains the updated conditions. Depending on with which arguments `WithConditionUpdater` was called, the existing conditions will be either updated with these ones (keeping the other ones), or be replaced by them. +- `Object` contains the object to be updated. + - If `Object` is nil, no status update will be performed. +- `OldObject` holds the version of the object that will be used as a base for constructing the patch during the status update. + - If this is nil, `Object` will be used instead. + - If this is non-nil, it must not point to the same instance as `Object` - use the `DeepCopy()` function to create a different instance. + - All changes to `Object`'s status that are not part to `OldObject`'s status will be included in the patch during the status update. This can be used to inject custom changes to the status into the status update (in addition to the `WithCustomUpdateFunc` mentioned above). + ### logging This package contains the logging library from the [Landscaper controller-utils module](https://github.com/gardener/landscaper/tree/master/controller-utils/pkg/logging). From 7044511663dbf40a321aa17543ef656214f70ade Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Tue, 22 Apr 2025 10:41:27 +0200 Subject: [PATCH 09/10] add documentation for errors package --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index 275ec10..cdeb7d3 100644 --- a/README.md +++ b/README.md @@ -248,6 +248,16 @@ The `ReconcileResult` that is passed into the status updater is expected to cont - If this is non-nil, it must not point to the same instance as `Object` - use the `DeepCopy()` function to create a different instance. - All changes to `Object`'s status that are not part to `OldObject`'s status will be included in the patch during the status update. This can be used to inject custom changes to the status into the status update (in addition to the `WithCustomUpdateFunc` mentioned above). +### errors + +The `errors` package contains the `ReasonableError` type, which combines a normal `error` with a reason string. This is useful for errors that happen during reconciliation for updating the resource's status with a reason for the error later on. + +#### Noteworthy Functions: + +- `WithReason(...)` can be used to wrap a standard error together with a reason into a `ReasonableError`. +- `Errorf(...)` can be used to wrap an existing `ReasonableError` together with a new error, similarly to how `fmt.Errorf(...)` does it for standard errors. +- `NewReasonableErrorList(...)` or `Join(...)` can be used to work with lists of errors. `Aggregate()` turns them into a single error again. + ### logging This package contains the logging library from the [Landscaper controller-utils module](https://github.com/gardener/landscaper/tree/master/controller-utils/pkg/logging). From a7b9bbaefe894caa2da4180b060745226bded752 Mon Sep 17 00:00:00 2001 From: Johannes Aubart Date: Tue, 22 Apr 2025 11:06:23 +0200 Subject: [PATCH 10/10] downgrade task version to avoid regression --- .github/workflows/ci.yaml | 2 +- .github/workflows/release.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 43a8ef6..b464845 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -26,7 +26,7 @@ jobs: - name: Install Task uses: arduino/setup-task@v2 with: - version: 3.x + version: 3.42.1 - name: task generate run: | diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 90793ea..37f9d5d 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -24,7 +24,7 @@ jobs: - name: Install Task uses: arduino/setup-task@v2 with: - version: 3.x + version: 3.42.1 - name: Read and validate VERSION id: version