docker_swarm_service: ensure idempotency when the user parameter is None #49235

dariko · 2018-11-28T08:41:17Z

SUMMARY

This solves the idempotency issue reported in #49199, when user parameter is None

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

docker_swarm_service.py

ADDITIONAL INFORMATION

ansibot · 2018-11-28T08:49:27Z

Hi @dariko, thank you for submitting this pull-request!

click here for bot help

ansibot · 2018-11-28T08:49:28Z

cc @akshay196 @danihodovic @felixfontein @jwitko @kassiansun @tbouvet
click here for bot help

felixfontein · 2018-11-28T20:59:43Z

lib/ansible/modules/cloud/docker/docker_swarm_service.py

@@ -696,7 +696,7 @@ def compare(self, os):
            differences.add('reserve_memory', parameter=self.reserve_memory, active=os.reserve_memory)
        if self.container_labels != os.container_labels:
            differences.add('container_labels', parameter=self.container_labels, active=os.container_labels)
-        if self.publish != os.publish:
+        if self.publish and self.publish != os.publish:


Why do you handle publish this way? If a user explicitly specifies publish: [], why should it behave differently than, for example, if the user explicitly specifies networks: []?

This was a change reverting problematic parts of #45674 (I've open #49285 to track the problem ) I should not have pushed this here.

I'm gonna push the correct changes.

felixfontein

Looks good! :)

felixfontein · 2018-11-29T19:39:55Z

Ah, I think you should add a little changelog. And maybe also add it to the documentation of user?

felixfontein · 2018-12-08T10:09:38Z

@dariko ping

dariko · 2018-12-09T09:27:31Z

@felixfontein I expanded the user parameter documentation and wrote the changelog entry

felixfontein

Almost there :)

changelogs/fragments/49235-docker_swarm_service-user-default.yaml

felixfontein · 2018-12-09T12:40:41Z

lib/ansible/modules/cloud/docker/docker_swarm_service.py

-    description: username or UID
+    description:
+    - username or UID.
+    - "If set to C(none) the docker daemon default value (or the one already


The value must be null (that's YAML's None), not none. It might be that Ansible does some magic to also convert "none" to None, but I guess it's safer to use the correct YAML keyword.

Thanks, I also wrote the same value in the changelog fragment.

felixfontein · 2018-12-09T14:55:26Z

changelogs/fragments/49235-docker_swarm_service-user-default.yaml

 minor_changes:
-  - 'docker_swarm_service: use docker defaults for the C(user) parameter if it is set to None'
+  - 'docker_swarm_service: use docker defaults for the C(user) parameter if it is set to C(null)'


Ah, sorry for not noticing it earlier, but here you have to use ReST syntax, i.e.

- 'docker_swarm_service: use docker defaults for the ``user`` parameter if it is set to ``null``'

No problem, thank you :)

felixfontein

LGTM shipit

felixfontein · 2018-12-10T08:42:58Z

@dariko thanks for your work! Do you want to create a backport for stable-2.7?
@amenonsen thanks for merging!

felixfontein · 2018-12-10T19:22:41Z

@dariko There's a merge window for 2.7.5 which ends today; I'll simply create a backport. I hope that's ok for you :)

