diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 6b1b3c1..b516576 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -730,13 +730,13 @@ func (r *resourceReconciler) updateResource( rm acktypes.AWSResourceManager, desired acktypes.AWSResource, latest acktypes.AWSResource, -) (acktypes.AWSResource, error) { - var err error +) (updated acktypes.AWSResource, err error) { rlog := ackrtlog.FromContext(ctx) exit := rlog.Trace("r.updateResource") defer func() { exit(err) }() + updated = latest // Ensure the resource is managed if err = r.failOnResourceUnmanaged(ctx, latest); err != nil { @@ -752,21 +752,24 @@ func (r *resourceReconciler) updateResource( "diff", delta.Differences, ) rlog.Enter("rm.Update") - latest, err = rm.Update(ctx, desired, latest, delta) + updated, err = rm.Update(ctx, desired, latest, delta) rlog.Exit("rm.Update", err, "latest", latest) if err != nil { - return latest, err + return updated, err } // Ensure that we are patching any changes to the annotations/metadata and // the Spec that may have been set by the resource manager's successful // Update call above. - latest, err = r.patchResourceMetadataAndSpec(ctx, rm, desired, latest) + if IsAdopted(latest) { + r.rd.MarkAdopted(updated) + } + updated, err = r.patchResourceMetadataAndSpec(ctx, rm, desired, updated) if err != nil { - return latest, err + return updated, err } rlog.Info("updated resource") } - return latest, nil + return updated, nil } // lateInitializeResource calls AWSResourceManager.LateInitialize() method and diff --git a/pkg/runtime/reconciler_test.go b/pkg/runtime/reconciler_test.go index 8f78560..0c2fe47 100644 --- a/pkg/runtime/reconciler_test.go +++ b/pkg/runtime/reconciler_test.go @@ -396,7 +396,6 @@ func TestReconcilerAdoptOrCreateResource_Create(t *testing.T) { ).Once() rm.On("IsSynced", ctx, latest).Return(true, nil) rmf, rd := managedResourceManagerFactoryMocks(desired, latest) - rd.On("MarkAdopted", latest).Return() rm.On("LateInitialize", ctx, latest).Return(latest, nil) rd.On("IsManaged", desired).Return(false).Once() @@ -421,31 +420,59 @@ func TestReconcilerAdoptOrCreateResource_Create(t *testing.T) { func TestReconcilerAdoptOrCreateResource_Adopt(t *testing.T) { require := require.New(t) + assert := assert.New(t) ctx := context.TODO() + adoptionFieldsString := "{\"arn\": \"my-adopt-book-arn\"}" + adoptionFields := map[string]string{ + "arn": "my-adopt-book-arn", + } desired, _, metaObj := resourceMocks() desired.On("ReplaceConditions", []*ackv1alpha1.Condition{}).Return() metaObj.SetAnnotations(map[string]string{ ackv1alpha1.AnnotationAdoptionPolicy: "adopt-or-create", - ackv1alpha1.AnnotationAdoptionFields: "{\"arn\": \"my-adopt-book-arn\"}", + ackv1alpha1.AnnotationAdoptionFields: adoptionFieldsString, }) ids := &ackmocks.AWSResourceIdentifiers{} delta := ackcompare.NewDelta() - delta.Add("Spec.A", "val1", "val2") + delta.Add("Spec", "val1", "val2") - latest, latestRTObj, _ := resourceMocks() + latest, latestRTObj, latestMetaObj := resourceMocks() latest.On("Identifiers").Return(ids) latest.On("Conditions").Return([]*ackv1alpha1.Condition{}) - latest.On("MetaObject").Return(metav1.ObjectMeta{ + latest.On( + "ReplaceConditions", + mock.AnythingOfType("[]*v1alpha1.Condition"), + ).Return().Run(func(args mock.Arguments) { + conditions := args.Get(0).([]*ackv1alpha1.Condition) + hasSynced := false + for _, condition := range conditions { + if condition.Type != ackv1alpha1.ConditionTypeResourceSynced { + continue + } + + hasSynced = true + assert.Equal(corev1.ConditionTrue, condition.Status) + assert.Equal(ackcondition.SyncedMessage, *condition.Message) + } + assert.True(hasSynced) + }) + latestMetaObj.SetAnnotations(map[string]string{ + 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\"}", }, }) - latest.On("Conditions").Return([]*ackv1alpha1.Condition{}) - latest.On( + updated.On( "ReplaceConditions", mock.AnythingOfType("[]*v1alpha1.Condition"), ).Return() @@ -454,36 +481,50 @@ func TestReconcilerAdoptOrCreateResource_Adopt(t *testing.T) { rm.On("ResolveReferences", ctx, nil, desired).Return( desired, false, nil, ).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("ReadOne", ctx, desired).Return( latest, nil, ).Once() rm.On("Update", ctx, desired, latest, delta).Return( - latest, nil, + updated, nil, ).Once() rm.On("IsSynced", ctx, latest).Return(true, nil) rmf, rd := managedResourceManagerFactoryMocks(desired, latest) - rm.On("LateInitialize", ctx, latest).Return(latest, nil) + rm.On("LateInitialize", ctx, updated).Return(latest, nil) + rd.On("IsManaged", desired).Return(false).Once() rd.On("IsManaged", desired).Return(true) - rd.On("Delta", desired, latest).Return( - delta, - ).Once() - rd.On("Delta", desired, latest).Return(ackcompare.NewDelta()) - rd.On("Delta", latest, latest).Return(ackcompare.NewDelta()) + rd.On("MarkAdopted", latest).Return().Once() + latestMetaObj.SetAnnotations(map[string]string{ + ackv1alpha1.AnnotationAdoptionPolicy: "adopt-or-create", + 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) - statusWriter.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) _, err := r.Sync(ctx, rm, desired) require.Nil(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) } @@ -1249,7 +1290,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...