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

docker_container: Always recreates the container when restart_policy is changed #65993

Closed
idelsink opened this issue Dec 20, 2019 · 15 comments · Fixed by #66192
Closed

docker_container: Always recreates the container when restart_policy is changed #65993

idelsink opened this issue Dec 20, 2019 · 15 comments · Fixed by #66192
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. cloud docker has_pr This issue has an associated PR. module This issue/PR relates to a module. python3 support:community This issue/PR relates to code supported by the Ansible community.

Comments

@idelsink
Copy link

SUMMARY

The docker_container module recreates a container when the restart_policy is changed. This can be changed using the docker update command.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

docker_container

ANSIBLE VERSION
$ ansible --version
ansible 2.9.2
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/ingmar/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.7.5 (default, Oct 17 2019, 12:09:47) [GCC 9.2.1 20190827 (Red Hat 9.2.1-1)]
CONFIGURATION
$ ansible-config dump --only-changed
<empty>
OS / ENVIRONMENT
$ lsb_release -a
LSB Version:    :core-4.1-amd64:core-4.1-noarch
Distributor ID: Fedora
Description:    Fedora release 30 (Thirty)
Release:        30
Codename:       Thirty
STEPS TO REPRODUCE
  1. Run the following playbook:
$ ansible-playbook docker-container-update.yml
- name: Container update restart_policy
  hosts: localhost
  tasks:
    - name: Container with restart_policy
      docker_container:
        name: restart_policy_example
        image: alpine
        state: present
        restart_policy: unless-stopped
        entrypoint: top
  1. Change the restart_policy to always and run it again.
$ ansible-playbook docker-container-update.yml
- name: Container update restart_policy
  hosts: localhost
  tasks:
    - name: Container with restart_policy
      docker_container:
        name: restart_policy_example
        image: alpine
        state: present
        restart_policy: unless-stopped
        entrypoint: top
  1. Notice by using the docker ps command that the container is recreated instead of updated.
EXPECTED RESULTS

I expect the container to be update with the new restart_policy by using the docker update command:

$ docker update --restart=<new policy>
ACTUAL RESULTS

The container is recreated instead of updated.
I currently run a command task before the container which updates the command.

# Update docker restart policy
- name: Update docker container restart policy
  command: "docker update --restart={{ (restart_policy == false) | ternary('no', restart_policy) }} {{ container }}"
  ignore_errors: true
@idelsink idelsink changed the title docker_container: Always recreate the container when restart_policy is changed docker_container: Always recreates the container when restart_policy is changed Dec 20, 2019
@ansibot
Copy link
Contributor

ansibot commented Dec 20, 2019

Files identified in the description:
None

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Dec 20, 2019
@idelsink
Copy link
Author

!component docker_container

@idelsink
Copy link
Author

!component docker_container

Hmm I think that's not how it's supposed to work

@winem
Copy link
Contributor

winem commented Dec 21, 2019

!component =lib/ansible/modules/cloud/docker/

@winem
Copy link
Contributor

winem commented Dec 21, 2019

I am already working on a PR. Actually not a big deal but I have to put bit more time to do it right and not violate the currently very clear structure of the module.

@ansibot
Copy link
Contributor

ansibot commented Dec 21, 2019

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added cloud docker module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Dec 21, 2019
@winem
Copy link
Contributor

winem commented Dec 21, 2019

!component =lib/ansible/modules/cloud/docker/docker_container.py

@ansibot
Copy link
Contributor

ansibot commented Dec 21, 2019

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Dec 21, 2019

@felixfontein
Copy link
Contributor

felixfontein commented Dec 22, 2019

Just put docker_container in the COMPONENT NAME field the next time.

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Dec 22, 2019
@idelsink
Copy link
Author

I am already working on a PR. Actually not a big deal but I have to put bit more time to do it right and not violate the currently very clear structure of the module.

Ah sounds great! I'm looking forward for the PR!

Just put docker_container in the COMPONENT NAME field the next time.

Sure, will do! A markdown link is not working then, noted.

@winem
Copy link
Contributor

winem commented Jan 4, 2020

@idelsink the Docker documentation does not mention anything like a minimum required API version for example. Do you know any restrictions or dependencies like that?

I am just wondering why the restart policy was not handled like the other supported Docker update versions.

I'm quite busy at the moment but will create the PR end of next week, latest. :)

@felixfontein
Copy link
Contributor

@winem does that mean you didn't start with the PR so far? In that case, do you mind if I don't wait and create a PR now?

@winem
Copy link
Contributor

winem commented Jan 5, 2020

@felixfontein, sorry, was not sure if I get to it today but I actually just did and created the PR. Any review and feedback is highly appreciated.

@ansibot ansibot added the has_pr This issue has an associated PR. label Jan 5, 2020
felixfontein pushed a commit that referenced this issue Jan 6, 2020
…estart retries) wit… (#66192)

* #65993 - update restart policy (restart policy & restart retries) without restarting the container

* - proper indentation on the continuation-line
- set restart_policy to the correct value independent from the api version

* - move restart_policy definitions into the if block
- add a new variable for the restart_policy configuration value

* add changelog fragment

* typo; minus -> underscore

* rename changelog fragment to contain the correct module name

* rename restart_policy_config_value to just restart_policy and refer to the correct dict values
felixfontein pushed a commit to felixfontein/ansible that referenced this issue Jan 6, 2020
…icy & restart retries) wit… (ansible#66192)

* ansible#65993 - update restart policy (restart policy & restart retries) without restarting the container

* - proper indentation on the continuation-line
- set restart_policy to the correct value independent from the api version

* - move restart_policy definitions into the if block
- add a new variable for the restart_policy configuration value

* add changelog fragment

* typo; minus -> underscore

* rename changelog fragment to contain the correct module name

* rename restart_policy_config_value to just restart_policy and refer to the correct dict values

(cherry picked from commit 02c126f)
felixfontein pushed a commit to felixfontein/ansible that referenced this issue Jan 6, 2020
…icy & restart retries) wit… (ansible#66192)

* ansible#65993 - update restart policy (restart policy & restart retries) without restarting the container

* - proper indentation on the continuation-line
- set restart_policy to the correct value independent from the api version

* - move restart_policy definitions into the if block
- add a new variable for the restart_policy configuration value

* add changelog fragment

* typo; minus -> underscore

* rename changelog fragment to contain the correct module name

* rename restart_policy_config_value to just restart_policy and refer to the correct dict values

(cherry picked from commit 02c126f)
mattclay pushed a commit that referenced this issue Jan 10, 2020
…estart retries) wit… (#66192)

* #65993 - update restart policy (restart policy & restart retries) without restarting the container

* - proper indentation on the continuation-line
- set restart_policy to the correct value independent from the api version

* - move restart_policy definitions into the if block
- add a new variable for the restart_policy configuration value

* add changelog fragment

* typo; minus -> underscore

* rename changelog fragment to contain the correct module name

* rename restart_policy_config_value to just restart_policy and refer to the correct dict values

(cherry picked from commit 02c126f)
mattclay pushed a commit that referenced this issue Jan 10, 2020
…estart retries) wit… (#66192)

* #65993 - update restart policy (restart policy & restart retries) without restarting the container

* - proper indentation on the continuation-line
- set restart_policy to the correct value independent from the api version

* - move restart_policy definitions into the if block
- add a new variable for the restart_policy configuration value

* add changelog fragment

* typo; minus -> underscore

* rename changelog fragment to contain the correct module name

* rename restart_policy_config_value to just restart_policy and refer to the correct dict values

(cherry picked from commit 02c126f)
@ansible ansible locked and limited conversation to collaborators Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. cloud docker has_pr This issue has an associated PR. module This issue/PR relates to a module. python3 support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants