Fix ICMP MEG Ingress e2es #4391
Conversation
|
@tssurya FYI |
There was a problem hiding this comment.
I might have missed that discussion. I'd rather not have us bulk up our repo unless you have motivations/reasons for doing this which are missing in the commit message. Can you please fill me in?
My reason for not wanting vendoring upstream is:
- Vendoring is usually needed if the go get or go download that needs internet access so if that connection doesn't work properly for whatever reason then its best to have all that code locally downloaded and shipped; so I see the need to do this in our core code but for tests ..?
- u/s tests have been working fine without vendoring in all this?
- it might have been something to do with d/s need? I have a vague memory but forgetting details; if we can avoid it let's do a d/s specific solution and not do it upstream.
There was a problem hiding this comment.
2M lines of code no less 😮
There was a problem hiding this comment.
kind of a selfish reason that it messes up my IDE, and i have to fix it everytime I check out a different branch (downstream or upstream) and then have to go mod vendor manually. I'll remove the commit.
| @@ -448,7 +448,8 @@ var _ = ginkgo.Describe("External Gateway", func() { | |||
|
|
|||
| ginkgo.By(fmt.Sprintf("Verifying connectivity to the pod [%s] from external gateways", addresses.srcPodIP)) | |||
| for _, gwContainer := range gwContainers { | |||
| _, err := runCommand(containerRuntime, "exec", gwContainer, "ping", "-c", testTimeout, addresses.srcPodIP) | |||
| // Ping from a common IP address that exists on both gateways to ensure test coverage where ingress reply goes back to the same host. | |||
There was a problem hiding this comment.
old traffic path was: externalGWContainer to pod inside the cluster ingress ping test ? and new traffic path check is
ping test to use a common
IP address that is emulated on a loopback acting as though it lives
behind the gateways. This ensures that packets sent from either gateway
will have the same hash.
the same loopback IP get's added to both the externalGW containers? and then we ping the same podIPs inside the cluster?
so instead of doing IPA->podIP and IPB->podIP we do IPLoopBack->podIP from gatewayA and same IPLoopBack->podIP from gatewayB so that the 3 tuple hash for ICMP is the same for dp_hash?
the -I targetIPs confuses me because -I is the srcIP here for ingress traffic (-I interface address
Set source address to specified interface address. Argument may be numeric IP address or name of device. When pinging IPv6 link-local address this option is required.) but the variable name is targetIPs which I guess is used in second half of the test
There was a problem hiding this comment.
yeah. I added the comment to try to make it less confusing.
tssurya
added
area/e2e-testing
feature/egress-gateway
This modifies the sender address for ingress ping test to use a common IP address that is emulated on a loopback acting as though it lives behind the gateways. This ensures that packets sent from either gateway will have the same hash. The point of this is to exercise test coverage that auto last hop is working correctly. If it is not working, then the ping reply will only go back to one of the gateways instead of both. Signed-off-by: Tim Rozet <[email protected]>
|
unrelated flakes: https://github.com/ovn-org/ovn-kubernetes/actions/runs/9271215057/job/25507789464?pr=4391 upgrade: https://github.com/ovn-org/ovn-kubernetes/actions/runs/9271215057/job/25507784564?pr=4391 ran over 2hrs and ended up getting cancelled both have nothing to do with the changes in this PR. I am going to merge this. |
Also, vendor the test directory like I think we agreed on a while ago and never did it.