From 79394a1cbd4847cb64c82f15edb1c7eddc4407ba Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 8 May 2024 10:24:44 -0400 Subject: [PATCH 1/2] Don't require ICMP reject on UDP LB with no endpoints Allow either drop or reject; we previously made the same change for TCP load balancers. --- test/e2e/network/loadbalancer.go | 4 ++-- test/e2e/network/service.go | 14 -------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/test/e2e/network/loadbalancer.go b/test/e2e/network/loadbalancer.go index a2dbe8335bc..ab25df8a5d2 100644 --- a/test/e2e/network/loadbalancer.go +++ b/test/e2e/network/loadbalancer.go @@ -387,8 +387,8 @@ var _ = common.SIGDescribe("LoadBalancers", feature.LoadBalancer, func() { err = udpJig.Scale(ctx, 0) framework.ExpectNoError(err) - ginkgo.By("looking for ICMP REJECT on the UDP service's LoadBalancer") - testRejectedUDP(ctx, udpIngressIP, svcPort, loadBalancerCreateTimeout) + ginkgo.By("checking that the UDP service's LoadBalancer is not reachable") + testNotReachableUDP(ctx, udpIngressIP, svcPort, loadBalancerCreateTimeout) ginkgo.By("Scaling the pods to 1") err = udpJig.Scale(ctx, 1) diff --git a/test/e2e/network/service.go b/test/e2e/network/service.go index cea17a7167c..4a9cbb6969a 100644 --- a/test/e2e/network/service.go +++ b/test/e2e/network/service.go @@ -606,20 +606,6 @@ func testNotReachableUDP(ctx context.Context, host string, port int, timeout tim } } -// testRejectedUDP tests that the given host rejects a UDP request on the given port. -func testRejectedUDP(ctx context.Context, host string, port int, timeout time.Duration) { - pollfn := func(ctx context.Context) (bool, error) { - result := pokeUDP(host, port, "echo hello", &UDPPokeParams{Timeout: 3 * time.Second}) - if result.Status == UDPRefused { - return true, nil - } - return false, nil // caller can retry - } - if err := wait.PollUntilContextTimeout(ctx, framework.Poll, timeout, true, pollfn); err != nil { - framework.Failf("UDP service %v:%v not rejected: %v", host, port, err) - } -} - // TestHTTPHealthCheckNodePort tests a HTTP connection by the given request to the given host and port. func TestHTTPHealthCheckNodePort(ctx context.Context, host string, port int, request string, timeout time.Duration, expectSucceed bool, threshold int) error { count := 0 From 85d5b4bd4aa8702fdec2cec470e97efa2d39a8b0 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 8 May 2024 10:25:49 -0400 Subject: [PATCH 2/2] Skip source IP preservation checks for Proxy-type load balancers To be revisited --- test/e2e/network/loadbalancer.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/test/e2e/network/loadbalancer.go b/test/e2e/network/loadbalancer.go index ab25df8a5d2..863a84bc7dd 100644 --- a/test/e2e/network/loadbalancer.go +++ b/test/e2e/network/loadbalancer.go @@ -927,11 +927,6 @@ var _ = common.SIGDescribe("LoadBalancers", feature.LoadBalancer, func() { }) var _ = common.SIGDescribe("LoadBalancers ExternalTrafficPolicy: Local", feature.LoadBalancer, framework.WithSlow(), func() { - // FIXME: What are the expected semantics of requesting an - // "ExternalTrafficPolicy: Local" service from a cloud provider that does not - // support that? What are the expected semantics of "ExternalTrafficPolicy: Local" - // on `IPMode: Proxy`-type LoadBalancers? - f := framework.NewDefaultFramework("esipp") f.NamespacePodSecurityLevel = admissionapi.LevelBaseline var loadBalancerCreateTimeout time.Duration @@ -987,6 +982,14 @@ var _ = common.SIGDescribe("LoadBalancers ExternalTrafficPolicy: Local", feature framework.ExpectNoError(err) }) + // FIXME: figure out the actual expected semantics for + // "ExternalTrafficPolicy: Local" + "IPMode: Proxy". + // https://issues.k8s.io/123714 + ingress := &svc.Status.LoadBalancer.Ingress[0] + if ingress.IP == "" || (ingress.IPMode != nil && *ingress.IPMode == v1.LoadBalancerIPModeProxy) { + e2eskipper.Skipf("LoadBalancer uses 'Proxy' IPMode") + } + svcTCPPort := int(svc.Spec.Ports[0].Port) ingressIP := e2eservice.GetIngressPoint(&svc.Status.LoadBalancer.Ingress[0]) @@ -1133,6 +1136,14 @@ var _ = common.SIGDescribe("LoadBalancers ExternalTrafficPolicy: Local", feature framework.ExpectNoError(err) }) + // FIXME: figure out the actual expected semantics for + // "ExternalTrafficPolicy: Local" + "IPMode: Proxy". + // https://issues.k8s.io/123714 + ingress := &svc.Status.LoadBalancer.Ingress[0] + if ingress.IP == "" || (ingress.IPMode != nil && *ingress.IPMode == v1.LoadBalancerIPModeProxy) { + e2eskipper.Skipf("LoadBalancer uses 'Proxy' IPMode") + } + ingressIP := e2eservice.GetIngressPoint(&svc.Status.LoadBalancer.Ingress[0]) port := strconv.Itoa(int(svc.Spec.Ports[0].Port)) ipPort := net.JoinHostPort(ingressIP, port)