Implement portable classes

apis/core/v1alpha1/resource.go

@@ -46,8 +46,9 @@ type ResourceClaimSpec struct {
 	// TODO(negz): Make the below references immutable once set? Doing so means
 	// we don't have to track what provisioner was used to create a resource.
 
-	ClassReference    *corev1.ObjectReference `json:"classRef,omitempty"`
-	ResourceReference *corev1.ObjectReference `json:"resourceRef,omitempty"`
+	// PortableClassReference must reference a portable class by name
+	PortableClassReference *corev1.LocalObjectReference `json:"classRef,omitempty"`
+	ResourceReference      *corev1.ObjectReference      `json:"resourceRef,omitempty"`
 }
 
 // ResourceClaimStatus represents the status of a resource claim. Claims should
@@ -63,9 +64,11 @@ type ResourceClaimStatus struct {
 type ResourceSpec struct {
 	WriteConnectionSecretToReference corev1.LocalObjectReference `json:"writeConnectionSecretToRef,omitempty"`
 
-	ClaimReference    *corev1.ObjectReference `json:"claimRef,omitempty"`
-	ClassReference    *corev1.ObjectReference `json:"classRef,omitempty"`
-	ProviderReference *corev1.ObjectReference `json:"providerRef"`
+	ClaimReference *corev1.ObjectReference `json:"claimRef,omitempty"`
+
+	// NonPortableClassReference must reference a non-portable class by GVK
+	NonPortableClassReference *corev1.ObjectReference `json:"classRef,omitempty"`
+	ProviderReference         *corev1.ObjectReference `json:"providerRef"`
 
 	ReclaimPolicy ReclaimPolicy `json:"reclaimPolicy,omitempty"`
 }
@@ -90,15 +93,17 @@ type ResourceStatus struct {
 // PortableClass contains standard fields that all portable classes should include. Class
 // should typically be embedded in a specific portable class.
 type PortableClass struct {
-	ClassReference *corev1.ObjectReference `json:"classRef,omitempty"`
+
+	// NonPortableClassReference must reference a non-portable class by GVK
+	NonPortableClassReference *corev1.ObjectReference `json:"classRef,omitempty"`
 }
 
-// SetClassReference of this Class
-func (c *PortableClass) SetClassReference(r *corev1.ObjectReference) {
-	c.ClassReference = r
+// SetNonPortableClassReference of this Class
+func (c *PortableClass) SetNonPortableClassReference(r *corev1.ObjectReference) {
+	c.NonPortableClassReference = r
 }
 
-// GetClassReference of this Class
-func (c *PortableClass) GetClassReference() *corev1.ObjectReference {
-	return c.ClassReference
+// GetNonPortableClassReference of this Class
+func (c *PortableClass) GetNonPortableClassReference() *corev1.ObjectReference {
+	return c.NonPortableClassReference
 }

pkg/resource/api.go

@@ -63,7 +63,7 @@ func (a *APIManagedCreator) Create(ctx context.Context, cm Claim, cs Class, mg M
 	mgr := meta.ReferenceTo(mg, MustGetKind(mg, a.typer))
 
 	mg.SetClaimReference(cmr)
-	mg.SetClassReference(csr)
+	mg.SetNonPortableClassReference(csr)
 	if err := a.client.Create(ctx, mg); err != nil {
 		return errors.Wrap(err, errCreateManaged)
 	}

pkg/resource/api_test.go

@@ -108,7 +108,7 @@ func TestCreate(t *testing.T) {
 							APIVersion: MockGVK(&MockClaim{}).GroupVersion().String(),
 							Kind:       MockGVK(&MockClaim{}).Kind,
 						})
-						want.SetClassReference(&corev1.ObjectReference{
+						want.SetNonPortableClassReference(&corev1.ObjectReference{
 							Name:       csname,
 							APIVersion: MockGVK(&MockClass{}).GroupVersion().String(),
 							Kind:       MockGVK(&MockClass{}).Kind,

pkg/resource/claim_reconciler.go

@@ -265,7 +265,7 @@ func NewClaimReconciler(m manager.Manager, of ClaimKind, using ClassKinds, with
 
 	// Panic early if we've been asked to reconcile a claim or resource kind
 	// that has not been registered with our controller manager's scheme.
-	_, _, _ = nc(), ns(), nr()
+	_, _, _, _ = nc(), ns(), np(), nr()
 
 	r := &ClaimReconciler{
 		client:           m.GetClient(),
@@ -350,15 +350,14 @@ func (r *ClaimReconciler) Reconcile(req reconcile.Request) (reconcile.Result, er
 			// implicitly due to the status update. Otherwise we want to retry
 			// after a brief wait, in case this was a transient error or the
 			// portable class is (re)created.
-			claim.SetClassReference(portable.GetClassReference())
 			claim.SetConditions(v1alpha1.Creating(), v1alpha1.ReconcileError(err))
 			return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, claim), errUpdateClaimStatus)
 		}
 
 		class := r.newClass()
 		// Class reference should always be set by the time we get this far; we
 		// set it on last reconciliation.
-		if err := r.client.Get(ctx, meta.NamespacedNameOf(claim.GetClassReference()), class); err != nil {
+		if err := r.client.Get(ctx, meta.NamespacedNameOf(portable.GetNonPortableClassReference()), class); err != nil {
 			// If we didn't hit this error last time we'll be requeued
 			// implicitly due to the status update. Otherwise we want to retry
 			// after a brief wait, in case this was a transient error or the

pkg/resource/claim_reconciler_test.go

@@ -319,12 +319,11 @@ func TestClaimReconciler(t *testing.T) {
 							case *MockClaim:
 								cm := &MockClaim{}
 								cm.SetPortableClassReference(&corev1.LocalObjectReference{})
-								cm.SetClassReference(&corev1.ObjectReference{})
 								*o = *cm
 								return nil
 							case *MockPortableClass:
 								pc := &MockPortableClass{}
-								pc.SetClassReference(&corev1.ObjectReference{})
+								pc.SetNonPortableClassReference(&corev1.ObjectReference{})
 								*o = *pc
 								return nil
 							case *MockClass:
@@ -336,7 +335,6 @@ func TestClaimReconciler(t *testing.T) {
 						MockStatusUpdate: test.NewMockStatusUpdateFn(nil, func(got runtime.Object) error {
 							want := &MockClaim{}
 							want.SetPortableClassReference(&corev1.LocalObjectReference{})
-							want.SetClassReference(&corev1.ObjectReference{})
 							want.SetConditions(v1alpha1.Creating(), v1alpha1.ReconcileError(errBoom))
 							if diff := cmp.Diff(want, got, test.EquateConditions()); diff != "" {
 								t.Errorf("-want, +got:\n%s", diff)
@@ -361,12 +359,11 @@ func TestClaimReconciler(t *testing.T) {
 							case *MockClaim:
 								cm := &MockClaim{}
 								cm.SetPortableClassReference(&corev1.LocalObjectReference{})
-								cm.SetClassReference(&corev1.ObjectReference{})
 								*o = *cm
 								return nil
 							case *MockPortableClass:
 								pc := &MockPortableClass{}
-								pc.SetClassReference(&corev1.ObjectReference{})
+								pc.SetNonPortableClassReference(&corev1.ObjectReference{})
 								*o = *pc
 								return nil
 							case *MockClass:
@@ -378,7 +375,6 @@ func TestClaimReconciler(t *testing.T) {
 						MockStatusUpdate: test.NewMockStatusUpdateFn(nil, func(got runtime.Object) error {
 							want := &MockClaim{}
 							want.SetPortableClassReference(&corev1.LocalObjectReference{})
-							want.SetClassReference(&corev1.ObjectReference{})
 							want.SetConditions(v1alpha1.Creating(), v1alpha1.ReconcileError(errBoom))
 							if diff := cmp.Diff(want, got, test.EquateConditions()); diff != "" {
 								t.Errorf("-want, +got:\n%s", diff)
@@ -406,12 +402,11 @@ func TestClaimReconciler(t *testing.T) {
 							case *MockClaim:
 								cm := &MockClaim{}
 								cm.SetPortableClassReference(&corev1.LocalObjectReference{})
-								cm.SetClassReference(&corev1.ObjectReference{})
 								*o = *cm
 								return nil
 							case *MockPortableClass:
 								pc := &MockPortableClass{}
-								pc.SetClassReference(&corev1.ObjectReference{})
+								pc.SetNonPortableClassReference(&corev1.ObjectReference{})
 								*o = *pc
 								return nil
 							case *MockClass:
@@ -423,7 +418,6 @@ func TestClaimReconciler(t *testing.T) {
 						MockStatusUpdate: test.NewMockStatusUpdateFn(nil, func(got runtime.Object) error {
 							want := &MockClaim{}
 							want.SetPortableClassReference(&corev1.LocalObjectReference{})
-							want.SetClassReference(&corev1.ObjectReference{})
 							want.SetConditions(v1alpha1.Creating(), v1alpha1.ReconcileError(errBoom))
 							if diff := cmp.Diff(want, got, test.EquateConditions()); diff != "" {
 								t.Errorf("-want, +got:\n%s", diff)

pkg/resource/defaultclass.go

@@ -21,7 +21,7 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -38,12 +38,17 @@ const (
 	defaultClassReconcileTimeout = 1 * time.Minute
 )
 
+// Label values for listing default portable classes
+const (
+	LabelKeyDefaultClass = "default"
+	LabelValueTrue       = "true"
+)
+
 // Error strings
 const (
-	errFailedList                    = "unable to list policies for claim kind"
-	errFailedPortableClassConversion = "unable to convert located portable class to correct kind"
-	errNoPortableClass               = "unable to locate a portable class that specifies a default class for claim kind"
-	errMultiplePortableClasses       = "multiple portable classes that specify a default class defined for claim kind"
+	errFailedList              = "unable to list portable classes for claim kind"
+	errNoPortableClass         = "unable to locate a portable class that specifies a default class for claim kind"
+	errMultiplePortableClasses = "multiple portable classes that specify a default class defined for claim kind"
 )
 
 // A PortableClassKind contains the type metadata for a kind of portable class.
@@ -54,16 +59,15 @@ type PortableClassKind struct {
 
 // DefaultClassReconciler reconciles resource claims to the
 // default resource class for their given kind according to existing
-// policies. Predicates ensure that only claims with no resource class
+// portable classes. Predicates ensure that only claims with no resource class
 // reference are reconciled.
 type DefaultClassReconciler struct {
-	client                client.Client
-	converter             runtime.ObjectConvertor
-	labels                map[string]string
-	portableClassKind     schema.GroupVersionKind
-	portableClassListKind schema.GroupVersionKind
-	newClaim              func() Claim
-	newPortableClass      func() PortableClass
+	client               client.Client
+	converter            runtime.ObjectConvertor
+	labels               map[string]string
+	newClaim             func() Claim
+	newPortableClass     func() PortableClass
+	newPortableClassList func() PortableClassList
 }
 
 // A DefaultClassReconcilerOption configures a DefaultClassReconciler.
@@ -77,26 +81,33 @@ func WithObjectConverter(oc runtime.ObjectConvertor) DefaultClassReconcilerOptio
 	}
 }
 
+// WithLabels specifies how the DefaultClassReconciler should search
+// for a default class
+func WithLabels(labels map[string]string) DefaultClassReconcilerOption {
+	return func(r *DefaultClassReconciler) {
+		r.labels = labels
+	}
+}
+
 // NewDefaultClassReconciler creates a new DefaultReconciler for the claim kind.
 func NewDefaultClassReconciler(m manager.Manager, of ClaimKind, by PortableClassKind, o ...DefaultClassReconcilerOption) *DefaultClassReconciler {
 	nc := func() Claim { return MustCreateObject(schema.GroupVersionKind(of), m.GetScheme()).(Claim) }
 	np := func() PortableClass { return MustCreateObject(by.Singular, m.GetScheme()).(PortableClass) }
 	npl := func() PortableClassList { return MustCreateObject(by.Plural, m.GetScheme()).(PortableClassList) }
 
-	// Panic early if we've been asked to reconcile a claim, policy, or policy list that has
+	// Panic early if we've been asked to reconcile a claim, portable class, or portable class list that has
 	// not been registered with our controller manager's scheme.
 	_, _, _ = nc(), np(), npl()
 
-	labels := map[string]string{"default": "true"}
+	labels := map[string]string{LabelKeyDefaultClass: LabelValueTrue}
 
 	r := &DefaultClassReconciler{
-		client:                m.GetClient(),
-		converter:             m.GetScheme(),
-		labels:                labels,
-		portableClassKind:     by.Singular,
-		portableClassListKind: by.Plural,
-		newClaim:              nc,
-		newPortableClass:      np,
+		client:               m.GetClient(),
+		converter:            m.GetScheme(),
+		labels:               labels,
+		newClaim:             nc,
+		newPortableClass:     np,
+		newPortableClassList: npl,
 	}
 
 	for _, ro := range o {
@@ -120,9 +131,8 @@ func (r *DefaultClassReconciler) Reconcile(req reconcile.Request) (reconcile.Res
 		return reconcile.Result{}, errors.Wrap(IgnoreNotFound(err), errGetClaim)
 	}
 
-	// Get policies for claim kind in claim's namespace
-	portables := &unstructured.UnstructuredList{}
-	portables.SetGroupVersionKind(r.portableClassListKind)
+	// Get portable classes for claim kind in claim's namespace
+	portables := r.newPortableClassList()
 	if err := r.client.List(ctx, portables, client.InNamespace(req.Namespace), client.MatchingLabels(r.labels)); err != nil {
 		// If this is the first time we encounter listing error we'll be
 		// requeued implicitly due to the status update. If not, we don't
@@ -131,38 +141,28 @@ func (r *DefaultClassReconciler) Reconcile(req reconcile.Request) (reconcile.Res
 		return reconcile.Result{}, errors.Wrap(IgnoreNotFound(r.client.Status().Update(ctx, claim)), errUpdateClaimStatus)
 	}
 
+	items := portables.GetPortableClassItems()
 	// Check to see if no defaults defined for claim kind.
-	if len(portables.Items) == 0 {
-		// If this is the first time we encounter no policies we'll be
+	if len(items) == 0 {
+		// If this is the first time we encounter no default portable classes we'll be
 		// requeued implicitly due to the status update. If not, we will requeue
-		// after a time to see if a policy has been created.
+		// after a time to see if a default portable class has been created.
 		claim.SetConditions(corev1alpha1.ReconcileError(errors.New(errNoPortableClass)))
 		return reconcile.Result{RequeueAfter: defaultClassWait}, errors.Wrap(IgnoreNotFound(r.client.Status().Update(ctx, claim)), errUpdateClaimStatus)
 	}
 
-	// Check to see if multiple policies defined for claim kind.
-	if len(portables.Items) > 1 {
-		// If this is the first time we encounter multiple policies we'll be
+	// Check to see if multiple defaults defined for claim kind.
+	if len(items) > 1 {
+		// If this is the first time we encounter multiple default portable classes we'll be
 		// requeued implicitly due to the status update. If not, we will requeue
-		// after a time to see if only one policy class exists.
+		// after a time to see if only one default portable class exists.
 		claim.SetConditions(corev1alpha1.ReconcileError(errors.New(errMultiplePortableClasses)))
 		return reconcile.Result{RequeueAfter: defaultClassWait}, errors.Wrap(IgnoreNotFound(r.client.Status().Update(ctx, claim)), errUpdateClaimStatus)
 	}
 
-	// Make sure single item is of correct policy kind.
-	portable := r.newPortableClass()
-	p := portables.Items[0]
-	p.SetGroupVersionKind(r.portableClassKind)
-	if err := r.converter.Convert(&p, portable, ctx); err != nil {
-		// If this is the first time we encounter conversion error we'll be
-		// requeued implicitly due to the status update. If not, we don't
-		// care to requeue because conversion will likely not change.
-		claim.SetConditions(corev1alpha1.ReconcileError(errors.New(errFailedPortableClassConversion)))
-		return reconcile.Result{}, errors.Wrap(IgnoreNotFound(r.client.Status().Update(ctx, claim)), errUpdateClaimStatus)
-	}
-
-	// Set class reference on claim to default resource class.
-	claim.SetClassReference(portable.GetClassReference())
+	// Set portable class reference on claim to default portable class.
+	portable := items[0]
+	claim.SetPortableClassReference(&corev1.LocalObjectReference{Name: portable.GetName()})
 
 	// Do not requeue, claim controller will see update and claim
 	// with class reference set will pass predicates.