From bb562fce2536d426b5191174f795874fb7ff91de Mon Sep 17 00:00:00 2001 From: hasheddan Date: Fri, 20 Mar 2020 09:48:15 -0500 Subject: [PATCH] Clean up OAM reconcilers Signed-off-by: hasheddan --- pkg/reconciler/oam/trait/reconciler.go | 5 +-- pkg/reconciler/oam/trait/reconciler_test.go | 36 +++++++++---------- pkg/reconciler/oam/workload/reconciler.go | 12 +------ .../oam/workload/reconciler_test.go | 26 +++++++------- pkg/reconciler/oam/workload/translate.go | 16 ++++----- pkg/resource/resource.go | 11 +++++- 6 files changed, 51 insertions(+), 55 deletions(-) diff --git a/pkg/reconciler/oam/trait/reconciler.go b/pkg/reconciler/oam/trait/reconciler.go index 128ee34c3..7c29fb11f 100644 --- 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 index beac7e0c2..1bf0c2951 100644 --- 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 index 6ac644994..8fcb43ab6 100644 --- 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 index 7f45dc312..7b08db8e9 100644 --- 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 index e4b1dc87c..8f7d9aff4 100644 --- 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,7 +33,7 @@ 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) } @@ -41,7 +41,7 @@ func (p *ObjectTranslator) Translate(ctx context.Context, w resource.Workload) ( // 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 index bf27484d1..7ec123305 100644 --- 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 {