diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 6b1b3c1..e830474 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -636,6 +636,16 @@ func (r *resourceReconciler) createResource( latest, err = rm.Create(ctx, desired) rlog.Exit("rm.Create", err) if err != nil { + // Here we're deciding to set a resource as unmanaged + // if the error is an AWS API Error. This will ensure + // that we're only managing (put finalizer) the resources + // that actually exist in AWS. + if _, ok := ackerr.AWSError(err); ok { + mErr := r.setResourceUnmanaged(ctx, rm, desired) + if mErr != nil { + return latest, err + } + } return latest, err } diff --git a/pkg/runtime/reconciler_test.go b/pkg/runtime/reconciler_test.go index 8f78560..4089a44 100644 --- a/pkg/runtime/reconciler_test.go +++ b/pkg/runtime/reconciler_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/aws/smithy-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -224,6 +225,68 @@ func TestReconcilerCreate_BackoffRetries(t *testing.T) { rm.AssertNumberOfCalls(t, "ReadOne", 6) } +type awsError struct { + smithy.APIError +} + +func (err awsError) Error() string { + return "mock error" +} + +func TestReconcilerCreate_UnmanageResourceOnAWSErrors(t *testing.T) { + require := require.New(t) + + ctx := context.TODO() + arn := ackv1alpha1.AWSResourceName("mybook-arn") + + desired, desiredRTObj, _ := resourceMocks() + desired.On("ReplaceConditions", []*ackv1alpha1.Condition{}).Return() + + ids := &ackmocks.AWSResourceIdentifiers{} + ids.On("ARN").Return(&arn) + + latest, latestRTObj, _ := resourceMocks() + latest.On("Identifiers").Return(ids) + + latest.On("Conditions").Return([]*ackv1alpha1.Condition{}) + latest.On( + "ReplaceConditions", + mock.AnythingOfType("[]*v1alpha1.Condition"), + ).Return() + + rm := &ackmocks.AWSResourceManager{} + rm.On("ResolveReferences", ctx, nil, desired).Return( + desired, false, nil, + ).Times(2) + rm.On("ClearResolvedReferences", desired).Return(desired) + rm.On("ClearResolvedReferences", latest).Return(latest) + rm.On("ReadOne", ctx, desired).Return( + latest, ackerr.NotFound, + ).Once() + rm.On("Create", ctx, desired).Return( + latest, awsError{}, + ) + rm.On("IsSynced", ctx, latest).Return(false, nil) + rmf, rd := managedResourceManagerFactoryMocks(desired, latest) + rd.On("IsManaged", desired).Return(false).Twice() + rd.On("IsManaged", desired).Return(true) + rd.On("MarkUnmanaged", desired) + rd.On("MarkManaged", desired) + rd.On("MarkUnmanaged", desired) + rd.On("ResourceFromRuntimeObject", desiredRTObj).Return(desired) + rd.On("Delta", desired, desired).Return(ackcompare.NewDelta()) + + r, kc, scmd := reconcilerMocks(rmf) + rm.On("EnsureTags", ctx, desired, scmd).Return(nil) + // Use specific matcher for WithoutCancel context instead of mock.Anything + kc.On("Patch", withoutCancelContextMatcher, latestRTObj, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil) + _, err := r.Sync(ctx, rm, desired) + require.NotNil(err) + rm.AssertNumberOfCalls(t, "ReadOne", 1) + rd.AssertCalled(t, "MarkUnmanaged", desired) + rd.AssertCalled(t, "MarkManaged", desired) +} + func TestReconcilerReadOnlyResource(t *testing.T) { require := require.New(t) @@ -1249,7 +1312,7 @@ func TestReconcilerHandleReconcilerError_PatchStatus_Latest(t *testing.T) { _, err := r.HandleReconcileError(ctx, desired, latest, nil) require.Nil(err) - statusWriter.AssertCalled(t, "Patch", withoutCancelContextMatcher, latestRTObj, mock.AnythingOfType("*client.mergeFromPatch")) + statusWriter.AssertCalled(t, "Patch", withoutCancelContextMatcher, latestRTObj, mock.AnythingOfType("*client.mergeFromPatch")) // The HandleReconcilerError function never updates spec or metadata, so // even though there is a change to the annotations we expect no call to // patch the spec/metadata...