search

crossplane / crossplane-runtime

A set of libraries for writing Crossplane controllers.
https://crossplane.io
Apache License 2.0
144 stars 95 forks source link

Disconnect ExternalClient #697

Open denniskniep opened 1 month ago

denniskniep commented 1 month ago

Problem

I am currently struggling with appropriately disconnecting the external client after it was used.

My Reconciler is created with WithExternalConnectDisconnecter here

I implemented the connect and disconnect method. But disconnect does not have any further specific parameters, so it seems to be intended to close the whole Connector. But there is only one Connector per Reconciler. Meaning if the Connectorś disconnect method is called and then the Reconciler.Reconcile gets another call it will call Connect() on an already Disconnected connector.

https://github.com/crossplane/crossplane-runtime/blob/fc036618ffd82023aeefe97554acdeb3a3db972f/pkg/reconciler/managed/reconciler.go#L892-L912

Currently the Disconnect call is: r.external.Disconnect(ctx);

Proposal

Wouldn´t it be better to pass either external as a parameter to disconnect r.external.Disconnect(ctx, external);

or directly call Disconnect on external (without r.) external.Disconnect(ctx)

Benefit is, that we can close precisely an reconcile call scoped client. And not disconnect the whole connector of the reconciler, though it is still in use and might run another reconcile