…one (ansible#49235) * ensure idempotency for user set to None * Update `user` documentation and add changelog fragment * clarify changelog fragments and parameters documentation * use restructuredtext syntax in changelog fragment (cherry picked from commit b183eb4)

…one (#49235) * ensure idempotency for user set to None * Update `user` documentation and add changelog fragment * clarify changelog fragments and parameters documentation * use restructuredtext syntax in changelog fragment (cherry picked from commit b183eb4)

dariko · 2018-12-10T21:22:11Z

@felixfontein Thank you :)

…one (ansible#49235) * ensure idempotency for user set to None * Update `user` documentation and add changelog fragment * clarify changelog fragments and parameters documentation * use restructuredtext syntax in changelog fragment

dariko mentioned this pull request Nov 28, 2018

docker_swarm_service fails because of default "user: root" #49199

Closed

gundalow changed the title ~~ensure idempotency when the user, publish and update_order parameter are not specified~~ docker_swarm_service: ensure idempotency when the user, publish and update_order parameter are not specified Nov 28, 2018

gundalow removed needs_triage Needs a first human triage before being processed. labels Nov 28, 2018

felixfontein reviewed Nov 28, 2018

View reviewed changes

dariko force-pushed the issue_49199_workaround branch from ce6a1c7 to b2ce9c5 Compare November 29, 2018 10:22

dariko changed the title ~~docker_swarm_service: ensure idempotency when the user, publish and update_order parameter are not specified~~ docker_swarm_service: ensure idempotency when the user parameter is None Nov 29, 2018

felixfontein approved these changes Nov 29, 2018

View reviewed changes

felixfontein mentioned this pull request Dec 3, 2018

Docker Agenda/Task list ansible/community#408

Closed

ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Dec 7, 2018

ensure idempotency for user set to None

4958d4e

dariko force-pushed the issue_49199_workaround branch from b2ce9c5 to e178242 Compare December 9, 2018 08:24

ansibot removed owner_pr This PR is made by the module's maintainer. small_patch stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 9, 2018

Update user documentation and add changelog fragment

b2e7fa8

dariko force-pushed the issue_49199_workaround branch from e178242 to b2e7fa8 Compare December 9, 2018 08:30

felixfontein reviewed Dec 9, 2018

View reviewed changes

clarify changelog fragments and parameters documentation

ccf9755

felixfontein reviewed Dec 9, 2018

View reviewed changes

use restructuredtext syntax in changelog fragment

29c5b11

felixfontein approved these changes Dec 9, 2018

View reviewed changes

ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Dec 9, 2018

amenonsen merged commit b183eb4 into ansible:devel Dec 10, 2018

felixfontein mentioned this pull request Dec 10, 2018

[2.7] docker_swarm_service: ensure idempotency when the user parameter is None #49742

Merged

ansible locked and limited conversation to collaborators Jul 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docker_swarm_service: ensure idempotency when the user parameter is None #49235

docker_swarm_service: ensure idempotency when the user parameter is None #49235

dariko commented Nov 28, 2018 •

edited

ansibot commented Nov 28, 2018

ansibot commented Nov 28, 2018

felixfontein Nov 28, 2018

dariko Nov 29, 2018 •

edited

felixfontein Nov 29, 2018

felixfontein left a comment •

edited

felixfontein commented Nov 29, 2018

felixfontein commented Dec 8, 2018

dariko commented Dec 9, 2018

felixfontein left a comment

felixfontein Dec 9, 2018

dariko Dec 9, 2018

felixfontein Dec 9, 2018

dariko Dec 9, 2018

felixfontein left a comment

felixfontein commented Dec 10, 2018

felixfontein commented Dec 10, 2018

dariko commented Dec 10, 2018

docker_swarm_service: ensure idempotency when the user parameter is None #49235

docker_swarm_service: ensure idempotency when the user parameter is None #49235

Conversation

dariko commented Nov 28, 2018 • edited

SUMMARY

ISSUE TYPE

COMPONENT NAME

ADDITIONAL INFORMATION

ansibot commented Nov 28, 2018

ansibot commented Nov 28, 2018

felixfontein Nov 28, 2018

Choose a reason for hiding this comment

dariko Nov 29, 2018 • edited

Choose a reason for hiding this comment

felixfontein Nov 29, 2018

Choose a reason for hiding this comment

felixfontein left a comment • edited

Choose a reason for hiding this comment

felixfontein commented Nov 29, 2018

felixfontein commented Dec 8, 2018

dariko commented Dec 9, 2018

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein Dec 9, 2018

Choose a reason for hiding this comment

dariko Dec 9, 2018

Choose a reason for hiding this comment

felixfontein Dec 9, 2018

Choose a reason for hiding this comment

dariko Dec 9, 2018

Choose a reason for hiding this comment

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein commented Dec 10, 2018

felixfontein commented Dec 10, 2018

dariko commented Dec 10, 2018

dariko commented Nov 28, 2018 •

edited

dariko Nov 29, 2018 •

edited

felixfontein left a comment •

edited