Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementating ReferenceResolver in managed reconciler #46

Merged
merged 10 commits into from
Oct 22, 2019
46 changes: 46 additions & 0 deletions apis/core/v1alpha1/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ const (
// TypeSynced managed resources are believed to be in sync with the
// Kubernetes resources that manage their lifecycle.
TypeSynced ConditionType = "Synced"

// TypeReferencesResolved managed resources' references are resolved
TypeReferencesResolved = "ReferencesResolved"
)

// A ConditionReason represents the reason a resource is in a condition.
Expand All @@ -53,6 +56,12 @@ const (
ReasonReconcileError ConditionReason = "Encountered an error during managed resource reconciliation"
)

// Reason references for a resource are or are not resolved
const (
ReasonReferenceResolveSuccess ConditionReason = "Successfully resolved managed resource references to other resources"
negz marked this conversation as resolved.
Show resolved Hide resolved
ReasonResolveReferencesBlocked ConditionReason = "One or more of referenced resources do not exist, or are not yet Ready"
)
soorena776 marked this conversation as resolved.
Show resolved Hide resolved

// A Condition that may apply to a managed resource.
type Condition struct {
// Type of this condition. At most one of each condition type may apply to
Expand Down Expand Up @@ -113,6 +122,18 @@ func NewConditionedStatus(c ...Condition) *ConditionedStatus {
return s
}

// GetCondition returns the condition for the given ConditionType if exists,
// otherwise returns nil
func (s *ConditionedStatus) GetCondition(ct ConditionType) Condition {
for _, c := range s.Conditions {
if c.Type == ct {
return c
}
}

return Condition{Type: ct, Status: corev1.ConditionUnknown}
}
soorena776 marked this conversation as resolved.
Show resolved Hide resolved

// SetConditions sets the supplied conditions, replacing any existing conditions
// of the same type. This is a no-op if all supplied conditions are identical,
// ignoring the last transition time, to those already set.
Expand Down Expand Up @@ -239,3 +260,28 @@ func ReconcileError(err error) Condition {
Message: err.Error(),
}
}

// ReferenceResolutionSuccess returns a condition indicating that Crossplane
// successfully resolved the references used in the managed resource
func ReferenceResolutionSuccess() Condition {
return Condition{
Type: TypeReferencesResolved,
Status: corev1.ConditionTrue,
LastTransitionTime: metav1.Now(),
Reason: ReasonReferenceResolveSuccess,
}
}

// ReferenceResolutionBlocked returns a condition indicating that Crossplane is
// unable to resolve the references used in the managed resource. This could
// mean that one or more of referred resources do not yet exist, or are not yet
// Ready
soorena776 marked this conversation as resolved.
Show resolved Hide resolved
func ReferenceResolutionBlocked(err error) Condition {
return Condition{
Type: TypeReferencesResolved,
Status: corev1.ConditionFalse,
LastTransitionTime: metav1.Now(),
Reason: ReasonResolveReferencesBlocked,
Message: err.Error(),
}
}
31 changes: 31 additions & 0 deletions apis/core/v1alpha1/condition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,37 @@ func TestSetConditions(t *testing.T) {
}
}

func TestGetCondition(t *testing.T) {
cases := map[string]struct {
cs *ConditionedStatus
t ConditionType
want Condition
}{
"ConditionExists": {
cs: NewConditionedStatus(Available()),
t: TypeReady,
want: Available(),
},
"ConditionDoesNotExist": {
cs: NewConditionedStatus(Available()),
t: TypeSynced,
want: Condition{
Type: TypeSynced,
Status: corev1.ConditionUnknown,
},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
got := tc.cs.GetCondition(tc.t)
if diff := cmp.Diff(tc.want, got); diff != "" {
t.Errorf("tc.cs.GetConditions(...): -want, +got:\n%s", diff)
}
})
}
}

