From 5ba5f67b6173a8e5848b44528f09982ebd9d57f5 Mon Sep 17 00:00:00 2001 From: Nico Schieder Date: Tue, 4 Nov 2025 14:35:08 +0100 Subject: [PATCH] Fix Collision error not reported when paused --- machinery/objects.go | 11 ++-- machinery/objects_test.go | 104 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/machinery/objects.go b/machinery/objects.go index 96b4fb24..8550e0b3 100644 --- a/machinery/objects.go +++ b/machinery/objects.go @@ -226,7 +226,7 @@ func (e *ObjectEngine) Reconcile( return nil, fmt.Errorf("creating resource: %w", err) } - if err := e.migrateFieldManagersToSSA(ctx, desiredObject); err != nil { + if err := e.migrateFieldManagersToSSA(ctx, desiredObject, options); err != nil { return nil, fmt.Errorf("migrating to SSA after create: %w", err) } @@ -441,7 +441,7 @@ func (e *ObjectEngine) create( options types.ObjectReconcileOptions, opts ...client.CreateOption, ) error { if options.Paused { - return nil + opts = append(opts, client.DryRunAll) } return e.writer.Create(ctx, obj, opts...) @@ -458,7 +458,7 @@ func (e *ObjectEngine) patch( return nil } - if err := e.migrateFieldManagersToSSA(ctx, obj); err != nil { + if err := e.migrateFieldManagersToSSA(ctx, obj, options); err != nil { return err } @@ -543,7 +543,12 @@ func (e *ObjectEngine) getObjectRevision(obj client.Object) (int64, error) { // SSA really is complicated: https://github.com/kubernetes/kubernetes/issues/99003 func (e *ObjectEngine) migrateFieldManagersToSSA( ctx context.Context, object Object, + options types.ObjectReconcileOptions, ) error { + if options.Paused { + return nil + } + patch, err := csaupgrade.UpgradeManagedFieldsPatch( object, sets.New(e.fieldOwner), e.fieldOwner) diff --git a/machinery/objects_test.go b/machinery/objects_test.go index 8b6af075..2527b26f 100644 --- a/machinery/objects_test.go +++ b/machinery/objects_test.go @@ -861,6 +861,110 @@ func TestObjectEngine(t *testing.T) { } } +func TestObjectEngine_Collision(t *testing.T) { + t.Parallel() + + owner := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + UID: "12345-678", + Name: "owner", + Namespace: "test", + }, + } + + t.Run("non paused", func(t *testing.T) { + t.Parallel() + + cache := &cacheMock{} + writer := testutil.NewClient() + ownerStrategy := ownerhandling.NewNative(scheme.Scheme) + divergeDetector := &comparatorMock{} + + oe := NewObjectEngine( + scheme.Scheme, + cache, writer, + ownerStrategy, divergeDetector, + testFieldOwner, + testSystemPrefix, + ) + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oe-test", + Namespace: "test", + }, + Data: map[string]string{ + "test1": "test", + "test2": "test", + }, + } + + ctx := t.Context() + + cache. + On("Get", mock.Anything, client.ObjectKeyFromObject(configMap), mock.Anything, mock.Anything). + Return(errors.NewNotFound(schema.GroupResource{}, "")) + + writer. + On("Create", mock.Anything, mock.Anything, mock.Anything). + Return(errors.NewAlreadyExists(schema.GroupResource{ + Group: "test", Resource: "banana", + }, "cavendish")) + + _, err := oe.Reconcile(ctx, owner, 1, configMap) + + var terr *CreateCollisionError + + require.ErrorAs(t, err, &terr) + require.EqualError(t, err, `banana.test "cavendish" already exists`) + }) + + t.Run("paused", func(t *testing.T) { + t.Parallel() + + cache := &cacheMock{} + writer := testutil.NewClient() + ownerStrategy := ownerhandling.NewNative(scheme.Scheme) + divergeDetector := &comparatorMock{} + + oe := NewObjectEngine( + scheme.Scheme, + cache, writer, + ownerStrategy, divergeDetector, + testFieldOwner, + testSystemPrefix, + ) + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "oe-test", + Namespace: "test", + }, + Data: map[string]string{ + "test1": "test", + "test2": "test", + }, + } + + ctx := t.Context() + + cache. + On("Get", mock.Anything, client.ObjectKeyFromObject(configMap), mock.Anything, mock.Anything). + Return(errors.NewNotFound(schema.GroupResource{}, "")) + + writer. + On("Create", mock.Anything, mock.Anything, mock.Anything). + Return(errors.NewAlreadyExists(schema.GroupResource{ + Group: "test", Resource: "banana", + }, "cavendish")) + + _, err := oe.Reconcile(ctx, owner, 1, configMap, types.WithPaused{}) + + var terr *CreateCollisionError + + require.ErrorAs(t, err, &terr) + require.EqualError(t, err, `banana.test "cavendish" already exists`) + }) +} + func TestObjectEngine_Reconcile_SanityChecks(t *testing.T) { t.Parallel()