Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions apis/core/v1alpha1/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions mocks/pkg/types/aws_resource.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 23 additions & 0 deletions mocks/pkg/types/aws_resource_manager.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 39 additions & 0 deletions pkg/condition/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
}
13 changes: 11 additions & 2 deletions pkg/requeue/requeue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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
}

Expand Down
15 changes: 13 additions & 2 deletions pkg/requeue/requeue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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())
}
42 changes: 42 additions & 0 deletions pkg/runtime/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}
Comment on lines +375 to +378
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine. We'll be shadowing any non-nil err that may have been returned on line 367, but that's OK I guess.

}
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(
Expand Down
67 changes: 67 additions & 0 deletions pkg/runtime/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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)

Expand All @@ -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) {
Expand Down Expand Up @@ -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)

Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}
7 changes: 7 additions & 0 deletions pkg/types/aws_resource_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down