From d0ec56b0a03dbfbe9df522cda0a41513a3132cce Mon Sep 17 00:00:00 2001 From: Patrick Derks Date: Wed, 28 Jan 2026 13:12:39 +0100 Subject: [PATCH 1/2] chore: change the predicate handeling and skip to many updates with deployment This will reduce the logs and remove reconciles if the deployment updates the status. This will also lead to not accurate deployment status. --- internal/controller/predicate.go | 200 ------------------ internal/controller/store_controller.go | 7 +- .../storesnapshot_create_controller.go | 9 +- .../storesnapshot_restore_controller.go | 7 +- 4 files changed, 5 insertions(+), 218 deletions(-) delete mode 100644 internal/controller/predicate.go 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) } From d146692fb4cb83ab8cdc5545f9bb6e0c9abb9e80 Mon Sep 17 00:00:00 2001 From: Patrick Derks Date: Wed, 28 Jan 2026 13:31:20 +0100 Subject: [PATCH 2/2] chore: remove store status for deployments We reduced the reconciles for shops so we only run if the resource changes (spec) instead of status updates. Then the deployment status is not accurate anymore, so we remove this for now. --- api/v1/store.go | 3 --- 1 file changed, 3 deletions(-) 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