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

Add sysctls option to docker_swarm_service #836

Merged
merged 7 commits into from
Apr 30, 2024

Conversation

NairolfL
Copy link
Contributor

SUMMARY

Add sysctls options to docker_swarm_service, as discussed in #190

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

docker_swarm_service

ADDITIONAL INFORMATION

@NairolfL
Copy link
Contributor Author

This is my first time contributing to this project. Don't hesitate to tell me if I'm missing something / doing something wrong.

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.

Thanks for your contribution! Can you add a changelog fragment? Thanks.

You also need to add a new entry for sysctls to option_minimal_versions (line 2747). The required API version is apparently 1.40, and it has been added to Docker SDK for Python 6.0.0 according to docker/docker-py@58aa62b.

plugins/modules/docker_swarm_service.py Outdated Show resolved Hide resolved
@felixfontein
Copy link
Collaborator

You also need to adjust the integration tests. You probably have to add sysctls: null to https://github.com/ansible-collections/community.docker/blob/main/tests/integration/targets/docker_swarm_service/vars/main.yml.

Co-authored-by: Felix Fontein <[email protected]>
@felixfontein felixfontein added the docker-swarm Docker Swarm label Apr 18, 2024
@NairolfL
Copy link
Contributor Author

Thank you for the feedback, I made the requested changes

@felixfontein
Copy link
Collaborator

Thanks, looks good to me! If nobody objects, I'll merge this in ~a week.

@felixfontein felixfontein merged commit 368d616 into ansible-collections:main Apr 30, 2024
130 checks passed
@felixfontein
Copy link
Collaborator

@NairolfL thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docker-swarm Docker Swarm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants