From 934ba686bf88048f3b29a10078344d6098afea86 Mon Sep 17 00:00:00 2001 From: vijtrip2 <62766875+vijtrip2@users.noreply.github.com> Date: Thu, 12 Aug 2021 13:35:55 -0700 Subject: [PATCH] Initial changes for late initializing resource fields. (#37) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue: https://github.com/aws-controllers-k8s/community/issues/812 Changes: * Introduce new condition ConditionTypeLateInitialized * Update reconciler to invoke `AWSResourceManager.LateInitialize()` method ------------------ Corresponding Code generator PR: https://github.com/aws-controllers-k8s/code-generator/pull/138 Please do not merge until code generator PR is merged ------------------ Testing * Unit tests successful ``` ➜ runtime git:(li-mlp) ✗ make test building mocks for pkg/types ... ok. building mocks for k8s.io/apimachinery/pkg/apis/meta/v1 ... ok. building mocks for k8s.io/apimachinery/runtime ... ok. building mocks for k8s.io/apimachinery/runtime/schema ... ok. building mocks for sigs.k8s.io/controller-runtime/pkg/client ... ok. go test ./... ? github.com/aws-controllers-k8s/runtime/apis/core/v1alpha1 [no test files] ? github.com/aws-controllers-k8s/runtime/mocks/apimachinery/pkg/apis/meta/v1 [no test files] ? github.com/aws-controllers-k8s/runtime/mocks/apimachinery/pkg/runtime [no test files] ? github.com/aws-controllers-k8s/runtime/mocks/apimachinery/pkg/runtime/schema [no test files] ? github.com/aws-controllers-k8s/runtime/mocks/controller-runtime/pkg/client [no test files] ? github.com/aws-controllers-k8s/runtime/mocks/pkg/types [no test files] ok github.com/aws-controllers-k8s/runtime/pkg/compare (cached) ok github.com/aws-controllers-k8s/runtime/pkg/condition (cached) ? github.com/aws-controllers-k8s/runtime/pkg/config [no test files] ? github.com/aws-controllers-k8s/runtime/pkg/errors [no test files] ? github.com/aws-controllers-k8s/runtime/pkg/metrics [no test files] ok github.com/aws-controllers-k8s/runtime/pkg/requeue (cached) ok github.com/aws-controllers-k8s/runtime/pkg/runtime 1.287s ok github.com/aws-controllers-k8s/runtime/pkg/runtime/cache (cached) ? github.com/aws-controllers-k8s/runtime/pkg/runtime/log [no test files] ? github.com/aws-controllers-k8s/runtime/pkg/types [no test files] ok github.com/aws-controllers-k8s/runtime/pkg/util (cached) ? github.com/aws-controllers-k8s/runtime/pkg/webhook [no test files] ``` * local-kind-e2e-test logs ``` │ │ 2021-07-26T19:38:08.701Z DEBUG ackrt > r.Sync {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "kind": "Repository" │ │ 2021-07-26T19:38:08.701Z DEBUG ackrt >> rm.ReadOne {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": f │ │ 2021-07-26T19:38:08.701Z DEBUG ackrt >>> rm.sdkFind {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": │ │ 2021-07-26T19:38:09.641Z DEBUG ackrt <<< rm.sdkFind {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": │ │ 2021-07-26T19:38:09.641Z DEBUG ackrt << rm.ReadOne {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": f │ │ 2021-07-26T19:38:09.641Z DEBUG ackrt >> r.setResourceManaged {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_a │ │ 2021-07-26T19:38:09.641Z DEBUG ackrt >>> kc.Patch (metadata + spec) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2" │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt <<< kc.Patch (metadata + spec) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2" │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt marked resource as managed {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "i │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt << r.setResourceManaged {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_a │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt >> rm.Create {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": fa │ │ 2021-07-26T19:38:09.694Z DEBUG ackrt >>> rm.sdkCreate {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted" │ │ 2021-07-26T19:38:09.725Z DEBUG ackrt <<< rm.sdkCreate {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted" │ │ 2021-07-26T19:38:09.725Z DEBUG ackrt << rm.Create {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": fa │ │ 2021-07-26T19:38:09.725Z INFO ackrt created new resource {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopt │ │ 2021-07-26T19:38:09.725Z DEBUG ackrt >> rm.LateInitialize {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adop │ │ 2021-07-26T19:38:09.725Z INFO ackrt calculated late initialization delay is 0 seconds {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "re │ │ 2021-07-26T19:38:09.725Z DEBUG ackrt >>> rm.sdkFind {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": │ │ 2021-07-26T19:38:09.748Z DEBUG ackrt <<< rm.sdkFind {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": │ │ 2021-07-26T19:38:09.748Z DEBUG ackrt << rm.LateInitialize {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adop │ │ 2021-07-26T19:38:09.748Z DEBUG ackrt >> r.patchResourceMetadataAndSpec {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west │ │ 2021-07-26T19:38:09.748Z DEBUG ackrt >>> kc.Patch (metadata + spec) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2" │ │ 2021-07-26T19:38:09.758Z DEBUG ackrt <<< kc.Patch (metadata + spec) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2" │ │ 2021-07-26T19:38:09.759Z DEBUG ackrt patched resource metadata and spec {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-wes │ │ 2021-07-26T19:38:09.759Z DEBUG ackrt << r.patchResourceMetadataAndSpec {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west │ │ 2021-07-26T19:38:09.759Z DEBUG ackrt >> r.patchResourceStatus {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_ │ │ 2021-07-26T19:38:09.759Z DEBUG ackrt >>> kc.Patch (status) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_ado │ │ 2021-07-26T19:38:09.794Z DEBUG ackrt <<< kc.Patch (status) {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_ado │ │ 2021-07-26T19:38:09.795Z DEBUG ackrt patched resource status {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_a │ │ 2021-07-26T19:38:09.795Z DEBUG ackrt << r.patchResourceStatus {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_ │ │ 2021-07-26T19:38:09.795Z DEBUG ackrt < r.Sync {"kind": "Repository", "namespace": "default", "name": "ecr-repository-wy6n6wjm8", "generation": 1, "account": "309117047740", "role": "", "region": "us-west-2", "is_adopted": false, │ │ 2021-07-26T19:38:09.795Z DEBUG controller-runtime.controller Successfully Reconciled {"controller": "repository", "request": "default/ecr-repository-wy6n6wjm8"} ``` --- apis/core/v1alpha1/conditions.go | 7 +++ mocks/pkg/types/aws_resource.go | 5 ++ mocks/pkg/types/aws_resource_manager.go | 23 +++++++++ pkg/condition/condition.go | 39 ++++++++++++++ pkg/requeue/requeue.go | 13 ++++- pkg/requeue/requeue_test.go | 15 +++++- pkg/runtime/reconciler.go | 42 ++++++++++++++++ pkg/runtime/reconciler_test.go | 67 +++++++++++++++++++++++++ pkg/types/aws_resource_manager.go | 7 +++ 9 files changed, 214 insertions(+), 4 deletions(-) diff --git a/apis/core/v1alpha1/conditions.go b/apis/core/v1alpha1/conditions.go index dbef4bf..c06a5e5 100644 --- a/apis/core/v1alpha1/conditions.go +++ b/apis/core/v1alpha1/conditions.go @@ -45,6 +45,13 @@ const ( // Examples include // - Modifying an immutable field after it was created ConditionTypeAdvisory ConditionType = "ACK.Advisory" + // ConditionTypeLateInitialized indicates whether the late initialization + // of fields is completed or is in progress. + // The absence of this condition indicates there is no late initalization + // needed for the k8s resource. + // "True" status indicates that the resource fields have been late initialized + // "False" status indicates that the resource fields are in process of being late initialized. + ConditionTypeLateInitialized ConditionType = "ACK.LateInitialized" ) // Condition is the common struct used by all CRDs managed by ACK service diff --git a/mocks/pkg/types/aws_resource.go b/mocks/pkg/types/aws_resource.go index 968f5de..107e1cf 100644 --- a/mocks/pkg/types/aws_resource.go +++ b/mocks/pkg/types/aws_resource.go @@ -135,3 +135,8 @@ func (_m *AWSResource) SetIdentifiers(_a0 *v1alpha1.AWSIdentifiers) error { func (_m *AWSResource) SetObjectMeta(meta v1.ObjectMeta) { _m.Called(meta) } + +// SetStatus provides a mock function with given fields: _a0 +func (_m *AWSResource) SetStatus(_a0 types.AWSResource) { + _m.Called(_a0) +} diff --git a/mocks/pkg/types/aws_resource_manager.go b/mocks/pkg/types/aws_resource_manager.go index c081352..1395dd6 100644 --- a/mocks/pkg/types/aws_resource_manager.go +++ b/mocks/pkg/types/aws_resource_manager.go @@ -77,6 +77,29 @@ func (_m *AWSResourceManager) Delete(_a0 context.Context, _a1 types.AWSResource) return r0, r1 } +// LateInitialize provides a mock function with given fields: _a0, _a1 +func (_m *AWSResourceManager) LateInitialize(_a0 context.Context, _a1 types.AWSResource) (types.AWSResource, error) { + ret := _m.Called(_a0, _a1) + + var r0 types.AWSResource + if rf, ok := ret.Get(0).(func(context.Context, types.AWSResource) types.AWSResource); ok { + r0 = rf(_a0, _a1) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(types.AWSResource) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, types.AWSResource) error); ok { + r1 = rf(_a0, _a1) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // ReadOne provides a mock function with given fields: _a0, _a1 func (_m *AWSResourceManager) ReadOne(_a0 context.Context, _a1 types.AWSResource) (types.AWSResource, error) { ret := _m.Called(_a0, _a1) diff --git a/pkg/condition/condition.go b/pkg/condition/condition.go index fd4e42e..a6f95b8 100644 --- a/pkg/condition/condition.go +++ b/pkg/condition/condition.go @@ -35,6 +35,13 @@ func Terminal(subject acktypes.ConditionManager) *ackv1alpha1.Condition { return FirstOfType(subject, ackv1alpha1.ConditionTypeTerminal) } +// LateInitialized returns the Condition in the resource's Conditions collection that +// is of type ConditionTypeLateInitialized. If no such condition is found, returns +// nil. +func LateInitialized(subject acktypes.ConditionManager) *ackv1alpha1.Condition { + return FirstOfType(subject, ackv1alpha1.ConditionTypeLateInitialized) +} + // FirstOfType returns the first Condition in the resource's Conditions // collection of the supplied type. If no such condition is found, returns nil. func FirstOfType( @@ -111,3 +118,35 @@ func SetTerminal( c.Reason = reason subject.ReplaceConditions(allConds) } + +// SetLateInitialized sets the resource's Condition of type ConditionTypeLateInitialized to +// the supplied status, optional message and reason. +func SetLateInitialized( + subject acktypes.ConditionManager, + status corev1.ConditionStatus, + message *string, + reason *string, +) { + allConds := subject.Conditions() + var c *ackv1alpha1.Condition + if c = LateInitialized(subject); c == nil { + c = &ackv1alpha1.Condition{ + Type: ackv1alpha1.ConditionTypeLateInitialized, + } + allConds = append(allConds, c) + } + now := metav1.Now() + c.LastTransitionTime = &now + c.Status = status + c.Message = message + c.Reason = reason + subject.ReplaceConditions(allConds) +} + +// LateInitializationInProgress return true if ConditionTypeLateInitialized has "False" status +// False status means that resource has LateInitializationConfig but has not been completely +// late initialized yet. +func LateInitializationInProgress(subject acktypes.ConditionManager) bool { + c := LateInitialized(subject) + return c != nil && c.Status == corev1.ConditionFalse +} diff --git a/pkg/requeue/requeue.go b/pkg/requeue/requeue.go index eb4b983..57e9c91 100644 --- a/pkg/requeue/requeue.go +++ b/pkg/requeue/requeue.go @@ -53,13 +53,16 @@ type RequeueNeeded struct { } func (e *RequeueNeeded) Error() string { - if e.err == nil { + if e == nil || e.err == nil { return "" } return e.err.Error() } func (e *RequeueNeeded) Unwrap() error { + if e == nil { + return nil + } return e.err } @@ -78,17 +81,23 @@ type RequeueNeededAfter struct { } func (e *RequeueNeededAfter) Error() string { - if e.err == nil { + if e == nil || e.err == nil { return "" } return e.err.Error() } func (e *RequeueNeededAfter) Duration() time.Duration { + if e == nil { + return time.Duration(0)*time.Second + } return e.duration } func (e *RequeueNeededAfter) Unwrap() error { + if e == nil { + return nil + } return e.err } diff --git a/pkg/requeue/requeue_test.go b/pkg/requeue/requeue_test.go index 82ff66d..51801cb 100644 --- a/pkg/requeue/requeue_test.go +++ b/pkg/requeue/requeue_test.go @@ -17,10 +17,9 @@ import ( "testing" "time" + "github.com/aws-controllers-k8s/runtime/pkg/requeue" "github.com/pkg/errors" "github.com/stretchr/testify/assert" - - "github.com/aws-controllers-k8s/runtime/pkg/requeue" ) func TestRequeueNeeded(t *testing.T) { @@ -109,3 +108,15 @@ func TestRequeueNeededAfter(t *testing.T) { }) } } + +func TestRequeueNeededAfter_Nil(t *testing.T) { + assert := assert.New(t) + var nilRequeueNeededAfter *requeue.RequeueNeededAfter + assert.Empty(nilRequeueNeededAfter.Error()) + assert.Nil(nilRequeueNeededAfter.Unwrap()) + assert.Equal("0s", nilRequeueNeededAfter.Duration().String()) + + var nilRequeueNeeded *requeue.RequeueNeeded + assert.Empty(nilRequeueNeeded.Error()) + assert.Nil(nilRequeueNeeded.Unwrap()) +} diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 8dec793..2b495ac 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -221,6 +221,11 @@ func (r *resourceReconciler) Sync( return latest, err } } + // 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, latest); err != nil { + return latest, err + } return r.handleRequeues(ctx, latest) } @@ -338,6 +343,43 @@ func (r *resourceReconciler) updateResource( return latest, nil } +// lateInitializeResource calls AWSResourceManager.LateInitialize() method and +// returns the AWSResource with late initialized fields. +// +// When the late initialization is delayed for an AWSResource, an error is returned +// with specific requeue delay to attempt lateInitialization again. +// +// This method also adds an annotation to K8s CR, indicating the number of +// late initialization attempts to correctly calculate exponential backoff delay +// +// This method also adds Condition to CR's status indicating status of late initialization. +func (r *resourceReconciler) lateInitializeResource( + ctx context.Context, + rm acktypes.AWSResourceManager, + latest acktypes.AWSResource, +) (acktypes.AWSResource, error) { + var err error + rlog := ackrtlog.FromContext(ctx) + exit := rlog.Trace("r.lateInitializeResource") + defer exit(err) + + rlog.Enter("rm.LateInitialize") + lateInitializedLatest, err := rm.LateInitialize(ctx, latest) + rlog.Exit("rm.LateInitialize", err) + // 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 +} + // patchResourceMetadataAndSpec patches the custom resource in the Kubernetes API to match the // supplied latest resource's metadata and spec. func (r *resourceReconciler) patchResourceMetadataAndSpec( diff --git a/pkg/runtime/reconciler_test.go b/pkg/runtime/reconciler_test.go index 7233dd4..3bb8942 100644 --- a/pkg/runtime/reconciler_test.go +++ b/pkg/runtime/reconciler_test.go @@ -15,8 +15,11 @@ package runtime_test import ( "context" + "errors" "testing" + "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap/zapcore" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,6 +32,7 @@ import ( ackcompare "github.com/aws-controllers-k8s/runtime/pkg/compare" ackcfg "github.com/aws-controllers-k8s/runtime/pkg/config" ackmetrics "github.com/aws-controllers-k8s/runtime/pkg/metrics" + "github.com/aws-controllers-k8s/runtime/pkg/requeue" ackrt "github.com/aws-controllers-k8s/runtime/pkg/runtime" ackrtcache "github.com/aws-controllers-k8s/runtime/pkg/runtime/cache" acktypes "github.com/aws-controllers-k8s/runtime/pkg/types" @@ -152,6 +156,9 @@ func TestReconcilerUpdate(t *testing.T) { ).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) @@ -170,6 +177,7 @@ func TestReconcilerUpdate(t *testing.T) { kc.AssertNotCalled(t, "Patch", ctx, latestRTObj, client.MergeFrom(desiredRTObj)) // Only the HandleReconcilerError wrapper function ever calls patchResourceStatus kc.AssertNotCalled(t, "Status") + rm.AssertCalled(t, "LateInitialize", ctx, latest) } func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInMetadata(t *testing.T) { @@ -206,6 +214,8 @@ func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInMetadata(t *testing.T) { rm.On("Update", ctx, desired, latest, delta).Return( latest, nil, ) + rm.On("LateInitialize", ctx, latest).Return(latest, nil) + rd.On("Delta", latest, latest).Return(ackcompare.NewDelta()) r, kc := reconcilerMocks(rmf) @@ -219,6 +229,7 @@ func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInMetadata(t *testing.T) { kc.AssertCalled(t, "Patch", ctx, latestRTObj, client.MergeFrom(desiredRTObj)) // Only the HandleReconcilerError wrapper function ever calls patchResourceStatus kc.AssertNotCalled(t, "Status") + rm.AssertCalled(t, "LateInitialize", ctx, latest) } func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInSpec(t *testing.T) { @@ -252,6 +263,8 @@ func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInSpec(t *testing.T) { rm.On("Update", ctx, desired, latest, delta).Return( latest, nil, ) + rm.On("LateInitialize", ctx, latest).Return(latest, nil) + rd.On("Delta", latest, latest).Return(ackcompare.NewDelta()) r, kc := reconcilerMocks(rmf) @@ -265,6 +278,7 @@ func TestReconcilerUpdate_PatchMetadataAndSpec_DiffInSpec(t *testing.T) { kc.AssertCalled(t, "Patch", ctx, latestRTObj, client.MergeFrom(desiredRTObj)) // Only the HandleReconcilerError wrapper function ever calls patchResourceStatus kc.AssertNotCalled(t, "Status") + rm.AssertCalled(t, "LateInitialize", ctx, latest) } func TestReconcilerHandleReconcilerError_PatchStatus_Latest(t *testing.T) { @@ -326,3 +340,56 @@ func TestReconcilerHandleReconcilerError_NoPatchStatus_NoLatest(t *testing.T) { // patch the spec/metadata... kc.AssertNotCalled(t, "Patch") } + +func TestReconcilerUpdate_ErrorInLateInitialization(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + + ctx := context.TODO() + arn := ackv1alpha1.AWSResourceName("mybook-arn") + + delta := ackcompare.NewDelta() + delta.Add("Spec.A", "val1", "val2") + + desired, desiredRTObj, _ := resourceMocks() + + ids := &ackmocks.AWSResourceIdentifiers{} + ids.On("ARN").Return(&arn) + + latest, latestRTObj, _ := resourceMocks() + latest.On("Identifiers").Return(ids) + latest.On("Conditions").Return([]*ackv1alpha1.Condition{}) + + rm := &ackmocks.AWSResourceManager{} + rm.On("ReadOne", ctx, desired).Return( + latest, nil, + ) + rm.On("Update", ctx, desired, latest, delta).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()) + + 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) + + _, err := r.Sync(ctx, rm, desired) + // Assert the error from late initialization + require.NotNil(err) + assert.Equal(requeueError, err) + rm.AssertCalled(t, "ReadOne", ctx, desired) + rd.AssertCalled(t, "Delta", desired, latest) + rm.AssertCalled(t, "Update", ctx, desired, latest, delta) + // No difference in desired, latest metadata and spec + kc.AssertNotCalled(t, "Patch", ctx, latestRTObj, client.MergeFrom(desiredRTObj)) + rm.AssertCalled(t, "LateInitialize", ctx, latest) +} diff --git a/pkg/types/aws_resource_manager.go b/pkg/types/aws_resource_manager.go index bb6acfd..487fa8a 100644 --- a/pkg/types/aws_resource_manager.go +++ b/pkg/types/aws_resource_manager.go @@ -67,6 +67,13 @@ type AWSResourceManager interface { // GetAttributes operations but all we have (for new CRs at least) is a // name for the resource ARNFromName(string) string + // LateInitialize returns an AWS Resource after setting the late initialized + // fields from the ReadOne call. This method will initialize the optional fields + // which were not provided by the k8s user but were defaulted by the AWS service. + // If there are no such fields to be initialized, the returned object is identical to + // object passed in the parameter. + // This method also adds/updates the ConditionTypeLateInitialized for the AWSResource. + LateInitialize(context.Context, AWSResource) (AWSResource, error) } // AWSResourceManagerFactory returns an AWSResourceManager that can be used to