Replace syscall.Unlink with os.Remove so that the directory(e.g. /run… #72
Conversation
…/docker.sock/) can be deleted Signed-off-by: Shukui Yang <[email protected]> Signed-off-by: Shukui Yang <[email protected]>
| @@ -79,7 +79,7 @@ func WithChmod(mask os.FileMode) SockOption { | |||
|
|
|||
| // NewUnixSocketWithOpts creates a unix socket with the specified options | |||
| func NewUnixSocketWithOpts(path string, opts ...SockOption) (net.Listener, error) { | |||
| if err := syscall.Unlink(path); err != nil && !os.IsNotExist(err) { | |||
| if err := os.Remove(path); err != nil && !os.IsNotExist(err) { | |||
There was a problem hiding this comment.
Thanks, so, this patch would not help getting into the situation (which is due to a race-condition between containers starting and the API being up), but would help recovering from that situation (by being able to remove the faulty directory instead of producing an error. Obviously, if there's containers still running that mount /var/run/docker.sock that would likely be problematic still 🤔
Change itself seems fine to me; if path is a directory it would attempt to remove it, which should be fairly safe, as it would fail if the directory is not empty, so there should be no real risk of accidentally removing files that we don't want to.
There was a problem hiding this comment.
@thaJeztah @tiborvass thanks.
For creating the dir of /var/run/docker.sock, the root cause of this is that dockerd can still handle the API when it shutdown, at that time, dockerd create a container and share the host path /var/run/docker.sock, but it is not exist，so dockerd create it. I have a pr moby/moby#38241 abort this，PTAL again, thanks very much.
|
Unlink does remove the file if it's not being used |
|
@cpuguy83 you mean that the |
|
The short term solution is to use --mount instead of "-v", because "--mount" will not attempt to create the directory. |
|
Yes. |
|
True; that would mostly prevent the situation. Once you're in there, things are more tricky (and something like this PR would help); Should we check if it's a directory and in that case do a |
|
I think it is difficult to properly remediate it from here. The correct fix would likely be in Dockerd.
|
|
Or just the tried and true "don't do that" approach. |
|
@cpuguy83 Thanks for your review.
Does this mean that Unlink does't remove the file if it's being used? I use -v and mount /run/docker.sock to a container，then I write some code to use Unlink and try delete /run/docker.sock. At last /run/docker.sock is deleted successfully. I want to know if I misunderstood something, PTAL, thanks very much. the follownig is the code And the details |
Docker start failed If docker's unix socket path exist，and it is a dir.
we can reproduce it like thks.
we can see the dir /run/docker.sock/ is created and It causes docker to start failed.
use os.Remove can fix this issue.