From fa27da777a4ebb9b9f633da1ef3f70add10108ce Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Thu, 4 Dec 2025 11:43:34 +0100 Subject: [PATCH 1/4] Do not ever update the status in case the generation has changed --- pkg/reconciler/internal/updater/updater.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/pkg/reconciler/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index 0908748..3dd9b20 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -144,7 +144,7 @@ func (u *Updater) Apply(ctx context.Context, baseObj *unstructured.Unstructured) updateErr := u.client.Status().Update(ctx, obj) if errors.IsConflict(updateErr) && u.enableAggressiveConflictResolution { u.logger.V(1).Info("Status update conflict detected") - resolved, resolveErr := u.tryRefresh(ctx, baseObj, isSafeForStatusUpdate) + resolved, resolveErr := u.tryRefresh(ctx, baseObj, isSafeForUpdate) if resolveErr != nil { return resolveErr } @@ -195,11 +195,17 @@ func (u *Updater) Apply(ctx context.Context, baseObj *unstructured.Unstructured) return err } -func isSafeForStatusUpdate(_ logr.Logger, _ *unstructured.Unstructured, _ *unstructured.Unstructured) bool { - return true -} - func isSafeForUpdate(logger logr.Logger, inMemory *unstructured.Unstructured, onCluster *unstructured.Unstructured) bool { + if inMemory.GetGeneration() != onCluster.GetGeneration() { + // Diff in generation. Nothing we can do about it -> Fail. + logger.V(1).Info("Not refreshing object due to generation mismatch", + "namespace", inMemory.GetNamespace(), + "name", inMemory.GetName(), + "gkv", inMemory.GroupVersionKind(), + ) + return false + } + // Extra verification to make sure that the spec has not changed. if !reflect.DeepEqual(inMemory.Object["spec"], onCluster.Object["spec"]) { // Diff in object spec. Nothing we can do about it -> Fail. logger.V(1).Info("Not refreshing object due to spec mismatch", From 0941cdd659969d7baf610c57c66f650d626d5ec5 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Thu, 4 Dec 2025 12:09:02 +0100 Subject: [PATCH 2/4] Fix test --- pkg/reconciler/internal/updater/updater_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/reconciler/internal/updater/updater_test.go b/pkg/reconciler/internal/updater/updater_test.go index 6fdfdc8..dec073f 100644 --- a/pkg/reconciler/internal/updater/updater_test.go +++ b/pkg/reconciler/internal/updater/updater_test.go @@ -237,6 +237,9 @@ var _ = Describe("Updater", func() { }) Context("when in-cluster object has been updated", func() { JustBeforeEach(func() { + // Refresh obj first with what is on the cluster, otherwise the following Apply() calls will fail due to + // subtle changes in obj's spec and its refreshed counterpart, both of which are empty, but not in the same way... + Expect(cl.Get(context.TODO(), types.NamespacedName{Namespace: "testNamespace", Name: "testDeployment"}, obj)).To(Succeed()) // Add external status condition on cluster. clusterObj := obj.DeepCopy() unknownCondition := map[string]interface{}{ From a1c496d6b0b5716e77d19d4c3608f301003b2bdb Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Thu, 4 Dec 2025 12:24:41 +0100 Subject: [PATCH 3/4] Compare whole metadata without resourceVersion --- pkg/reconciler/internal/updater/updater.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/pkg/reconciler/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index 3dd9b20..65daf4e 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -196,7 +196,10 @@ func (u *Updater) Apply(ctx context.Context, baseObj *unstructured.Unstructured) } func isSafeForUpdate(logger logr.Logger, inMemory *unstructured.Unstructured, onCluster *unstructured.Unstructured) bool { - if inMemory.GetGeneration() != onCluster.GetGeneration() { + // Compare metadata (excluding resourceVersion). + inMemoryMetadata := metadataWithoutResourceVersion(inMemory) + onClusterMetadata := metadataWithoutResourceVersion(onCluster) + if !reflect.DeepEqual(inMemoryMetadata, onClusterMetadata) { // Diff in generation. Nothing we can do about it -> Fail. logger.V(1).Info("Not refreshing object due to generation mismatch", "namespace", inMemory.GetNamespace(), @@ -218,6 +221,21 @@ func isSafeForUpdate(logger logr.Logger, inMemory *unstructured.Unstructured, on return true } +func metadataWithoutResourceVersion(u *unstructured.Unstructured) map[string]interface{} { + metadata, ok := u.Object["metadata"].(map[string]interface{}) + if !ok { + return nil + } + modifiedMetadata := make(map[string]interface{}, len(metadata)) + for k, v := range metadata { + if k == "resourceVersion" { + continue + } + modifiedMetadata[k] = v + } + return modifiedMetadata +} + func (u *Updater) tryRefresh(ctx context.Context, obj *unstructured.Unstructured, isSafe func(logger logr.Logger, inMemory *unstructured.Unstructured, onCluster *unstructured.Unstructured) bool) (bool, error) { // Re-fetch object with client. current := &unstructured.Unstructured{} From 1d5d3fc956496e28d8756e5dd5d4b36cc52400c0 Mon Sep 17 00:00:00 2001 From: Moritz Clasmeier Date: Thu, 4 Dec 2025 12:26:54 +0100 Subject: [PATCH 4/4] Simplified code a bit --- pkg/reconciler/internal/updater/updater.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/reconciler/internal/updater/updater.go b/pkg/reconciler/internal/updater/updater.go index 65daf4e..6558dc1 100644 --- a/pkg/reconciler/internal/updater/updater.go +++ b/pkg/reconciler/internal/updater/updater.go @@ -144,7 +144,7 @@ func (u *Updater) Apply(ctx context.Context, baseObj *unstructured.Unstructured) updateErr := u.client.Status().Update(ctx, obj) if errors.IsConflict(updateErr) && u.enableAggressiveConflictResolution { u.logger.V(1).Info("Status update conflict detected") - resolved, resolveErr := u.tryRefresh(ctx, baseObj, isSafeForUpdate) + resolved, resolveErr := u.tryRefresh(ctx, baseObj) if resolveErr != nil { return resolveErr } @@ -177,7 +177,7 @@ func (u *Updater) Apply(ctx context.Context, baseObj *unstructured.Unstructured) updateErr := u.client.Update(ctx, obj) if errors.IsConflict(updateErr) && u.enableAggressiveConflictResolution { u.logger.V(1).Info("Update conflict detected") - resolved, resolveErr := u.tryRefresh(ctx, baseObj, isSafeForUpdate) + resolved, resolveErr := u.tryRefresh(ctx, baseObj) if resolveErr != nil { return resolveErr } @@ -236,7 +236,7 @@ func metadataWithoutResourceVersion(u *unstructured.Unstructured) map[string]int return modifiedMetadata } -func (u *Updater) tryRefresh(ctx context.Context, obj *unstructured.Unstructured, isSafe func(logger logr.Logger, inMemory *unstructured.Unstructured, onCluster *unstructured.Unstructured) bool) (bool, error) { +func (u *Updater) tryRefresh(ctx context.Context, obj *unstructured.Unstructured) (bool, error) { // Re-fetch object with client. current := &unstructured.Unstructured{} current.SetGroupVersionKind(obj.GroupVersionKind()) @@ -246,7 +246,7 @@ func (u *Updater) tryRefresh(ctx context.Context, obj *unstructured.Unstructured return false, err } - if !isSafe(u.logger, obj, current) { + if !isSafeForUpdate(u.logger, obj, current) { return false, nil }