From 120472032cb0f17e8bbea895b58d409166092c8e Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Fri, 6 Nov 2020 09:41:00 +0100 Subject: [PATCH 1/2] kube-proxy: treat ExternalIPs as ClusterIP Currently kube-proxy treat ExternalIPs differently depending on: - the traffic origin - if the ExternalIP is present or not in the system. It also depends on the CNI implementation to discriminate between local and non-local traffic. Since the ExternalIP belongs to a Service, we can avoid the roundtrip of sending outside the traffic originated in the cluster. Also, we leverage the new LocalTrafficDetector to detect the local traffic and not rely on the CNI implementations for this. --- pkg/proxy/iptables/proxier.go | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index 50d13c57308..7184dcb5048 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -1128,24 +1128,22 @@ func (proxier *Proxier) syncProxyRules() { ) destChain := svcXlbChain - // We have to SNAT packets to external IPs if externalTrafficPolicy is cluster. + // We have to SNAT packets to external IPs if externalTrafficPolicy is cluster + // and the traffic is NOT Local. Local traffic coming from Pods and Nodes will + // be always forwarded to the corresponding Service, so no need to SNAT + // If we can't differentiate the local traffic we always SNAT. if !svcInfo.OnlyNodeLocalEndpoints() { destChain = svcChain - writeLine(proxier.natRules, append(args, "-j", string(KubeMarkMasqChain))...) + // This masquerades off-cluster traffic to a External IP. + if proxier.localDetector.IsImplemented() { + writeLine(proxier.natRules, proxier.localDetector.JumpIfNotLocal(args, string(KubeMarkMasqChain))...) + } else { + writeLine(proxier.natRules, append(args, "-j", string(KubeMarkMasqChain))...) + } } + // Sent traffic bound for external IPs to the service chain. + writeLine(proxier.natRules, append(args, "-j", string(destChain))...) - // Allow traffic for external IPs that does not come from a bridge (i.e. not from a container) - // nor from a local process to be forwarded to the service. - // This rule roughly translates to "all traffic from off-machine". - // This is imperfect in the face of network plugins that might not use a bridge, but we can revisit that later. - externalTrafficOnlyArgs := append(args, - "-m", "physdev", "!", "--physdev-is-in", - "-m", "addrtype", "!", "--src-type", "LOCAL") - writeLine(proxier.natRules, append(externalTrafficOnlyArgs, "-j", string(destChain))...) - dstLocalOnlyArgs := append(args, "-m", "addrtype", "--dst-type", "LOCAL") - // Allow traffic bound for external IPs that happen to be recognized as local IPs to stay local. - // This covers cases like GCE load-balancers which get added to the local routing table. - writeLine(proxier.natRules, append(dstLocalOnlyArgs, "-j", string(destChain))...) } else { // No endpoints. writeLine(proxier.filterRules, From 9e4642211ac6dc239b6ec2335f86e87696bdedf4 Mon Sep 17 00:00:00 2001 From: DP19 Date: Mon, 2 Nov 2020 11:36:22 -0500 Subject: [PATCH 2/2] add e2e test for Service ExternalIPs --- test/e2e/framework/service/jig.go | 13 +++++++++++++ test/e2e/network/service.go | 31 +++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/test/e2e/framework/service/jig.go b/test/e2e/framework/service/jig.go index 5eaf83b7523..227c8721f9f 100644 --- a/test/e2e/framework/service/jig.go +++ b/test/e2e/framework/service/jig.go @@ -829,6 +829,10 @@ func testReachabilityOverClusterIP(clusterIP string, sp v1.ServicePort, execPod return nil } +func testReachabilityOverExternalIP(externalIP string, sp v1.ServicePort, execPod *v1.Pod) error { + return testEndpointReachability(externalIP, sp.Port, sp.Protocol, execPod) +} + func testReachabilityOverNodePorts(nodes *v1.NodeList, sp v1.ServicePort, pod *v1.Pod, clusterIP string) error { internalAddrs := e2enode.CollectAddresses(nodes, v1.NodeInternalIP) externalAddrs := e2enode.CollectAddresses(nodes, v1.NodeExternalIP) @@ -911,6 +915,7 @@ func testEndpointReachability(endpoint string, port int32, protocol v1.Protocol, func (j *TestJig) checkClusterIPServiceReachability(svc *v1.Service, pod *v1.Pod) error { clusterIP := svc.Spec.ClusterIP servicePorts := svc.Spec.Ports + externalIPs := svc.Spec.ExternalIPs err := j.waitForAvailableEndpoint(ServiceEndpointsTimeout) if err != nil { @@ -926,6 +931,14 @@ func (j *TestJig) checkClusterIPServiceReachability(svc *v1.Service, pod *v1.Pod if err != nil { return err } + if len(externalIPs) > 0 { + for _, externalIP := range externalIPs { + err = testReachabilityOverExternalIP(externalIP, servicePort, pod) + if err != nil { + return err + } + } + } } return nil } diff --git a/test/e2e/network/service.go b/test/e2e/network/service.go index a73a2d4a309..7de9953a2b6 100644 --- a/test/e2e/network/service.go +++ b/test/e2e/network/service.go @@ -1179,6 +1179,37 @@ var _ = SIGDescribe("Services", func() { framework.ExpectNoError(err) }) + /* + Create a ClusterIP service with an External IP that is not assigned to an interface. + The IP ranges here are reserved for documentation according to + [RFC 5737](https://tools.ietf.org/html/rfc5737) Section 3 and should not be used by any host. + */ + ginkgo.It("should be possible to connect to a service via ExternalIP when the external IP is not assigned to a node", func() { + serviceName := "externalip-test" + ns := f.Namespace.Name + externalIP := "203.0.113.250" + if framework.TestContext.ClusterIsIPv6() { + externalIP = "2001:DB8::cb00:71fa" + } + + jig := e2eservice.NewTestJig(cs, ns, serviceName) + + ginkgo.By("creating service " + serviceName + " with type=clusterIP in namespace " + ns) + clusterIPService, err := jig.CreateTCPService(func(svc *v1.Service) { + svc.Spec.Type = v1.ServiceTypeClusterIP + svc.Spec.ExternalIPs = []string{externalIP} + svc.Spec.Ports = []v1.ServicePort{ + {Port: 80, Name: "http", Protocol: v1.ProtocolTCP, TargetPort: intstr.FromInt(9376)}, + } + }) + framework.ExpectNoError(err) + err = jig.CreateServicePods(2) + framework.ExpectNoError(err) + execPod := e2epod.CreateExecPodOrFail(cs, ns, "execpod", nil) + err = jig.CheckServiceReachability(clusterIPService, execPod) + framework.ExpectNoError(err) + }) + // TODO: Get rid of [DisabledForLargeClusters] tag when issue #56138 is fixed. ginkgo.It("should be able to change the type and ports of a service [Slow] [DisabledForLargeClusters]", func() { // requires cloud load-balancer support