Merge pull request #133 from hasheddan/useobj

Clean up OAM reconcilers
crossplane · Mar 20, 2020 · da385a7 · da385a7
2 parents 946bbdd + bb562fc
commit da385a7
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 55 deletions.
diff --git a/pkg/reconciler/oam/trait/reconciler.go b/pkg/reconciler/oam/trait/reconciler.go
@@ -104,12 +104,9 @@ type Reconciler struct {
 	record event.Recorder
 }
 
-// Kind is an OAM trait kind.
-type Kind schema.GroupVersionKind
-
 // NewReconciler returns a Reconciler that reconciles OAM traits by fetching
 // their referenced workload's translation and applying modifications.
-func NewReconciler(m ctrl.Manager, trait Kind, trans Kind, o ...ReconcilerOption) *Reconciler {
+func NewReconciler(m ctrl.Manager, trait resource.TraitKind, trans resource.ObjectKind, o ...ReconcilerOption) *Reconciler {
 	nt := func() resource.Trait {
 		return resource.MustCreateObject(schema.GroupVersionKind(trait), m.GetScheme()).(resource.Trait)
 	}

diff --git a/pkg/reconciler/oam/trait/reconciler_test.go b/pkg/reconciler/oam/trait/reconciler_test.go
@@ -40,8 +40,8 @@ var _ reconcile.Reconciler = &Reconciler{}
 func TestReconciler(t *testing.T) {
 	type args struct {
 		m manager.Manager
-		t Kind
-		p Kind
+		t resource.TraitKind
+		p resource.ObjectKind
 		o []ReconcilerOption
 	}
 
@@ -68,8 +68,8 @@ func TestReconciler(t *testing.T) {
 					Client: &test.MockClient{MockGet: test.NewMockGetFn(errBoom)},
 					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
 				},
-				t: Kind(fake.GVK(&fake.Trait{})),
-				p: Kind(fake.GVK(&fake.Object{})),
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 			},
 			want: want{err: errors.Wrap(errBoom, errGetTrait)},
 		},
@@ -80,8 +80,8 @@ func TestReconciler(t *testing.T) {
 					Client: &test.MockClient{MockGet: test.NewMockGetFn(kerrors.NewNotFound(schema.GroupResource{}, ""))},
 					Scheme: fake.SchemeWith(&fake.Trait{}),
 				},
-				t: Kind(fake.GVK(&fake.Trait{})),
-				p: Kind(fake.GVK(&fake.Object{})),
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 			},
 			want: want{result: reconcile.Result{}},
 		},
@@ -113,8 +113,8 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
 				},
-				t: Kind(fake.GVK(&fake.Trait{})),
-				p: Kind(fake.GVK(&fake.Object{})),
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 			},
 			want: want{result: reconcile.Result{RequeueAfter: shortWait}},
 		},
@@ -150,8 +150,8 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
 				},
-				t: Kind(fake.GVK(&fake.Trait{})),
-				p: Kind(fake.GVK(&fake.Object{})),
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 			},
 			want: want{result: reconcile.Result{RequeueAfter: shortWait}},
 		},
@@ -190,8 +190,8 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
 				},
-				t: Kind(fake.GVK(&fake.Trait{})),
-				p: Kind(fake.GVK(&fake.Object{})),
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 				o: []ReconcilerOption{WithModifier(ModifyFn(func(_ context.Context, _ runtime.Object, _ resource.Trait) error {
 					return errBoom
 				}))},
@@ -233,8 +233,8 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
 				},
-				t: Kind(fake.GVK(&fake.Trait{})),
-				p: Kind(fake.GVK(&fake.Object{})),
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 				o: []ReconcilerOption{WithApplicator(resource.ApplyFn(func(_ context.Context, _ client.Client, _ runtime.Object, _ ...resource.ApplyOption) error {
 					return errBoom
 				}))},
@@ -276,8 +276,8 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
 				},
-				t: Kind(fake.GVK(&fake.Trait{})),
-				p: Kind(fake.GVK(&fake.Object{})),
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 				o: []ReconcilerOption{WithApplicator(resource.ApplyFn(func(_ context.Context, _ client.Client, _ runtime.Object, _ ...resource.ApplyOption) error {
 					return nil
 				}))},
@@ -307,8 +307,8 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
 				},
-				t: Kind(fake.GVK(&fake.Trait{})),
-				p: Kind(fake.GVK(&fake.Object{})),
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 				o: []ReconcilerOption{WithApplicator(resource.ApplyFn(func(_ context.Context, _ client.Client, _ runtime.Object, _ ...resource.ApplyOption) error {
 					return nil
 				}))},

