Skip to content

Commit

Permalink
docker: provide alternatives to ansible_facts results (#51192)
Browse files Browse the repository at this point in the history
* Try to stop using ansible_facts for docker modules.

* Stop using facts returned from regular modules.
  • Loading branch information
felixfontein authored and gundalow committed Feb 18, 2019
1 parent 8222ebd commit 37b0f5c
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 26 deletions.
6 changes: 6 additions & 0 deletions changelogs/fragments/docker-facts.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
minor_changes:
- "docker_container, docker_network, docker_volume - return facts as regular variable additionally to
facts. This is now the preferred way to obtain results."
- "docker_service - return facts as regular variable ``service_facts``; this is a dictionary mapping
service names to container dictionaries. The facts are still returned, but it is recommended to
use ``register`` and ``service_facts`` in the future."
16 changes: 11 additions & 5 deletions lib/ansible/modules/cloud/docker/docker_compose.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,8 +293,12 @@
'''

RETURN = '''
service:
description: Name of the service.
service_facts:
description:
- A dictionary mapping the service's name to a dictionary of containers.
- Note that facts are part of the registered vars since Ansible 2.8. For compatibility reasons, the facts
are also accessible directly. The service's name is the variable with which the container dictionary
can be accessed.
returned: success
type: complex
contains:
Expand Down Expand Up @@ -673,7 +677,7 @@ def cmd_up(self):
start_deps = self.dependencies
service_names = self.services
detached = True
result = dict(changed=False, actions=[], ansible_facts=dict())
result = dict(changed=False, actions=[], ansible_facts=dict(), service_facts=dict())

up_options = {
u'--no-recreate': False,
Expand Down Expand Up @@ -757,7 +761,9 @@ def cmd_up(self):
result['actions'] += scale_output['actions']

for service in self.project.services:
result['ansible_facts'][service.name] = dict()
service_facts = dict()
result['ansible_facts'][service.name] = service_facts
result['service_facts'][service.name] = service_facts
for container in service.containers(stopped=True):
inspection = container.inspect()
# pare down the inspection data to the most useful bits
Expand Down Expand Up @@ -809,7 +815,7 @@ def cmd_up(self):
if networks[key].get('MacAddress', None) is not None:
facts['networks'][key]['macAddress'] = networks[key]['MacAddress']

result['ansible_facts'][service.name][container.name] = facts
service_facts[container.name] = facts

return result

Expand Down
4 changes: 3 additions & 1 deletion lib/ansible/modules/cloud/docker/docker_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,8 @@
description:
- Before 2.3 this was 'ansible_docker_container' but was renamed due to conflicts with the connection plugin.
- Facts representing the current state of the container. Matches the docker inspection output.
- Note that facts are not part of registered vars but accessible directly.
- Note that facts are part of the registered vars since Ansible 2.8. For compatibility reasons, the facts
are also accessible directly.
- Empty if C(state) is I(absent)
- If detached is I(False), will include Output attribute containing any output from container run.
returned: always
Expand Down Expand Up @@ -2196,6 +2197,7 @@ def __init__(self, client):

if self.facts:
self.results['ansible_facts'] = {'docker_container': self.facts}
self.results['docker_container'] = self.facts

def present(self, state):
container = self._get_container(self.parameters.name)
Expand Down
11 changes: 8 additions & 3 deletions lib/ansible/modules/cloud/docker/docker_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,11 @@
'''

RETURN = '''
facts:
description: Network inspection results for the affected network.
docker_network:
description:
- Network inspection results for the affected network.
- Note that facts are part of the registered vars since Ansible 2.8. For compatibility reasons, the facts
are also accessible directly.
returned: success
type: dict
sample: {}
Expand Down Expand Up @@ -575,7 +578,9 @@ def present(self):
if not self.check_mode and not self.parameters.debug:
self.results.pop('actions')

self.results['ansible_facts'] = {u'docker_network': self.get_existing_network()}
network_facts = self.get_existing_network()
self.results['ansible_facts'] = {u'docker_network': network_facts}
self.results['docker_network'] = network_facts

def absent(self):
self.diff_tracker.add('exists', parameter=False, active=self.existing_network is not None)
Expand Down
11 changes: 8 additions & 3 deletions lib/ansible/modules/cloud/docker/docker_volume.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,11 @@
'''

RETURN = '''
facts:
description: Volume inspection results for the affected volume.
docker_volume:
description:
- Volume inspection results for the affected volume.
- Note that facts are part of the registered vars since Ansible 2.8. For compatibility reasons, the facts
are also accessible directly.
returned: success
type: dict
sample: {}
Expand Down Expand Up @@ -284,7 +287,9 @@ def present(self):
if not self.check_mode and not self.parameters.debug:
self.results.pop('actions')

self.results['ansible_facts'] = {u'docker_volume': self.get_existing_volume()}
volume_facts = self.get_existing_volume()
self.results['ansible_facts'] = {u'docker_volume': volume_facts}
self.results['docker_volume'] = volume_facts

def absent(self):
self.diff_tracker.add('exists', parameter=False, active=self.existing_volume is not None)
Expand Down
22 changes: 11 additions & 11 deletions test/integration/targets/docker_container/tasks/tests/options.yml
Original file line number Diff line number Diff line change
Expand Up @@ -543,15 +543,15 @@
# of hello-world. We don't know why this happens, but it happens
# often enough to be annoying. That's why we disable this for now,
# and simply test that 'Output' is contained in the result.
- "'Output' in detach_no_cleanup.ansible_facts.docker_container"
# - "'Hello from Docker!' in detach_no_cleanup.ansible_facts.docker_container.Output"
- "'Output' in detach_no_cleanup.docker_container"
# - "'Hello from Docker!' in detach_no_cleanup.docker_container.Output"
- detach_no_cleanup_cleanup is changed
- "'Output' in detach_cleanup.ansible_facts.docker_container"
# - "'Hello from Docker!' in detach_cleanup.ansible_facts.docker_container.Output"
- "'Output' in detach_cleanup.docker_container"
# - "'Hello from Docker!' in detach_cleanup.docker_container.Output"
- detach_cleanup_cleanup is not changed
- assert:
that:
- "'Cannot retrieve result as auto_remove is enabled' == detach_auto_remove.ansible_facts.docker_container.Output"
- "'Cannot retrieve result as auto_remove is enabled' == detach_auto_remove.docker_container.Output"
- detach_auto_remove_cleanup is not changed
when: docker_py_version is version('2.1.0', '>=')

Expand Down Expand Up @@ -2373,7 +2373,7 @@
- memory_swap_1 is changed
# Sometimes (in particular during integration tests, maybe when not running
# on a proper VM), memory_swap cannot be set and will be -1 afterwards.
- memory_swap_2 is not changed or memory_swap_2.ansible_facts.docker_container.HostConfig.MemorySwap == -1
- memory_swap_2 is not changed or memory_swap_2.docker_container.HostConfig.MemorySwap == -1
- memory_swap_3 is changed

- debug: var=memory_swap_1
Expand Down Expand Up @@ -2775,7 +2775,7 @@
command: '/bin/sh -c "sleep 10m"'
name: "{{ cname }}"
state: started
pid_mode: "container:{{ pid_mode_helper.ansible_facts.docker_container.Id }}"
pid_mode: "container:{{ pid_mode_helper.docker_container.Id }}"
register: pid_mode_1
ignore_errors: yes
# docker-py < 2.0 does not support "arbitrary" pid_mode values
Expand Down Expand Up @@ -3098,9 +3098,9 @@
- recreate_2 is changed
- recreate_3 is changed
- recreate_4 is changed
- recreate_1.ansible_facts.docker_container.Id != recreate_2.ansible_facts.docker_container.Id
- recreate_2.ansible_facts.docker_container.Id == recreate_3.ansible_facts.docker_container.Id
- recreate_3.ansible_facts.docker_container.Id != recreate_4.ansible_facts.docker_container.Id
- recreate_1.docker_container.Id != recreate_2.docker_container.Id
- recreate_2.docker_container.Id == recreate_3.docker_container.Id
- recreate_3.docker_container.Id != recreate_4.docker_container.Id

####################################################################
## restart #########################################################
Expand Down Expand Up @@ -3139,7 +3139,7 @@
that:
- restart_1 is changed
- restart_2 is changed
- restart_1.ansible_facts.docker_container.Id == restart_2.ansible_facts.docker_container.Id
- restart_1.docker_container.Id == restart_2.docker_container.Id

####################################################################
## restart_policy ##################################################
Expand Down
6 changes: 3 additions & 3 deletions test/integration/targets/docker_prune/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@
- assert:
that:
# containers
- container.ansible_facts.docker_container.Id in result.containers
- container.docker_container.Id in result.containers
- "'containers_space_reclaimed' in result"
# images
- "'images_space_reclaimed' in result"
# networks
- network.ansible_facts.docker_network.Name in result.networks
- network.docker_network.Name in result.networks
# volumes
- volume.ansible_facts.docker_volume.Name in result.volumes
- volume.docker_volume.Name in result.volumes
- "'volumes_space_reclaimed' in result"
# builder_cache
- "'builder_cache_space_reclaimed' in result"
Expand Down

0 comments on commit 37b0f5c

Please sign in to comment.