always patchResourceMetadataAndSpec after rm.LateInitialization() call.

Author: vijtrip2

mocks/pkg/types/aws_resource.go

@@ -223,7 +223,7 @@ func (r *resourceReconciler) Sync(
 	}
 	// Attempt to late initialize the resource. If there are no fields to
 	// late initialize, this operation will be a no-op.
-	if latest, err = r.lateInitializeResource(ctx, rm, desired, latest); err != nil {
+	if latest, err = r.lateInitializeResource(ctx, rm, latest); err != nil {
 		return latest, err
 	}
 	return r.handleRequeues(ctx, latest)
@@ -356,7 +356,6 @@ func (r *resourceReconciler) updateResource(
 func (r *resourceReconciler) lateInitializeResource(
 	ctx context.Context,
 	rm acktypes.AWSResourceManager,
-	desired acktypes.AWSResource,
 	latest acktypes.AWSResource,
 ) (acktypes.AWSResource, error) {
 	var err error
@@ -367,11 +366,15 @@ func (r *resourceReconciler) lateInitializeResource(
 	rlog.Enter("rm.LateInitialize")
 	lateInitializedLatest, err := rm.LateInitialize(ctx, latest)
 	rlog.Exit("rm.LateInitialize", err)
-	if err != nil {
-		// If there was error in late initialization, still patch the resource metadata and spec
-		// to reflect changes in k8s resource.
-		if ackcompare.IsNotNil(lateInitializedLatest) {
-			_ = r.patchResourceMetadataAndSpec(ctx, desired, lateInitializedLatest)
+	// Always patch after late initialize because some fields may have been initialized while
+	// others require a retry after some delay.
+	// This patching does not hurt because if there is no diff then 'patchResourceMetadataAndSpec'
+	// acts as a no-op.
+	if ackcompare.IsNotNil(lateInitializedLatest) {
+		patchErr := r.patchResourceMetadataAndSpec(ctx, latest, lateInitializedLatest)
+		// Throw the patching error if reconciler is unable to patch the resource with late initializations
+		if patchErr != nil {
+			err = patchErr
 		}
 	}
 	return lateInitializedLatest, err

pkg/runtime/reconciler.go

@@ -149,14 +149,16 @@ func TestReconcilerUpdate(t *testing.T) {
 	rm.On("Update", ctx, desired, latest, delta).Return(
 		latest, nil,
 	)
-	rm.On("LateInitialize", ctx, latest).Return(latest, nil)
 
 	rmf, rd := managerFactoryMocks(desired, latest, delta)
 	rd.On("Delta", desired, latest).Return(
 		delta,
 	).Once()
 	rd.On("Delta", desired, latest).Return(ackcompare.NewDelta())
 
+	rm.On("LateInitialize", ctx, latest).Return(latest, nil)
+	rd.On("Delta", latest, latest).Return(ackcompare.NewDelta())
+
 	r, kc := reconcilerMocks(rmf)
 
 	kc.On("Patch", ctx, latestRTObj, client.MergeFrom(desiredRTObj)).Return(nil)
@@ -213,6 +215,7 @@ func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInMetadata(t *testing.T) {
 		latest, nil,
 	)
 	rm.On("LateInitialize", ctx, latest).Return(latest, nil)
+	rd.On("Delta", latest, latest).Return(ackcompare.NewDelta())
 
 	r, kc := reconcilerMocks(rmf)
 
@@ -261,6 +264,7 @@ func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInSpec(t *testing.T) {
 		latest, nil,
 	)
 	rm.On("LateInitialize", ctx, latest).Return(latest, nil)
+	rd.On("Delta", latest, latest).Return(ackcompare.NewDelta())
 
 	r, kc := reconcilerMocks(rmf)
 
@@ -363,15 +367,17 @@ func TestReconcilerUpdate_ErrorInLateInitialization(t *testing.T) {
 	rm.On("Update", ctx, desired, latest, delta).Return(
 		latest, nil,
 	)
-	requeueError := requeue.NeededAfter(errors.New("error from late initialization"), time.Duration(0)*time.Second)
-	rm.On("LateInitialize", ctx, latest).Return(latest, requeueError)
 
 	rmf, rd := managerFactoryMocks(desired, latest, delta)
 	rd.On("Delta", desired, latest).Return(
 		delta,
 	).Once()
 	rd.On("Delta", desired, latest).Return(ackcompare.NewDelta())
 
+	requeueError := requeue.NeededAfter(errors.New("error from late initialization"), time.Duration(0)*time.Second)
+	rm.On("LateInitialize", ctx, latest).Return(latest, requeueError)
+	rd.On("Delta", latest, latest).Return(ackcompare.NewDelta())
+
 	r, kc := reconcilerMocks(rmf)
 
 	kc.On("Patch", ctx, latestRTObj, client.MergeFrom(desiredRTObj)).Return(nil)