diff --git a/pkg/reconciler/managed/reconciler.go b/pkg/reconciler/managed/reconciler.go index f559e77a4..b479f72e5 100644 --- a/pkg/reconciler/managed/reconciler.go +++ b/pkg/reconciler/managed/reconciler.go @@ -230,6 +230,9 @@ type ExternalConnecter interface { } // An ExternalDisconnecter disconnects from a provider. +// +// Deprecated: Please use Disconnect() on the ExternalClient for disconnecting +// from the provider. type ExternalDisconnecter interface { // Disconnect from the provider and close the ExternalClient. Disconnect(ctx context.Context) error @@ -330,15 +333,22 @@ type ExternalClient interface { // Delete the external resource upon deletion of its associated Managed // resource. Called when the managed resource has been deleted. Delete(ctx context.Context, mg resource.Managed) error + + // Disconnect from the provider and close the ExternalClient. + // Called at the end of reconcile loop. An ExternalClient not requiring + // to explicitly disconnect to cleanup it resources, can provide a no-op + // implementation which just return nil. + Disconnect(ctx context.Context) error } // ExternalClientFns are a series of functions that satisfy the ExternalClient // interface. type ExternalClientFns struct { - ObserveFn func(ctx context.Context, mg resource.Managed) (ExternalObservation, error) - CreateFn func(ctx context.Context, mg resource.Managed) (ExternalCreation, error) - UpdateFn func(ctx context.Context, mg resource.Managed) (ExternalUpdate, error) - DeleteFn func(ctx context.Context, mg resource.Managed) error + ObserveFn func(ctx context.Context, mg resource.Managed) (ExternalObservation, error) + CreateFn func(ctx context.Context, mg resource.Managed) (ExternalCreation, error) + UpdateFn func(ctx context.Context, mg resource.Managed) (ExternalUpdate, error) + DeleteFn func(ctx context.Context, mg resource.Managed) error + DisconnectFn func(ctx context.Context) error } // Observe the external resource the supplied Managed resource represents, if @@ -365,6 +375,11 @@ func (e ExternalClientFns) Delete(ctx context.Context, mg resource.Managed) erro return e.DeleteFn(ctx, mg) } +// Disconnect the external client. +func (e ExternalClientFns) Disconnect(ctx context.Context) error { + return e.DisconnectFn(ctx) +} + // A NopConnecter does nothing. type NopConnecter struct{} @@ -394,6 +409,9 @@ func (c *NopClient) Update(_ context.Context, _ resource.Managed) (ExternalUpdat // Delete does nothing. It never returns an error. func (c *NopClient) Delete(_ context.Context, _ resource.Managed) error { return nil } +// Disconnect does nothing. It never returns an error. +func (c *NopClient) Disconnect(_ context.Context) error { return nil } + // An ExternalObservation is the result of an observation of an external // resource. type ExternalObservation struct { @@ -603,6 +621,8 @@ func WithExternalConnecter(c ExternalConnecter) ReconcilerOption { // WithExternalConnectDisconnecter specifies how the Reconciler should connect and disconnect to the API // used to sync and delete external resources. +// +// Deprecated: Please use Disconnect() on the ExternalClient for disconnecting from the provider. func WithExternalConnectDisconnecter(c ExternalConnectDisconnecter) ReconcilerOption { return func(r *Reconciler) { r.external.ExternalConnectDisconnecter = c @@ -909,6 +929,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (resu log.Debug("Cannot disconnect from provider", "error", err) record.Event(managed, event.Warning(reasonCannotDisconnect, err)) } + + if err := external.Disconnect(ctx); err != nil { + log.Debug("Cannot disconnect from provider", "error", err) + record.Event(managed, event.Warning(reasonCannotDisconnect, err)) + } }() observation, err := external.Observe(externalCtx, managed) diff --git a/pkg/reconciler/managed/reconciler_test.go b/pkg/reconciler/managed/reconciler_test.go index 60973e931..3467fab79 100644 --- a/pkg/reconciler/managed/reconciler_test.go +++ b/pkg/reconciler/managed/reconciler_test.go @@ -310,17 +310,17 @@ func TestReconciler(t *testing.T) { o: []ReconcilerOption{ WithInitializers(), WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), - WithExternalConnectDisconnecter(ExternalConnectDisconnecterFns{ - ConnectFn: func(_ context.Context, _ resource.Managed) (ExternalClient, error) { - c := &ExternalClientFns{ - ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { - return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil - }, - } - return c, nil - }, - DisconnectFn: func(_ context.Context) error { return errBoom }, - }), + WithExternalConnecter(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) { + c := &ExternalClientFns{ + ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { + return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil + }, + DisconnectFn: func(_ context.Context) error { + return errBoom + }, + } + return c, nil + })), WithConnectionPublishers(), WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), }, @@ -353,6 +353,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{}, errBoom }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -381,6 +384,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: false}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -426,6 +432,9 @@ func TestReconciler(t *testing.T) { DeleteFn: func(_ context.Context, _ resource.Managed) error { return errBoom }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -471,6 +480,9 @@ func TestReconciler(t *testing.T) { DeleteFn: func(_ context.Context, _ resource.Managed) error { return nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -513,6 +525,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: false}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -558,6 +573,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: false}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -590,6 +608,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: false}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -692,6 +713,9 @@ func TestReconciler(t *testing.T) { CreateFn: func(_ context.Context, _ resource.Managed) (ExternalCreation, error) { return ExternalCreation{}, errBoom }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -735,6 +759,9 @@ func TestReconciler(t *testing.T) { CreateFn: func(_ context.Context, _ resource.Managed) (ExternalCreation, error) { return ExternalCreation{}, errBoom }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -781,6 +808,9 @@ func TestReconciler(t *testing.T) { CreateFn: func(_ context.Context, _ resource.Managed) (ExternalCreation, error) { return ExternalCreation{}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -825,6 +855,9 @@ func TestReconciler(t *testing.T) { cd := ConnectionDetails{"create": []byte{}} return ExternalCreation{ConnectionDetails: cd}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -907,6 +940,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: true, ResourceUpToDate: true, ResourceLateInitialized: true}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -943,6 +979,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -973,6 +1012,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1013,6 +1055,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1048,6 +1093,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1096,6 +1144,9 @@ func TestReconciler(t *testing.T) { UpdateFn: func(_ context.Context, _ resource.Managed) (ExternalUpdate, error) { return ExternalUpdate{}, errBoom }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1136,6 +1187,9 @@ func TestReconciler(t *testing.T) { cd := ConnectionDetails{"update": []byte{}} return ExternalUpdate{ConnectionDetails: cd}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1185,6 +1239,9 @@ func TestReconciler(t *testing.T) { UpdateFn: func(_ context.Context, _ resource.Managed) (ExternalUpdate, error) { return ExternalUpdate{}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1282,17 +1339,17 @@ func TestReconciler(t *testing.T) { o: []ReconcilerOption{ WithInitializers(), WithReferenceResolver(ReferenceResolverFn(func(_ context.Context, _ resource.Managed) error { return nil })), - WithExternalConnectDisconnecter(ExternalConnectDisconnecterFns{ - ConnectFn: func(_ context.Context, _ resource.Managed) (ExternalClient, error) { - c := &ExternalClientFns{ - ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { - return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil - }, - } - return c, nil - }, - DisconnectFn: func(_ context.Context) error { return errBoom }, - }), + WithExternalConnecter(ExternalConnectorFn(func(_ context.Context, _ resource.Managed) (ExternalClient, error) { + c := &ExternalClientFns{ + ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { + return ExternalObservation{ResourceExists: true, ResourceUpToDate: true}, nil + }, + DisconnectFn: func(_ context.Context) error { + return nil + }, + } + return c, nil + })), WithConnectionPublishers(), WithFinalizer(resource.FinalizerFns{AddFinalizerFn: func(_ context.Context, _ resource.Object) error { return nil }}), }, @@ -1439,6 +1496,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: false}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1478,6 +1538,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: true}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1522,6 +1585,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: true}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1652,6 +1718,9 @@ func TestReconciler(t *testing.T) { UpdateFn: func(_ context.Context, _ resource.Managed) (ExternalUpdate, error) { return ExternalUpdate{}, errBoom }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1697,6 +1766,9 @@ func TestReconciler(t *testing.T) { UpdateFn: func(_ context.Context, _ resource.Managed) (ExternalUpdate, error) { return ExternalUpdate{}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1742,6 +1814,9 @@ func TestReconciler(t *testing.T) { UpdateFn: func(_ context.Context, _ resource.Managed) (ExternalUpdate, error) { return ExternalUpdate{}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })), @@ -1785,6 +1860,9 @@ func TestReconciler(t *testing.T) { ObserveFn: func(_ context.Context, _ resource.Managed) (ExternalObservation, error) { return ExternalObservation{ResourceExists: true, ResourceUpToDate: true, ResourceLateInitialized: true}, nil }, + DisconnectFn: func(_ context.Context) error { + return nil + }, } return c, nil })),