Skip to content

Commit d3aafb2

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#92609 from andrewsykim/ipvs-binded-addr
proxier/ipvs: check already binded addresses in the IPVS dummy interface
2 parents 6eca9f6 + de2ecd7 commit d3aafb2

File tree

2 files changed

+69
-18
lines changed

2 files changed

+69
-18
lines changed

‎pkg/proxy/ipvs/proxier.go‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,10 @@ type Proxier struct {
262262
gracefuldeleteManager *GracefulTerminationManager
263263
}
264264

265-
// IPGetter helps get node network interface IP
265+
// IPGetter helps get node network interface IP and IPs binded to the IPVS dummy interface
266266
type IPGetter interface {
267267
NodeIPs() ([]net.IP, error)
268+
BindedIPs() (sets.String, error)
268269
}
269270

270271
// realIPGetter is a real NodeIP handler, it implements IPGetter.
@@ -300,6 +301,11 @@ func (r *realIPGetter) NodeIPs() (ips []net.IP, err error) {
300301
return ips, nil
301302
}
302303

304+
// BindedIPs returns all addresses that are binded to the IPVS dummy interface kube-ipvs0
305+
func (r *realIPGetter) BindedIPs() (sets.String, error) {
306+
return r.nl.GetLocalAddresses(DefaultDummyDevice, "")
307+
}
308+
303309
// Proxier implements proxy.Provider
304310
var _ proxy.Provider = &Proxier{}
305311

@@ -1089,6 +1095,11 @@ func (proxier *Proxier) syncProxyRules() {
10891095
// activeBindAddrs represents ip address successfully bind to DefaultDummyDevice in this round of sync
10901096
activeBindAddrs := map[string]bool{}
10911097

1098+
bindedAddresses, err := proxier.ipGetter.BindedIPs()
1099+
if err != nil {
1100+
klog.Errorf("error listing addresses binded to dummy interface, error: %v", err)
1101+
}
1102+
10921103
hasNodePort := false
10931104
for _, svc := range proxier.serviceMap {
10941105
svcInfo, ok := svc.(*serviceInfo)
@@ -1197,7 +1208,7 @@ func (proxier *Proxier) syncProxyRules() {
11971208
serv.Timeout = uint32(svcInfo.StickyMaxAgeSeconds())
11981209
}
11991210
// We need to bind ClusterIP to dummy interface, so set `bindAddr` parameter to `true` in syncService()
1200-
if err := proxier.syncService(svcNameString, serv, true); err == nil {
1211+
if err := proxier.syncService(svcNameString, serv, true, bindedAddresses); err == nil {
12011212
activeIPVSServices[serv.String()] = true
12021213
activeBindAddrs[serv.Address.String()] = true
12031214
// ExternalTrafficPolicy only works for NodePort and external LB traffic, does not affect ClusterIP
@@ -1278,7 +1289,7 @@ func (proxier *Proxier) syncProxyRules() {
12781289
serv.Flags |= utilipvs.FlagPersistent
12791290
serv.Timeout = uint32(svcInfo.StickyMaxAgeSeconds())
12801291
}
1281-
if err := proxier.syncService(svcNameString, serv, true); err == nil {
1292+
if err := proxier.syncService(svcNameString, serv, true, bindedAddresses); err == nil {
12821293
activeIPVSServices[serv.String()] = true
12831294
activeBindAddrs[serv.Address.String()] = true
12841295

@@ -1384,7 +1395,7 @@ func (proxier *Proxier) syncProxyRules() {
13841395
serv.Flags |= utilipvs.FlagPersistent
13851396
serv.Timeout = uint32(svcInfo.StickyMaxAgeSeconds())
13861397
}
1387-
if err := proxier.syncService(svcNameString, serv, true); err == nil {
1398+
if err := proxier.syncService(svcNameString, serv, true, bindedAddresses); err == nil {
13881399
activeIPVSServices[serv.String()] = true
13891400
activeBindAddrs[serv.Address.String()] = true
13901401
if err := proxier.syncEndpoint(svcName, svcInfo.OnlyNodeLocalEndpoints(), serv); err != nil {
@@ -1541,7 +1552,7 @@ func (proxier *Proxier) syncProxyRules() {
15411552
serv.Timeout = uint32(svcInfo.StickyMaxAgeSeconds())
15421553
}
15431554
// There is no need to bind Node IP to dummy interface, so set parameter `bindAddr` to `false`.
1544-
if err := proxier.syncService(svcNameString, serv, false); err == nil {
1555+
if err := proxier.syncService(svcNameString, serv, false, bindedAddresses); err == nil {
15451556
activeIPVSServices[serv.String()] = true
15461557
if err := proxier.syncEndpoint(svcName, svcInfo.OnlyNodeLocalEndpoints(), serv); err != nil {
15471558
klog.Errorf("Failed to sync endpoint for service: %v, err: %v", serv, err)
@@ -1922,7 +1933,7 @@ func (proxier *Proxier) deleteEndpointConnections(connectionMap []proxy.ServiceE
19221933
}
19231934
}
19241935

1925-
func (proxier *Proxier) syncService(svcName string, vs *utilipvs.VirtualServer, bindAddr bool) error {
1936+
func (proxier *Proxier) syncService(svcName string, vs *utilipvs.VirtualServer, bindAddr bool, bindedAddresses sets.String) error {
19261937
appliedVirtualServer, _ := proxier.ipvs.GetVirtualServer(vs)
19271938
if appliedVirtualServer == nil || !appliedVirtualServer.Equal(vs) {
19281939
if appliedVirtualServer == nil {
@@ -1943,16 +1954,22 @@ func (proxier *Proxier) syncService(svcName string, vs *utilipvs.VirtualServer,
19431954
}
19441955
}
19451956

1946-
// bind service address to dummy interface even if service not changed,
1947-
// in case that service IP was removed by other processes
1957+
// bind service address to dummy interface
19481958
if bindAddr {
1959+
// always attempt to bind if bindedAddresses is nil,
1960+
// otherwise check if it's already binded and return early
1961+
if bindedAddresses != nil && bindedAddresses.Has(vs.Address.String()) {
1962+
return nil
1963+
}
1964+
19491965
klog.V(4).Infof("Bind addr %s", vs.Address.String())
19501966
_, err := proxier.netlinkHandle.EnsureAddressBind(vs.Address.String(), DefaultDummyDevice)
19511967
if err != nil {
19521968
klog.Errorf("Failed to bind service address to dummy device %q: %v", svcName, err)
19531969
return err
19541970
}
19551971
}
1972+
19561973
return nil
19571974
}
19581975

‎pkg/proxy/ipvs/proxier_test.go‎

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,13 +57,18 @@ import (
5757
const testHostname = "test-hostname"
5858

5959
type fakeIPGetter struct {
60-
nodeIPs []net.IP
60+
nodeIPs []net.IP
61+
bindedIPs sets.String
6162
}
6263

6364
func (f *fakeIPGetter) NodeIPs() ([]net.IP, error) {
6465
return f.nodeIPs, nil
6566
}
6667

68+
func (f *fakeIPGetter) BindedIPs() (sets.String, error) {
69+
return f.bindedIPs, nil
70+
}
71+
6772
// fakePortOpener implements portOpener.
6873
type fakePortOpener struct {
6974
openPorts []*utilproxy.LocalPort
@@ -3056,6 +3061,7 @@ func Test_syncService(t *testing.T) {
30563061
svcName string
30573062
newVirtualServer *utilipvs.VirtualServer
30583063
bindAddr bool
3064+
bindedAddrs sets.String
30593065
}{
30603066
{
30613067
// case 0, old virtual server is same as new virtual server
@@ -3074,7 +3080,8 @@ func Test_syncService(t *testing.T) {
30743080
Scheduler: "rr",
30753081
Flags: utilipvs.FlagHashed,
30763082
},
3077-
bindAddr: false,
3083+
bindAddr: false,
3084+
bindedAddrs: sets.NewString(),
30783085
},
30793086
{
30803087
// case 1, old virtual server is different from new virtual server
@@ -3093,7 +3100,8 @@ func Test_syncService(t *testing.T) {
30933100
Scheduler: "rr",
30943101
Flags: utilipvs.FlagPersistent,
30953102
},
3096-
bindAddr: false,
3103+
bindAddr: false,
3104+
bindedAddrs: sets.NewString(),
30973105
},
30983106
{
30993107
// case 2, old virtual server is different from new virtual server
@@ -3112,7 +3120,8 @@ func Test_syncService(t *testing.T) {
31123120
Scheduler: "wlc",
31133121
Flags: utilipvs.FlagHashed,
31143122
},
3115-
bindAddr: false,
3123+
bindAddr: false,
3124+
bindedAddrs: sets.NewString(),
31163125
},
31173126
{
31183127
// case 3, old virtual server is nil, and create new virtual server
@@ -3125,7 +3134,8 @@ func Test_syncService(t *testing.T) {
31253134
Scheduler: "rr",
31263135
Flags: utilipvs.FlagHashed,
31273136
},
3128-
bindAddr: true,
3137+
bindAddr: true,
3138+
bindedAddrs: sets.NewString(),
31293139
},
31303140
{
31313141
// case 4, SCTP, old virtual server is same as new virtual server
@@ -3144,7 +3154,8 @@ func Test_syncService(t *testing.T) {
31443154
Scheduler: "rr",
31453155
Flags: utilipvs.FlagHashed,
31463156
},
3147-
bindAddr: false,
3157+
bindAddr: false,
3158+
bindedAddrs: sets.NewString(),
31483159
},
31493160
{
31503161
// case 5, old virtual server is different from new virtual server
@@ -3163,7 +3174,8 @@ func Test_syncService(t *testing.T) {
31633174
Scheduler: "rr",
31643175
Flags: utilipvs.FlagPersistent,
31653176
},
3166-
bindAddr: false,
3177+
bindAddr: false,
3178+
bindedAddrs: sets.NewString(),
31673179
},
31683180
{
31693181
// case 6, old virtual server is different from new virtual server
@@ -3182,7 +3194,8 @@ func Test_syncService(t *testing.T) {
31823194
Scheduler: "wlc",
31833195
Flags: utilipvs.FlagHashed,
31843196
},
3185-
bindAddr: false,
3197+
bindAddr: false,
3198+
bindedAddrs: sets.NewString(),
31863199
},
31873200
{
31883201
// case 7, old virtual server is nil, and create new virtual server
@@ -3195,7 +3208,28 @@ func Test_syncService(t *testing.T) {
31953208
Scheduler: "rr",
31963209
Flags: utilipvs.FlagHashed,
31973210
},
3198-
bindAddr: true,
3211+
bindAddr: true,
3212+
bindedAddrs: sets.NewString(),
3213+
},
3214+
{
3215+
// case 8, virtual server address already binded, skip sync
3216+
oldVirtualServer: &utilipvs.VirtualServer{
3217+
Address: net.ParseIP("1.2.3.4"),
3218+
Protocol: string(v1.ProtocolSCTP),
3219+
Port: 53,
3220+
Scheduler: "rr",
3221+
Flags: utilipvs.FlagHashed,
3222+
},
3223+
svcName: "baz",
3224+
newVirtualServer: &utilipvs.VirtualServer{
3225+
Address: net.ParseIP("1.2.3.4"),
3226+
Protocol: string(v1.ProtocolSCTP),
3227+
Port: 53,
3228+
Scheduler: "rr",
3229+
Flags: utilipvs.FlagHashed,
3230+
},
3231+
bindAddr: true,
3232+
bindedAddrs: sets.NewString("1.2.3.4"),
31993233
},
32003234
}
32013235

@@ -3211,7 +3245,7 @@ func Test_syncService(t *testing.T) {
32113245
t.Errorf("Case [%d], unexpected add IPVS virtual server error: %v", i, err)
32123246
}
32133247
}
3214-
if err := proxier.syncService(testCases[i].svcName, testCases[i].newVirtualServer, testCases[i].bindAddr); err != nil {
3248+
if err := proxier.syncService(testCases[i].svcName, testCases[i].newVirtualServer, testCases[i].bindAddr, testCases[i].bindedAddrs); err != nil {
32153249
t.Errorf("Case [%d], unexpected sync IPVS virtual server error: %v", i, err)
32163250
}
32173251
// check

0 commit comments

Comments
 (0)