kubernetes: EndpointSlice controller sync fails when Node is not found

I think there are some exceptions where pod garbage collection does not happen immediately (cc @bobbypage).

pod garbage collection does not happen immediately - it will only run the when threshold is reached - terminatedPodThreshold. This defaults to 12500 in kube-controller-manager today. Pod GC will also happen for pods bound to a node object which no longer exists, so called “orphaned pods” xref.

heh, this is easy to repro with an unit test

• Log but don’t return error if node is not found, resulting in endpoints lacking a zone

Since the Node object is only required for topology, I think that is more resilient to log and keep going, in an edge case, with current implementation, a Service with one endpoint will not work at all … Also, I see there is a handler for Node objects, so I think that when the Node event is received the controller will reconcile and set the topology information…

I thought in something like

diff --git a/pkg/controller/endpointslice/reconciler.go b/pkg/controller/endpointslice/reconciler.go
index 4756a0d3448..8aaab7b3a43 100644
--- a/pkg/controller/endpointslice/reconciler.go
+++ b/pkg/controller/endpointslice/reconciler.go
@@ -173,7 +173,7 @@ func (r *reconciler) reconcileByAddressType(service *corev1.Service, pods []*cor
 
                node, err := r.nodeLister.Get(pod.Spec.NodeName)
                if err != nil {
-                       return err
+                       klog.Warningf("Skipping topology information for pod %s: %v", pod.Namespace+"/"+pod.Name, err)
                }
                endpoint := podToEndpoint(pod, node, service, addressType)
                if len(endpoint.Addresses) > 0 {
diff --git a/pkg/controller/endpointslice/reconciler_test.go b/pkg/controller/endpointslice/reconciler_test.go
index 2e71419f352..b83ebedea63 100644
--- a/pkg/controller/endpointslice/reconciler_test.go
+++ b/pkg/controller/endpointslice/reconciler_test.go
@@ -460,6 +460,47 @@ func TestReconcile1Pod(t *testing.T) {
        }
 }
 
+// given an existing endpoint slice and 1 pods matching the service but no Node present on the informer cache
+// an slice should be created and log a message about the missing topology information
+func TestReconcileCreateEndpointSliceNodeMissing(t *testing.T) {
+       client := newClientset()
+       setupMetrics()
+       namespace := "test"
+       pods := []*corev1.Pod{newPod(1, namespace, true, 1, false)}
+       svc, _ := newServiceAndEndpointMeta("foo", namespace)
+
+       numActionsBefore := len(client.Actions())
+
+       r := newReconciler(client, []*corev1.Node{}, defaultMaxEndpointsPerSlice)
+       reconcileHelper(t, r, &svc, pods, []*discovery.EndpointSlice{}, time.Now())
+
+       assert.Len(t, client.Actions(), numActionsBefore+1, "Expected 1 additional clientset action")
+       actions := client.Actions()
+       assert.True(t, actions[numActionsBefore].Matches("create", "endpointslices"), "Action should be create endpoint slice")
+
+       slices := fetchEndpointSlices(t, client, namespace)
+
+       assert.Len(t, slices, 1, "Expected 1 endpoint slices")
+       assert.Regexp(t, "^"+svc.Name, slices[0].Name)
+       assert.Equal(t, svc.Name, slices[0].Labels[discovery.LabelServiceName])
+       protoTCP := corev1.ProtocolTCP
+       assert.EqualValues(t, []discovery.EndpointPort{{Name: utilpointer.StringPtr("foo"), Protocol: &protoTCP, Port: utilpointer.Int32Ptr(80)}}, slices[0].Ports)
+       expectedEndpoints := []discovery.Endpoint{{
+               Addresses:  []string{"1.2.3.5"},
+               Conditions: discovery.EndpointConditions{Ready: utilpointer.BoolPtr(true), Serving: utilpointer.BoolPtr(true), Terminating: utilpointer.BoolPtr(false)},
+               Zone:       nil,
+               NodeName:   utilpointer.StringPtr("node-1"),
+               TargetRef: &corev1.ObjectReference{
+                       Kind:      "Pod",
+                       Namespace: namespace,
+                       Name:      "pod1",
+               },
+       }}
+       assert.EqualValues(t, expectedEndpoints, slices[0].Endpoints)
+       expectTrackedGeneration(t, r.endpointSliceTracker, &slices[0], 1)
+       expectMetrics(t, expectedMetrics{desiredSlices: 1, actualSlices: 1, desiredEndpoints: 1, addedPerSync: 1, removedPerSync: 0, numCreated: 1, numUpdated: 0, numDeleted: 0, slicesChangedPerSync: 1})
+}
+

The following appears in EndpointSlice controller logs when a Node specified by pod.spec.nodeName is not present in the Node informer cache:

that is a bit surprising, isn’t it? for the pod to be created there had to be a node 🤔 I wonder if we have something else here