diff --git a/api/v1/store.go b/api/v1/store.go index 8556bf6d..ad3ca948 100644 --- a/api/v1/store.go +++ b/api/v1/store.go @@ -11,9 +11,6 @@ import ( // Store is the Schema for the stores API // +kubebuilder:object:root=true // +kubebuilder:subresource:status -// +kubebuilder:printcolumn:name="Storefront",type=string,JSONPath=".status.storefrontState.ready" -// +kubebuilder:printcolumn:name="Worker",type=string,JSONPath=".status.workerState.ready" -// +kubebuilder:printcolumn:name="Admin",type=string,JSONPath=".status.adminState.ready" // +kubebuilder:printcolumn:name="State",type=string,JSONPath=".status.state" // +kubebuilder:printcolumn:name="Message",type=string,JSONPath=".status.message" // +kubebuilder:resource:scope=Namespaced diff --git a/internal/controller/predicate.go b/internal/controller/predicate.go deleted file mode 100644 index ffe9abfc..00000000 --- a/internal/controller/predicate.go +++ /dev/null @@ -1,200 +0,0 @@ -package controller - -import ( - "encoding/json" - "errors" - "reflect" - - "github.com/pmezard/go-difflib/difflib" - "go.uber.org/zap" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/event" - "sigs.k8s.io/controller-runtime/pkg/predicate" -) - -type SkipStatusUpdates = TypedSkipStatusPredicate[client.Object] - -// NewSkipStatusUpdates creates a new SkipStatusUpdates predicate with the given logger and allow list. -// The logger is required to prevent nil pointer panics during logging operations. -func NewSkipStatusUpdates(logger *zap.SugaredLogger, allowList ...client.Object) (SkipStatusUpdates, error) { - if logger == nil { - return SkipStatusUpdates{}, errors.New("logger is required") - } - return TypedSkipStatusPredicate[client.Object]{ - Logger: logger, - AllowList: allowList, - }, nil -} - -// TypedSkipStatusPredicate filters out status-only updates. -type TypedSkipStatusPredicate[object client.Object] struct { - predicate.TypedFuncs[object] - Logger *zap.SugaredLogger - AllowList []client.Object -} - -// Update returns false if only the status has changed, skipping reconciliation. -func (t TypedSkipStatusPredicate[object]) Update(e event.TypedUpdateEvent[object]) (update bool) { - var oldStatusJson []byte - var newStatusJson []byte - var oldObjectJson []byte - var newObjectJson []byte - var err error - - // We need to use reflection because the GetObjectKind is empty - kind := "unknown" - objType := reflect.TypeOf(e.ObjectNew) - if objType != nil { - if objType.Kind() == reflect.Ptr { - objType = objType.Elem() - } - kind = objType.Name() - } - - defer func() { - if t.Logger == nil || !update { - return - } - - // Log the update trigger - t.Logger.Debugw("Update trigger", - zap.Bool("triggerReconcile", update), - zap.String("name", e.ObjectNew.GetName()), - zap.String("kind", kind), - ) - - // Generate and log status diff - statusDiff := difflib.UnifiedDiff{ - A: difflib.SplitLines(string(oldStatusJson)), - B: difflib.SplitLines(string(newStatusJson)), - FromFile: "Old Status", - ToFile: "New Status", - Context: 3, - } - statusDiffText, _ := difflib.GetUnifiedDiffString(statusDiff) - if statusDiffText != "" { - t.Logger.Debugf("Status diff: \n%s", statusDiffText) - } - - // Generate and log spec diff - specDiff := difflib.UnifiedDiff{ - A: difflib.SplitLines(string(oldObjectJson)), - B: difflib.SplitLines(string(newObjectJson)), - FromFile: "Old Spec", - ToFile: "New Spec", - Context: 3, - } - specDiffText, _ := difflib.GetUnifiedDiffString(specDiff) - if specDiffText != "" { - t.Logger.Debugf("Spec Diff: \n%s", specDiffText) - } - }() - if isNil(e.ObjectOld) || isNil(e.ObjectNew) { - return false - } - - oldSpec, okOld := getSpec(e.ObjectOld) - newSpec, okNew := getSpec(e.ObjectNew) - if okOld && okNew { - oldObjectJson, err = json.MarshalIndent(oldSpec, "", " ") - if t.Logger != nil && err != nil { - t.Logger.Warnw("parse old spec json", zap.Error(err)) - } - newObjectJson, err = json.MarshalIndent(newSpec, "", " ") - if t.Logger != nil && err != nil { - t.Logger.Warnw("parse new spec json", zap.Error(err)) - } - } - - oldStatus, okOld := getStatus(e.ObjectOld) - newStatus, okNew := getStatus(e.ObjectNew) - if !okOld || !okNew { - // Fallback to allow reconcile if we can't extract status - return true - } - - oldStatusJson, err = json.MarshalIndent(oldStatus, "", " ") - if t.Logger != nil && err != nil { - t.Logger.Warnw("parse old status json", zap.Error(err)) - } - newStatusJson, err = json.MarshalIndent(newStatus, "", " ") - if t.Logger != nil && err != nil { - t.Logger.Warnw("parse new status json", zap.Error(err)) - } - - // If status changed check if object is in allow list and allow reconcile - if !reflect.DeepEqual(oldStatus, newStatus) { - return t.isInAllowList(kind) - } - - return true -} - -// getStatus extracts the .Status field using reflection. -func getStatus(obj any) (any, bool) { - v := reflect.ValueOf(obj) - if !v.IsValid() { - return nil, false - } - if v.Kind() == reflect.Pointer { - v = v.Elem() - } - if !v.IsValid() || v.Kind() != reflect.Struct { - return nil, false - } - - status := v.FieldByName("Status") - if !status.IsValid() { - return nil, false - } - return status.Interface(), true -} - -func getSpec(obj any) (any, bool) { - v := reflect.ValueOf(obj) - if !v.IsValid() { - return nil, false - } - if v.Kind() == reflect.Pointer { - v = v.Elem() - } - if !v.IsValid() || v.Kind() != reflect.Struct { - return nil, false - } - - spec := v.FieldByName("Spec") - if !spec.IsValid() { - return nil, false - } - return spec.Interface(), true -} - -// isNil safely checks for nil values via reflection. -func isNil(arg any) bool { - if arg == nil { - return true - } - v := reflect.ValueOf(arg) - switch v.Kind() { - case reflect.Ptr, reflect.Interface, reflect.Slice, reflect.Map, reflect.Chan, reflect.Func: - return v.IsNil() - default: - return false - } -} - -// isInAllowList checks if the given kind matches any object type in the allow list. -func (t TypedSkipStatusPredicate[object]) isInAllowList(kind string) bool { - for _, allowedObj := range t.AllowList { - objType := reflect.TypeOf(allowedObj) - if objType != nil { - if objType.Kind() == reflect.Ptr { - objType = objType.Elem() - } - if objType.Name() == kind { - return true - } - } - } - return false -} diff --git a/internal/controller/store_controller.go b/internal/controller/store_controller.go index b80e3d26..b94eee5d 100644 --- a/internal/controller/store_controller.go +++ b/internal/controller/store_controller.go @@ -59,10 +59,6 @@ type StoreReconciler struct { // SetupWithManager sets up the controller with the Manager. func (r *StoreReconciler) SetupWithManager(mgr ctrl.Manager, logger *zap.SugaredLogger) error { - skipStatusUpdates, err := NewSkipStatusUpdates(logger, &appsv1.Deployment{}) - if err != nil { - return err - } return ctrl.NewControllerManagedBy(mgr). For(&v1.Store{}). // We get triggerd by every update on the created resources, this leeads to high reconciles at the start. @@ -74,8 +70,7 @@ func (r *StoreReconciler) SetupWithManager(mgr ctrl.Manager, logger *zap.Sugared Owns(&appsv1.Deployment{}). Owns(&batchv1.Job{}). Owns(&batchv1.CronJob{}). - // Skip status updates of all resources - WithEventFilter(skipStatusUpdates). + WithEventFilter(predicate.GenerationChangedPredicate{}). // This will watch the db secret and run a reconcile if the db secret will change. Watches( &corev1.Secret{}, diff --git a/internal/controller/storesnapshot_create_controller.go b/internal/controller/storesnapshot_create_controller.go index 3b354e9a..cf1f12d0 100644 --- a/internal/controller/storesnapshot_create_controller.go +++ b/internal/controller/storesnapshot_create_controller.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/predicate" v1 "github.com/shopware/shopware-operator/api/v1" ) @@ -35,17 +36,11 @@ type StoreSnapshotCreateReconciler struct { StoreSnapshotBaseReconciler } -// TODO: Filter if the state is failed or succeeded, because then we don't reconcile finished snapshots -// SetupWithManager sets up the controller with the Manager. func (r *StoreSnapshotCreateReconciler) SetupWithManager(mgr ctrl.Manager) error { - skipStatusUpdates, err := NewSkipStatusUpdates(r.Logger) - if err != nil { - return err - } return ctrl.NewControllerManagedBy(mgr). For(&v1.StoreSnapshotCreate{}). Owns(&batchv1.Job{}). - WithEventFilter(skipStatusUpdates). + WithEventFilter(predicate.GenerationChangedPredicate{}). Complete(r) } diff --git a/internal/controller/storesnapshot_restore_controller.go b/internal/controller/storesnapshot_restore_controller.go index 62ebcc9c..ca02d431 100644 --- a/internal/controller/storesnapshot_restore_controller.go +++ b/internal/controller/storesnapshot_restore_controller.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/predicate" v1 "github.com/shopware/shopware-operator/api/v1" ) @@ -36,14 +37,10 @@ type StoreSnapshotRestoreReconciler struct { // SetupWithManager sets up the controller with the Manager. func (r *StoreSnapshotRestoreReconciler) SetupWithManager(mgr ctrl.Manager) error { - skipStatusUpdates, err := NewSkipStatusUpdates(r.Logger) - if err != nil { - return err - } return ctrl.NewControllerManagedBy(mgr). For(&v1.StoreSnapshotRestore{}). Owns(&batchv1.Job{}). - WithEventFilter(skipStatusUpdates). + WithEventFilter(predicate.GenerationChangedPredicate{}). Complete(r) }