test: Fix fragment tracking test under KUBEPROXY=1 #11098
Conversation
pchaigno
added
kind/bug/CI
|
test-me-please |
There was a problem hiding this comment.
Thanks a lot! This looks great and looks correct in regard to my understanding of the issue.
The test does become more complicated to read, but I see no ideal solution. We could decouple the test function for the kubeproxy/kube-free cases, but we would end up with something longer in total and with a lot of duplicated code. Still, a few suggestions inline.
Could you please explain what you mean about the fromNode == helpers.K8s2 case? I didn't get your point :/.
|
Thanks for taking a look!
What about:
Do we really risk having flakes if we match on the 3-tuple instead of the 4-tuple? I've had tcpdump running for a long time during those tests and never saw anything else using UDP port 12345. I think it would simplify the test a lot. |
Probably not. I used as much as I could for a maximum of caution, but it sounds very unlikely that we get a duplicate flow by matching just on the 3-tuple. That's a good idea. |
The fragment tracking test sends a couple fragments to a K8s service and checks the ctmap content to ensure they were properly sent and received. It used to filter based on the 4-tuple. That is however making the test convoluted when adding support for kube-proxy to the test (new commit). This commit therefore switches to filtering based on the 3-tuple (source port, destination IP and port). Support for sending from k8s2 is also removed since it wasn't used. Signed-off-by: Paul Chaignon <paul@cilium.io>
The fragment tracking test currently fails when kube-proxy is enabled because the destination IP address and port are sometimes not DNATed. The awk filter on the ctmap dump output fails in that case. This commit fixes this issue by checking if kube-proxy is enabled and adapting the destination on which to match as a result. Fixes: #10929 Signed-off-by: Paul Chaignon <paul@cilium.io>
pchaigno
force-pushed
the
pr/pchaigno/fix-fragment-test-with-kubeproxy
branch
from
acd9ae4
to
e7c48a2
Compare
pchaigno
changed the title
|
I ran the fragment tracking test locally, 29 times with kube-proxy and 160 times without kube-proxy. Haven't seen any flakes, so matching on 3-tuple is probably safe. We could have issues once we start running the test in the context of the test suites though. |
|
We look at the number of packets right before and right after sending the datagram, so to have a conflict not only would we need to have another flow with the same IP and ports which is not very likely, but it would also have to send frames within a very short window. I think it's safe to match on the 3-tuple. |
First commit simplifies fragment tracking test to match on 3-tuple instead of 4-tuple. The second commit fixes the test to support
KUBEPROXY=1. See commit messages for details.Fixes: #10929
/cc @qmonnet