Merge pull request #135593 from danwinship/proxy-duplicate-ips

Handle the case of a pod IP being reused while the old Pod still exists
This commit is contained in:
Kubernetes Prow Robot 2025-12-17 23:28:02 -08:00 committed by GitHub
commit 2fa93a995e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 61 additions and 10 deletions

View file

@ -1321,8 +1321,8 @@ func TestEndpointSliceUpdate(t *testing.T) {
// test additions to existing state with partially overlapping slices and ports
"add a slice that overlaps with existing state and partial ports": {
startingSlices: []*discovery.EndpointSlice{
generateEndpointSlice("svc1", "ns1", 1, 3, 999, 999, []string{"host1", "host2"}, []*int32{ptr.To[int32](80), ptr.To[int32](443)}),
generateEndpointSlice("svc1", "ns1", 2, 2, 999, 999, []string{"host1", "host2"}, []*int32{ptr.To[int32](80), ptr.To[int32](443)}),
generateEndpointSlice("svc1", "ns1", 1, 3, 999, 999, []string{"host1"}, []*int32{ptr.To[int32](80), ptr.To[int32](443)}),
generateEndpointSlice("svc1", "ns1", 2, 2, 999, 999, []string{"host1"}, []*int32{ptr.To[int32](80), ptr.To[int32](443)}),
},
endpointsChangeTracker: NewEndpointsChangeTracker(v1.IPv4Protocol, "host1", nil, nil),
namespacedName: types.NamespacedName{Name: "svc1", Namespace: "ns1"},
@ -1336,14 +1336,14 @@ func TestEndpointSliceUpdate(t *testing.T) {
&BaseEndpointInfo{ip: "10.0.1.3", port: 80, endpoint: "10.0.1.3:80", isLocal: true, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.1.4", port: 80, endpoint: "10.0.1.4:80", isLocal: true, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.1.5", port: 80, endpoint: "10.0.1.5:80", isLocal: true, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.2.1", port: 80, endpoint: "10.0.2.1:80", isLocal: false, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.2.1", port: 80, endpoint: "10.0.2.1:80", isLocal: true, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.2.2", port: 80, endpoint: "10.0.2.2:80", isLocal: true, ready: true, serving: true, terminating: false},
},
makeServicePortName("ns1", "svc1", "port-1", v1.ProtocolTCP): {
&BaseEndpointInfo{ip: "10.0.1.1", port: 443, endpoint: "10.0.1.1:443", isLocal: false, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.1.1", port: 443, endpoint: "10.0.1.1:443", isLocal: true, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.1.2", port: 443, endpoint: "10.0.1.2:443", isLocal: true, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.1.3", port: 443, endpoint: "10.0.1.3:443", isLocal: false, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.2.1", port: 443, endpoint: "10.0.2.1:443", isLocal: false, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.1.3", port: 443, endpoint: "10.0.1.3:443", isLocal: true, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.2.1", port: 443, endpoint: "10.0.2.1:443", isLocal: true, ready: true, serving: true, terminating: false},
&BaseEndpointInfo{ip: "10.0.2.2", port: 443, endpoint: "10.0.2.2:443", isLocal: true, ready: true, serving: true, terminating: false},
},
},

View file

@ -232,10 +232,19 @@ func (cache *EndpointSliceCache) addEndpoints(svcPortName *ServicePortName, port
endpointInfo := newBaseEndpointInfo(endpointIP, portNum, isLocal,
ready, serving, terminating, zoneHints, nodeHints)
// This logic ensures we're deduplicating potential overlapping endpoints
// isLocal should not vary between matching endpoints, but if it does, we
// favor a true value here if it exists.
if _, exists := endpointSet[endpointInfo.String()]; !exists || isLocal {
// If an Endpoint gets moved from one slice to another, we may temporarily
// see it in both slices. Ideally we want to prefer the Endpoint from the
// more-recently-updated EndpointSlice, since it may have newer
// conditions. But we can't easily figure that out, and the situation will
// resolve itself once we receive the second EndpointSlice update anyway.
//
// On the other hand, there maybe also be two *different* Endpoints (i.e.,
// with different targetRefs) that point to the same IP, if the pod
// network reuses the IP from a terminating pod before the Pod object is
// fully deleted. In this case we want to prefer the running pod over the
// terminating one. (If there are multiple non-terminating pods with the
// same podIP, then the result is undefined.)
if _, exists := endpointSet[endpointInfo.String()]; !exists || !terminating {
endpointSet[endpointInfo.String()] = cache.makeEndpointInfo(endpointInfo, svcPortName)
}
}

View file

@ -311,6 +311,48 @@ func TestEndpointInfoByServicePort(t *testing.T) {
},
},
},
"with duplicate Endpoints, prefer non-terminating": {
namespacedName: types.NamespacedName{Name: "svc1", Namespace: "ns1"},
hostname: "host1",
endpointSlices: []*discovery.EndpointSlice{
func() *discovery.EndpointSlice {
es := generateEndpointSliceWithOffset("svc1", "ns1", 1, 1, 1, 999, 999, []string{"host1"}, []*int32{ptr.To[int32](80)})
// Duplicate the endpoint
ep1 := es.Endpoints[0]
ep1.Conditions.Ready = ptr.To(false)
ep1.Conditions.Serving = ptr.To(true)
ep1.Conditions.Terminating = ptr.To(true)
ep2 := es.Endpoints[0]
ep2.Conditions.Ready = ptr.To(true)
ep2.Conditions.Serving = ptr.To(true)
ep2.Conditions.Terminating = ptr.To(false)
ep3 := es.Endpoints[0]
ep3.Conditions.Ready = ptr.To(false)
ep3.Conditions.Serving = ptr.To(false)
ep3.Conditions.Terminating = ptr.To(true)
// ep2 should win since it's the non-terminating one
es.Endpoints = []discovery.Endpoint{ep1, ep2, ep3}
return es
}(),
},
expectedMap: spToEndpointMap{
makeServicePortName("ns1", "svc1", "port-0", v1.ProtocolTCP): {
"10.0.1.1:80": &BaseEndpointInfo{
ip: "10.0.1.1",
port: 80,
endpoint: "10.0.1.1:80",
isLocal: true,
ready: true,
serving: true,
terminating: false,
},
},
},
},
}
for name, tc := range testCases {