func TestConditionWithMessage(t *testing.T) {
testMsg := "Something went wrong on cloud side"
cases := map[string]struct {
Expand Down
12 changes: 6 additions & 6 deletions pkg/resource/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ type Bindable interface {
GetBindingPhase() v1alpha1.BindingPhase
}

// A ConditionSetter may have conditions set. Conditions are informational, and
// typically indicate the status of both a resource and its reconciliation
// process.
type ConditionSetter interface {
// A Conditioned may have conditions set or retrieved. Conditions are typically
// indicate the status of both a resource and its reconciliation process.
type Conditioned interface {
SetConditions(c ...v1alpha1.Condition)
GetCondition(v1alpha1.ConditionType) v1alpha1.Condition
}

// A ClaimReferencer may reference a resource claim.
Expand Down Expand Up @@ -91,7 +91,7 @@ type Claim interface {
ManagedResourceReferencer
ConnectionSecretWriterTo

ConditionSetter
Conditioned
Bindable
}

Expand All @@ -115,7 +115,7 @@ type Managed interface {
ConnectionSecretWriterTo
Reclaimer

ConditionSetter
Conditioned
Bindable
}

Expand Down
11 changes: 7 additions & 4 deletions pkg/resource/interfaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@ type MockBindable struct{ Phase v1alpha1.BindingPhase }
func (m *MockBindable) SetBindingPhase(p v1alpha1.BindingPhase) { m.Phase = p }
func (m *MockBindable) GetBindingPhase() v1alpha1.BindingPhase { return m.Phase }

type MockConditionSetter struct{ Conditions []v1alpha1.Condition }
type MockConditioned struct{ Conditions []v1alpha1.Condition }

func (m *MockConditionSetter) SetConditions(c ...v1alpha1.Condition) { m.Conditions = c }
func (m *MockConditioned) SetConditions(c ...v1alpha1.Condition) { m.Conditions = c }
func (m *MockConditioned) GetCondition(ct v1alpha1.ConditionType) v1alpha1.Condition {
return v1alpha1.Condition{Type: ct, Status: corev1.ConditionUnknown}
}

type MockClaimReferencer struct{ Ref *corev1.ObjectReference }

Expand Down Expand Up @@ -89,7 +92,7 @@ type MockClaim struct {
MockPortableClassReferencer
MockManagedResourceReferencer
MockConnectionSecretWriterTo
MockConditionSetter
MockConditioned
MockBindable
}

Expand All @@ -112,7 +115,7 @@ type MockManaged struct {
MockClaimReferencer
MockConnectionSecretWriterTo
MockReclaimer
MockConditionSetter
MockConditioned
MockBindable
}

Expand Down
31 changes: 29 additions & 2 deletions pkg/resource/managed_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ import (
)

const (
managedControllerName = "managedresource.crossplane.io"
managedFinalizerName = "finalizer." + managedControllerName
managedControllerName = "managedresource.crossplane.io"
managedFinalizerName = "finalizer." + managedControllerName
managedResourceStructTagPackageName = "resource"

managedReconcileTimeout = 1 * time.Minute

defaultManagedShortWait = 30 * time.Second
Expand Down Expand Up @@ -87,6 +89,14 @@ func (m ManagedInitializerFn) Initialize(ctx context.Context, mg Managed) error
return m(ctx, mg)
}

// ManagedReferenceResolverFn is the pluggable struct to produce objects with ManagedReferenceResolver interface.
type ManagedReferenceResolverFn func(context.Context, CanReference) error

// ResolveReferences calls ManagedReferenceResolverFn function
func (m ManagedReferenceResolverFn) ResolveReferences(ctx context.Context, res CanReference) error {
return m(ctx, res)
}

// An ExternalConnecter produces a new ExternalClient given the supplied
// Managed resource.
type ExternalConnecter interface {
Expand Down Expand Up @@ -224,6 +234,7 @@ type mrManaged struct {
ManagedConnectionPublisher
ManagedFinalizer
ManagedInitializer
ManagedReferenceResolver
}

func defaultMRManaged(m manager.Manager) mrManaged {
Expand All @@ -234,6 +245,7 @@ func defaultMRManaged(m manager.Manager) mrManaged {
NewManagedNameAsExternalName(m.GetClient()),
NewAPIManagedFinalizerAdder(m.GetClient()),
},
ManagedReferenceResolver: NewAPIManagedReferenceResolver(m.GetClient()),
}
}

Expand Down Expand Up @@ -360,6 +372,21 @@ func (r *ManagedReconciler) Reconcile(req reconcile.Request) (reconcile.Result,
return reconcile.Result{RequeueAfter: r.shortWait}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

if !IsConditionTrue(managed.GetCondition(v1alpha1.TypeReferencesResolved)) {
soorena776 marked this conversation as resolved.
Show resolved Hide resolved
if err := r.managed.ResolveReferences(ctx, managed); err != nil {
condition := v1alpha1.ReconcileError(err)
if IsReferencesAccessError(err) {
condition = v1alpha1.ReferenceResolutionBlocked(err)
}

managed.SetConditions(condition)
return reconcile.Result{RequeueAfter: r.longWait}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

// Add ReferenceResolutionSuccess to the conditions
managed.SetConditions(v1alpha1.ReferenceResolutionSuccess())
}

observation, err := external.Observe(ctx, managed)
if err != nil {
// We'll usually hit this case if our Provider credentials are invalid
Expand Down
75 changes: 73 additions & 2 deletions pkg/resource/managed_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func TestManagedReconciler(t *testing.T) {
}

errBoom := errors.New("boom")
errNotReady := &referencesAccessErr{[]ReferenceStatus{{Name: "cool-res", Status: ReferenceNotReady}}}
now := metav1.Now()
testFinalizers := []string{"finalizer.crossplane.io"}
testConnectionDetails := ConnectionDetails{
Expand Down Expand Up @@ -517,14 +518,15 @@ func TestManagedReconciler(t *testing.T) {
},
want: want{result: reconcile.Result{RequeueAfter: defaultManagedShortWait}},
},
"EstablishResourceDoesNotExistError": {
"ResolveReferences_NotReadyErr_ReturnsExpected": {
args: args{
m: &MockManager{
c: &test.MockClient{
MockGet: test.NewMockGetFn(nil),
MockStatusUpdate: test.MockStatusUpdateFn(func(_ context.Context, obj runtime.Object, _ ...client.UpdateOption) error {
want := &MockManaged{}
want.SetConditions(v1alpha1.ReconcileError(errBoom))
want.SetConditions(v1alpha1.ReferenceResolutionBlocked(errNotReady))
want.SetFinalizers(testFinalizers)
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
t.Errorf("-want, +got:\n%s", diff)
}
Expand All @@ -541,6 +543,75 @@ func TestManagedReconciler(t *testing.T) {
}, nil
},
},
o: []ManagedReconcilerOption{
func(r *ManagedReconciler) {
r.managed.ManagedInitializer = ManagedInitializerFn(func(_ context.Context, mg Managed) error {
mg.SetFinalizers(testFinalizers)
return nil
})
},
func(r *ManagedReconciler) {
r.managed.ManagedReferenceResolver = ManagedReferenceResolverFn(func(_ context.Context, res CanReference) error {
return errNotReady
})
},
},
},
want: want{result: reconcile.Result{RequeueAfter: defaultManagedLongWait}},
},
"ResolveReferences_OtherError_ReturnsExpected": {
args: args{
m: &MockManager{
c: &test.MockClient{
MockGet: test.NewMockGetFn(nil),
MockStatusUpdate: test.MockStatusUpdateFn(func(_ context.Context, obj runtime.Object, _ ...client.UpdateOption) error {
want := &MockManaged{}
want.SetConditions(v1alpha1.ReconcileError(errBoom))
want.SetFinalizers(testFinalizers)
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
t.Errorf("-want, +got:\n%s", diff)
}
return nil
}),
},
s: MockSchemeWith(&MockManaged{}),
},
mg: ManagedKind(MockGVK(&MockManaged{})),
e: &ExternalClientFns{},
o: []ManagedReconcilerOption{
func(r *ManagedReconciler) {
r.managed.ManagedInitializer = ManagedInitializerFn(func(_ context.Context, mg Managed) error {
mg.SetFinalizers(testFinalizers)
return nil
})
},
func(r *ManagedReconciler) {
r.managed.ManagedReferenceResolver = ManagedReferenceResolverFn(func(_ context.Context, res CanReference) error {
return errBoom
})
},
},
},
want: want{result: reconcile.Result{RequeueAfter: defaultManagedLongWait}},
},
"EstablishResourceDoesNotExistError": {
args: args{
m: &MockManager{
c: &test.MockClient{
MockGet: test.NewMockGetFn(nil),
MockStatusUpdate: test.MockStatusUpdateFn(func(_ context.Context, obj runtime.Object, _ ...client.UpdateOption) error {
want := &MockManaged{}
want.SetConditions(v1alpha1.ReconcileError(errBoom))
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
t.Errorf("-want, +got:\n%s", diff)
}
return nil
}),
},
s: MockSchemeWith(&MockManaged{}),
},
mg: ManagedKind(MockGVK(&MockManaged{})),
e: &ExternalClientFns{},
o: []ManagedReconcilerOption{
func(r *ManagedReconciler) {
r.managed.ManagedConnectionPublisher = ManagedConnectionPublisherFns{
Expand Down
Loading