docker_network IPAM test / general docker test cleanup

[felixfontein]

SUMMARY

The test had to be deactivated (in #50477) since it caused a lot of spurious failures; see also here.

Fixes #50527, fixes #50478.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

docker_network

[felixfontein]

(Copying from #50354, since that's closed now)

CC @mattclay as he knows a lot more about the CI system.

@smueller18 I think this can only happen in one of two instances:

1. An earlier run of the test died and the network wasn't cleaned up; this should only happen in case the test run was interrupted (or the instance of the docker_network module used was horribly broken, but there hasn't been any such change to it recently) so that the test role didn't finish (it contains cleanup code). I don't know what happens if running tests are cancelled;

   @mattclay is it possible that cancelling interrupts the role and doesn't run cleanup? And if that happens, is it possible that the same CI node is used for another test (without "cleaning up" docker in some way first)?

2. Two docker_network tests are running on the same CI node simulatenously, or are somehow else using the same docker daemon. @mattclay is this possible?

[ansibot]

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

[felixfontein]

Idea: modify setup_docker to clean up docker:

• force remove all containers starting with ansible-test-
• force remove all networks starting with ansible-test-
• force remove all volumes starting with ansible-test-

That should prevent errors like left-over containers or networks from before reserving resources (like IP addresses, subnets, ...) which are needed.

(I think the CI system already clears up docker containers, but probably doesn't clean up docker networks or docker volumes.)

[felixfontein]

I think this should do it (at least as a first approximation).

ready_for_review

[smueller18]

I would wait with merging until we get more information about the underlying CI system.

(Copying from #50354, since that's closed now)

CC @mattclay as he knows a lot more about the CI system.

@smueller18 I think this can only happen in one of two instances:

1. An earlier run of the test died and the network wasn't cleaned up; this should only happen in case the test run was interrupted (or the instance of the docker_network module used was horribly broken, but there hasn't been any such change to it recently) so that the test role didn't finish (it contains cleanup code). I don't know what happens if running tests are cancelled;
   @mattclay is it possible that cancelling interrupts the role and doesn't run cleanup? And if that happens, is it possible that the same CI node is used for another test (without "cleaning up" docker in some way first)?
2. Two docker_network tests are running on the same CI node simulatenously, or are somehow else using the same docker daemon. @mattclay is this possible?

If 2 is possible, than cleaning up docker networks might cause other issues. Have a look at

ansible/test/integration/targets/docker_network/tasks/tests/ipam.yml

Lines 63 to 114 in 5091ba4

	- name: Create network with custom IPAM config
	docker_network:
	name: "{{ nname_ipam_1 }}"
	ipam_config:
	- subnet: 172.3.27.0/24
	gateway: 172.3.27.2
	iprange: 172.3.27.0/26
	aux_addresses:
	host1: 172.3.27.3
	host2: 172.3.27.4
	register: network
	- assert:
	that:
	- network is changed
	- name: Change subnet, gateway, iprange and auxiliary addresses of network with custom IPAM config
	docker_network:
	name: "{{ nname_ipam_1 }}"
	ipam_config:
	- subnet: 172.3.28.0/24
	gateway: 172.3.28.2
	iprange: 172.3.28.0/26
	aux_addresses:
	host1: 172.3.28.3
	register: network
	diff: yes
	- assert:
	that:
	- network is changed
	- network.diff.differences | length == 4
	- '"ipam_config[0].subnet" in network.diff.differences'
	- '"ipam_config[0].gateway" in network.diff.differences'
	- '"ipam_config[0].iprange" in network.diff.differences'
	- '"ipam_config[0].aux_addresses" in network.diff.differences'
	- name: Remove gateway and iprange of network with custom IPAM config
	docker_network:
	name: "{{ nname_ipam_1 }}"
	ipam_config:
	- subnet: 172.3.28.0/24
	register: network
	- assert:
	that:
	- network is not changed
	- name: Cleanup network with custom IPAM config
	docker_network:
	name: "{{ nname_ipam_1 }}"
	state: absent

Imagine there are 2 tests, both with docker_network and sharing the same docker host. Test 1 is creating and modifying the docker network in multiple steps (see script excerpt). So there is a little time window of some seconds the network must not be deleted). At this moment, test 2 starts running and therefore force the cleanup of every network (also that one test 1 is working on). Then the steps of test 1 that are not executed so far will fail because the network was removed in the meantime.

edit:
Just figured out that the same behavior is happening if 2 tests run in parallel with a minimal offset.

[smueller18]

While I developed the tests for the IPAM options, I overtook the existing IP addresses for tests. But the chosen IPv4 addresses are not in the private network segments described in https://en.wikipedia.org/wiki/Private_network#Private_IPv4_addresses. There could be an interference.

[smueller18]

Here are all observed tests failures that appeared so far (that they do not get lost in the other closed issues):

docker.errors.APIError: 500 Server Error: Internal Server Error ("failed to allocate gateway (172.3.28.2): Address already in use")

docker.errors.APIError: 403 Client Error: Forbidden ("cannot create network 6cf2f23a9499ff3904eb21d0524250fd5f7abae8fff42dfdd1cc794c4643b9b8 (br-6cf2f23a9499): conflicts with network d501a521181cc354889484d36dd0901e3f035a07589bf48fee2add1b27abb57c (br-d501a521181c): networks have overlapping IPv6")

[mattclay]

@felixfontein You are correct. If a Shippable run is cancelled, none of our cleanup code will execute. Test nodes on Shippable are used for running only one job at a time, so there's no need to worry about parallel execution on a single host.

[felixfontein]

Great, thanks! I think this is ready to be merged, then. @smueller18 any more objections? :)

[smueller18]

@felixfontein LGTM

[jborean93]

Thanks @felixfontein and @smueller18 for working through this one.

[felixfontein]

Thanks @smueller18 @jborean93 @mattclay for suggestions and reviewing and merging!

[felixfontein]

I created a backport to stable-2.7 in #50696.