diff --git a/pkg/reconciler/oam/workload/reconciler.go b/pkg/reconciler/oam/workload/reconciler.go
@@ -23,7 +23,6 @@ import (
 
 	"github.com/pkg/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -110,18 +109,9 @@ type Reconciler struct {
 	record event.Recorder
 }
 
-// Kind is a kind of OAM workload.
-type Kind schema.GroupVersionKind
-
-// An Object is a Kubernetes object.
-type Object interface {
-	metav1.Object
-	runtime.Object
-}
-
 // NewReconciler returns a Reconciler that reconciles an OAM workload type by
 // packaging it into a KubernetesApplication.
-func NewReconciler(m ctrl.Manager, workload Kind, o ...ReconcilerOption) *Reconciler {
+func NewReconciler(m ctrl.Manager, workload resource.WorkloadKind, o ...ReconcilerOption) *Reconciler {
 	nw := func() resource.Workload {
 		return resource.MustCreateObject(schema.GroupVersionKind(workload), m.GetScheme()).(resource.Workload)
 	}

diff --git a/pkg/reconciler/oam/workload/reconciler_test.go b/pkg/reconciler/oam/workload/reconciler_test.go
@@ -41,7 +41,7 @@ var _ reconcile.Reconciler = &Reconciler{}
 func TestReconciler(t *testing.T) {
 	type args struct {
 		m manager.Manager
-		w Kind
+		w resource.WorkloadKind
 		o []ReconcilerOption
 	}
 
@@ -64,7 +64,7 @@ func TestReconciler(t *testing.T) {
 					Client: &test.MockClient{MockGet: test.NewMockGetFn(errBoom)},
 					Scheme: fake.SchemeWith(&fake.Workload{}),
 				},
-				w: Kind(fake.GVK(&fake.Workload{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 			},
 			want: want{err: errors.Wrap(errBoom, errGetWorkload)},
 		},
@@ -75,7 +75,7 @@ func TestReconciler(t *testing.T) {
 					Client: &test.MockClient{MockGet: test.NewMockGetFn(kerrors.NewNotFound(schema.GroupResource{}, ""))},
 					Scheme: fake.SchemeWith(&fake.Workload{}),
 				},
-				w: Kind(fake.GVK(&fake.Workload{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 			},
 			want: want{result: reconcile.Result{}},
 		},
@@ -101,8 +101,8 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Workload{}),
 				},
-				w: Kind(fake.GVK(&fake.Workload{})),
-				o: []ReconcilerOption{WithTranslator(TranslateFn(func(_ context.Context, _ resource.Workload) ([]Object, error) {
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
+				o: []ReconcilerOption{WithTranslator(TranslateFn(func(_ context.Context, _ resource.Workload) ([]resource.Object, error) {
 					return nil, errBoom
 				}))},
 			},
@@ -130,11 +130,11 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Workload{}),
 				},
-				w: Kind(fake.GVK(&fake.Workload{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				o: []ReconcilerOption{
-					WithTranslator(NewObjectTranslatorWithWrappers(func(_ context.Context, _ resource.Workload) ([]Object, error) {
+					WithTranslator(NewObjectTranslatorWithWrappers(func(_ context.Context, _ resource.Workload) ([]resource.Object, error) {
 						return nil, nil
-					}, func(ctx context.Context, w resource.Workload, obj []Object) ([]Object, error) {
+					}, func(ctx context.Context, w resource.Workload, obj []resource.Object) ([]resource.Object, error) {
 						return nil, errBoom
 					})),
 				},
@@ -163,10 +163,10 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Workload{}),
 				},
-				w: Kind(fake.GVK(&fake.Workload{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				o: []ReconcilerOption{
-					WithTranslator(TranslateFn(func(ctx context.Context, w resource.Workload) ([]Object, error) {
-						return []Object{
+					WithTranslator(TranslateFn(func(ctx context.Context, w resource.Workload) ([]resource.Object, error) {
+						return []resource.Object{
 							&appsv1.Deployment{},
 							&appsv1.Deployment{},
 						}, nil
@@ -187,7 +187,7 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Workload{}),
 				},
-				w: Kind(fake.GVK(&fake.Workload{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				o: []ReconcilerOption{WithApplicator(resource.ApplyFn(func(_ context.Context, _ client.Client, _ runtime.Object, _ ...resource.ApplyOption) error {
 					return nil
 				}))},
@@ -204,7 +204,7 @@ func TestReconciler(t *testing.T) {
 					},
 					Scheme: fake.SchemeWith(&fake.Workload{}),
 				},
-				w: Kind(fake.GVK(&fake.Workload{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				o: []ReconcilerOption{WithApplicator(resource.ApplyFn(func(_ context.Context, _ client.Client, _ runtime.Object, _ ...resource.ApplyOption) error {
 					return nil
 				}))},

diff --git a/pkg/reconciler/oam/workload/translate.go b/pkg/reconciler/oam/workload/translate.go
@@ -24,7 +24,7 @@ import (
 
 // A Translator is responsible for packaging workloads into other objects.
 type Translator interface {
-	Translate(context.Context, resource.Workload) ([]Object, error)
+	Translate(context.Context, resource.Workload) ([]resource.Object, error)
 }
 
 // An ObjectTranslator is a concrete implementation of a Translator.
@@ -33,15 +33,15 @@ type ObjectTranslator struct {
 }
 
 // Translate a workload into other objects.
-func (p *ObjectTranslator) Translate(ctx context.Context, w resource.Workload) ([]Object, error) {
+func (p *ObjectTranslator) Translate(ctx context.Context, w resource.Workload) ([]resource.Object, error) {
 	return p.TranslateFn(ctx, w)
 }
 
 // NewObjectTranslatorWithWrappers returns a Translator that translates and wraps
 // a workload.
 func NewObjectTranslatorWithWrappers(t TranslateFn, wp ...TranslationWrapper) Translator {
 	return &ObjectTranslator{
-		TranslateFn: func(ctx context.Context, w resource.Workload) ([]Object, error) {
+		TranslateFn: func(ctx context.Context, w resource.Workload) ([]resource.Object, error) {
 			objs, err := t(ctx, w)
 			if err != nil {
 				return nil, err
@@ -57,27 +57,27 @@ func NewObjectTranslatorWithWrappers(t TranslateFn, wp ...TranslationWrapper) Tr
 }
 
 // A TranslateFn translates a workload into an object.
-type TranslateFn func(context.Context, resource.Workload) ([]Object, error)
+type TranslateFn func(context.Context, resource.Workload) ([]resource.Object, error)
 
 // Translate workload into object or objects with no wrappers.
-func (fn TranslateFn) Translate(ctx context.Context, w resource.Workload) ([]Object, error) {
+func (fn TranslateFn) Translate(ctx context.Context, w resource.Workload) ([]resource.Object, error) {
 	return fn(ctx, w)
 }
 
 var _ Translator = TranslateFn(NoopTranslate)
 
 // NoopTranslate does not translate the workload and does not return error.
-func NoopTranslate(ctx context.Context, w resource.Workload) ([]Object, error) {
+func NoopTranslate(ctx context.Context, w resource.Workload) ([]resource.Object, error) {
 	return nil, nil
 }
 
 // A TranslationWrapper wraps the output of a workload translation in another
 // object or adds addition object.
-type TranslationWrapper func(context.Context, resource.Workload, []Object) ([]Object, error)
+type TranslationWrapper func(context.Context, resource.Workload, []resource.Object) ([]resource.Object, error)
 
 var _ TranslationWrapper = NoopWrapper
 
 // NoopWrapper does not wrap the workload translation and does not return error.
-func NoopWrapper(ctx context.Context, w resource.Workload, objs []Object) ([]Object, error) {
+func NoopWrapper(ctx context.Context, w resource.Workload, objs []resource.Object) ([]resource.Object, error) {
 	return objs, nil
 }
diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go
@@ -71,12 +71,21 @@ func (k ClassKind) List() schema.GroupVersionKind {
 	}
 }
 
-// A ManagedKind contains the type metadata for a kind of managed
+// A ManagedKind contains the type metadata for a kind of managed.
 type ManagedKind schema.GroupVersionKind
 
 // A TargetKind contains the type metadata for a kind of target resource.
 type TargetKind schema.GroupVersionKind
 
+// ObjectKind contains the type metadata for a kind of an object resource.
+type ObjectKind schema.GroupVersionKind
+
+// TraitKind contains the type metadata for a kind of an OAM trait resource.
+type TraitKind schema.GroupVersionKind
+
+// WorkloadKind contains the type metadata for a kind of an OAM workload resource.
+type WorkloadKind schema.GroupVersionKind
+
 // A LocalConnectionSecretOwner may create and manage a connection secret in its
 // own namespace.
 type LocalConnectionSecretOwner interface {