Skip to content
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.

Sign up for GitHub

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

iptables proxier: route local traffic to LB IPs to service chain #77523

Merged
merged 2 commits into from
May 31, 2019
Merged

iptables proxier: route local traffic to LB IPs to service chain #77523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b926fb9
iptables proxier: route local traffic to LB IPs to service chain
andrewsykim May 6, 2019
8dfd4de
add unit tests for -src-type=LOCAL from LB chain
andrewsykim May 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions pkg/proxy/iptables/proxier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor point - the comment here says "first rule", but you broke that :)

Can you put the new block after this block, and word the comments in similar fashion?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, thanks Tim!

// Service's ClusterIP instead. This happens whether or not we have local
// endpoints; only if clusterCIDR is specified
Expand Down
21 changes: 14 additions & 7 deletions pkg/proxy/iptables/proxier_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -942,7 +954,6 @@ func TestOnlyLocalNodePorts(t *testing.T) {
}

func onlyLocalNodePorts(t *testing.T, fp *Proxier, ipt *iptablestest.FakeIPTables) {
shouldLBTOSVCRuleExist := len(fp.clusterCIDR) > 0
svcIP := "10.20.30.41"
svcPort := 80
svcNodePort := 3001
Expand Down Expand Up @@ -1018,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)
Expand Down
3 changes: 2 additions & 1 deletion pkg/util/iptables/testing/fake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const (
ToDest = "--to-destination "
Recent = "recent "
MatchSet = "--match-set "
SrcType = "--src-type "
)

type Rule map[string]string
Expand Down Expand Up @@ -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
Expand Down