From ba21583a90a07b25f0c9c73a3d3837bb799fab53 Mon Sep 17 00:00:00 2001
From: Hasan Turken <turkenh@gmail.com>
Date: Thu, 2 Nov 2023 12:05:19 +0300
Subject: [PATCH] Fix retry in update critical annotations

- A successful get after a conflict should not hide the conflict
error and prevent retries.

Signed-off-by: Hasan Turken <turkenh@gmail.com>
(cherry picked from commit 9aa102409748a25edf195704d947f9c42f335e3f)
---
 pkg/reconciler/managed/api.go      |  4 +++-
 pkg/reconciler/managed/api_test.go | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/pkg/reconciler/managed/api.go b/pkg/reconciler/managed/api.go
index a374a9a11..8ab4aeeaf 100644
--- a/pkg/reconciler/managed/api.go
+++ b/pkg/reconciler/managed/api.go
@@ -177,7 +177,9 @@ func (u *RetryingCriticalAnnotationUpdater) UpdateCriticalAnnotations(ctx contex
 	err := retry.OnError(retry.DefaultRetry, resource.IsAPIError, func() error {
 		err := u.client.Update(ctx, o)
 		if kerrors.IsConflict(err) {
-			err = u.client.Get(ctx, types.NamespacedName{Name: o.GetName()}, o)
+			if getErr := u.client.Get(ctx, types.NamespacedName{Name: o.GetName()}, o); getErr != nil {
+				return getErr
+			}
 			meta.AddAnnotations(o, a)
 		}
 		return err
diff --git a/pkg/reconciler/managed/api_test.go b/pkg/reconciler/managed/api_test.go
index 699b21325..1c22149d9 100644
--- a/pkg/reconciler/managed/api_test.go
+++ b/pkg/reconciler/managed/api_test.go
@@ -413,6 +413,26 @@ func TestRetryingCriticalAnnotationUpdater(t *testing.T) {
 				o:   &fake.Managed{},
 			},
 		},
+		"SuccessfulGetAfterAConflict": {
+			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{
+					Group:    "foo.com",
+					Resource: "bars",
+				}, "abc", errBoom)),
+			},
+			args: args{
+				o: &fake.Managed{},
+			},
+			want: want{
+				err: errors.Wrap(kerrors.NewConflict(schema.GroupResource{
+					Group:    "foo.com",
+					Resource: "bars",
+				}, "abc", errBoom), errUpdateCriticalAnnotations),
+				o: objectReturnedByGet,
+			},
+		},
 		"Success": {
 			reason: "We should return without error if we successfully update our annotations",
 			c: &test.MockClient{