Skip to content

Commit

Permalink
Merge pull request #595 from crossplane/backport-594-to-release-1.14
Browse files Browse the repository at this point in the history
[Backport release-1.14] Fix retry in update critical annotations after a confict
  • Loading branch information
turkenh committed Nov 2, 2023
2 parents 288a6e5 + ba21583 commit aaa202d
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 1 deletion.
4 changes: 3 additions & 1 deletion pkg/reconciler/managed/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions pkg/reconciler/managed/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit aaa202d

Please sign in to comment.