-
Notifications
You must be signed in to change notification settings - Fork 252
Use Docker CLI socket from home #2171
Use Docker CLI socket from home #2171
Conversation
cli/metrics/conn_darwin.go
Outdated
|
||
func init() { | ||
// Attempt to retrieve the Docker CLI socket for the current user. | ||
if home, err := os.UserHomeDir(); err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to take into account situations where HOME
isn't set? I know the docker cli uses that information, and falls back to looking up the user's config; https://github.com/moby/moby/blob/686be57d0a6e514c0cddb2f3ac9cbb3cbef87f5f/pkg/homedir/homedir_unix.go#L25-L33
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point :) thanks
|
||
/* | ||
Copyright 2020 Docker Compose CLI authors | ||
Copyright 2020, 2022 Docker Compose CLI authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the validation currently doesn't allow for the years to differ between files, so all should have the same years
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah :( I even did a quick google for some more decent copyright header checker, but couldn't find one which does the right thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In https://github.com/docker/cli-docs-tool/, we've been trying Google addlicense, which seemed to work (a bit more "loosely" in checking); see docker/cli-docs-tool#4
17bfc3c
to
4d4b6cf
Compare
4d4b6cf
to
718f596
Compare
.gitignore
Outdated
@@ -1,3 +1,24 @@ | |||
bin/ | |||
dist/ | |||
/.vscode/ | |||
|
|||
## VIM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn’t all that be in your global gitignore on your machine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that will work as well. Putting those in project-specific configs is force of habit for me.
718f596
to
1798796
Compare
cli/metrics/conn_windows.go
Outdated
@@ -30,6 +31,10 @@ var ( | |||
socket = `\\.\pipe\docker_cli` | |||
) | |||
|
|||
func init() { | |||
overrideSocket() // nop, unless built for e2e testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean
overrideSocket() // nop, unless built for e2e testing | |
overrideSocket() // no-op, unless built for e2e testing |
or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nop
is the mnemonic for no-op in most assemblers (and tends to be used as a neologism for "do nothing"), but yeah that would be clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, I'm just waiting for an approval from someone of the Pos team to merge it
We should not rely on having a global path for the Docker CLI socket. On macOS this forces Docker Desktop to access directories which require raised privileges. Whereas on Linux we do not create sockets in that location at all, currently. So look for the Docker CLI socket in the User's home directory. Signed-off-by: Piotr Stankiewicz <[email protected]>
Signed-off-by: Piotr Stankiewicz <[email protected]>
1798796
to
19ac0ad
Compare
func init() { | ||
// Attempt to retrieve the Docker CLI socket for the current user. | ||
if home := homedir.Get(); home != "" { | ||
socket = filepath.Join(home, "/Library/Containers/com.docker.docker/Data/docker-cli.sock") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should use a shorter Linux-style path in $HOME/.docker/desktop/docker-cli.sock
even on Darwin because we've had some problems where the maximum path in the sockaddr is 104 characters (even shorter than Linux's 108 characters). We work around this in some of our binaries by calling os.Chdir("Library/Containers/com.docker.docker")
and then using a relative path... but I suspect this trick doesn't work for the CLI because changing the current directory probably breaks something.
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this is a tough one.
The best thing I can think of from the top of my head is:
pushd ...
connect with relative path
popd
but not sure that would be safe to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reflecting on it a bit, I'm not as concerned as I was because the socket is best-effort (and it will work probably 99% of the time). So I'm happy with it as-is.
What I did
We should not rely on having a global path for the Docker CLI socket. On
macOS this forces Docker Desktop to access directories which require
raised privileges. Whereas on Linux we do not create sockets in that
location at all, currently. So look for the Docker CLI socket in the
User's home directory.
Related issue
(not mandatory) A picture of a cute animal, if possible in relation with what you did