Ensure only annotations are updated

As the UpdateCriticialAnnotations function is now not exclusively called in the creation process, we have to ensure no other fields like the spec are updated, so we don't interfer with the normal LateInitialize logic

Signed-off-by: twobiers <[email protected]>

Author: twobiers

pkg/reconciler/managed/api.go

@@ -277,15 +277,25 @@ func NewRetryingCriticalAnnotationUpdater(c client.Client) *RetryingCriticalAnno
 // UpdateCriticalAnnotations updates (i.e. persists) the annotations of the
 // supplied Object. It retries in the face of any API server error several times
 // in order to ensure annotations that contain critical state are persisted.
-// Pending changes to the supplied Object's spec, status, or other metadata
-// might get reset to their current state according to the API server, e.g. in
-// case of a conflict error.
+// Only annotations will be updated as part of this operation, other fields of the
+// supplied Object will not be modified.
 func (u *RetryingCriticalAnnotationUpdater) UpdateCriticalAnnotations(ctx context.Context, o client.Object) error {
 	a := o.GetAnnotations()
 	err := retry.OnError(retry.DefaultRetry, func(err error) bool {
 		return !errors.Is(err, context.Canceled)
 	}, func() error {
-		err := u.client.Update(ctx, o)
+		patchMap := map[string]interface{}{
+			"metadata": map[string]any{
+				"annotations": a,
+			},
+		}
+
+		patchData, err := json.Marshal(patchMap)
+		if err != nil {
+			return err
+		}
+
+		err = u.client.Patch(ctx, o, client.RawPatch(types.MergePatchType, patchData), client.FieldOwner(fieldOwnerAPISimpleRefResolver), client.ForceOwnership)
 		if kerrors.IsConflict(err) {
 			if getErr := u.client.Get(ctx, client.ObjectKeyFromObject(o), o); getErr != nil {
 				return getErr

pkg/reconciler/managed/api_test.go

@@ -560,7 +560,7 @@ func TestRetryingCriticalAnnotationUpdater(t *testing.T) {
 			reason: "We should return any error we encounter getting the supplied object",
 			c: &test.MockClient{
 				MockGet: test.NewMockGetFn(errBoom, setLabels),
-				MockUpdate: test.NewMockUpdateFn(kerrors.NewConflict(schema.GroupResource{
+				MockPatch: test.NewMockPatchFn(kerrors.NewConflict(schema.GroupResource{
 					Group:    "foo.com",
 					Resource: "bars",
 				}, "abc", errBoom)),
@@ -576,8 +576,8 @@ func TestRetryingCriticalAnnotationUpdater(t *testing.T) {
 		"UpdateError": {
 			reason: "We should return any error we encounter updating the supplied object",
 			c: &test.MockClient{
-				MockGet:    test.NewMockGetFn(nil, setLabels),
-				MockUpdate: test.NewMockUpdateFn(errBoom),
+				MockGet:   test.NewMockGetFn(nil, setLabels),
+				MockPatch: test.NewMockPatchFn(errBoom),
 			},
 			args: args{
 				o: &fake.LegacyManaged{},
@@ -591,7 +591,7 @@ func TestRetryingCriticalAnnotationUpdater(t *testing.T) {
 			reason: "A successful get after a conflict should not hide the conflict error and prevent retries",
 			c: &test.MockClient{
 				MockGet: test.NewMockGetFn(nil, setLabels),
-				MockUpdate: test.NewMockUpdateFn(kerrors.NewConflict(schema.GroupResource{
+				MockPatch: test.NewMockPatchFn(kerrors.NewConflict(schema.GroupResource{
 					Group:    "foo.com",
 					Resource: "bars",
 				}, "abc", errBoom)),
@@ -610,8 +610,8 @@ func TestRetryingCriticalAnnotationUpdater(t *testing.T) {
 		"Success": {
 			reason: "We should return without error if we successfully update our annotations",
 			c: &test.MockClient{
-				MockGet:    test.NewMockGetFn(nil, setLabels),
-				MockUpdate: test.NewMockUpdateFn(errBoom),
+				MockGet:   test.NewMockGetFn(nil, setLabels),
+				MockPatch: test.NewMockPatchFn(errBoom),
 			},
 			args: args{
 				o: &fake.LegacyManaged{},