Skip to content

Commit ed0d6ee

Browse files
authored
Merge pull request kubernetes#85617 from andrewsykim/optimize-external-ips
proxier: only get local addresses once per sync loop
2 parents 919871e + 1653476 commit ed0d6ee

File tree

42 files changed

+333
-416
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+333
-416
lines changed

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ require (
164164
k8s.io/repo-infra v0.0.1-alpha.1
165165
k8s.io/sample-apiserver v0.0.0
166166
k8s.io/system-validators v1.0.4
167-
k8s.io/utils v0.0.0-20191217005138-9e5e9d854fcc
167+
k8s.io/utils v0.0.0-20200117235808-5f6fbceb4c31
168168
sigs.k8s.io/kustomize v2.0.3+incompatible
169169
sigs.k8s.io/yaml v1.2.0
170170
)
@@ -563,7 +563,7 @@ replace (
563563
k8s.io/sample-cli-plugin => ./staging/src/k8s.io/sample-cli-plugin
564564
k8s.io/sample-controller => ./staging/src/k8s.io/sample-controller
565565
k8s.io/system-validators => k8s.io/system-validators v1.0.4
566-
k8s.io/utils => k8s.io/utils v0.0.0-20191217005138-9e5e9d854fcc
566+
k8s.io/utils => k8s.io/utils v0.0.0-20200117235808-5f6fbceb4c31
567567
modernc.org/cc => modernc.org/cc v1.0.0
568568
modernc.org/golex => modernc.org/golex v1.0.0
569569
modernc.org/mathutil => modernc.org/mathutil v1.0.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -608,8 +608,8 @@ k8s.io/repo-infra v0.0.1-alpha.1 h1:2us1n30u3cOcoPsacNfCvCssS9B9Yldr1ZGOdK0728U=
608608
k8s.io/repo-infra v0.0.1-alpha.1/go.mod h1:wO1t9WaB99V80ljbeENTnayuEEwNZt7gECYh/CEyOJ8=
609609
k8s.io/system-validators v1.0.4 h1:sW57tJ/ciqOVbbTLN+ZNy64MJMNqUuiwrirQv8IR2Kw=
610610
k8s.io/system-validators v1.0.4/go.mod h1:HgSgTg4NAGNoYYjKsUyk52gdNi2PVDswQ9Iyn66R7NI=
611-
k8s.io/utils v0.0.0-20191217005138-9e5e9d854fcc h1:MUttqhwRgupMiA5ps5F3d2/NLkU8EZSECTGxrQxqM54=
612-
k8s.io/utils v0.0.0-20191217005138-9e5e9d854fcc/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
611+
k8s.io/utils v0.0.0-20200117235808-5f6fbceb4c31 h1:KCcLuc/HD1RogJgEbZi9ObRuLv1bgiRCfAbidLKrUpg=
612+
k8s.io/utils v0.0.0-20200117235808-5f6fbceb4c31/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew=
613613
modernc.org/cc v1.0.0/go.mod h1:1Sk4//wdnYJiUIxnW8ddKpaOJCF37yAdqYnkxUpaYxw=
614614
modernc.org/golex v1.0.0/go.mod h1:b/QX9oBD/LhixY6NDh+IdGv17hgB+51fET1i2kPSmvk=
615615
modernc.org/mathutil v1.0.0/go.mod h1:wU0vUrJsVWBZ4P6e7xtFJEhFSNsfRLJ8H458uRjg03k=

pkg/proxy/iptables/proxier.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,16 @@ func (proxier *Proxier) syncProxyRules() {
796796
klog.V(4).Infof("syncProxyRules took %v", time.Since(start))
797797
}()
798798

799+
localAddrs, err := utilproxy.GetLocalAddrs()
800+
if err != nil {
801+
klog.Errorf("Failed to get local addresses during proxy sync: %v, assuming external IPs are not local", err)
802+
} else if len(localAddrs) == 0 {
803+
klog.Warning("No local addresses found, assuming all external IPs are not local")
804+
}
805+
806+
localAddrSet := utilnet.IPSet{}
807+
localAddrSet.Insert(localAddrs...)
808+
799809
// We assume that if this was called, we really want to sync them,
800810
// even if nothing changed in the meantime. In other words, callers are
801811
// responsible for detecting no-op changes and not calling this function.
@@ -848,7 +858,7 @@ func (proxier *Proxier) syncProxyRules() {
848858
// This will be a map of chain name to chain with rules as stored in iptables-save/iptables-restore
849859
existingFilterChains := make(map[utiliptables.Chain][]byte)
850860
proxier.existingFilterChainsData.Reset()
851-
err := proxier.iptables.SaveInto(utiliptables.TableFilter, proxier.existingFilterChainsData)
861+
err = proxier.iptables.SaveInto(utiliptables.TableFilter, proxier.existingFilterChainsData)
852862
if err != nil { // if we failed to get any rules
853863
klog.Errorf("Failed to execute iptables-save, syncing all rules: %v", err)
854864
} else { // otherwise parse the output
@@ -1030,9 +1040,7 @@ func (proxier *Proxier) syncProxyRules() {
10301040
// If the "external" IP happens to be an IP that is local to this
10311041
// machine, hold the local port open so no other process can open it
10321042
// (because the socket might open but it would never work).
1033-
if local, err := utilproxy.IsLocalIP(externalIP); err != nil {
1034-
klog.Errorf("can't determine if IP is local, assuming not: %v", err)
1035-
} else if local && (svcInfo.Protocol() != v1.ProtocolSCTP) {
1043+
if localAddrSet.Len() > 0 && (svcInfo.Protocol() != v1.ProtocolSCTP) && localAddrSet.Has(net.ParseIP(externalIP)) {
10361044
lp := utilproxy.LocalPort{
10371045
Description: "externalIP for " + svcNameString,
10381046
IP: externalIP,

pkg/proxy/ipvs/proxier.go

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -990,33 +990,6 @@ func (proxier *Proxier) OnNodeSynced() {
990990
// EntryInvalidErr indicates if an ipset entry is invalid or not
991991
const EntryInvalidErr = "error adding entry %s to ipset %s"
992992

993-
func getLocalAddrs() ([]net.IP, error) {
994-
var localAddrs []net.IP
995-
996-
addrs, err := net.InterfaceAddrs()
997-
if err != nil {
998-
return nil, err
999-
}
1000-
1001-
for _, addr := range addrs {
1002-
ip, _, err := net.ParseCIDR(addr.String())
1003-
if err != nil {
1004-
return nil, err
1005-
}
1006-
localAddrs = append(localAddrs, ip)
1007-
}
1008-
return localAddrs, nil
1009-
}
1010-
1011-
func ipExists(ip net.IP, addrs []net.IP) bool {
1012-
for _, addr := range addrs {
1013-
if ip.Equal(addr) {
1014-
return true
1015-
}
1016-
}
1017-
return false
1018-
}
1019-
1020993
// This is where all of the ipvs calls happen.
1021994
// assumes proxier.mu is held
1022995
func (proxier *Proxier) syncProxyRules() {
@@ -1036,11 +1009,16 @@ func (proxier *Proxier) syncProxyRules() {
10361009
klog.V(4).Infof("syncProxyRules took %v", time.Since(start))
10371010
}()
10381011

1039-
localAddrs, err := getLocalAddrs()
1012+
localAddrs, err := utilproxy.GetLocalAddrs()
10401013
if err != nil {
1041-
klog.Errorf("Failed to get local addresses during proxy sync: %v", err)
1014+
klog.Errorf("Failed to get local addresses during proxy sync: %v, assuming external IPs are not local", err)
1015+
} else if len(localAddrs) == 0 {
1016+
klog.Warning("No local addresses found, assuming all external IPs are not local")
10421017
}
10431018

1019+
localAddrSet := utilnet.IPSet{}
1020+
localAddrSet.Insert(localAddrs...)
1021+
10441022
// We assume that if this was called, we really want to sync them,
10451023
// even if nothing changed in the meantime. In other words, callers are
10461024
// responsible for detecting no-op changes and not calling this function.
@@ -1222,9 +1200,10 @@ func (proxier *Proxier) syncProxyRules() {
12221200

12231201
// Capture externalIPs.
12241202
for _, externalIP := range svcInfo.ExternalIPStrings() {
1225-
if len(localAddrs) == 0 {
1226-
klog.Errorf("couldn't find any local IPs, assuming %s is not local", externalIP)
1227-
} else if (svcInfo.Protocol() != v1.ProtocolSCTP) && ipExists(net.ParseIP(externalIP), localAddrs) {
1203+
// If the "external" IP happens to be an IP that is local to this
1204+
// machine, hold the local port open so no other process can open it
1205+
// (because the socket might open but it would never work).
1206+
if localAddrSet.Len() > 0 && (svcInfo.Protocol() != v1.ProtocolSCTP) && localAddrSet.Has(net.ParseIP(externalIP)) {
12281207
// We do not start listening on SCTP ports, according to our agreement in the SCTP support KEP
12291208
lp := utilproxy.LocalPort{
12301209
Description: "externalIP for " + svcNameString,

pkg/proxy/userspace/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ go_library(
3636
"//staging/src/k8s.io/cloud-provider/service/helpers:go_default_library",
3737
"//vendor/k8s.io/klog:go_default_library",
3838
"//vendor/k8s.io/utils/exec:go_default_library",
39+
"//vendor/k8s.io/utils/net:go_default_library",
3940
] + select({
4041
"@io_bazel_rules_go//go/platform:android": [
4142
"//vendor/golang.org/x/sys/unix:go_default_library",

pkg/proxy/userspace/proxier.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
"k8s.io/kubernetes/pkg/util/conntrack"
4242
"k8s.io/kubernetes/pkg/util/iptables"
4343
utilexec "k8s.io/utils/exec"
44+
netutils "k8s.io/utils/net"
4445
)
4546

4647
type portal struct {
@@ -127,6 +128,7 @@ type Proxier struct {
127128
listenIP net.IP
128129
iptables iptables.Interface
129130
hostIP net.IP
131+
localAddrs netutils.IPSet
130132
proxyPorts PortAllocator
131133
makeProxySocket ProxySocketFunc
132134
exec utilexec.Interface
@@ -371,6 +373,17 @@ func (proxier *Proxier) syncProxyRules() {
371373
proxier.unmergeService(change.previous, existingPorts)
372374
}
373375

376+
localAddrs, err := utilproxy.GetLocalAddrs()
377+
if err != nil {
378+
klog.Errorf("Failed to get local addresses during proxy sync: %s, assuming IPs are not local", err)
379+
} else if len(localAddrs) == 0 {
380+
klog.Warning("No local addresses were found, assuming all external IPs are not local")
381+
}
382+
383+
localAddrSet := netutils.IPSet{}
384+
localAddrSet.Insert(localAddrs...)
385+
proxier.localAddrs = localAddrSet
386+
374387
proxier.ensurePortals()
375388
proxier.cleanupStaleStickySessions()
376389
}
@@ -725,9 +738,7 @@ func (proxier *Proxier) openPortal(service proxy.ServicePortName, info *ServiceI
725738
}
726739

727740
func (proxier *Proxier) openOnePortal(portal portal, protocol v1.Protocol, proxyIP net.IP, proxyPort int, name proxy.ServicePortName) error {
728-
if local, err := utilproxy.IsLocalIP(portal.ip.String()); err != nil {
729-
return fmt.Errorf("can't determine if IP %s is local, assuming not: %v", portal.ip, err)
730-
} else if local {
741+
if proxier.localAddrs.Len() > 0 && proxier.localAddrs.Has(portal.ip) {
731742
err := proxier.claimNodePort(portal.ip, portal.port, protocol, name)
732743
if err != nil {
733744
return err
@@ -903,10 +914,7 @@ func (proxier *Proxier) closePortal(service proxy.ServicePortName, info *Service
903914

904915
func (proxier *Proxier) closeOnePortal(portal portal, protocol v1.Protocol, proxyIP net.IP, proxyPort int, name proxy.ServicePortName) []error {
905916
el := []error{}
906-
907-
if local, err := utilproxy.IsLocalIP(portal.ip.String()); err != nil {
908-
el = append(el, fmt.Errorf("can't determine if IP %s is local, assuming not: %v", portal.ip, err))
909-
} else if local {
917+
if proxier.localAddrs.Len() > 0 && proxier.localAddrs.Has(portal.ip) {
910918
if err := proxier.releaseNodePort(portal.ip, portal.port, protocol, name); err != nil {
911919
el = append(el, err)
912920
}

pkg/proxy/util/utils.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -123,23 +123,25 @@ func IsProxyableHostname(ctx context.Context, resolv Resolver, hostname string)
123123
return nil
124124
}
125125

126-
// IsLocalIP checks if a given IP address is bound to an interface
127-
// on the local system
128-
func IsLocalIP(ip string) (bool, error) {
126+
// GetLocalAddrs returns a list of all network addresses on the local system
127+
func GetLocalAddrs() ([]net.IP, error) {
128+
var localAddrs []net.IP
129+
129130
addrs, err := net.InterfaceAddrs()
130131
if err != nil {
131-
return false, err
132+
return nil, err
132133
}
133-
for i := range addrs {
134-
intf, _, err := net.ParseCIDR(addrs[i].String())
134+
135+
for _, addr := range addrs {
136+
ip, _, err := net.ParseCIDR(addr.String())
135137
if err != nil {
136-
return false, err
137-
}
138-
if net.ParseIP(ip).Equal(intf) {
139-
return true, nil
138+
return nil, err
140139
}
140+
141+
localAddrs = append(localAddrs, ip)
141142
}
142-
return false, nil
143+
144+
return localAddrs, nil
143145
}
144146

145147
// ShouldSkipService checks if a given service should skip proxying

staging/src/k8s.io/api/go.sum

Lines changed: 0 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

staging/src/k8s.io/apiextensions-apiserver/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ require (
2929
k8s.io/component-base v0.0.0
3030
k8s.io/klog v1.0.0
3131
k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c
32-
k8s.io/utils v0.0.0-20191217005138-9e5e9d854fcc
32+
k8s.io/utils v0.0.0-20200117235808-5f6fbceb4c31
3333
sigs.k8s.io/yaml v1.2.0
3434
)
3535

0 commit comments

Comments
 (0)