Fixes #19103 - Set Volumes field only for anonymous volumes

[rsanr]

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

docker_container

ANSIBLE VERSION

ansible 2.3.0 (devel 7e84bcec30) last updated 2016/12/06 19:10:54 (GMT +000)
  config file = 
  configured module search path = ['/opt/ansible/ansible/library']

SUMMARY

Currently docker_container module sets Volumes field for all volumes passed. This behavior is inconsistent with docker run command. This commit fixes docker_container.py to set Volumes field only when anonymous volumes are passed.

This behavior is also clarified in the following comment:
moby/moby#2949 (comment)

Playbook:

- name: assert state of my container
  docker_container:
    name: mc
    image: "my_container:1.0"
    command: "execute"
    state: started
    restart_policy: always
    exposed_ports:
    - 12345
    network_mode: host
    detach: True
    volumes:
    - "/a/b:/a/b"
    log_driver: "json-file"
    log_options:
      max-size: "10m"
      max-file: "10"

Before:

DEBU[0111] form data: {"AttachStderr":false,"AttachStdin":false,"AttachStdout":false,"Cmd":["execute"],"Env":[],"ExposedPorts":{"12345/tcp":{}},"HostConfig":{"Binds":["/a/b:/a/b:rw"],"LogConfig":{"Config":{"max-file":"10","max-size":"10m"},"Type":"json-file"},"Memory":0,"NetworkMode":"host","ReadonlyRootfs":false,"RestartPolicy":{"MaximumRetryCount":null,"Name":"always"}},"Image":"my_container:1.0","NetworkDisabled":false,"OpenStdin":false,"StdinOnce":false,"Tty":false,"Volumes":{"/a/b":{}}} 

After:

DEBU[0111] form data: {"AttachStderr":false,"AttachStdin":false,"AttachStdout":false,"Cmd":["execute"],"Env":[],"ExposedPorts":{"12345/tcp":{}},"HostConfig":{"Binds":["/a/b:/a/b:rw"],"LogConfig":{"Config":{"max-file":"10","max-size":"10m"},"Type":"json-file"},"Memory":0,"NetworkMode":"host","ReadonlyRootfs":false,"RestartPolicy":{"MaximumRetryCount":null,"Name":"always"}},"Image":"my_container:1.0","NetworkDisabled":false,"OpenStdin":false,"StdinOnce":false,"Tty":false,"Volumes":{}} 

Fixes #19103

[rsanr]

Migrated from ansible/ansible-modules-core#5848

[amcrn]

cc @chouseknecht

[amcrn]

Hey @jimi-c , not sure if there's an active maintainer for this. Should we be pinging someone in particular to take a look at this? Thanks!

[amcrn]

@abadger @chouseknecht

[amcrn]

Hey @jimi-c , congrats on the Ansible 2.3 release! I'm re-ping'ing you on this pull request given your comments on https://news.ycombinator.com/item?id=14100434

Given this has been open for 4 months, and it's not a new module (it's a bug fix), I think the issue that's unique here is that there isn't really an active maintainer for Docker-related Ansible changes. Thoughts? Thanks!

[ansibot]

cc @ThomasSteinbach @cove @dusdanig @jctanner @joshuaconner @kassiansun @softzilla @zfil
click here for bot help

[chouseknecht]

@amcrn

Testing this. It seems your change is causing the module to no longer be idempotent. It returns the following differences:

"differences": [
            {
                "expected_volumes": {
                    "container": {
                        "/things": {}
                    },
                    "parameter": {
                        "/projects": {},
                        "/testing": {},
                        "/things": {}
                    }
                }
            }
        ]

Here's my playbook:

- name: Test
  hosts: localhost
  connection: local
  tasks:
    - name: Assert state of my container
      docker_container:
        name: mc
        image: 'centos:7'
        command: ['/bin/sleep', '1d']
        state: started
        restart_policy: always
        exposed_ports:
        - 12345
        network_mode: host
        detach: True
        volumes:
        - /Users/chouseknecht/projects:/projects:rw
        - test-volume:/testing:rw
        - /things
        log_driver: 'json-file'
        log_options:
          max-size: '10m'
          max-file: '10'
        debug: yes

[chouseknecht]

@amcrn

I think the _get_expected_volumes method at line 1552 should look something like this:

def _get_expected_volumes(self, image):
    self.log('_get_expected_volumes')
    expected_vols = dict()
    if image and image['ContainerConfig'].get('Volumes'):
        expected_vols.update(image['ContainerConfig'].get('Volumes'))

    if self.parameters.volumes:
        for vol in self.parameters.volumes:
            parts = vol.split(':')
            if len(parts) == 1:
                new_vol = dict()
                new_vol[vol] = dict()
                expected_vols.update(new_vol)

    if not expected_vols:
        expected_vols = None
    self.log("expected_volumes:")
    self.log(expected_vols, pretty_print=True)
    return expected_vols

[chouseknecht]

needs_revision

[ansibot]

cc @felixfontein
click here for bot help

[ansibot]

cc @akshay196 @danihodovic @dariko @jwitko @tbouvet
click here for bot help

[felixfontein]

@rsanr are you still around? Can you rebase this PR?

[gundalow]

Hi,
This PR seems to have gotten stuck, so we are going to close it. We like to include community contributions, so this is a difficult call for us. Here's why we're closing this one:

• Given the discussion in Docker Agenda/Task list community#408 (comment)
• It was created from an old version of the code
• We want to clear out the backlog so it's easier to find bug fixes
• There haven't been any comments or other activity in a long time

If you're the PR author, and you want to start the conversation back up, please:

• reopen the PR and
• rebase the branch

If you're a community member, and you want to see this change merged, please:

• Create a new PR and
• Reference this author in the git commit & PR and
• Reference this PR in the new PR

If you've like to help with the Docker modules please comment on the Docker Agenda/Task List

How to rebase a branch: https://docs.ansible.com/ansible/latest/dev_guide/developing_rebasing.html
How to communicate with the Ansible community: https://docs.ansible.com/ansible/devel/community/communication.html
How to contribute to Ansible: https://docs.ansible.com/ansible/devel/community/index.html

needs_info

[ansibot]

cc @DBendit
click here for bot help

[ansibot]

@rsanr This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help