Docker does not unmount on mount failure if -w option is passed with docker run. #22564
Comments
|
/cc @cpuguy83 |
|
Ping, is this issue going to be fixed in the upcoming RCs for Docker 1.12? Our plugin does reference counting on its own and when mount fails we have the volume ref. counted and unless an unmount gets sent (which happens always except when "-w" is passed) the volume is unusable till at least the plugin is restarted. /cc @cpuguy83 |
|
@govint specifying |
|
@cpuguy83, correct, but that's the bug though, the behavior is inconsistent with when docker sends the unmount to the volume driver. If -w is in the command line to run the container and the mount fails, then an unmount isn't issued to the volume driver.But if -w isn't provided and the mount fails then an unmount is issued to the volume driver. |
|
I think I see the problem. |
|
I lied, I don't see it, but I can reproduce it. It only happens when |
|
Ok, I think I've found it... It seems to be related to whether or not the path in the container exists or not. Later when we get to if _, err = ioutil.ReadDir(rootfs); err != nil {
if os.IsNotExist(err) {
return nil
}
return err
}
id := stringid.GenerateNonCryptoID()
path, err := v.Mount(id)
if err != nil {
return err
}
defer func() {
if err := v.Unmount(id); err != nil {
logrus.Warnf("error while unmounting volume %s: %v", v.Name(), err)
}
}()Basically, if the volume path in the container does not exist, it will return immediately here and the error you get from When the volume path does exist, the error is returned during the I think we should not unmount if |
|
Actually, it would be difficult to track what is mounted and what is not. We can just send the unmount and this can effectively be a no-op for plugins if they don't recognize the ID. |
|
Here we go: #27116 |
|
that would change the current behavior though. Before the fix, Docker sends unmount on mount failures (sans -w existing_path case) , so the volume driver increments refcounts on mount and decrements on refcount, no matter if mount succeeds of fails. With the suggested behavior, the driver(s) would need to be changed to skip refcount increment on failed mount, and this would have to be dependent on Docker version. Is there any way just to send an umount on all mount failures (including "-w existing_path") ? |
|
@msterin Right now the behavior is inconsistent. A change as you are suggesting is the same problem, just in the other direction (re: changing behavior). |
|
@cpuguy83 I don't think you get multiple unmounts on existing path when mount fails, I think you get none. Now we'll adjust to the correct one. |
Fixes moby#22564 When an error occurs on mount, there should not be any call later to unmount. This can throw off refcounting in the underlying driver unexpectedly. Consider these two cases: ``` $ docker run -v foo:/bar busybox true ``` ``` $ docker run -v foo:/bar -w /foo busybox true ``` In the first case, if mounting `foo` fails, the volume driver will not get a call to unmount (this is the incorrect behavior). In the second case, the volume driver will not get a call to unmount (correct behavior). This occurs because in the first case, `/bar` does not exist in the container, and as such there is no call to `volume.Mount()` during the `create` phase. It will error out during the `start` phase. In the second case `/bar` is created before dealing with the volume because of the `-w`. Because of this, when the volume is being setup docker will try to copy the image path contents in the volume, in which case it will attempt to mount the volume and fail. This happens during the `create` phase. This makes it so the container will not be created (or at least fully created) and the user gets the error on `create` instead of `start`. The error handling is different in these two phases. Changed to only send `unmount` if the volume is mounted. While investigating the cause of the reported issue I found some odd behavior in unmount calls so I've cleaned those up a bit here as well. Signed-off-by: Brian Goff <[email protected]>
Output of
docker version:On VM1
On VM2
Output of
docker info:On VM1
On VM2
Additional environment details (AWS, VirtualBox, physical, etc.):
On VM1
VM2
Steps to reproduce the issue:
docker volume create --driver= --name=datavol -o size=10gb
docker run -it --rm -v datavol:/data -w /data busybox
Describe the results you received:
Command:
Logs:
Describe the results you expected:
Command:
Logs
Additional information you deem important (e.g. issue happens only occasionally):
This happens on both 1.10 and 1.11. We are writing a plugin for volume and expect to get an umount request on failed mounts. This is needed to maintain refcount within the plugin. The expectation is that docker will always send an unmount independent of the status of mount command and this is what we see when we do not pass in the -w option on run. Discussions part of https://github.com/docker/docker/pull/21015/files also indicate that umount will always be sent.
Either this is a bug in the clean up handling when -w is sent or the behavior of mount and umount needs to be specified.
The text was updated successfully, but these errors were encountered: