crossplane
/
crossplane-runtime
mirror of https://github.com/crossplane/crossplane-runtime.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #228 from muvaf/creation-with-rollback 

managed reconciler: more resilient update call for saving identifiers retrieved from provider
This commit is contained in:
muvaffak onuş 2020-11-17 02:23:34 +03:00 committed by GitHub
parent 31eef9b01e 91317e3e5c
commit 1b691efff4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 187 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

65
pkg/reconciler/managed/reconciler.go
View File

@ -21,6 +21,9 @@ import (
"strings"
"time"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/util/retry"
"github.com/pkg/errors"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
@ -45,25 +48,27 @@ const (
// Error strings.
const (
errGetManaged = "cannot get managed resource"
errReconcileConnect = "connect failed"
errReconcileObserve = "observe failed"
errReconcileCreate = "create failed"
errReconcileUpdate = "update failed"
errReconcileDelete = "delete failed"
errGetManaged = "cannot get managed resource"
errUpdateManagedAfterCreate = "cannot update managed resource. this may have resulted in a leaked external resource"
errReconcileConnect = "connect failed"
errReconcileObserve = "observe failed"
errReconcileCreate = "create failed"
errReconcileUpdate = "update failed"
errReconcileDelete = "delete failed"
)
// Event reasons.
const (
reasonCannotConnect event.Reason = "CannotConnectToProvider"
reasonCannotInitialize event.Reason = "CannotInitializeManagedResource"
reasonCannotResolveRefs event.Reason = "CannotResolveResourceReferences"
reasonCannotObserve event.Reason = "CannotObserveExternalResource"
reasonCannotCreate event.Reason = "CannotCreateExternalResource"
reasonCannotDelete event.Reason = "CannotDeleteExternalResource"
reasonCannotPublish event.Reason = "CannotPublishConnectionDetails"
reasonCannotUnpublish event.Reason = "CannotUnpublishConnectionDetails"
reasonCannotUpdate event.Reason = "CannotUpdateExternalResource"
reasonCannotConnect event.Reason = "CannotConnectToProvider"
reasonCannotInitialize event.Reason = "CannotInitializeManagedResource"
reasonCannotResolveRefs event.Reason = "CannotResolveResourceReferences"
reasonCannotObserve event.Reason = "CannotObserveExternalResource"
reasonCannotCreate event.Reason = "CannotCreateExternalResource"
reasonCannotDelete event.Reason = "CannotDeleteExternalResource"
reasonCannotPublish event.Reason = "CannotPublishConnectionDetails"
reasonCannotUnpublish event.Reason = "CannotUnpublishConnectionDetails"
reasonCannotUpdate event.Reason = "CannotUpdateExternalResource"
reasonCannotUpdateManaged event.Reason = "CannotUpdateManagedResource"
reasonDeleted event.Reason = "DeletedExternalResource"
reasonCreated event.Reason = "CreatedExternalResource"
@ -306,6 +311,12 @@ type ExternalObservation struct {
// An ExternalCreation is the result of the creation of an external resource.
type ExternalCreation struct {
// ExternalNameAssigned is true if the Create operation resulted in a change
// in the external name annotation. If that's the case, we need to issue a
// spec update and make sure it goes through so that we don't lose the identifier
// of the resource we just created.
ExternalNameAssigned bool
// ConnectionDetails required to connect to this resource. These details
// are a set that is collated throughout the managed resource's lifecycle -
// i.e. returning new connection details will have no affect on old details
@ -676,6 +687,28 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
return reconcile.Result{RequeueAfter: r.shortWait}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}
if creation.ExternalNameAssigned {
en := meta.GetExternalName(managed)
// We will retry in all cases where the error comes from the api-server.
// At one point, context deadline will be exceeded and we'll get out
// of the loop. In that case, we warn the user that the external resource
// might be leaked.
err := retry.OnError(retry.DefaultRetry, resource.IsAPIError, func() error {
nn := types.NamespacedName{Name: managed.GetName()}
if err := r.client.Get(ctx, nn, managed); err != nil {
return err
}
meta.SetExternalName(managed, en)
return r.client.Update(ctx, managed)
})
if err != nil {
log.Debug("Cannot update managed resource", "error", err, "requeue-after", time.Now().Add(r.shortWait))
record.Event(managed, event.Warning(reasonCannotUpdateManaged, errors.Wrap(err, errUpdateManagedAfterCreate)))
managed.SetConditions(v1alpha1.ReconcileError(errors.Wrap(err, errUpdateManagedAfterCreate)))
return reconcile.Result{RequeueAfter: r.shortWait}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}
}
if err := r.managed.PublishConnection(ctx, managed, creation.ConnectionDetails); err != nil {
// If this is the first time we encounter this issue we'll be
// requeued implicitly when we update our status with the new error
@ -706,7 +739,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
// and persist its status.
if err := r.client.Update(ctx, managed); err != nil {
log.Debug(errUpdateManaged, "error", err, "requeue-after", time.Now().Add(r.shortWait))
record.Event(managed, event.Warning(reasonCannotUpdate, err))
record.Event(managed, event.Warning(reasonCannotUpdateManaged, err))
managed.SetConditions(v1alpha1.ReconcileError(errors.Wrap(err, errUpdateManaged)))
return reconcile.Result{RequeueAfter: r.shortWait}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

134
pkg/reconciler/managed/reconciler_test.go
View File

@ -20,11 +20,10 @@ import (
"context"
"testing"
"sigs.k8s.io/controller-runtime/pkg/client"
kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"sigs.k8s.io/controller-runtime/pkg/client"
"github.com/google/go-cmp/cmp"
"github.com/pkg/errors"
@ -33,6 +32,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/reconcile"
"github.com/crossplane/crossplane-runtime/apis/core/v1alpha1"
"github.com/crossplane/crossplane-runtime/pkg/meta"
"github.com/crossplane/crossplane-runtime/pkg/resource"
"github.com/crossplane/crossplane-runtime/pkg/resource/fake"
"github.com/crossplane/crossplane-runtime/pkg/test"
@ -577,6 +577,136 @@ func TestReconciler(t *testing.T) {
},
want: want{result: reconcile.Result{RequeueAfter: defaultManagedShortWait}},
},
"CreateWithExternalNameAssignmentSuccessful": {
reason: "Successful managed resource creation with external name assignment should trigger an update.",
args: args{
m: &fake.Manager{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil),
MockUpdate: test.NewMockUpdateFn(nil),
MockStatusUpdate: test.MockStatusUpdateFn(func(_ context.Context, obj runtime.Object, _ ...client.UpdateOption) error {
want := &fake.Managed{}
meta.SetExternalName(want, "test")
want.SetConditions(v1alpha1.ReconcileSuccess())
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
reason := "Successful managed resource creation should be reported as a conditioned status."
t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff)
}
return nil
}),
},
Scheme: fake.SchemeWith(&fake.Managed{}),
},
mg: resource.ManagedKind(fake.GVK(&fake.Managed{})),
o: []ReconcilerOption{
WithInitializers(),
WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })),
WithExternalConnecter(ExternalConnectorFn(func(_ context.Context, mg resource.Managed) (ExternalClient, error) {
c := &ExternalClientFns{
CreateFn: func(_ context.Context, mg resource.Managed) (ExternalCreation, error) {
meta.SetExternalName(mg, "test")
return ExternalCreation{ExternalNameAssigned: true}, nil
},
ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) {
return ExternalObservation{}, nil
},
}
return c, nil
})),
WithConnectionPublishers(),
WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}),
},
},
want: want{result: reconcile.Result{RequeueAfter: defaultManagedShortWait}},
},
"CreateWithExternalNameAssignmentGetError": {
reason: "If the Get call during the update after Create does not go through, we need to inform the user and requeue shortly.",
args: args{
m: &fake.Manager{
Client: &test.MockClient{
MockGet: func(_ context.Context, _ client.ObjectKey, obj runtime.Object) error {
if meta.GetExternalName(obj.(metav1.Object)) == "test" {
return errBoom
}
return nil
},
MockStatusUpdate: test.MockStatusUpdateFn(func(_ context.Context, obj runtime.Object, _ ...client.UpdateOption) error {
want := &fake.Managed{}
meta.SetExternalName(want, "test")
want.SetConditions(v1alpha1.ReconcileError(errors.Wrap(errBoom, errUpdateManagedAfterCreate)))
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
reason := "Successful managed resource creation should be reported as a conditioned status."
t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff)
}
return nil
}),
},
Scheme: fake.SchemeWith(&fake.Managed{}),
},
mg: resource.ManagedKind(fake.GVK(&fake.Managed{})),
o: []ReconcilerOption{
WithInitializers(),
WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })),
WithExternalConnecter(ExternalConnectorFn(func(_ context.Context, mg resource.Managed) (ExternalClient, error) {
c := &ExternalClientFns{
CreateFn: func(_ context.Context, mg resource.Managed) (ExternalCreation, error) {
meta.SetExternalName(mg, "test")
return ExternalCreation{ExternalNameAssigned: true}, nil
},
ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) {
return ExternalObservation{}, nil
},
}
return c, nil
})),
WithConnectionPublishers(),
WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}),
},
},
want: want{result: reconcile.Result{RequeueAfter: defaultManagedShortWait}},
},
"CreateWithExternalNameAssignmentUpdateError": {
reason: "If the update after Create does not go through, we need to inform the user and requeue shortly.",
args: args{
m: &fake.Manager{
Client: &test.MockClient{
MockGet: test.NewMockGetFn(nil),
MockUpdate: test.NewMockUpdateFn(errBoom),
MockStatusUpdate: test.MockStatusUpdateFn(func(_ context.Context, obj runtime.Object, _ ...client.UpdateOption) error {
want := &fake.Managed{}
meta.SetExternalName(want, "test")
want.SetConditions(v1alpha1.ReconcileError(errors.Wrap(errBoom, errUpdateManagedAfterCreate)))
if diff := cmp.Diff(want, obj, test.EquateConditions()); diff != "" {
reason := "Successful managed resource creation should be reported as a conditioned status."
t.Errorf("\nReason: %s\n-want, +got:\n%s", reason, diff)
}
return nil
}),
},
Scheme: fake.SchemeWith(&fake.Managed{}),
},
mg: resource.ManagedKind(fake.GVK(&fake.Managed{})),
o: []ReconcilerOption{
WithInitializers(),
WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })),
WithExternalConnecter(ExternalConnectorFn(func(_ context.Context, mg resource.Managed) (ExternalClient, error) {
c := &ExternalClientFns{
CreateFn: func(_ context.Context, mg resource.Managed) (ExternalCreation, error) {
meta.SetExternalName(mg, "test")
return ExternalCreation{ExternalNameAssigned: true}, nil
},
ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) {
return ExternalObservation{}, nil
},
}
return c, nil
})),
WithConnectionPublishers(),
WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}),
},
},
want: want{result: reconcile.Result{RequeueAfter: defaultManagedShortWait}},
},
"LateInitializeUpdateError": {
reason: "Errors updating a managed resource to persist late initialized fields should trigger a requeue after a short wait.",
args: args{

6
pkg/resource/resource.go
View File

@ -194,6 +194,12 @@ func IgnoreNotFound(err error) error {
return Ignore(kerrors.IsNotFound, err)
}
// IsAPIError returns true if the given error's type is of Kubernetes API error.
func IsAPIError(err error) bool {
_, ok := err.(kerrors.APIStatus)
return ok
}
// IsConditionTrue returns if condition status is true
func IsConditionTrue(c v1alpha1.Condition) bool {
return c.Status == corev1.ConditionTrue