From b926fb9d2b161642f9cd524aaa06d7dda5d5aab6 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Mon, 6 May 2019 17:19:31 -0400 Subject: [PATCH 1/2] iptables proxier: route local traffic to LB IPs to service chain Signed-off-by: Andrew Sy Kim --- pkg/proxy/iptables/proxier.go | 10 ++++++++++ pkg/proxy/iptables/proxier_test.go | 5 ++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go index e4b13b17c9f..ba64d48d178 100644 --- a/pkg/proxy/iptables/proxier.go +++ b/pkg/proxy/iptables/proxier.go @@ -1202,6 +1202,16 @@ func (proxier *Proxier) syncProxyRules() { continue } + // For LBs with externalTrafficPolicy=Local, we need to re-route any local traffic to the service chain masqueraded. + // Masqueraded traffic in this scenario is okay since source IP preservation only applies to external traffic anyways. + args = append(args[:0], "-A", string(svcXlbChain)) + writeLine(proxier.natRules, append(args, + "-m", "comment", "--comment", fmt.Sprintf(`"masquerade LOCAL traffic for %s LB IP"`, svcNameString), + "-m", "addrtype", "--src-type", "LOCAL", "-j", string(KubeMarkMasqChain))...) + writeLine(proxier.natRules, append(args, + "-m", "comment", "--comment", fmt.Sprintf(`"route LOCAL traffic for %s LB IP to service chain"`, svcNameString), + "-m", "addrtype", "--src-type", "LOCAL", "-j", string(svcChain))...) + // First rule in the chain redirects all pod -> external VIP traffic to the // Service's ClusterIP instead. This happens whether or not we have local // endpoints; only if clusterCIDR is specified diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index c096753d0d9..1eb672e3756 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -942,7 +942,10 @@ func TestOnlyLocalNodePorts(t *testing.T) { } func onlyLocalNodePorts(t *testing.T, fp *Proxier, ipt *iptablestest.FakeIPTables) { - shouldLBTOSVCRuleExist := len(fp.clusterCIDR) > 0 + // LB to SVC rule should always exist for local only since + // any traffic with `--src-type LOCAL` now routes to service chain + shouldLBTOSVCRuleExist := true + svcIP := "10.20.30.41" svcPort := 80 svcNodePort := 3001 From 8dfd4def99ea1cc6296b25bc2121af9a72661fdf Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Tue, 7 May 2019 15:13:37 -0400 Subject: [PATCH 2/2] add unit tests for -src-type=LOCAL from LB chain Signed-off-by: Andrew Sy Kim --- pkg/proxy/iptables/proxier_test.go | 24 ++++++++++++++---------- pkg/util/iptables/testing/fake.go | 3 ++- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go index 1eb672e3756..7baa2ec2e45 100644 --- a/pkg/proxy/iptables/proxier_test.go +++ b/pkg/proxy/iptables/proxier_test.go @@ -424,6 +424,18 @@ func hasJump(rules []iptablestest.Rule, destChain, destIP string, destPort int) return match } +func hasSrcType(rules []iptablestest.Rule, srcType string) bool { + for _, r := range rules { + if r[iptablestest.SrcType] != srcType { + continue + } + + return true + } + + return false +} + func TestHasJump(t *testing.T) { testCases := map[string]struct { rules []iptablestest.Rule @@ -942,10 +954,6 @@ func TestOnlyLocalNodePorts(t *testing.T) { } func onlyLocalNodePorts(t *testing.T, fp *Proxier, ipt *iptablestest.FakeIPTables) { - // LB to SVC rule should always exist for local only since - // any traffic with `--src-type LOCAL` now routes to service chain - shouldLBTOSVCRuleExist := true - svcIP := "10.20.30.41" svcPort := 80 svcNodePort := 3001 @@ -1021,12 +1029,8 @@ func onlyLocalNodePorts(t *testing.T, fp *Proxier, ipt *iptablestest.FakeIPTable if hasJump(lbRules, nonLocalEpChain, "", 0) { errorf(fmt.Sprintf("Found jump from lb chain %v to non-local ep %v", lbChain, epStrLocal), lbRules, t) } - if hasJump(lbRules, svcChain, "", 0) != shouldLBTOSVCRuleExist { - prefix := "Did not find " - if !shouldLBTOSVCRuleExist { - prefix = "Found " - } - errorf(fmt.Sprintf("%s jump from lb chain %v to svc %v", prefix, lbChain, svcChain), lbRules, t) + if !hasJump(lbRules, svcChain, "", 0) || !hasSrcType(lbRules, "LOCAL") { + errorf(fmt.Sprintf("Did not find jump from lb chain %v to svc %v with src-type LOCAL", lbChain, svcChain), lbRules, t) } if !hasJump(lbRules, localEpChain, "", 0) { errorf(fmt.Sprintf("Didn't find jump from lb chain %v to local ep %v", lbChain, epStrLocal), lbRules, t) diff --git a/pkg/util/iptables/testing/fake.go b/pkg/util/iptables/testing/fake.go index cb504f90471..66adc1a275e 100644 --- a/pkg/util/iptables/testing/fake.go +++ b/pkg/util/iptables/testing/fake.go @@ -34,6 +34,7 @@ const ( ToDest = "--to-destination " Recent = "recent " MatchSet = "--match-set " + SrcType = "--src-type " ) type Rule map[string]string @@ -113,7 +114,7 @@ func (f *FakeIPTables) GetRules(chainName string) (rules []Rule) { for _, l := range strings.Split(string(f.Lines), "\n") { if strings.Contains(l, fmt.Sprintf("-A %v", chainName)) { newRule := Rule(map[string]string{}) - for _, arg := range []string{Destination, Source, DPort, Protocol, Jump, ToDest, Recent, MatchSet} { + for _, arg := range []string{Destination, Source, DPort, Protocol, Jump, ToDest, Recent, MatchSet, SrcType} { tok := getToken(l, arg) if tok != "" { newRule[arg] = tok