plugins: cleanup sockets when done

[laurazard]

• relates to plugin: drop explicit unlink #4963

- What I did

Since 509123f, we've been leaking sockets in the filesystem on platforms where abstract sockets aren't supported.

That change relied on Go to cleanup our sockets for us, which Go will happily do as long as we make sure to close the listener, which we weren't previously doing unless to signal the plugin to terminate.

- How I did it

This change adds a deferred call to PluginServer.Close(), which makes sure we close the plugin server at the end of the plugin execution, so that we never exit without cleaning up.

- How to verify it

Execute a plugin command with debug logging enabled (e.g. docker -D compose convert) and look for the added debug log. Also check $TMPDIR for leaked sockets.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 61.67%. Comparing base (591bd17) to head (3dcc653).
Report is 14 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5146      +/-   ##
==========================================
- Coverage   61.82%   61.67%   -0.16%     
==========================================
  Files         298      295       -3     
  Lines       20730    20778      +48     
==========================================
- Hits        12817    12815       -2     
- Misses       7000     7043      +43     
- Partials      913      920       +7     

[laurazard]

Would you still rather I remove this line @tonistiigi? I don't really mind either way, but I expect debug logs to be quite verbose.

[tonistiigi]

Currently, we don't have many debug logs for commands, and for example in buildx where we use --debug for extra verbosity for build errors, this can be confusing to users who have no context for "plugin server".

[thaJeztah]

Yeah the tricky bit with the CLI (unlike the daemon, where it usually ends up in a log file) is that logging in the CLI is.. printing in the user's terminal.

Would be nice if we had a location to store logs, but .. we don't, so we should not use it except for exceptional cases.

We could use "trace" level logging (one level more verbose than debug), but not sure if we pass that through currently (and not sure if containerd logs had it as option)

[laurazard]

Oh, that works!

logrus.Trace("Closing plugin server")

./build/docker -l trace compose version
TRAC[0000] Plugin server listening on /var/folders/zj/khjjyd1n0zjd9xc44815szxc0000gn/T/docker_cli_9e516c003cda3c20ab79b88afc18434b 
Docker Compose version v2.27.1-desktop.1
TRAC[0000] Closing plugin server                                              

./build/docker -D compose version    
Docker Compose version v2.27.1-desktop.1

[laurazard]

@tonistiigi this work for you?

[thaJeztah]

I think it should be fine; we can remove in a follow-up if it's a problem (but we need to look a bit at logging overall; I also did a quick attempt to see what it'd could look like if we switched over to containerd/log, as we did on the engine side; #5149

[thaJeztah]

@laurazard how difficult would it be to write a test-case for this? (could be in a follow-up if we want to get this in)

[thaJeztah]

Had a quick chat; let's bring this one in, and do a follow-up for the test and any other changes we may need.