Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement portable classes #13

Merged
merged 4 commits into from
Sep 12, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions apis/core/v1alpha1/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
negz marked this conversation as resolved.
Show resolved Hide resolved
PortableClassReference *corev1.LocalObjectReference `json:"classRef,omitempty"`
ResourceReference *corev1.ObjectReference `json:"resourceRef,omitempty"`
}

// ResourceClaimStatus represents the status of a resource claim. Claims should
Expand All @@ -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
negz marked this conversation as resolved.
Show resolved Hide resolved
NonPortableClassReference *corev1.ObjectReference `json:"classRef,omitempty"`
ProviderReference *corev1.ObjectReference `json:"providerRef"`

ReclaimPolicy ReclaimPolicy `json:"reclaimPolicy,omitempty"`
}
Expand All @@ -87,18 +90,20 @@ type ResourceStatus struct {
BindingStatus `json:",inline"`
}

// Policy contains standard fields that all policies should include. Policy
// should typically be embedded in a specific resource claim policy.
type Policy struct {
DefaultClassReference *corev1.ObjectReference `json:"defaultClassRef,omitempty"`
// PortableClass contains standard fields that all portable classes should include. Class
negz marked this conversation as resolved.
Show resolved Hide resolved
// should typically be embedded in a specific portable class.
type PortableClass struct {

// NonPortableClassReference must reference a non-portable class by GVK
NonPortableClassReference *corev1.ObjectReference `json:"classRef,omitempty"`
}

// SetDefaultClassReference of this Policy
func (p *Policy) SetDefaultClassReference(r *corev1.ObjectReference) {
p.DefaultClassReference = r
// SetNonPortableClassReference of this Class
func (c *PortableClass) SetNonPortableClassReference(r *corev1.ObjectReference) {
c.NonPortableClassReference = r
}

// GetDefaultClassReference of this Policy
func (p *Policy) GetDefaultClassReference() *corev1.ObjectReference {
return p.DefaultClassReference
// GetNonPortableClassReference of this Class
func (c *PortableClass) GetNonPortableClassReference() *corev1.ObjectReference {
return c.NonPortableClassReference
}
22 changes: 11 additions & 11 deletions apis/core/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/resource/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/resource/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
59 changes: 41 additions & 18 deletions pkg/resource/claim_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -56,8 +57,11 @@ var log = logging.Logger.WithName("controller")
// A ClaimKind contains the type metadata for a kind of resource claim.
type ClaimKind schema.GroupVersionKind

// A ClassKind contains the type metadata for a kind of resource class.
type ClassKind schema.GroupVersionKind
// A ClassKinds contains the type metadata for a kind of resource class.
type ClassKinds struct {
Portable schema.GroupVersionKind
NonPortable schema.GroupVersionKind
negz marked this conversation as resolved.
Show resolved Hide resolved
}

// A ManagedKind contains the type metadata for a kind of managed resource.
type ManagedKind schema.GroupVersionKind
Expand Down Expand Up @@ -156,10 +160,11 @@ func (fn ClaimFinalizerFn) Finalize(ctx context.Context, cm Claim) error {
// type of resource class provisioner. Each controller must watch its subset of
// resource claims and any managed resources they control.
type ClaimReconciler struct {
client client.Client
newClaim func() Claim
newClass func() Class
newManaged func() Managed
client client.Client
newClaim func() Claim
newClass func() Class
negz marked this conversation as resolved.
Show resolved Hide resolved
newPortableClass func() PortableClass
newManaged func() Managed

// The below structs embed the set of interfaces used to implement the
// resource claim reconciler. We do this primarily for readability, so that
Expand Down Expand Up @@ -252,22 +257,24 @@ func WithClaimFinalizer(f ClaimFinalizer) ClaimReconcilerOption {
// with the supplied manager's runtime.Scheme. The returned ClaimReconciler will
// apply only the ObjectMetaConfigurator by default; most callers should supply
// one or more ManagedConfigurators to configure their managed resources.
func NewClaimReconciler(m manager.Manager, of ClaimKind, using ClassKind, with ManagedKind, o ...ClaimReconcilerOption) *ClaimReconciler {
func NewClaimReconciler(m manager.Manager, of ClaimKind, using ClassKinds, with ManagedKind, o ...ClaimReconcilerOption) *ClaimReconciler {
nc := func() Claim { return MustCreateObject(schema.GroupVersionKind(of), m.GetScheme()).(Claim) }
ns := func() Class { return MustCreateObject(schema.GroupVersionKind(using), m.GetScheme()).(Class) }
ns := func() Class { return MustCreateObject(using.NonPortable, m.GetScheme()).(Class) }
np := func() PortableClass { return MustCreateObject(using.Portable, m.GetScheme()).(PortableClass) }
nr := func() Managed { return MustCreateObject(schema.GroupVersionKind(with), m.GetScheme()).(Managed) }

// 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(),
newClaim: nc,
newClass: ns,
newManaged: nr,
managed: defaultCRManaged(m),
claim: defaultCRClaim(m),
client: m.GetClient(),
newClaim: nc,
newClass: ns,
newPortableClass: np,
newManaged: nr,
managed: defaultCRManaged(m),
claim: defaultCRClaim(m),
}

for _, ro := range o {
Expand Down Expand Up @@ -331,10 +338,26 @@ func (r *ClaimReconciler) Reconcile(req reconcile.Request) (reconcile.Result, er
}

if !meta.WasCreated(managed) {
portable := r.newPortableClass()
// Portable class reference should always be set by the time we get this far;
// Our watch predicates require it.
p := types.NamespacedName{
Namespace: claim.GetNamespace(),
Name: claim.GetPortableClassReference().Name,
}
if err := r.client.Get(ctx, p, portable); 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
// portable class is (re)created.
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; our
// watch predicates require it.
if err := r.client.Get(ctx, meta.NamespacedNameOf(claim.GetClassReference()), class); err != nil {
// 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(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
Expand Down
Loading