Skip to content
This repository was archived by the owner on Oct 30, 2018. It is now read-only.

Fix bind-volumes on docker >= 1.4.0 #547

Merged
merged 3 commits into from
Jun 30, 2015
Merged

Fix bind-volumes on docker >= 1.4.0 #547

merged 3 commits into from
Jun 30, 2015

Conversation

steveej
Copy link
Contributor

@steveej steveej commented Dec 21, 2014

If bind-volumes are submitted to docker >= 1.4.0 with the volumes set in addition to the binds, docker will create a regular volume and not bind-mount the specified path.

If bind-volumes are submitted to docker >= 1.4.0 with the volumes set in addition to the binds, docker will create a regular volume and not bind-mount the specified path.
@lorin
Copy link
Contributor

lorin commented Jan 3, 2015

I tried out this patch locally since I needed the functionality, and I hit a case where this fix didn't work.

If the image specifies a volume directory, and you try to bind a local directory to the volume directory, Docker won't bind the container directory to the host directory.

For example, the Dockerfile of the official nginx image contains:

VOLUME ["/var/cache/nginx"]

If you try to bind this directory to a local directory using the docker module, Docker still won't do the bind correctly:

- docker:
    image: nginx
    name: mynginx
    volumes: /home/lorin/nginx-cache:/var/cache/nginx

I worked around it by passing the bind info in the host_config parameter that gets passed to the create_container method.

diff --git a/cloud/docker/docker.py b/cloud/docker/docker.py
index f71bad4..4c74b1c 100644
--- a/cloud/docker/docker.py
+++ b/cloud/docker/docker.py
@@ -721,6 +721,7 @@ class DockerManager(object):
                   'volumes':      self.volumes,
                   'mem_limit':    _human_to_bytes(self.module.params.get('memory_limit')),
                   'environment':  self.env,
+                  'host_config':  docker.utils.create_host_config(binds=self.binds),
                   'hostname':     self.module.params.get('hostname'),
                   'detach':       self.module.params.get('detach'),
                   'name':         self.module.params.get('name'),

This seems to be discussed in docker/docker-py#419 as bug in docker-py.

lorin pushed a commit to lorin/ansible-mezzanine-docker that referenced this pull request Jan 3, 2015
@steveej
Copy link
Contributor Author

steveej commented Mar 12, 2015

I've updated my PR with additional changes.

Things I've tested successfully with the official nginx image

running the image without additional binds

$ ansible -i 'localhost,' localhost -m docker -a 'image=nginx state=running'

                "Volumes": {
                    "/var/cache/nginx": "/var/lib/docker/vfs/dir/4442d05a9d1b51eb82bb27f293809d60c0ad5eecfbe047f7e28db91c45e36c84"
                },
                "VolumesRW": {
                    "/var/cache/nginx": true
                }

running the image and overriding the VOLUME from the Dockerfile

$ ansible -i 'localhost,' localhost -m docker -a 'image=nginx state=running volumes=/var/tmp/nginx-cache:/var/cache/nginx'

                "Volumes": {
                    "/var/cache/nginx": "/var/tmp/nginx-cache"
                },
                "VolumesRW": {
                    "/var/cache/nginx": true
                }

$ ls -l /var/tmp/nginx-cache/

total 0
drwx------ 1 messagebus root 0 12. Mär 13:27 client_temp
drwx------ 1 messagebus root 0 12. Mär 13:27 fastcgi_temp
drwx------ 1 messagebus root 0 12. Mär 13:27 proxy_temp
drwx------ 1 messagebus root 0 12. Mär 13:27 scgi_temp
drwx------ 1 messagebus root 0 12. Mär 13:27 uwsgi_temp

This looks like correct behavior to me.

@lorin
Could you please verify that the problem is gone with my latest commits?

@lorin
Copy link
Contributor

lorin commented Mar 15, 2015

@steveej Just tested it out, this commit resolves the issue. 👍

@magicrobotmonkey
Copy link

This would be really nice to see fixed,makes it difficult to use ansible to orchestrate containers

@stefanvanwouw
Copy link

Have the same problem. Please accept this PR. +1

@maliskovik
Copy link

same issue here.

@lchorbadjiev
Copy link

+1

@gregdek gregdek added the shipit label Jun 25, 2015
@gregdek
Copy link
Contributor

gregdek commented Jun 25, 2015

Lots of love for this PR, including a +1 from @lorin -- promoting to P1_reviewed.

@gregdek gregdek removed the P3 label Jun 25, 2015
bcoca added a commit that referenced this pull request Jun 30, 2015
Fix bind-volumes on docker >= 1.4.0
@bcoca bcoca merged commit ec15b67 into ansible:devel Jun 30, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants