Merge pull request #145 from hasheddan/ownoam

Pass workload kind to trait reconciler

pkg/reconciler/oam/trait/reconciler.go

@@ -18,6 +18,7 @@ package trait
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"github.com/pkg/errors"
@@ -45,6 +46,7 @@ const (
 	errGetTrait               = "cannot get trait"
 	errUpdateTraitStatus      = "cannot update trait status"
 	errTraitModify            = "cannot apply trait modification"
+	errGetWorkload            = "cannot get workload referenced in trait"
 	errGetTranslation         = "cannot get translation for workload reference in trait"
 	errApplyTraitModification = "cannot apply trait modification to workload translation"
 )
@@ -54,6 +56,7 @@ const (
 	reasonTraitWait   = "WaitingForWorkloadTranslation"
 	reasonTraitModify = "PackageModified"
 
+	reasonCannotGetWorkload       = "CannotGetReferencedWorkload"
 	reasonCannotGetTranslation    = "CannotGetReferencedWorkloadTranslation"
 	reasonCannotModifyTranslation = "CannotModifyTranslation"
 	reasonCannotApplyModification = "CannotApplyModification"
@@ -96,6 +99,7 @@ func WithApplicator(a resource.Applicator) ReconcilerOption {
 type Reconciler struct {
 	client         client.Client
 	newTrait       func() resource.Trait
+	newWorkload    func() resource.Workload
 	newTranslation func() resource.Object
 	trait          Modifier
 	applicator     resource.Applicator
@@ -106,11 +110,13 @@ type Reconciler struct {
 
 // NewReconciler returns a Reconciler that reconciles OAM traits by fetching
 // their referenced workload's translation and applying modifications.
-func NewReconciler(m ctrl.Manager, trait resource.TraitKind, trans resource.ObjectKind, o ...ReconcilerOption) *Reconciler {
+func NewReconciler(m ctrl.Manager, trait resource.TraitKind, workload resource.WorkloadKind, trans resource.ObjectKind, o ...ReconcilerOption) *Reconciler {
 	nt := func() resource.Trait {
 		return resource.MustCreateObject(schema.GroupVersionKind(trait), m.GetScheme()).(resource.Trait)
 	}
-
+	nw := func() resource.Workload {
+		return resource.MustCreateObject(schema.GroupVersionKind(workload), m.GetScheme()).(resource.Workload)
+	}
 	nr := func() resource.Object {
 		return resource.MustCreateObject(schema.GroupVersionKind(trans), m.GetScheme()).(resource.Object)
 	}
@@ -118,6 +124,7 @@ func NewReconciler(m ctrl.Manager, trait resource.TraitKind, trans resource.Obje
 	r := &Reconciler{
 		client:         m.GetClient(),
 		newTrait:       nt,
+		newWorkload:    nw,
 		newTranslation: nr,
 		trait:          ModifyFn(NoopModifier),
 		applicator:     resource.NewAPIPatchingApplicator(m.GetClient()),
@@ -149,6 +156,22 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 
 	log = log.WithValues("uid", trait.GetUID(), "version", trait.GetResourceVersion())
 
+	workload := r.newWorkload()
+	err := r.client.Get(ctx, types.NamespacedName{Name: trait.GetWorkloadReference().Name, Namespace: trait.GetNamespace()}, workload)
+	if kerrors.IsNotFound(err) {
+		log.Debug("Waiting for referenced workload to exist", "kind", trait.GetObjectKind().GroupVersionKind().String())
+		r.record.Event(trait, event.Normal(reasonTraitWait, "Waiting for workload to exist"))
+		trait.SetConditions(v1alpha1.ReconcileSuccess())
+		return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, trait), errUpdateTraitStatus)
+	}
+	if err != nil {
+		fmt.Println("HEEERREE")
+		log.Debug("Cannot get referenced workload", "error", err, "requeue-after", time.Now().Add(shortWait))
+		r.record.Event(trait, event.Warning(reasonCannotGetWorkload, err))
+		trait.SetConditions(v1alpha1.ReconcileError(errors.Wrap(err, errGetWorkload)))
+		return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, trait), errUpdateTraitStatus)
+	}
+
 	translation := r.newTranslation()
 
 	// TODO(hasheddan): we make the assumption here that the workload
@@ -156,7 +179,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 	// workload itself. This would not work if a translation produced multiple
 	// objects of the same kind as they would not be permitted to have the same
 	// name.
-	err := r.client.Get(ctx, types.NamespacedName{Name: trait.GetWorkloadReference().Name, Namespace: trait.GetNamespace()}, translation)
+	err = r.client.Get(ctx, types.NamespacedName{Name: workload.GetName(), Namespace: trait.GetNamespace()}, translation)
 	if kerrors.IsNotFound(err) {
 		log.Debug("Waiting for referenced workload's translation", "kind", trait.GetObjectKind().GroupVersionKind().String())
 		r.record.Event(trait, event.Normal(reasonTraitWait, "Waiting for workload translation to exist"))
@@ -182,7 +205,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 	// object(s) already exists in the same namespace, with the same name, and
 	// with a different controller before it is created, this wll guard against
 	// modifying it.
-	if err := r.applicator.Apply(ctx, translation, resource.MustBeControllableBy(trait.GetWorkloadReference().UID)); err != nil { // nolint:staticcheck
+	if err := r.applicator.Apply(ctx, translation, resource.MustBeControllableBy(workload.GetUID())); err != nil { // nolint:staticcheck
 		log.Debug("Cannot apply workload translation", "error", err, "requeue-after", time.Now().Add(shortWait))
 		r.record.Event(trait, event.Warning(reasonCannotApplyModification, err))
 		trait.SetConditions(v1alpha1.ReconcileError(errors.Wrap(err, errApplyTraitModification)))

pkg/reconciler/oam/trait/reconciler_test.go

@@ -41,6 +41,7 @@ func TestReconciler(t *testing.T) {
 	type args struct {
 		m manager.Manager
 		t resource.TraitKind
+		w resource.WorkloadKind
 		p resource.ObjectKind
 		o []ReconcilerOption
 	}
@@ -66,9 +67,10 @@ func TestReconciler(t *testing.T) {
 			args: args{
 				m: &fake.Manager{
 					Client: &test.MockClient{MockGet: test.NewMockGetFn(errBoom)},
-					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
+					Scheme: fake.SchemeWith(&fake.Trait{}),
 				},
 				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 			},
 			want: want{err: errors.Wrap(errBoom, errGetTrait)},
@@ -81,10 +83,89 @@ func TestReconciler(t *testing.T) {
 					Scheme: fake.SchemeWith(&fake.Trait{}),
 				},
 				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 			},
 			want: want{result: reconcile.Result{}},
 		},
+		"WorkloadNotFound": {
+			reason: "Status should report successful reconcile and we should requeue after short wait if workload is not found.",
+			args: args{
+				m: &fake.Manager{
+					Client: &test.MockClient{
+						MockGet: func(_ context.Context, key client.ObjectKey, obj runtime.Object) error {
+							if t, ok := obj.(resource.Trait); ok {
+								t.SetWorkloadReference(v1alpha1.TypedReference{
+									APIVersion: workloadAPIVersion,
+									Kind:       workloadKind,
+									Name:       workloadName,
+								})
+								return nil
+							}
+							if _, ok := obj.(resource.Workload); ok {
+								return kerrors.NewNotFound(schema.GroupResource{}, "")
+							}
+							return errBoom
+						},
+						MockStatusUpdate: func(_ context.Context, obj runtime.Object, _ ...client.UpdateOption) error {
+							got := obj.(resource.Trait)
+
+							if diff := cmp.Diff(v1alpha1.ReasonReconcileSuccess, got.GetCondition(v1alpha1.TypeSynced).Reason); diff != "" {
+								return errors.Errorf("MockStatusUpdate: -want, +got: %s", diff)
+							}
+
+							return nil
+						},
+					},
+					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Workload{}),
+				},
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
+			},
+			want: want{result: reconcile.Result{RequeueAfter: shortWait}},
+		},
+		"GetWorkloadError": {
+			reason: "Status should report reconcile error and we should requeue after short wait if we encounter an error getting the workload.",
+			args: args{
+				m: &fake.Manager{
+					Client: &test.MockClient{
+						MockGet: func(_ context.Context, key client.ObjectKey, obj runtime.Object) error {
+							if t, ok := obj.(resource.Trait); ok {
+								t.SetWorkloadReference(v1alpha1.TypedReference{
+									APIVersion: workloadAPIVersion,
+									Kind:       workloadKind,
+									Name:       workloadName,
+								})
+								return nil
+							}
+							if _, ok := obj.(resource.Workload); ok {
+								return errBoom
+							}
+							return nil
+						},
+						MockStatusUpdate: func(_ context.Context, obj runtime.Object, _ ...client.UpdateOption) error {
+							got := obj.(resource.Trait)
+
+							if diff := cmp.Diff(v1alpha1.ReasonReconcileError, got.GetCondition(v1alpha1.TypeSynced).Reason); diff != "" {
+								return errors.Errorf("MockStatusUpdate: -want, +got: %s", diff)
+							}
+
+							if diff := cmp.Diff(errors.Wrap(errBoom, errGetWorkload).Error(), got.GetCondition(v1alpha1.TypeSynced).Message); diff != "" {
+								return errors.Errorf("MockStatusUpdate: -want, +got: %s", diff)
+							}
+
+							return nil
+						},
+					},
+					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Workload{}),
+				},
+				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
+				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
+			},
+			want: want{result: reconcile.Result{RequeueAfter: shortWait}},
+		},
 		"TranslationNotFound": {
 			reason: "Status should report successful reconcile and we should requeue after short wait if translation is not found.",
 			args: args{
@@ -111,9 +192,10 @@ func TestReconciler(t *testing.T) {
 							return nil
 						},
 					},
-					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
+					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Workload{}, &fake.Object{}),
 				},
 				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 			},
 			want: want{result: reconcile.Result{RequeueAfter: shortWait}},
@@ -132,6 +214,10 @@ func TestReconciler(t *testing.T) {
 								})
 								return nil
 							}
+							if w, ok := obj.(resource.Workload); ok {
+								w.SetName(workloadName)
+								return nil
+							}
 							return errBoom
 						},
 						MockStatusUpdate: func(_ context.Context, obj runtime.Object, _ ...client.UpdateOption) error {
@@ -148,9 +234,10 @@ func TestReconciler(t *testing.T) {
 							return nil
 						},
 					},
-					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
+					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Workload{}, &fake.Object{}),
 				},
 				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 			},
 			want: want{result: reconcile.Result{RequeueAfter: shortWait}},
@@ -188,9 +275,10 @@ func TestReconciler(t *testing.T) {
 							return nil
 						},
 					},
-					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
+					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Workload{}, &fake.Object{}),
 				},
 				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 				o: []ReconcilerOption{WithModifier(ModifyFn(func(_ context.Context, _ runtime.Object, _ resource.Trait) error {
 					return errBoom
@@ -231,9 +319,10 @@ func TestReconciler(t *testing.T) {
 							return nil
 						},
 					},
-					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
+					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Workload{}, &fake.Object{}),
 				},
 				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 				o: []ReconcilerOption{WithApplicator(resource.ApplyFn(func(_ context.Context, _ runtime.Object, _ ...resource.ApplyOption) error {
 					return errBoom
@@ -274,9 +363,10 @@ func TestReconciler(t *testing.T) {
 							return nil
 						},
 					},
-					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
+					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Workload{}, &fake.Object{}),
 				},
 				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 				o: []ReconcilerOption{WithApplicator(resource.ApplyFn(func(_ context.Context, _ runtime.Object, _ ...resource.ApplyOption) error {
 					return nil
@@ -305,9 +395,10 @@ func TestReconciler(t *testing.T) {
 						},
 						MockStatusUpdate: test.NewMockStatusUpdateFn(errBoom),
 					},
-					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Object{}),
+					Scheme: fake.SchemeWith(&fake.Trait{}, &fake.Workload{}, &fake.Object{}),
 				},
 				t: resource.TraitKind(fake.GVK(&fake.Trait{})),
+				w: resource.WorkloadKind(fake.GVK(&fake.Workload{})),
 				p: resource.ObjectKind(fake.GVK(&fake.Object{})),
 				o: []ReconcilerOption{WithApplicator(resource.ApplyFn(func(_ context.Context, _ runtime.Object, _ ...resource.ApplyOption) error {
 					return nil
@@ -322,7 +413,7 @@ func TestReconciler(t *testing.T) {
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			r := NewReconciler(tc.args.m, tc.args.t, tc.args.p, tc.args.o...)
+			r := NewReconciler(tc.args.m, tc.args.t, tc.args.w, tc.args.p, tc.args.o...)
 			got, err := r.Reconcile(reconcile.Request{})
 
 			if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {