Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using volumes does not work as explained in docs. This breaks projects like fig. #419

Closed
bercab opened this issue Dec 12, 2014 · 12 comments · Fixed by #428
Closed

Using volumes does not work as explained in docs. This breaks projects like fig. #419

bercab opened this issue Dec 12, 2014 · 12 comments · Fixed by #428

Comments

@bercab
Copy link

bercab commented Dec 12, 2014

In newer versions of docker (>= v1.4.0), docker-py does not bind-mount volumes.

If volume is specified in volumes parameter to c.create_container(), then volume is created. But according to docs, when binds parameter is specified to c.start(), volume should be bind mounted.

http://docker-py.readthedocs.org/en/latest/volumes/

But it seems that start() only binds volumes NOT specified on volumes parameter of c.create_volumes().

I think this is breaking projects like fig (can't mount volumes) or saltstack dockerio module:
docker/compose#622

Code example reproducing issue for me:

from pprint import pprint
from docker import Client
c = Client(base_url='unix://var/run/docker.sock')

res = c.create_container('busybox', name='pytest', detach=True, 
    command='sh -c "while true;do echo test;sleep 1;done"',
    volumes=['/mnt/test',])
binds={
    '/test':
        {
            'bind': '/mnt/test',
            'ro': False
        },
    '/var/www':
        {
            'bind': '/mnt/vol1',
            'ro': True
        }
}
# start
c.start(res['Id'], binds=binds)

# inspect container
pprint(c.inspect_container(res['Id'])['Volumes'])

#result
{u'/mnt/test': u'/var/lib/docker/vfs/dir/3727ff0ae94652889ad5d1095b38c62fe24defd0aa952787cf1e1b0b6b04faf7',
 u'/mnt/vol1': u'/var/www'}

You can see as /var/www is correctly bind-mounted, but /mnt/test is not. The difference is that /mnt/vol1 was not specified in volumes parameter to c.create_container()

Do you think is a docker-py issue with newer docker engine?

@cpuguy83
Copy link
Contributor

This shouldn't be an issue on < 1.4
In 1.4 passing in HostConfig (ie, bind mounts, volumes-from, other host specific things) on start is deprecated... that said you can still pass in hostconfig on start.
.. However, in 1.4 volumes are now created when at container creation instead of on container start... so if the Binds parameter is not passed in on create, the bind-mounts are not created.

@bercab
Copy link
Author

bercab commented Dec 12, 2014

.. However, in 1.4 volumes are now created when at container creation instead of on container start...
so if the Binds parameter is not passed in on create, the bind-mounts are not created.

Are you sure? , In my example, binds parameter is passed on start(), not on create_container(), and it works.

Any way, seems clear that API does not work as stated on docs, at least with docker v1.4

@cpuguy83
Copy link
Contributor

I'm writing some tests for this right now on the docker side.
But I'm pretty sure that if a volume was passed into the create command (without specifying the binds in the HostConfig), if you then pass the Binds parameter on start, it will be ignored, since a volume would already be initialized for the given volume path.

@bercab
Copy link
Author

bercab commented Dec 12, 2014

Yes, I agree, this is what happens in my test.
But also, I see that if you don't specify volumes nor binds to create(), and only pass binds to start(), it works (see /mnt/vol1 in example above)

Confirmed, only happens in docker >=1.4 , I have modified my issue description, sorry.

@cpuguy83
Copy link
Contributor

I'm going to work on a fix on the Docker side since this was an unexpected breaking change.
However, docker-py should definitely be updated to pass all container configuration on create.

@dnephin
Copy link
Contributor

dnephin commented Dec 12, 2014

@cpuguy83

In 1.4 passing in HostConfig on start is deprecated

It sounds like it's actually been removed entirely, not just deprecated

@cpuguy83
Copy link
Contributor

No, just deprecated. The issue is with the volume initialization.
I'll have a PR up momentarily that fixes this on the Docker side.

@dnephin
Copy link
Contributor

dnephin commented Dec 12, 2014

@cpuguy83 awesome, thanks!

@cpuguy83
Copy link
Contributor

Issue is detailed here: moby/moby#9628

@cpuguy83
Copy link
Contributor

And it's actually not deprecated, this was bad information.

@virtuald
Copy link

+1

@ghost
Copy link

ghost commented Dec 14, 2014

this issue also breaks ansible docker module (

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants