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_network: fix multiple subnet (of same IP version) idempotence #65839

Merged
merged 7 commits into from Dec 29, 2019
Merged

docker_network: fix multiple subnet (of same IP version) idempotence #65839

merged 7 commits into from Dec 29, 2019

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Fixes #65815.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

docker_network

@felixfontein felixfontein mentioned this pull request Dec 14, 2019
Closed
@ansibot
Copy link
Contributor

ansibot commented Dec 14, 2019

cc @DBendit @WojciechowskiPiotr @akshay196 @chouseknecht @danihodovic @dariko @jwitko @kassiansun @keitwb @olsaki @tbouvet
click here for bot help

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 bug This issue/PR relates to a bug. cloud community_review In order to be merged, this PR must follow the community review workflow. docker has_issue 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. test This PR relates to tests. labels Dec 14, 2019
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Dec 14, 2019
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 15, 2019
@felixfontein
Copy link
Contributor Author

ready_for_review

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. community_review In order to be merged, this PR must follow the community review workflow. and removed community_review In order to be merged, this PR must follow the community review workflow. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 16, 2019
@felixfontein
Copy link
Contributor Author

@WojciechowskiPiotr could you also take a look at this one, and #65854? Thanks :)

@ansibot 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 29, 2019
@WojciechowskiPiotr
Copy link
Contributor

Are you sure the new integration tests are correct and executed on Shippable? I think some looks skipped due to conditional reasons. Like test number 112. But it may be just a case of Fedora tests not working correctly.

@felixfontein
Copy link
Contributor Author

They are executed, but only in the VMs, i.e. rhel/7.6 and rhel/8.1 (https://app.shippable.com/github/ansible/ansible/runs/153788/104/console, https://app.shippable.com/github/ansible/ansible/runs/153788/105/console). macvlan networking doesn't work inside docker containers, unfortunately.

@WojciechowskiPiotr
Copy link
Contributor

They are executed, but only in the VMs, i.e. rhel/7.6 and rhel/8.1 (https://app.shippable.com/github/ansible/ansible/runs/153788/104/console, https://app.shippable.com/github/ansible/ansible/runs/153788/105/console). macvlan networking doesn't work inside docker containers, unfortunately.

ok :)

@WojciechowskiPiotr
Copy link
Contributor

shipit

@felixfontein
Copy link
Contributor Author

bot_status

@ansibot
Copy link
Contributor

ansibot commented Dec 29, 2019

Components

changelogs/fragments/65839-docker_network-idempotence.yml
support: community
maintainers:

lib/ansible/modules/cloud/docker/docker_network.py
support: community
maintainers: DBendit WojciechowskiPiotr akshay196 chouseknecht danihodovic dariko felixfontein jwitko kassiansun keitwb olsaki tbouvet

test/integration/targets/docker_network/tasks/tests/ipam.yml
support: community
maintainers: DBendit WojciechowskiPiotr akshay196 chouseknecht danihodovic dariko felixfontein jwitko kassiansun keitwb olsaki tbouvet

test/units/modules/cloud/docker/test_docker_network.py
support: community
maintainers: DBendit WojciechowskiPiotr akshay196 danihodovic dariko felixfontein jwitko kassiansun tbouvet

Metadata

waiting_on: maintainer
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): WojciechowskiPiotr felixfontein
shipit_actors_other: []
automerge: automerge ci_stale test failed

click here for bot help

@ansibot 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. needs_triage Needs a first human triage before being processed. 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 29, 2019
@felixfontein felixfontein merged commit 17ef253 into ansible:devel Dec 29, 2019
@felixfontein felixfontein deleted the docker_network-idempotence branch December 29, 2019 22:16
@felixfontein
Copy link
Contributor Author

@WojciechowskiPiotr thanks a lot for reviewing this!

felixfontein added a commit to felixfontein/ansible that referenced this pull request Dec 29, 2019
@felixfontein
docker_network: fix multiple subnet (of same IP version) idempotence (a…
3914246 
…nsible#65839)

* Fix multiple subnet (of same IP version) idempotence for docker_network.

* Add changelog.

* Unit tests no longer make sense, since the part of the code they test has been removed.

* Re-add CIDR validation. Move it to better position (module setup instead of idempotence check).

* Update changelog.

* Only run new tests on VM test images.

* Actually do what is documented. Especially since an empty object is a valid value for aux_addresses.

(cherry picked from commit 17ef253)
felixfontein added a commit to felixfontein/ansible that referenced this pull request Dec 29, 2019
@felixfontein
docker_network: fix multiple subnet (of same IP version) idempotence (a…
debab10 
…nsible#65839)

* Fix multiple subnet (of same IP version) idempotence for docker_network.

* Add changelog.

* Unit tests no longer make sense, since the part of the code they test has been removed.

* Re-add CIDR validation. Move it to better position (module setup instead of idempotence check).

* Update changelog.

* Only run new tests on VM test images.

* Actually do what is documented. Especially since an empty object is a valid value for aux_addresses.

(cherry picked from commit 17ef253)
This was referenced Dec 29, 2019
Merged
Merged
countless-integers pushed a commit to countless-integers/ansible that referenced this pull request Jan 6, 2020
@felixfontein @countless-integers
docker_network: fix multiple subnet (of same IP version) idempotence (a…
b063a58 
…nsible#65839)

* Fix multiple subnet (of same IP version) idempotence for docker_network.

* Add changelog.

* Unit tests no longer make sense, since the part of the code they test has been removed.

* Re-add CIDR validation. Move it to better position (module setup instead of idempotence check).

* Update changelog.

* Only run new tests on VM test images.

* Actually do what is documented. Especially since an empty object is a valid value for aux_addresses.
mattclay pushed a commit that referenced this pull request Jan 10, 2020
@felixfontein @mattclay
docker_network: fix multiple subnet (of same IP version) idempotence (#…
519846a 
…65839)

* Fix multiple subnet (of same IP version) idempotence for docker_network.

* Add changelog.

* Unit tests no longer make sense, since the part of the code they test has been removed.

* Re-add CIDR validation. Move it to better position (module setup instead of idempotence check).

* Update changelog.

* Only run new tests on VM test images.

* Actually do what is documented. Especially since an empty object is a valid value for aux_addresses.

(cherry picked from commit 17ef253)
mattclay pushed a commit that referenced this pull request Jan 10, 2020
@felixfontein @mattclay
docker_network: fix multiple subnet (of same IP version) idempotence (#…
6586623 
…65839)

* Fix multiple subnet (of same IP version) idempotence for docker_network.

* Add changelog.

* Unit tests no longer make sense, since the part of the code they test has been removed.

* Re-add CIDR validation. Move it to better position (module setup instead of idempotence check).

* Update changelog.

* Only run new tests on VM test images.

* Actually do what is documented. Especially since an empty object is a valid value for aux_addresses.

(cherry picked from commit 17ef253)
@ansible ansible locked and limited conversation to collaborators Jan 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
affects_2.10 This issue/PR affects Ansible v2.10 bug This issue/PR relates to a bug. cloud docker has_issue module This issue/PR relates to a module. shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

docker_network with multiple subnets always changes
3 participants
@felixfontein @ansibot @WojciechowskiPiotr