test/l4lb,nat46x64: Replace Kind/Helm with DinD #22653

brb · 2022-12-09T14:39:24Z

See commit msgs.

Successful run - https://github.com/cilium/cilium/actions/runs/3685163577/jobs/6235811777.

borkmann

💯

brb · 2022-12-13T11:48:06Z

The nat46x64 test case is failing after the rewrite :-(

The following curl fails - https://github.com/cilium/cilium/pull/22653/files#diff-a971eb8f1b155111301db7bc65b14d8f369bc570b09cec47f67ff24cbd54e914R125:

+[11:37:33] curl -o /dev/null 10.0.0.4:80

The cilium monitor output (started just before the curl):

<- network flow 0x75fdd831 , identity unknown->unknown state unknown ifindex eth0 orig-ip 0.0.0.0: 172.12.42.1:47222 -> 10.0.0.4:80 tcp SYN

xx drop (FIB lookup failed, 7) flow 0x0 to endpoint 0, ifindex 13, file 105:833, , identity unknown->unknown: [2001:db8:1::2]:47222 -> [2001:db8:1::3]:80 tcp SYN

Err=7 == BPF_FIB_LKUP_RET_NO_NEIGH.

The PERM neigh entry was added before:

+[11:36:52] nsenter -t 5725 -n ip neigh add 172.12.42.3 dev eth0 lladdr 02:42:ac:0c:2a:03
+[11:36:52] nsenter -t 5725 -n ip neigh add 2001:db8:1::3 dev eth0 lladdr 02:42:ac:0c:2a:03

~~@borkmann Any idea?~~

UPDATE: the neigh entries got removed:

+[11:54:11] docker exec -t lb-node ip neigh
172.12.42.1 dev eth0 lladdr 02:42:83:af:b2:42 used 0/0/0 probes 1 STALE
fe80::42:acff:fe0c:2a03 dev eth0 lladdr 02:42:ac:0c:2a:03 used 0/0/0 probes 0 STALE
fe80::1 dev eth0 lladdr 02:42:83:af:b2:42 router used 0/0/0 probes 1 STALE
2001:db8:1::1 dev eth0 lladdr 02:42:83:af:b2:42 router used 0/0/0 probes 4 STALE

borkmann · 2022-12-13T11:55:00Z

[...]

xx drop (FIB lookup failed, 7) flow 0x0 to endpoint 0, ifindex 13, file 105:833, , identity unknown->unknown: [2001:db8:1::2]:47222 -> [2001:db8:1::3]:80 tcp SYN

Err=7 == BPF_FIB_LKUP_RET_NO_NEIGH.

The PERM neigh entry was added before:

+[11:36:52] nsenter -t 5725 -n ip neigh add 172.12.42.3 dev eth0 lladdr 02:42:ac:0c:2a:03
+[11:36:52] nsenter -t 5725 -n ip neigh add 2001:db8:1::3 dev eth0 lladdr 02:42:ac:0c:2a:03

@borkmann Any idea?

Could you dump the next hop address that fib lookup returned? Maybe its different than expected.

The usage of Kind/Helm when testing the Cilium in the standalone L4LB mode started to be a source of confusion. In particular, this gave a wrong impression that in that mode Cilium still has a connectivity to the kube-apiserver. This is not true. Previously, we used Kind/Helm for these tests just to create Docker containers acting as nodes, and then to install Cilium. This can be achieved by simply using docker/dind (aka Docker-in-Docker), and then starting the L4LB Cilium by execing into the dind container and running "docker run". Signed-off-by: Martynas Pumputis <m@lambda.lt>

The same reasoning applies as in the previous commit. Signed-off-by: Martynas Pumputis <m@lambda.lt>

nbusseneau

LGTM. I like this change, clarifies the purpose of the tests.

aspsk

Left two questions, but more for my curiosity

aspsk · 2022-12-14T09:35:46Z

test/l4lb/test.sh

-PORT=$(echo ${HOSTPORT} | cut -d: -f2)
-
+# Wait until Docker is ready in the lb-node node
+while ! docker exec -t lb-node docker ps >/dev/null; do sleep 1; done


Does this test have external timeout? If not, you can do something like

TIMEOUT=60 # 1 minute or so for dummy in $(seq $TIMEOUT); do docker exec -t lb-node docker ps >/dev/null && break done if [ "$dummy" = "$TIMEOUT" ]; then blah-blah-blah exit 1 fi

aspsk · 2022-12-14T09:38:12Z

test/l4lb/test.sh

-kind delete cluster
+docker rm -f lb-node
+docker rm -f nginx
+docker network rm cilium-l4lb


May this test break any consequent test if we don't clean the docker setup? If yes, then we better do a trap EXIT to cleanup

tklauser

🚀

The DinD for the tests was introduced in [1]. However, it never made into the v1.12 branch which made the GHA always to fail. Fix this by taking the test.sh files from the main branch. [1]: #22653 Signed-off-by: Martynas Pumputis <m@lambda.lt>

The DinD for the tests was introduced in [1]. However, it never made into the v1.11 branch which made the GHA always to fail. Fix this by taking the test.sh files from the main branch. [1]: #22653 Signed-off-by: Martynas Pumputis <m@lambda.lt>

The DinD for the tests was introduced in [1]. However, it never made into the v1.12 branch which made the GHA always to fail. Fix this by taking the test.sh files from the main branch. [1]: #22653 Signed-off-by: Martynas Pumputis <m@lambda.lt>

The DinD for the tests was introduced in [1]. However, it never made into the v1.11 branch which made the GHA always to fail. Fix this by taking the test.sh files from the main branch. [1]: #22653 Signed-off-by: Martynas Pumputis <m@lambda.lt>

brb added release-note/ci This PR makes changes to the CI. needs-backport/1.13 labels Dec 9, 2022

brb force-pushed the pr/brb/ci-dind-l4lb branch from 87bc448 to 74a05da Compare December 9, 2022 14:43

borkmann approved these changes Dec 9, 2022

View reviewed changes

This was referenced Dec 9, 2022

cilium: completion of nat46/64 gateway #22421

Merged

daemon: Do not remove PERM L2 entries in L4LB #22676

Merged

brb force-pushed the pr/brb/ci-dind-l4lb branch from edcded8 to 66d2f41 Compare December 12, 2022 14:29

brb changed the title ~~test/l4lb: Replace Kind/Helm with DinD~~ test/l4lb,nat46x64: Replace Kind/Helm with DinD Dec 12, 2022

brb force-pushed the pr/brb/ci-dind-l4lb branch 2 times, most recently from cda609d to b7e39d5 Compare December 12, 2022 15:06

brb force-pushed the pr/brb/ci-dind-l4lb branch from 4092f1d to 3e926ee Compare December 13, 2022 11:59

brb added 2 commits December 13, 2022 15:52

test/nat46x64: Use DinD instead of Helm/Kind

72b70a8

The same reasoning applies as in the previous commit. Signed-off-by: Martynas Pumputis <m@lambda.lt>

brb force-pushed the pr/brb/ci-dind-l4lb branch from 3e926ee to 72b70a8 Compare December 13, 2022 14:52

brb marked this pull request as ready for review December 13, 2022 14:52

brb requested review from a team as code owners December 13, 2022 14:52

brb requested review from aspsk, tklauser and nbusseneau December 13, 2022 14:52

nbusseneau approved these changes Dec 13, 2022

View reviewed changes

aspsk approved these changes Dec 14, 2022

View reviewed changes

tklauser approved these changes Dec 14, 2022

View reviewed changes

brb added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 14, 2022

pchaigno merged commit ed94b24 into master Dec 14, 2022

pchaigno deleted the pr/brb/ci-dind-l4lb branch December 14, 2022 12:29

joestringer mentioned this pull request Dec 21, 2022

v1.13 backports 2022-12-21 #22822

Merged

joestringer mentioned this pull request Dec 22, 2022

CI: v1.13: Cilium L4LB XDP (ci-l4lb-1.13) fails Run LoadBalancing test: timed out waiting for the condition on pods #22833

Closed

joestringer added the backport/author The backport will be carried out by the author of the PR. label Dec 22, 2022

borkmann mentioned this pull request Jan 23, 2023

v1.13 backport 2023-01-23 #23242

Merged

2 tasks

borkmann added backport-pending/1.13 backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed needs-backport/1.13 labels Jan 23, 2023

joestringer mentioned this pull request Jan 24, 2023

Prepare for release v1.13.0-rc5 #23270

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test/l4lb,nat46x64: Replace Kind/Helm with DinD #22653

test/l4lb,nat46x64: Replace Kind/Helm with DinD #22653

Uh oh!

brb commented Dec 9, 2022 •

edited

Loading

Uh oh!

borkmann left a comment

Uh oh!

brb commented Dec 13, 2022 •

edited

Loading

Uh oh!

borkmann commented Dec 13, 2022

Uh oh!

nbusseneau left a comment

Uh oh!

aspsk left a comment

Uh oh!

aspsk Dec 14, 2022

Uh oh!

aspsk Dec 14, 2022

Uh oh!

tklauser left a comment

Uh oh!

Uh oh!

test/l4lb,nat46x64: Replace Kind/Helm with DinD #22653

test/l4lb,nat46x64: Replace Kind/Helm with DinD #22653

Uh oh!

Conversation

brb commented Dec 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

borkmann left a comment

Choose a reason for hiding this comment

Uh oh!

brb commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

borkmann commented Dec 13, 2022

Uh oh!

nbusseneau left a comment

Choose a reason for hiding this comment

Uh oh!

aspsk left a comment

Choose a reason for hiding this comment

Uh oh!

aspsk Dec 14, 2022

Choose a reason for hiding this comment

Uh oh!

aspsk Dec 14, 2022

Choose a reason for hiding this comment

Uh oh!

tklauser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brb commented Dec 9, 2022 •

edited

Loading

brb commented Dec 13, 2022 •

edited

Loading