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

Make docker_swarm_service option publish.published_port optional #101

Conversation

sgpinkus
Copy link
Contributor

@sgpinkus sgpinkus commented Mar 12, 2021

SUMMARY

Change docker_swarm_service option publish.published_ported to optional. If not specified random high port is assigned by docker. Fixes #99.

ISSUE TYPE
  • Bugfix Pull Request
  • Feature Pull Request
COMPONENT NAME

docker_swarm_service

ADDITIONAL INFORMATION

This now works A free high port above 30000 will be assigned by docker:

- hosts: swarm-master
  tasks:
    - name: add swarm service
      community.docker.docker_swarm_service:
        state: present
        name: my-test-service
        replicas: 1
        image: nginx
        publish:
          - target_port: 80

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This needs a changelog fragment.
  2. The remaining sanity error (no-log-required) will be fixed by docker_swarm_service: flag secrets option as not secret #102, just ignore it.

CC maintainers @dariko @jwitko @hannseman @WojciechowskiPiotr

@WojciechowskiPiotr
Copy link
Collaborator

Happy to merge it when the changelog is updated.

@sgpinkus sgpinkus force-pushed the service-published-port-optional branch from f2d79a5 to c89c72b Compare March 12, 2021 22:42
@sgpinkus
Copy link
Contributor Author

Added, and squashed into one commit. Thanks!

…ed to optional. If not specified random high port is assigned by docker.
@sgpinkus sgpinkus force-pushed the service-published-port-optional branch from c89c72b to b8ea529 Compare March 12, 2021 22:46
@WojciechowskiPiotr
Copy link
Collaborator

@felixfontein can you take a look at why tests are still failing? #102 has been merged.

@felixfontein
Copy link
Collaborator

@WojciechowskiPiotr they failed because collection installation from galaxy failed (after three trials). That seems to happen a lot more since yesterday or so (ansible/galaxy#2429). Once the suggestion is merged tests will run another time anyway, and if some fail again I'll kick them until they pass :)

@felixfontein
Copy link
Collaborator

Happy to merge it when the changelog is updated.

Everything looks good to me (but then I'm not that familiar with Docker swarm), so feel free to merge.

I'll create a new release once this is merged to get the security fix out.

@WojciechowskiPiotr WojciechowskiPiotr merged commit 5b74313 into ansible-collections:main Mar 13, 2021
@felixfontein
Copy link
Collaborator

@sgpinkus thanks for fixing this!
@WojciechowskiPiotr thanks for reviewing and merging!

@sgpinkus sgpinkus deleted the service-published-port-optional branch March 14, 2021 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker_swarm_service publish.published_port should not be mandatory.
3 participants