-
Notifications
You must be signed in to change notification settings - Fork 210
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.
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
connectivity: Fix iface derivation in encrypt tests #1304
Conversation
This commit fixes two issues: * grep's "-P" is not available on busybox. * In the direct routing mode, "ip r g $DST_POD_IP from $SRC_POD_IP" fails with "RTNETLINK answers: Network unreachable". Fixes: 998ef8a ("connectivity: Add encryption test") Signed-off-by: Martynas Pumputis <[email protected]>
@@ -59,8 +59,8 @@ func (s *podToPodEncryption) Run(ctx context.Context, t *check.Test) { | |||
iface = "cilium_" + tunnelFeat.Mode // E.g. cilium_vxlan | |||
} else { | |||
cmd := []string{"/bin/sh", "-c", | |||
fmt.Sprintf("ip -o r g %s from %s | grep -oP '(?<=dev )[^ ]+'", | |||
server.Pod.Status.PodIP, client.Pod.Status.PodIP)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did the tests pass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the direct routing mode, "ip r g $DST_POD_IP from $SRC_POD_IP"
fails with "RTNETLINK answers: Network unreachable".
Do you know why? The revert means it may break on EKS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did the tests pass?
The direct routing was never tested. However, it's going to change in cilium/cilium#22803.
Do you know why? The revert means it may break on EKS.
My initial thought was that the rp_filter is to blame, but setting it to 0 didn't resolve the issue. I haven't spent much time digging into, as for now my priority is to get many different DP configurations tested "on prem" (=Kind on LVH VMs). Once it's been done - managed K8s. However, it doesn't mean that others cannot pick it meanwhile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Might be worth creating an issue for that on this repository, just so we don't waste time when we start running this test on EKS in > 6months.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth creating an issue for that on this repository
Yep, I'm going to do in a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for posterity - #1306.
The CI with the changes passed - https://github.com/cilium/cilium/actions/runs/3739715885/jobs/6347256459. |
Multicluster hit the infamous 1.1.1.1 flake. |
This commit fixes two issues:
Fixes: 998ef8a ("connectivity: Add encryption test")
Signed-off-by: Martynas Pumputis [email protected]