api/client: add Close to Client

[thaJeztah]

• relates to Close RootlessKit API connections after version requests moby/moby#53815 (comment)

Add Close to the Client interface to allow callers to release resources owned by the client without accessing the underlying HTTP client directly.

The current implementation closes idle HTTP connections associated with the client.

[thaJeztah]

cc @AkihiroSuda

[AkihiroSuda]

Needs rebasing

[thaJeztah]

FWIW; ideally, we should change this to return a concrete type instead of an interface (interfaces should be long on the consumer side);

func New(socketPath) (*Client, error) {

Doing so probably would be a breaking change, so require a v4?

Alternatively, we could add a NewWithOptions (e.g.) variant that does return a concrete type, and which could allow functional arguments to be passed.

Perhaps you have thoughts on some of that (a bit out of scope for this PR though)

[AkihiroSuda]

Why do you need a concrete type?

[thaJeztah]

It's often done wrong (and I definitely have made the mistake), but core Go design is for interfaces to belong to the consumer (this is the functionality I need) instead of the producer, as it otherwise encourages "interface first" design; https://go.dev/wiki/CodeReviewComments#interfaces

Go interfaces generally belong in the package that uses values of the interface type, not the package that implements those values. The implementing package should return concrete (usually pointer or struct) types: that way, new methods can be added to implementations without requiring extensive refactoring.

In this case that also would've avoided having to modify the interface (which also technically could be a breaking change, because if someone implemented the interface, that's no longer implemented now); see https://go.dev/blog/module-compatibility (which also describes some bits about "don't return interfaces; use concrete types to keep it compatible").

Returning a concrete type means that we could add methods without breaking a contract. If a consumer needs those methods, they can add it to their interface;

type infoProvider interface {
	Info(context.Context) (*api.Info, error)
}

So that some function that needs "info" (but doesn't handle port-mapping or anything) can just define "this is what I need";

func getInfo(c infoProvider) {
....
}

And if they want to write some unit-test, they only need to implement those methods, and don't have to update code if rootlesskit adds new methods to its client that are unrelated.

[AkihiroSuda]

No strong opinion from me.
Can be revisited when releasing v4 (ETA: after Docker v25 EOL)

[thaJeztah]

Rebased 👍

[thaJeztah]

Oh, just to explain; I added this to prevent some future visitor from thinking "Close never returns an error, so let's remove the error return"; I intentionally want it to implement io.Closer so that users are encouraged to Close the client (to prevent re-introducing the issues that were reported in Moby)

[AkihiroSuda]

Suggested change

	Close() error
	io.Closer

Then you can remove the var _ io.Closer = (*client)(nil) line