diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 318a679..7ac078d 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -589,10 +589,9 @@ func (r *resourceReconciler) Sync( } } else if adoptionPolicy == AdoptionPolicy_Adopt { rm.FilterSystemTags(latest) - if err = r.setResourceManaged(ctx, rm, latest); err != nil { + if err = r.setResourceManagedAndAdopted(ctx, rm, latest); err != nil { return latest, err } - r.rd.MarkAdopted(latest) latest, err = r.patchResourceMetadataAndSpec(ctx, rm, desired, latest) if err != nil { return latest, err @@ -609,12 +608,13 @@ func (r *resourceReconciler) Sync( return latest, nil } else { if adoptionPolicy == AdoptionPolicy_AdoptOrCreate { - // set adopt-or-create resource as managed before attempting - // update - if err = r.setResourceManaged(ctx, rm, latest); err != nil { + // set adopt-or-create resource as managed + // and requeue to ensure status is patched + if err = r.setResourceManagedAndAdopted(ctx, rm, latest); err != nil { return latest, err } - r.rd.MarkAdopted(latest) + err = requeue.Needed(fmt.Errorf("adopted resource, requeuing to check for updates")) + return latest, err } if latest, err = r.updateResource(ctx, rm, resolved, latest); err != nil { return latest, err @@ -1173,6 +1173,34 @@ func (r *resourceReconciler) setResourceManaged( rlog.Debug("marked resource as managed") return nil } +// setResourceManagedAndAdopted marks the underlying CR in the supplied AWSResource with +// a finalizer and adopted annotation that indicates the object is under ACK management and will not +// be deleted until that finalizer is removed (in setResourceUnmanaged()) +func (r *resourceReconciler) setResourceManagedAndAdopted( + ctx context.Context, + rm acktypes.AWSResourceManager, + res acktypes.AWSResource, +) error { + if r.rd.IsManaged(res) { + return nil + } + var err error + rlog := ackrtlog.FromContext(ctx) + exit := rlog.Trace("r.setResourceManagedAndAdopted") + defer func() { + exit(err) + }() + + orig := res.DeepCopy().RuntimeObject() + r.rd.MarkManaged(res) + r.rd.MarkAdopted(res) + res, err = r.patchResourceMetadataAndSpec(ctx, rm, r.rd.ResourceFromRuntimeObject(orig), res) + if err != nil { + return err + } + rlog.Debug("marked resource as managed and adopted") + return nil +} // setResourceUnmanaged removes a finalizer from the underlying CR in the // supplied AWSResource that indicates the object is under ACK management. This diff --git a/pkg/runtime/reconciler_test.go b/pkg/runtime/reconciler_test.go index e3c3dc7..246bd26 100644 --- a/pkg/runtime/reconciler_test.go +++ b/pkg/runtime/reconciler_test.go @@ -518,8 +518,8 @@ func TestReconcilerAdoptOrCreateResource_Adopt(t *testing.T) { } hasSynced = true - assert.Equal(corev1.ConditionTrue, condition.Status) - assert.Equal(ackcondition.SyncedMessage, *condition.Message) + assert.Equal(corev1.ConditionUnknown, condition.Status) + assert.Equal(ackcondition.UnknownSyncedMessage, *condition.Message) } assert.True(hasSynced) }).Once() @@ -532,19 +532,6 @@ func TestReconcilerAdoptOrCreateResource_Adopt(t *testing.T) { ackv1alpha1.AnnotationAdoptionPolicy: "adopt-or-create", ackv1alpha1.AnnotationAdoptionFields: adoptionFieldsString, }) - updated, updatedRTObj, _ := resourceMocks() - updated.On("Identifiers").Return(ids) - updated.On("Conditions").Return([]*ackv1alpha1.Condition{}) - updated.On("MetaObject").Return(metav1.ObjectMeta{ - Annotations: map[string]string{ - ackv1alpha1.AnnotationAdoptionPolicy: "adopt-or-create", - ackv1alpha1.AnnotationAdoptionFields: "{\"arn\": \"my-adopt-book-arn\"}", - }, - }) - updated.On( - "ReplaceConditions", - mock.AnythingOfType("[]*v1alpha1.Condition"), - ).Return() rm := &ackmocks.AWSResourceManager{} rm.On("ResolveReferences", ctx, nil, desired).Return( @@ -552,18 +539,14 @@ func TestReconcilerAdoptOrCreateResource_Adopt(t *testing.T) { ).Times(2) desired.On("PopulateResourceFromAnnotation", adoptionFields).Return(nil) rm.On("ClearResolvedReferences", desired).Return(desired) - rm.On("ClearResolvedReferences", updated).Return(updated) + rm.On("ClearResolvedReferences", latest).Return(latest) rm.On("ClearResolvedReferences", latest).Return(latest) rm.On("ReadOne", ctx, desired).Return( latest, nil, ).Once() - rm.On("Update", ctx, desired, latest, delta).Return( - updated, nil, - ).Once() rm.On("IsSynced", ctx, latest).Return(true, nil) rmf, rd := managedResourceManagerFactoryMocks(desired, latest) - rm.On("LateInitialize", ctx, updated).Return(latest, nil) rd.On("IsManaged", desired).Return(false).Once() rd.On("IsManaged", desired).Return(true) rd.On("MarkAdopted", latest).Return().Once() @@ -572,28 +555,16 @@ func TestReconcilerAdoptOrCreateResource_Adopt(t *testing.T) { ackv1alpha1.AnnotationAdoptionFields: adoptionFieldsString, ackv1alpha1.AnnotationAdopted: "true", }) - // setManaged - rd.On("Delta", latest, latest).Return(ackcompare.NewDelta()).Once() - // update - rd.On("Delta", desired, latest).Return(delta).Once() - // - rd.On("Delta", desired, updated).Return(ackcompare.NewDelta()) - rd.On("Delta", updated, updated).Return(ackcompare.NewDelta()) - rd.On("MarkAdopted", updated).Return().Once() r, kc, scmd := reconcilerMocks(rmf) rm.On("EnsureTags", ctx, desired, scmd).Return(nil) statusWriter := &ctrlrtclientmock.SubResourceWriter{} kc.On("Status").Return(statusWriter) kc.On("Patch", withoutCancelContextMatcher, latestRTObj, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil) - kc.On("Patch", withoutCancelContextMatcher, updatedRTObj, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil) - statusWriter.On("Patch", withoutCancelContextMatcher, updatedRTObj, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil) + statusWriter.On("Patch", withoutCancelContextMatcher, latestRTObj, mock.AnythingOfType("*client.mergeFromPatch")).Return(nil).Once() _, err := r.Sync(ctx, rm, desired) - require.Nil(err) + require.NotNil(err) rm.AssertNumberOfCalls(t, "ReadOne", 1) - rm.AssertCalled(t, "Update", ctx, desired, latest, delta) - rd.AssertCalled(t, "Delta", desired, latest) - rd.AssertNumberOfCalls(t, "MarkAdopted", 2) // Assert that the resource is not created or updated rm.AssertNumberOfCalls(t, "Create", 0) }