Skip to content

Commit

Permalink
Merge pull request #55 from negz/nonsense
Browse files Browse the repository at this point in the history 
Fix static provisioning edge cases
  • Loading branch information
@negz
negz committed Oct 25, 2019
2 parents 0f37bea + 551efff commit 78072ef
Show file tree
Hide file tree
Showing 4 changed files with 322 additions and 36 deletions.
56 changes: 37 additions & 19 deletions pkg/resource/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,28 +147,35 @@ func (a *APIManagedConnectionPropagator) PropagateConnection(ctx context.Context
// using the status subresource; such objects should use APIManagedStatusBinder.
type APIManagedBinder struct {
client client.Client
typer runtime.ObjectTyper
}

// NewAPIManagedBinder returns a new APIManagedBinder.
func NewAPIManagedBinder(c client.Client) *APIManagedBinder {
return &APIManagedBinder{client: c}
func NewAPIManagedBinder(c client.Client, t runtime.ObjectTyper) *APIManagedBinder {
return &APIManagedBinder{client: c, typer: t}
}

// Bind the supplied resource to the supplied claim.
func (a *APIManagedBinder) Bind(ctx context.Context, cm Claim, mg Managed) error {
cm.SetBindingPhase(v1alpha1.BindingPhaseBound)
// Propagate back the final name of the external resource to the claim.
if meta.GetExternalName(mg) != "" {
meta.SetExternalName(cm, meta.GetExternalName(mg))
if err := a.client.Update(ctx, cm); err != nil {
return errors.Wrap(err, errUpdateClaim)
}
}

// This claim reference will already be set for dynamically provisioned
// managed resources, but we need to make sure it's set for statically
// provisioned resources too.
cmr := meta.ReferenceTo(cm, MustGetKind(cm, a.typer))
mg.SetClaimReference(cmr)
mg.SetBindingPhase(v1alpha1.BindingPhaseBound)
if err := a.client.Update(ctx, mg); err != nil {
return errors.Wrap(err, errUpdateManaged)
}
return nil

if meta.GetExternalName(mg) == "" {
return nil
}

// Propagate back the final name of the external resource to the claim.
meta.SetExternalName(cm, meta.GetExternalName(mg))
return errors.Wrap(a.client.Update(ctx, cm), errUpdateClaim)
}

// An APIManagedStatusBinder binds resources to claims by updating them in a
Expand All @@ -177,28 +184,39 @@ func (a *APIManagedBinder) Bind(ctx context.Context, cm Claim, mg Managed) error
// APIManagedBinder.
type APIManagedStatusBinder struct {
client client.Client
typer runtime.ObjectTyper
}

// NewAPIManagedStatusBinder returns a new APIManagedStatusBinder.
func NewAPIManagedStatusBinder(c client.Client) *APIManagedStatusBinder {
return &APIManagedStatusBinder{client: c}
func NewAPIManagedStatusBinder(c client.Client, t runtime.ObjectTyper) *APIManagedStatusBinder {
return &APIManagedStatusBinder{client: c, typer: t}
}

// Bind the supplied resource to the supplied claim.
func (a *APIManagedStatusBinder) Bind(ctx context.Context, cm Claim, mg Managed) error {
cm.SetBindingPhase(v1alpha1.BindingPhaseBound)
// Propagate back the final name of the external resource to the claim.
if meta.GetExternalName(mg) != "" {
meta.SetExternalName(cm, meta.GetExternalName(mg))
if err := a.client.Update(ctx, cm); err != nil {
return errors.Wrap(err, errUpdateClaim)
}

// This claim reference will already be set for dynamically provisioned
// managed resources, but we need to make sure it's set for statically
// provisioned resources too.
cmr := meta.ReferenceTo(cm, MustGetKind(cm, a.typer))
mg.SetClaimReference(cmr)
if err := a.client.Update(ctx, mg); err != nil {
return errors.Wrap(err, errUpdateManaged)
}

mg.SetBindingPhase(v1alpha1.BindingPhaseBound)
if err := a.client.Status().Update(ctx, mg); err != nil {
return errors.Wrap(err, errUpdateManagedStatus)
}
return nil

if meta.GetExternalName(mg) == "" {
return nil
}

// Propagate back the final name of the external resource to the claim.
meta.SetExternalName(cm, meta.GetExternalName(mg))
return errors.Wrap(a.client.Update(ctx, cm), errUpdateClaim)
}

// An APIManagedUnbinder finalizes the deletion of a managed resource by
Expand Down
193 changes: 182 additions & 11 deletions pkg/resource/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,14 +464,24 @@ func TestBind(t *testing.T) {
}

errBoom := errors.New("boom")
externalName := "very-cool-external-resource"

cases := map[string]struct {
client client.Client
typer runtime.ObjectTyper
args args
want want
}{
"UpdateManagedError": {
client: &test.MockClient{MockUpdate: test.NewMockUpdateFn(errBoom)},
client: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil, func(obj runtime.Object) error {
switch obj.(type) {
case *MockManaged:
return errBoom
default:
return errors.New("unexpected object kind")
}
})},
typer: MockSchemeWith(&MockClaim{}),
args: args{
ctx: context.Background(),
cm: &MockClaim{},
Expand All @@ -480,11 +490,47 @@ func TestBind(t *testing.T) {
want: want{
err: errors.Wrap(errBoom, errUpdateManaged),
cm: &MockClaim{MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{
MockClaimReferencer: MockClaimReferencer{meta.ReferenceTo(&MockClaim{}, MockGVK(&MockClaim{}))},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
},
},
"Successful": {
"UpdateClaimError": {
client: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil, func(obj runtime.Object) error {
switch obj.(type) {
case *MockManaged:
return nil
case *MockClaim:
return errBoom
default:
return errors.New("unexpected object kind")
}
})},
typer: MockSchemeWith(&MockClaim{}),
args: args{
ctx: context.Background(),
cm: &MockClaim{},
mg: &MockManaged{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
},
},
want: want{
err: errors.Wrap(errBoom, errUpdateClaim),
cm: &MockClaim{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
mg: &MockManaged{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
MockClaimReferencer: MockClaimReferencer{meta.ReferenceTo(&MockClaim{}, MockGVK(&MockClaim{}))},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
},
},
"SuccessfulWithoutExternalName": {
client: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
typer: MockSchemeWith(&MockClaim{}),
args: args{
ctx: context.Background(),
cm: &MockClaim{},
Expand All @@ -493,14 +539,39 @@ func TestBind(t *testing.T) {
want: want{
err: nil,
cm: &MockClaim{MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{
MockClaimReferencer: MockClaimReferencer{meta.ReferenceTo(&MockClaim{}, MockGVK(&MockClaim{}))},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
},
},
"SuccessfulWithExternalName": {
client: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil)},
typer: MockSchemeWith(&MockClaim{}),
args: args{
ctx: context.Background(),
cm: &MockClaim{},
mg: &MockManaged{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
},
},
want: want{
err: nil,
cm: &MockClaim{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
MockClaimReferencer: MockClaimReferencer{meta.ReferenceTo(&MockClaim{}, MockGVK(&MockClaim{}))},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
api := NewAPIManagedBinder(tc.client)
api := NewAPIManagedBinder(tc.client, tc.typer)
err := api.Bind(tc.args.ctx, tc.args.cm, tc.args.mg)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("api.Bind(...): -want error, +got error:\n%s", diff)
Expand Down Expand Up @@ -529,14 +600,43 @@ func TestStatusBind(t *testing.T) {
}

errBoom := errors.New("boom")
externalName := "very-cool-external-resource"

cases := map[string]struct {
client client.Client
typer runtime.ObjectTyper
args args
want want
}{
"UpdateManagedError": {
client: &test.MockClient{MockUpdate: test.NewMockUpdateFn(nil, func(obj runtime.Object) error {
switch obj.(type) {
case *MockManaged:
return errBoom
default:
return errors.New("unexpected object kind")
}
})},
typer: MockSchemeWith(&MockClaim{}),
args: args{
ctx: context.Background(),
cm: &MockClaim{},
mg: &MockManaged{},
},
want: want{
err: errors.Wrap(errBoom, errUpdateManaged),
cm: &MockClaim{MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{
MockClaimReferencer: MockClaimReferencer{meta.ReferenceTo(&MockClaim{}, MockGVK(&MockClaim{}))},
},
},
},
"UpdateManagedStatusError": {
client: &test.MockClient{MockStatusUpdate: test.NewMockStatusUpdateFn(errBoom)},
client: &test.MockClient{
MockUpdate: test.NewMockUpdateFn(nil),
MockStatusUpdate: test.NewMockStatusUpdateFn(errBoom),
},
typer: MockSchemeWith(&MockClaim{}),
args: args{
ctx: context.Background(),
cm: &MockClaim{},
Expand All @@ -545,11 +645,53 @@ func TestStatusBind(t *testing.T) {
want: want{
err: errors.Wrap(errBoom, errUpdateManagedStatus),
cm: &MockClaim{MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{
MockClaimReferencer: MockClaimReferencer{meta.ReferenceTo(&MockClaim{}, MockGVK(&MockClaim{}))},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
},
},
"Successful": {
client: &test.MockClient{MockStatusUpdate: test.NewMockStatusUpdateFn(nil)},
"UpdateClaimError": {
client: &test.MockClient{
MockUpdate: test.NewMockUpdateFn(nil, func(obj runtime.Object) error {
switch obj.(type) {
case *MockManaged:
return nil
case *MockClaim:
return errBoom
default:
return errors.New("unexpected object kind")
}
}),
MockStatusUpdate: test.NewMockStatusUpdateFn(nil),
},
typer: MockSchemeWith(&MockClaim{}),
args: args{
ctx: context.Background(),
cm: &MockClaim{},
mg: &MockManaged{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
},
},
want: want{
err: errors.Wrap(errBoom, errUpdateClaim),
cm: &MockClaim{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
mg: &MockManaged{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
MockClaimReferencer: MockClaimReferencer{meta.ReferenceTo(&MockClaim{}, MockGVK(&MockClaim{}))},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
},
},
"SuccessfulWithoutExternalName": {
client: &test.MockClient{
MockUpdate: test.NewMockUpdateFn(nil),
MockStatusUpdate: test.NewMockStatusUpdateFn(nil),
},
typer: MockSchemeWith(&MockClaim{}),
args: args{
ctx: context.Background(),
cm: &MockClaim{},
Expand All @@ -558,14 +700,43 @@ func TestStatusBind(t *testing.T) {
want: want{
err: nil,
cm: &MockClaim{MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound}},
mg: &MockManaged{
MockClaimReferencer: MockClaimReferencer{meta.ReferenceTo(&MockClaim{}, MockGVK(&MockClaim{}))},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
},
},
"SuccessfulWithExternalName": {
client: &test.MockClient{
MockUpdate: test.NewMockUpdateFn(nil),
MockStatusUpdate: test.NewMockStatusUpdateFn(nil),
},
typer: MockSchemeWith(&MockClaim{}),
args: args{
ctx: context.Background(),
cm: &MockClaim{},
mg: &MockManaged{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
},
},
want: want{
err: nil,
cm: &MockClaim{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
mg: &MockManaged{
ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{meta.ExternalNameAnnotationKey: externalName}},
MockClaimReferencer: MockClaimReferencer{meta.ReferenceTo(&MockClaim{}, MockGVK(&MockClaim{}))},
MockBindable: MockBindable{Phase: v1alpha1.BindingPhaseBound},
},
},
},
}

for name, tc := range cases {
t.Run(name, func(t *testing.T) {
api := NewAPIManagedStatusBinder(tc.client)
api := NewAPIManagedStatusBinder(tc.client, tc.typer)
err := api.Bind(tc.args.ctx, tc.args.cm, tc.args.mg)
if diff := cmp.Diff(tc.want.err, err, test.EquateErrors()); diff != "" {
t.Errorf("api.Bind(...): -want error, +got error:\n%s", diff)
Expand Down
Loading

0 comments on commit 78072ef

Please sign in to comment.