Fix deleteEndpointSlice when there are multiple EndpointSlices (#10370)

When processing a `delete` event for an EndpointSlice, regardless of the outcome (whether there are still addresses/endpoints alive or whether we have no endpoints left) we make a call to `noEndpoints` on the subscriber. This will send a `Destination.Get` update to the listeners to advertise a `NoEndpoints` event.

If there are still addresses left, NoEndpoints { exists: true } will be sent (to signal the service still exists). Until an update is registered (i.e a new add) there will be no available endpoints -- this is incorrect, since other EndpointSlices may exist. This change fixes the problem by handling `noEndpoints` in a more specialized way for EndpointSlices.

Signed-off-by: Yannick Utard <yannickutard@gmail.com>
This commit is contained in:
Yannick Utard 2023-02-22 14:35:02 +01:00 committed by GitHub
parent d213a9d5e6
commit 95a3d19b3f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 77 additions and 26 deletions

View File

@ -1017,8 +1017,14 @@ func (pp *portPublisher) deleteEndpointSlice(es *discovery.EndpointSlice) {
listener.Remove(addrSet)
}
svcExists := len(pp.addresses.Addresses) > 0
pp.noEndpoints(svcExists)
if len(pp.addresses.Addresses) == 0 {
pp.noEndpoints(false)
} else {
pp.exists = true
pp.metrics.incUpdates()
pp.metrics.setPods(len(pp.addresses.Addresses))
pp.metrics.setExists(true)
}
}
func (pp *portPublisher) noEndpoints(exists bool) {

View File

@ -1520,33 +1520,77 @@ status:
phase: Running
podIP: 172.17.0.12`}
k8sConfigWithMultipleES := append(k8sConfigsWithES, []string{`
addressType: IPv4
apiVersion: discovery.k8s.io/v1
endpoints:
- addresses:
- 172.17.0.13
conditions:
ready: true
targetRef:
kind: Pod
name: name1-2
namespace: ns
topology:
kubernetes.io/hostname: node-1
kind: EndpointSlice
metadata:
labels:
kubernetes.io/service-name: name1
name: name1-live
namespace: ns
ports:
- name: ""
port: 8989`, `apiVersion: v1
kind: Pod
metadata:
name: name1-2
namespace: ns
status:
phase: Running
podIP: 172.17.0.13`}...)
for _, tt := range []struct {
serviceType string
k8sConfigs []string
id ServiceID
hostname string
port Port
objectToDelete interface{}
deletingServices bool
hasSliceAccess bool
serviceType string
k8sConfigs []string
id ServiceID
hostname string
port Port
objectToDelete interface{}
deletingServices bool
hasSliceAccess bool
noEndpointsCalled bool
}{
{
serviceType: "can delete EndpointSlices",
k8sConfigs: k8sConfigsWithES,
id: ServiceID{Name: "name1", Namespace: "ns"},
port: 8989,
hostname: "name1-1",
objectToDelete: createTestEndpointSlice(),
hasSliceAccess: true,
serviceType: "can delete an EndpointSlice",
k8sConfigs: k8sConfigsWithES,
id: ServiceID{Name: "name1", Namespace: "ns"},
port: 8989,
hostname: "name1-1",
objectToDelete: createTestEndpointSlice(),
hasSliceAccess: true,
noEndpointsCalled: true,
},
{
serviceType: "can delete EndpointSlices when wrapped in a DeletedFinalStateUnknown",
k8sConfigs: k8sConfigsWithES,
id: ServiceID{Name: "name1", Namespace: "ns"},
port: 8989,
hostname: "name1-1",
objectToDelete: createTestEndpointSlice(),
hasSliceAccess: true,
serviceType: "can delete an EndpointSlice when wrapped in a DeletedFinalStateUnknown",
k8sConfigs: k8sConfigsWithES,
id: ServiceID{Name: "name1", Namespace: "ns"},
port: 8989,
hostname: "name1-1",
objectToDelete: createTestEndpointSlice(),
hasSliceAccess: true,
noEndpointsCalled: true,
},
{
serviceType: "can delete an EndpointSlice when there are multiple ones",
k8sConfigs: k8sConfigWithMultipleES,
id: ServiceID{Name: "name1", Namespace: "ns"},
port: 8989,
hostname: "name1-1",
objectToDelete: createTestEndpointSlice(),
hasSliceAccess: true,
noEndpointsCalled: false,
},
} {
tt := tt // pin
@ -1578,8 +1622,9 @@ status:
watcher.deleteEndpointSlice(tt.objectToDelete)
if !listener.endpointsAreNotCalled() {
t.Fatal("Expected NoEndpoints to be Called")
if listener.endpointsAreNotCalled() != tt.noEndpointsCalled {
t.Fatalf("Expected noEndpointsCalled to be [%t], got [%t]",
tt.noEndpointsCalled, listener.endpointsAreNotCalled())
}
})
}