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.

Sign up for GitHub

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

Jump to bottom

docker_container: fix various idempotency problems and non-working options #45905

Merged
merged 20 commits into from Sep 24, 2018
Merged

docker_container: fix various idempotency problems and non-working options #45905

merged 20 commits into from Sep 24, 2018

Conversation

felixfontein
Copy link
Contributor

SUMMARY

This is a subset of fixes from #44789 which fix various problems with idempotency, and some options which apparently never worked in the first place. It also adds tests for all module options which belong to the state of the docker container.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

docker_container

ANSIBLE VERSION
2.7.0

@felixfontein felixfontein mentioned this pull request Sep 20, 2018
Merged
@ansibot
Copy link
Contributor

ansibot commented Sep 20, 2018

cc @chouseknecht @cove @dusdanig @joshuaconner @kassiansun @softzilla @zfil
click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. cloud core_review In order to be merged, this PR must follow the core review workflow. docker module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 20, 2018
@felixfontein
Copy link
Contributor Author

bot_status

@ansibot
Copy link
Contributor

ansibot commented Sep 20, 2018

Components

changelogs/fragments/docker_container-idempotency.yaml
support: core
maintainers:

lib/ansible/modules/cloud/docker/docker_container.py
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/files/env-file
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/tasks/main.yml
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/tasks/tests/options.yml
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/tasks/tests/regression-45700-dont-parse-on-absent.yml
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/tasks/tests/start-stop.yml
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 0
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 1
shipit_actors (maintainers or core team members): felixfontein
shipit_actors_other: []
automerge: automerge shipit test failed

click here for bot help

@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Sep 21, 2018
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Sep 21, 2018
@felixfontein felixfontein force-pushed the docker_container-idempotency-options-fixes branch from c63a702 to 69c4e83 Compare September 21, 2018 15:51
@felixfontein felixfontein force-pushed the docker_container-idempotency-options-fixes branch from 69c4e83 to 4c941b1 Compare September 21, 2018 20:12
@felixfontein
Copy link
Contributor Author

bot_status

@ansibot
Copy link
Contributor

ansibot commented Sep 23, 2018

Components

changelogs/fragments/docker_container-idempotency.yaml
support: core
maintainers:

lib/ansible/modules/cloud/docker/docker_container.py
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/files/env-file
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/tasks/main.yml
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/tasks/tests/options.yml
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/tasks/tests/regression-45700-dont-parse-on-absent.yml
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

test/integration/targets/docker_container/tasks/tests/start-stop.yml
support: community
maintainers: chouseknecht cove dusdanig felixfontein joshuaconner kassiansun softzilla zfil

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
too many files or commits: False
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): 1
community_shipits (namespace maintainers): 0
ansible_shipits (core team members): 1
shipit_actors (maintainers or core team members): felixfontein kassiansun
shipit_actors_other: []
automerge: automerge !module file(s) test failed

click here for bot help

@felixfontein felixfontein mentioned this pull request Sep 23, 2018
Closed
@felixfontein
Copy link
Contributor Author

@kassiansun Sorry, I had to add something more to avoid the use of not-yet-existing parameters for older docker-py versions.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed shipit This PR is ready to be merged by Core labels Sep 23, 2018
@felixfontein felixfontein force-pushed the docker_container-idempotency-options-fixes branch from dabf48c to 2c8c987 Compare September 23, 2018 16:42
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Sep 23, 2018
@felixfontein
Copy link
Contributor Author

@kassiansun Can you add another shipit? :)

This week there should be another 2.6.x release (the previous one was three weeks ago), it would be nice if we can get this in there.

@gundalow gundalow merged commit 7caf70d into ansible:devel Sep 24, 2018
@felixfontein felixfontein deleted the docker_container-idempotency-options-fixes branch September 24, 2018 11:14
@felixfontein
Copy link
Contributor Author

Thanks all for reviewing and merging! :)

This was referenced Sep 24, 2018
Closed
Merged
felixfontein added a commit to felixfontein/ansible that referenced this pull request Sep 24, 2018
@felixfontein
docker_container: fix various idempotency problems and non-working op…
f3a2d22 
…tions (ansible#45905)

* Sorting args.

* Doing comparisons of options with container parameters in a more context-sensitive way.

This prevents unnecessary restarts, or missing restarts (f.ex. if parameters are removed from ``cmd``).

* Make blkio_weight work.

* Fix cap_drop idempotency problem.

* Making groups idempotent if it contains integers.

* Make cpuset_mems work.

* Make dns_opts work.

* Fixing log_opts: docker expects string values, returns error for integer.

* Adding tests from felixfontein/ansible-docker_container-test#2.

* Make uts work.

* Adding changelog entry.

* Forgot option security_opts.

* Fixing typo.

* Explain strict set(dict) comparison a bit more.

* Improving idempotency tests.

* Making dns_servers a list, since the ordering is relevant.

* Making dns_search_domains a list, since the ordering is relevant.

* Improving dns_search_domains/dns_servers.

* Fixing entrypoint test.

* Making sure options are only supported for correct docker-py versions.
@felixfontein felixfontein mentioned this pull request Sep 24, 2018
Merged
felixfontein added a commit to felixfontein/ansible that referenced this pull request Sep 24, 2018
@felixfontein
docker_container: fix various idempotency problems and non-working op…
8814ff3 
…tions (ansible#45905)

* Sorting args.

* Doing comparisons of options with container parameters in a more context-sensitive way.

This prevents unnecessary restarts, or missing restarts (f.ex. if parameters are removed from ``cmd``).

* Make blkio_weight work.

* Fix cap_drop idempotency problem.

* Making groups idempotent if it contains integers.

* Make cpuset_mems work.

* Make dns_opts work.

* Fixing log_opts: docker expects string values, returns error for integer.

* Adding tests from felixfontein/ansible-docker_container-test#2.

* Make uts work.

* Adding changelog entry.

* Forgot option security_opts.

* Fixing typo.

* Explain strict set(dict) comparison a bit more.

* Improving idempotency tests.

* Making dns_servers a list, since the ordering is relevant.

* Making dns_search_domains a list, since the ordering is relevant.

* Improving dns_search_domains/dns_servers.

* Fixing entrypoint test.

* Making sure options are only supported for correct docker-py versions.
mattclay pushed a commit that referenced this pull request Sep 25, 2018
@felixfontein @mattclay
docker_container: fix various idempotency problems and non-working op…
8562629 
…tions (#45905)

* Sorting args.

* Doing comparisons of options with container parameters in a more context-sensitive way.

This prevents unnecessary restarts, or missing restarts (f.ex. if parameters are removed from ``cmd``).

* Make blkio_weight work.

* Fix cap_drop idempotency problem.

* Making groups idempotent if it contains integers.

* Make cpuset_mems work.

* Make dns_opts work.

* Fixing log_opts: docker expects string values, returns error for integer.

* Adding tests from felixfontein/ansible-docker_container-test#2.

* Make uts work.

* Adding changelog entry.

* Forgot option security_opts.

* Fixing typo.

* Explain strict set(dict) comparison a bit more.

* Improving idempotency tests.

* Making dns_servers a list, since the ordering is relevant.

* Making dns_search_domains a list, since the ordering is relevant.

* Improving dns_search_domains/dns_servers.

* Fixing entrypoint test.

* Making sure options are only supported for correct docker-py versions.
felixfontein added a commit to felixfontein/ansible that referenced this pull request Oct 9, 2018
@felixfontein
docker_container: fix various idempotency problems and non-working op…
eb218a3 
…tions (ansible#45905)

* Sorting args.

* Doing comparisons of options with container parameters in a more context-sensitive way.

This prevents unnecessary restarts, or missing restarts (f.ex. if parameters are removed from ``cmd``).

* Make blkio_weight work.

* Fix cap_drop idempotency problem.

* Making groups idempotent if it contains integers.

* Make cpuset_mems work.

* Make dns_opts work.

* Fixing log_opts: docker expects string values, returns error for integer.

* Adding tests from felixfontein/ansible-docker_container-test#2.

* Make uts work.

* Adding changelog entry.

* Forgot option security_opts.

* Fixing typo.

* Explain strict set(dict) comparison a bit more.

* Improving idempotency tests.

* Making dns_servers a list, since the ordering is relevant.

* Making dns_search_domains a list, since the ordering is relevant.

* Improving dns_search_domains/dns_servers.

* Fixing entrypoint test.

* Making sure options are only supported for correct docker-py versions.
abadger pushed a commit that referenced this pull request Oct 10, 2018
@felixfontein @abadger
docker_container: fix various idempotency problems and non-working op…
1d01543 
…tions (#45905)

* Sorting args.

* Doing comparisons of options with container parameters in a more context-sensitive way.

This prevents unnecessary restarts, or missing restarts (f.ex. if parameters are removed from ``cmd``).

* Make blkio_weight work.

* Fix cap_drop idempotency problem.

* Making groups idempotent if it contains integers.

* Make cpuset_mems work.

* Make dns_opts work.

* Fixing log_opts: docker expects string values, returns error for integer.

* Adding tests from felixfontein/ansible-docker_container-test#2.

* Make uts work.

* Adding changelog entry.

* Forgot option security_opts.

* Fixing typo.

* Explain strict set(dict) comparison a bit more.

* Improving idempotency tests.

* Making dns_servers a list, since the ordering is relevant.

* Making dns_search_domains a list, since the ordering is relevant.

* Improving dns_search_domains/dns_servers.

* Fixing entrypoint test.

* Making sure options are only supported for correct docker-py versions.
@felixfontein felixfontein mentioned this pull request Oct 27, 2018
Merged
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kassiansun kassiansun kassiansun approved these changes

Assignees
No one assigned
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. cloud core_review In order to be merged, this PR must follow the core review workflow. docker module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@felixfontein @ansibot @kassiansun @gundalow @mkrizek