Skip to content

Commit 48434c3

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#87117 from aojea/proxyv6LB
kube-proxy crash when load balancers use a different IP family
2 parents 3273cd9 + 11263bb commit 48434c3

File tree

3 files changed

+185
-22
lines changed

3 files changed

+185
-22
lines changed

‎pkg/proxy/service.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,10 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
136136
stickyMaxAgeSeconds = int(*service.Spec.SessionAffinityConfig.ClientIP.TimeoutSeconds)
137137
}
138138
info := &BaseServiceInfo{
139-
clusterIP: net.ParseIP(service.Spec.ClusterIP),
140-
port: int(port.Port),
141-
protocol: port.Protocol,
142-
nodePort: int(port.NodePort),
143-
// Deep-copy in case the service instance changes
144-
loadBalancerStatus: *service.Status.LoadBalancer.DeepCopy(),
139+
clusterIP: net.ParseIP(service.Spec.ClusterIP),
140+
port: int(port.Port),
141+
protocol: port.Protocol,
142+
nodePort: int(port.NodePort),
145143
sessionAffinityType: service.Spec.SessionAffinity,
146144
stickyMaxAgeSeconds: stickyMaxAgeSeconds,
147145
onlyNodeLocalEndpoints: onlyNodeLocalEndpoints,
@@ -153,9 +151,11 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
153151
info.loadBalancerSourceRanges = make([]string, len(service.Spec.LoadBalancerSourceRanges))
154152
copy(info.loadBalancerSourceRanges, service.Spec.LoadBalancerSourceRanges)
155153
copy(info.externalIPs, service.Spec.ExternalIPs)
154+
// Deep-copy in case the service instance changes
155+
info.loadBalancerStatus = *service.Status.LoadBalancer.DeepCopy()
156156
} else {
157157
// Filter out the incorrect IP version case.
158-
// If ExternalIPs and LoadBalancerSourceRanges on service contains incorrect IP versions,
158+
// If ExternalIPs, LoadBalancerSourceRanges and LoadBalancerStatus Ingress on service contains incorrect IP versions,
159159
// only filter out the incorrect ones.
160160
var incorrectIPs []string
161161
info.externalIPs, incorrectIPs = utilproxy.FilterIncorrectIPVersion(service.Spec.ExternalIPs, *sct.isIPv6Mode)
@@ -166,6 +166,22 @@ func (sct *ServiceChangeTracker) newBaseServiceInfo(port *v1.ServicePort, servic
166166
if len(incorrectIPs) > 0 {
167167
utilproxy.LogAndEmitIncorrectIPVersionEvent(sct.recorder, "loadBalancerSourceRanges", strings.Join(incorrectIPs, ","), service.Namespace, service.Name, service.UID)
168168
}
169+
// Obtain Load Balancer Ingress IPs
170+
var ips []string
171+
for _, ing := range service.Status.LoadBalancer.Ingress {
172+
ips = append(ips, ing.IP)
173+
}
174+
if len(ips) > 0 {
175+
correctIPs, incorrectIPs := utilproxy.FilterIncorrectIPVersion(ips, *sct.isIPv6Mode)
176+
if len(incorrectIPs) > 0 {
177+
utilproxy.LogAndEmitIncorrectIPVersionEvent(sct.recorder, "Load Balancer ingress IPs", strings.Join(incorrectIPs, ","), service.Namespace, service.Name, service.UID)
178+
}
179+
// Create the LoadBalancerStatus with the filtererd IPs
180+
for _, ip := range correctIPs {
181+
info.loadBalancerStatus.Ingress = append(info.loadBalancerStatus.Ingress, v1.LoadBalancerIngress{IP: ip})
182+
183+
}
184+
}
169185
}
170186

171187
if apiservice.NeedsHealthCheck(service) {

‎pkg/proxy/service_test.go‎

Lines changed: 69 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package proxy
1818

1919
import (
2020
"net"
21+
"reflect"
2122
"testing"
2223

2324
"github.com/davecgh/go-spew/spew"
@@ -166,15 +167,15 @@ func TestServiceToServiceMap(t *testing.T) {
166167
svc.Spec.LoadBalancerIP = "5.6.7.8"
167168
svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port3", "UDP", 8675, 30061, 7000)
168169
svc.Spec.Ports = addTestPort(svc.Spec.Ports, "port4", "UDP", 8676, 30062, 7001)
169-
svc.Status.LoadBalancer = v1.LoadBalancerStatus{
170-
Ingress: []v1.LoadBalancerIngress{
171-
{IP: "10.1.2.4"},
172-
},
173-
}
170+
svc.Status.LoadBalancer.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.4"}}
174171
}),
175172
expected: map[ServicePortName]*BaseServiceInfo{
176-
makeServicePortName("ns1", "load-balancer", "port3", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.11", 8675, "UDP", 0),
177-
makeServicePortName("ns1", "load-balancer", "port4", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.11", 8676, "UDP", 0),
173+
makeServicePortName("ns1", "load-balancer", "port3", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.11", 8675, "UDP", 0, func(info *BaseServiceInfo) {
174+
info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.4"}}
175+
}),
176+
makeServicePortName("ns1", "load-balancer", "port4", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.11", 8676, "UDP", 0, func(info *BaseServiceInfo) {
177+
info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.4"}}
178+
}),
178179
},
179180
},
180181
{
@@ -185,17 +186,17 @@ func TestServiceToServiceMap(t *testing.T) {
185186
svc.Spec.LoadBalancerIP = "5.6.7.8"
186187
svc.Spec.Ports = addTestPort(svc.Spec.Ports, "portx", "UDP", 8677, 30063, 7002)
187188
svc.Spec.Ports = addTestPort(svc.Spec.Ports, "porty", "UDP", 8678, 30064, 7003)
188-
svc.Status.LoadBalancer = v1.LoadBalancerStatus{
189-
Ingress: []v1.LoadBalancerIngress{
190-
{IP: "10.1.2.3"},
191-
},
192-
}
189+
svc.Status.LoadBalancer.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.3"}}
193190
svc.Spec.ExternalTrafficPolicy = v1.ServiceExternalTrafficPolicyTypeLocal
194191
svc.Spec.HealthCheckNodePort = 345
195192
}),
196193
expected: map[ServicePortName]*BaseServiceInfo{
197-
makeServicePortName("ns1", "only-local-load-balancer", "portx", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.12", 8677, "UDP", 345),
198-
makeServicePortName("ns1", "only-local-load-balancer", "porty", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.12", 8678, "UDP", 345),
194+
makeServicePortName("ns1", "only-local-load-balancer", "portx", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.12", 8677, "UDP", 345, func(info *BaseServiceInfo) {
195+
info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.3"}}
196+
}),
197+
makeServicePortName("ns1", "only-local-load-balancer", "porty", v1.ProtocolUDP): makeTestServiceInfo("172.16.55.12", 8678, "UDP", 345, func(info *BaseServiceInfo) {
198+
info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: "10.1.2.3"}}
199+
}),
199200
},
200201
},
201202
{
@@ -225,6 +226,14 @@ func TestServiceToServiceMap(t *testing.T) {
225226
},
226227
},
227228
},
229+
Status: v1.ServiceStatus{
230+
LoadBalancer: v1.LoadBalancerStatus{
231+
Ingress: []v1.LoadBalancerIngress{
232+
{IP: testExternalIPv4},
233+
{IP: testExternalIPv6},
234+
},
235+
},
236+
},
228237
},
229238
isIPv6Mode: &falseVal,
230239
},
@@ -245,6 +254,14 @@ func TestServiceToServiceMap(t *testing.T) {
245254
},
246255
},
247256
},
257+
Status: v1.ServiceStatus{
258+
LoadBalancer: v1.LoadBalancerStatus{
259+
Ingress: []v1.LoadBalancerIngress{
260+
{IP: testExternalIPv4},
261+
{IP: testExternalIPv6},
262+
},
263+
},
264+
},
248265
},
249266
isIPv6Mode: &trueVal,
250267
},
@@ -267,11 +284,20 @@ func TestServiceToServiceMap(t *testing.T) {
267284
},
268285
},
269286
},
287+
Status: v1.ServiceStatus{
288+
LoadBalancer: v1.LoadBalancerStatus{
289+
Ingress: []v1.LoadBalancerIngress{
290+
{IP: testExternalIPv4},
291+
{IP: testExternalIPv6},
292+
},
293+
},
294+
},
270295
},
271296
expected: map[ServicePortName]*BaseServiceInfo{
272297
makeServicePortName("test", "validIPv4", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv4, 12345, "TCP", 0, func(info *BaseServiceInfo) {
273298
info.externalIPs = []string{testExternalIPv4}
274299
info.loadBalancerSourceRanges = []string{testSourceRangeIPv4}
300+
info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: testExternalIPv4}}
275301
}),
276302
},
277303
isIPv6Mode: &falseVal,
@@ -295,11 +321,20 @@ func TestServiceToServiceMap(t *testing.T) {
295321
},
296322
},
297323
},
324+
Status: v1.ServiceStatus{
325+
LoadBalancer: v1.LoadBalancerStatus{
326+
Ingress: []v1.LoadBalancerIngress{
327+
{IP: testExternalIPv4},
328+
{IP: testExternalIPv6},
329+
},
330+
},
331+
},
298332
},
299333
expected: map[ServicePortName]*BaseServiceInfo{
300334
makeServicePortName("test", "validIPv6", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv6, 12345, "TCP", 0, func(info *BaseServiceInfo) {
301335
info.externalIPs = []string{testExternalIPv6}
302336
info.loadBalancerSourceRanges = []string{testSourceRangeIPv6}
337+
info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: testExternalIPv6}}
303338
}),
304339
},
305340
isIPv6Mode: &trueVal,
@@ -323,11 +358,20 @@ func TestServiceToServiceMap(t *testing.T) {
323358
},
324359
},
325360
},
361+
Status: v1.ServiceStatus{
362+
LoadBalancer: v1.LoadBalancerStatus{
363+
Ingress: []v1.LoadBalancerIngress{
364+
{IP: testExternalIPv4},
365+
{IP: testExternalIPv6},
366+
},
367+
},
368+
},
326369
},
327370
expected: map[ServicePortName]*BaseServiceInfo{
328371
makeServicePortName("test", "filterIPv6InIPV4Mode", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv4, 12345, "TCP", 0, func(info *BaseServiceInfo) {
329372
info.externalIPs = []string{testExternalIPv4}
330373
info.loadBalancerSourceRanges = []string{testSourceRangeIPv4}
374+
info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: testExternalIPv4}}
331375
}),
332376
},
333377
isIPv6Mode: &falseVal,
@@ -351,11 +395,20 @@ func TestServiceToServiceMap(t *testing.T) {
351395
},
352396
},
353397
},
398+
Status: v1.ServiceStatus{
399+
LoadBalancer: v1.LoadBalancerStatus{
400+
Ingress: []v1.LoadBalancerIngress{
401+
{IP: testExternalIPv4},
402+
{IP: testExternalIPv6},
403+
},
404+
},
405+
},
354406
},
355407
expected: map[ServicePortName]*BaseServiceInfo{
356408
makeServicePortName("test", "filterIPv4InIPV6Mode", "testPort", v1.ProtocolTCP): makeTestServiceInfo(testClusterIPv6, 12345, "TCP", 0, func(info *BaseServiceInfo) {
357409
info.externalIPs = []string{testExternalIPv6}
358410
info.loadBalancerSourceRanges = []string{testSourceRangeIPv6}
411+
info.loadBalancerStatus.Ingress = []v1.LoadBalancerIngress{{IP: testExternalIPv6}}
359412
}),
360413
},
361414
isIPv6Mode: &trueVal,
@@ -377,7 +430,8 @@ func TestServiceToServiceMap(t *testing.T) {
377430
svcInfo.protocol != expectedInfo.protocol ||
378431
svcInfo.healthCheckNodePort != expectedInfo.healthCheckNodePort ||
379432
!sets.NewString(svcInfo.externalIPs...).Equal(sets.NewString(expectedInfo.externalIPs...)) ||
380-
!sets.NewString(svcInfo.loadBalancerSourceRanges...).Equal(sets.NewString(expectedInfo.loadBalancerSourceRanges...)) {
433+
!sets.NewString(svcInfo.loadBalancerSourceRanges...).Equal(sets.NewString(expectedInfo.loadBalancerSourceRanges...)) ||
434+
!reflect.DeepEqual(svcInfo.loadBalancerStatus, expectedInfo.loadBalancerStatus) {
381435
t.Errorf("[%s] expected new[%v]to be %v, got %v", tc.desc, svcKey, expectedInfo, *svcInfo)
382436
}
383437
}

‎pkg/proxy/util/utils_test.go‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,3 +537,96 @@ func TestShuffleStrings(t *testing.T) {
537537
}
538538
}
539539
}
540+
541+
func TestFilterIncorrectIPVersion(t *testing.T) {
542+
testCases := []struct {
543+
desc string
544+
ipString []string
545+
wantIPv6 bool
546+
expectCorrect []string
547+
expectIncorrect []string
548+
}{
549+
{
550+
desc: "empty input IPv4",
551+
ipString: []string{},
552+
wantIPv6: false,
553+
expectCorrect: nil,
554+
expectIncorrect: nil,
555+
},
556+
{
557+
desc: "empty input IPv6",
558+
ipString: []string{},
559+
wantIPv6: true,
560+
expectCorrect: nil,
561+
expectIncorrect: nil,
562+
},
563+
{
564+
desc: "want IPv4 and receive IPv6",
565+
ipString: []string{"fd00:20::1"},
566+
wantIPv6: false,
567+
expectCorrect: nil,
568+
expectIncorrect: []string{"fd00:20::1"},
569+
},
570+
{
571+
desc: "want IPv6 and receive IPv4",
572+
ipString: []string{"192.168.200.2"},
573+
wantIPv6: true,
574+
expectCorrect: nil,
575+
expectIncorrect: []string{"192.168.200.2"},
576+
},
577+
{
578+
desc: "want IPv6 and receive IPv4 and IPv6",
579+
ipString: []string{"192.168.200.2", "192.1.34.23", "fd00:20::1", "2001:db9::3"},
580+
wantIPv6: true,
581+
expectCorrect: []string{"fd00:20::1", "2001:db9::3"},
582+
expectIncorrect: []string{"192.168.200.2", "192.1.34.23"},
583+
},
584+
{
585+
desc: "want IPv4 and receive IPv4 and IPv6",
586+
ipString: []string{"192.168.200.2", "192.1.34.23", "fd00:20::1", "2001:db9::3"},
587+
wantIPv6: false,
588+
expectCorrect: []string{"192.168.200.2", "192.1.34.23"},
589+
expectIncorrect: []string{"fd00:20::1", "2001:db9::3"},
590+
},
591+
{
592+
desc: "want IPv4 and receive IPv4 only",
593+
ipString: []string{"192.168.200.2", "192.1.34.23"},
594+
wantIPv6: false,
595+
expectCorrect: []string{"192.168.200.2", "192.1.34.23"},
596+
expectIncorrect: nil,
597+
},
598+
{
599+
desc: "want IPv6 and receive IPv4 only",
600+
ipString: []string{"192.168.200.2", "192.1.34.23"},
601+
wantIPv6: true,
602+
expectCorrect: nil,
603+
expectIncorrect: []string{"192.168.200.2", "192.1.34.23"},
604+
},
605+
{
606+
desc: "want IPv4 and receive IPv6 only",
607+
ipString: []string{"fd00:20::1", "2001:db9::3"},
608+
wantIPv6: false,
609+
expectCorrect: nil,
610+
expectIncorrect: []string{"fd00:20::1", "2001:db9::3"},
611+
},
612+
{
613+
desc: "want IPv6 and receive IPv6 only",
614+
ipString: []string{"fd00:20::1", "2001:db9::3"},
615+
wantIPv6: true,
616+
expectCorrect: []string{"fd00:20::1", "2001:db9::3"},
617+
expectIncorrect: nil,
618+
},
619+
}
620+
621+
for _, testcase := range testCases {
622+
t.Run(testcase.desc, func(t *testing.T) {
623+
correct, incorrect := FilterIncorrectIPVersion(testcase.ipString, testcase.wantIPv6)
624+
if !reflect.DeepEqual(testcase.expectCorrect, correct) {
625+
t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectCorrect, correct)
626+
}
627+
if !reflect.DeepEqual(testcase.expectIncorrect, incorrect) {
628+
t.Errorf("Test %v failed: expected %v, got %v", testcase.desc, testcase.expectIncorrect, incorrect)
629+
}
630+
})
631+
}
632+
}

0 commit comments

Comments
 (0)