Merge pull request #46 from soorena776/issue_887

Implementating ReferenceResolver in managed reconciler

apis/core/v1alpha1/condition.go

@@ -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.
@@ -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"
+	ReasonResolveReferencesBlocked ConditionReason = "One or more of referenced resources do not exist, or are not yet Ready"
+)
+
 // 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
@@ -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}
+}
+
 // 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.
@@ -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
+func ReferenceResolutionBlocked(err error) Condition {
+	return Condition{
+		Type:               TypeReferencesResolved,
+		Status:             corev1.ConditionFalse,
+		LastTransitionTime: metav1.Now(),
+		Reason:             ReasonResolveReferencesBlocked,
+		Message:            err.Error(),
+	}
+}

apis/core/v1alpha1/condition_test.go

@@ -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 {

pkg/resource/interfaces.go

@@ -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.
@@ -91,7 +91,7 @@ type Claim interface {
 	ManagedResourceReferencer
 	ConnectionSecretWriterTo
 
-	ConditionSetter
+	Conditioned
 	Bindable
 }
 
@@ -115,7 +115,7 @@ type Managed interface {
 	ConnectionSecretWriterTo
 	Reclaimer
 
-	ConditionSetter
+	Conditioned
 	Bindable
 }
 

pkg/resource/interfaces_test.go

@@ -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 }
 
@@ -89,7 +92,7 @@ type MockClaim struct {
 	MockPortableClassReferencer
 	MockManagedResourceReferencer
 	MockConnectionSecretWriterTo
-	MockConditionSetter
+	MockConditioned
 	MockBindable
 }
 
@@ -112,7 +115,7 @@ type MockManaged struct {
 	MockClaimReferencer
 	MockConnectionSecretWriterTo
 	MockReclaimer
-	MockConditionSetter
+	MockConditioned
 	MockBindable
 }
 

pkg/resource/managed_reconciler.go

@@ -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
@@ -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 {
@@ -224,6 +234,7 @@ type mrManaged struct {
 	ManagedConnectionPublisher
 	ManagedFinalizer
 	ManagedInitializer
+	ManagedReferenceResolver
 }
 
 func defaultMRManaged(m manager.Manager) mrManaged {
@@ -234,6 +245,7 @@ func defaultMRManaged(m manager.Manager) mrManaged {
 			NewManagedNameAsExternalName(m.GetClient()),
 			NewAPIManagedFinalizerAdder(m.GetClient()),
 		},
+		ManagedReferenceResolver: NewAPIManagedReferenceResolver(m.GetClient()),
 	}
 }
 
@@ -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)) {
+		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

pkg/resource/managed_reconciler_test.go

@@ -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{
@@ -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)
 							}
@@ -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{