feat(desktop): add Docker Desktop detection and client skeleton #11593
Conversation
milas
requested review from
ndeloof,
glours and
jhrotko
and removed request for
a team
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11593 +/- ##
==========================================
- Coverage 58.20% 57.75% -0.45%
==========================================
Files 135 138 +3
Lines 11611 11717 +106
==========================================
+ Hits 6758 6767 +9
- Misses 4182 4277 +95
- Partials 671 673 +2 ☔ View full report in Codecov by Sentry. |
| if s.dockerCli != nil { | ||
| errs = append(errs, s.dockerCli.Client().Close()) | ||
| } | ||
| if s.desktopCli != nil { | ||
| errs = append(errs, s.desktopCli.Close()) | ||
| } |
There was a problem hiding this comment.
I dont know how to read :)
|
Could we also add an e2e test for this cases? unix, windows and darwin? |
| if err != nil { | ||
| return err | ||
| } | ||
| cmd.SetContext(ctx) | ||
|
|
||
| // (6) Desktop integration | ||
| if db, ok := backend.(desktop.IntegrationService); ok { |
There was a problem hiding this comment.
I wish we refactor this init sequence so we don't mutate backend but use setup builder according to options:
backend := compose.NewComposeService(dockerCli, WithDryRun(xx), WithDesktopIntegration(xx))
There was a problem hiding this comment.
Agreed 😞 the init is overly complicated, especially because of the split between Docker/CLI/plugins, Compose itself, and Cobra 🐍
This was already kind of hacky - I didn't want to add this to api.Service but the concrete composeService implementation is unexported
I'll see if I can refactor this in another PR to swap to options an consolidate some things, e.g. we have several lazy/memoized components but they all happen in different ways currently
There was a problem hiding this comment.
I didn't want to add this to api.Service
why not ?
Yes, this is a great call out! This will require me to sort out a couple things first - need to get updated Docker Desktop builds on our CI runners & service accounts on Hub for them to log in as, so I made a ticket in JIRA. I'll go ahead and get started on the test and |
Use the system info Engine API to auto-discover the Desktop integration API endpoint (a local `AF_UNIX` socket or named pipe). If present and responding to a `/ping`, the client is populated on the Compose service for use. Otherwise, it's left `nil`, and Compose will not try to use any functionality that relies on Docker Desktop (e.g. opening `docker-desktop://` deep links to the GUI). Signed-off-by: Milas Bowman <milas.bowman@docker.com>
milas
force-pushed
the
desktop-detection
branch
from
cab1ac5
to
7452a14
Compare
| var d net.Dialer | ||
| switch network { | ||
| case "unix": | ||
| if err := validateSocketPath(addr); err != nil { |
There was a problem hiding this comment.
build is complaining this is undefined for windows arch
internal/memnet/conn_windows.go
Outdated
| return winio.DialPipeContext(ctx, addr) | ||
| } | ||
|
|
||
| func validateUnixSocketPath(addr string) error { |
There was a problem hiding this comment.
rename to validateSocketPath to avoid build errors
| if s.dockerCli != nil { | ||
| errs = append(errs, s.dockerCli.Client().Close()) | ||
| } | ||
| if s.desktopCli != nil { | ||
| errs = append(errs, s.desktopCli.Close()) | ||
| } |
There was a problem hiding this comment.
I dont know how to read :)
What I did
Use the system info Engine API to auto-discover the Desktop integration API endpoint (a local
AF_UNIXsocket or named pipe).If present and responding to a
/ping, the client is populated on the Compose service for use. Otherwise, it's leftnil, and Compose will not try to use any functionality that relies on Docker Desktop (e.g. openingdocker-desktop://deep links to the GUI).This refactors some of the in-memory socket dialer code, so we can share it easier (time to bring back
docker/go-connections? 🤔) and make it slightly more robust.Related issue
JIRA: COMP-457
(not mandatory) A picture of a cute animal, if possible in relation to what you did
