-
Notifications
You must be signed in to change notification settings - Fork 23.7k
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
fix: Parse docker_service scale parameter to int #45508
fix: Parse docker_service scale parameter to int #45508
Conversation
de92c0a
to
0f4a73a
Compare
1a88cff
to
fe177f6
Compare
The test
|
fe177f6
to
1ee6238
Compare
I'm unsure how to prove that this works with tests as I saw no unit test file corresponding to Here's an example of the fix running locally:
|
1ee6238
to
55ecce9
Compare
bot_status |
Componentslib/ansible/modules/cloud/docker/docker_service.py Metadatawaiting_on: maintainer |
ready_for_review |
@chouseknecht any chance you could have a look at this? |
55ecce9
to
23ff14d
Compare
f9b4c0b
to
bd5e0f0
Compare
shipit (since the last one probably was cleared because of the rebase) |
bot_status |
1 similar comment
bot_status |
Componentslib/ansible/modules/cloud/docker/docker_service.py Metadatawaiting_on: maintainer |
@danihodovic ansibot seems to be pretty slow for a couple of days now... @dariko @jwitko @kassiansun @tbouvet Can anyone of you take a look here? |
LGTM |
Awesome! @danihodovic thanks for your work on this! @jwitko thanks for reviewing! @danihodovic Do you want to create backport PRs (for Ansible 2.6 and 2.7) for this? I can do that, but in case you want to try this yourself... :) Here are some instructions: https://docs.ansible.com/ansible/latest/community/development_process.html#backport-pull-request-process Also, we forgot to add a changelog for this fix. I guess that's ok for devel, but we should really add something for the backports. You can find an example here: https://github.com/ansible/ansible/blob/devel/changelogs/fragments/46595-docker_container-expected_ports.yml |
* fix: Parse docker_service scale parameter to int (#45508) * Changelog: add fragment for docker_service scale fix
* fix: Parse docker_service scale parameter to int (#45508) * Changelog: add fragment for docker_service scale fix
SUMMARY
Parses the
scale
parameter to an int. If scale is interpolated with jinja(e.g when performing arithmetics) it will be a string.
ISSUE TYPE
COMPONENT NAME
docker_service
ANSIBLE VERSION
ADDITIONAL INFORMATION
When passing
scale
to thedocker_service
module to start N number of containersit expects an int. For directly defined values, e.g
5
this works. However for morecomplex values, e.g
'{{ (ansible_processor_vcpus - 1) }}'
it doesn't because a stringis passed to the module due to jinja interpolation.
This commit resolves the issue by attempting to parse scale to an int and
failing with an informative error message if the value cannot be parsed.