docker_container - #65993 - update restart policy (restart policy & restart retries) wit… #66192
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hout restarting the container
ansibot
added
affects_2.10
|
The test |
ansibot
added
needs_revision
felixfontein
reviewed
Jan 5, 2020
There was a problem hiding this comment.
Besides the comments below, you also need to add a changelog fragment: https://docs.ansible.com/ansible/latest/reference_appendices/release_and_maintenance.html#changelogs-how-to
Something like
bugfixes:
- "docker_container - changes to ``restart_policy`` and ``restart_retries`` now update the container instead of recreating it, if supported by the docker daemon."
felixfontein
changed the title
ansibot
removed
the
needs_triage
- set restart_policy to the correct value independent from the api version
- add a new variable for the restart_policy configuration value
|
All done. Many thanks for the review and all your input! Some of them are smart solutions to things I was not completely satisfied by before. |
ansibot
added
community_review
ansibot
added
the
needs_revision
felixfontein
reviewed
Jan 6, 2020
changelogs/fragments/65993-restart-docker-container-on-restart-policy-updates.yaml
Outdated
Show resolved
Hide resolved
ansibot
added
community_review
ansibot
added
automerge
|
@winem thanks a lot for fixing this! I'll create backport PRs so this fix will propagate to 2.9.x and 2.8.x. |
felixfontein
pushed a commit
to felixfontein/ansible
that referenced
this pull request
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 pull request
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)
|
Thanks for your input. This was really helpful to get "back" into programming and is appreciated. I'll take a look at your activities regarding the backport, too. Great chance to learn. |
mattclay
pushed a commit
that referenced
this pull request
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 pull request
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)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Fixes #65993 and allows to change the restart policy without restarting the container.
ISSUE TYPE
COMPONENT NAME
lib/ansible/modules/cloud/docker/docker_container.py
ADDITIONAL INFORMATION
I ran several test cases on my own dev environment and hope that I covered all cases.
Let me share one of the minified inventories to test it: