diff --git a/pkg/neg/syncers/utils.go b/pkg/neg/syncers/utils.go index 29363dc161..90c4445c92 100644 --- a/pkg/neg/syncers/utils.go +++ b/pkg/neg/syncers/utils.go @@ -290,7 +290,9 @@ func toZoneNetworkEndpointMap(eds []negtypes.EndpointsData, zoneGetter negtypes. return zoneNetworkEndpointMap, networkEndpointPodMap, dupCount, nil } -func toZoneNetworkEndpointMapDegradedMode(eds []negtypes.EndpointsData, zoneGetter negtypes.ZoneGetter, podLister cache.Indexer, servicePortName string, networkEndpointType negtypes.NetworkEndpointType) (map[string]negtypes.NetworkEndpointSet, negtypes.EndpointPodMap, int, error) { +func toZoneNetworkEndpointMapDegradedMode(eds []negtypes.EndpointsData, zoneGetter negtypes.ZoneGetter, + podLister, nodeLister cache.Indexer, servicePortName string, + networkEndpointType negtypes.NetworkEndpointType) (map[string]negtypes.NetworkEndpointSet, negtypes.EndpointPodMap, int, error) { targetMap := map[string]negtypes.NetworkEndpointSet{} endpointPodMap := negtypes.EndpointPodMap{} var dupCount int @@ -310,14 +312,14 @@ func toZoneNetworkEndpointMapDegradedMode(eds []negtypes.EndpointsData, zoneGett klog.V(2).Infof("Endpoint %q in Endpoints %s/%s does not have an associated pod. Skipping", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name) continue } - dupCount += validateAndAddEndpoints(endpointAddress, zoneGetter, podLister, matchPort, networkEndpointType, targetMap, endpointPodMap) + dupCount += validateAndAddEndpoints(endpointAddress, zoneGetter, podLister, nodeLister, matchPort, networkEndpointType, targetMap, endpointPodMap) } } return targetMap, endpointPodMap, dupCount, nil } // validateAndAddEndpoints fills in missing information and creates network endpoint for each endpoint addresss -func validateAndAddEndpoints(ep negtypes.AddressData, zoneGetter negtypes.ZoneGetter, podLister cache.Indexer, matchPort string, endpointType negtypes.NetworkEndpointType, targetMap map[string]negtypes.NetworkEndpointSet, endpointPodMap negtypes.EndpointPodMap) int { +func validateAndAddEndpoints(ep negtypes.AddressData, zoneGetter negtypes.ZoneGetter, podLister, nodeLister cache.Indexer, matchPort string, endpointType negtypes.NetworkEndpointType, targetMap map[string]negtypes.NetworkEndpointSet, endpointPodMap negtypes.EndpointPodMap) int { var dupCount int for _, address := range ep.Addresses { key := fmt.Sprintf("%s/%s", ep.TargetRef.Namespace, ep.TargetRef.Name) @@ -331,7 +333,7 @@ func validateAndAddEndpoints(ep negtypes.AddressData, zoneGetter negtypes.ZoneGe klog.V(2).Infof("Endpoint %q does not correspond to a pod object. Skipping", address) continue } - if !validatePod(pod) { + if !validatePod(pod, nodeLister) { klog.V(2).Infof("Endpoint %q does not correspond to a valid pod resource. Skipping", address) continue } @@ -363,13 +365,24 @@ func validateAndAddEndpoints(ep negtypes.AddressData, zoneGetter negtypes.ZoneGe // validatePod checks if this pod is a valid pod resource // it returns false if the pod: // 1. is in terminal state -func validatePod(pod *apiv1.Pod) bool { +// 2. corresponds to a non-existent node +func validatePod(pod *apiv1.Pod, nodeLister cache.Indexer) bool { // Terminal Pod means a pod is in PodFailed or PodSucceeded phase phase := pod.Status.Phase if phase == apiv1.PodFailed || phase == apiv1.PodSucceeded { klog.V(2).Info("Pod %s/%s is a terminal pod with status %v, skipping", pod.ObjectMeta.Namespace, pod.ObjectMeta.Name, phase) return false } + obj, exists, err := nodeLister.GetByKey(pod.Spec.NodeName) + if err != nil || !exists { + klog.V(2).Info("Pod %s/%s corresponds to a non-existing node %s, skipping", pod.ObjectMeta.Namespace, pod.ObjectMeta.Name, pod.Spec.NodeName) + return false + } + _, isNode := obj.(*apiv1.Node) + if !isNode { + klog.V(2).Info("Pod %s/%s does not correspond to a valid node resource, skipping", pod.ObjectMeta.Namespace, pod.ObjectMeta.Name) + return false + } return true } diff --git a/pkg/neg/syncers/utils_test.go b/pkg/neg/syncers/utils_test.go index 663142e00a..0f5d64516c 100644 --- a/pkg/neg/syncers/utils_test.go +++ b/pkg/neg/syncers/utils_test.go @@ -1196,9 +1196,19 @@ func TestToZoneNetworkEndpointMapDegradedMode(t *testing.T) { t.Parallel() fakeZoneGetter := negtypes.NewFakeZoneGetter() - podLister := negtypes.NewTestContext().PodInformer.GetIndexer() + testContext := negtypes.NewTestContext() + podLister := testContext.PodInformer.GetIndexer() addPodsToLister(podLister) + nodeLister := testContext.NodeInformer.GetIndexer() + for i := 1; i <= 4; i++ { + nodeLister.Add(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("instance%v", i), + }, + }) + } + testNonExistPort := "non-exists" testEmptyNamedPort := "" testNamedPort := "named-Port" @@ -1291,7 +1301,7 @@ func TestToZoneNetworkEndpointMapDegradedMode(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { testEndpointSlices := getTestEndpointSlices(testService, testNamespace) - targetMap, endpointPodMap, _, err := toZoneNetworkEndpointMapDegradedMode(negtypes.EndpointsDataFromEndpointSlices(testEndpointSlices), fakeZoneGetter, podLister, tc.portName, tc.networkEndpointType) + targetMap, endpointPodMap, _, err := toZoneNetworkEndpointMapDegradedMode(negtypes.EndpointsDataFromEndpointSlices(testEndpointSlices), fakeZoneGetter, podLister, nodeLister, tc.portName, tc.networkEndpointType) if err != nil { t.Errorf("expected error=nil, but got %v", err) } @@ -1321,9 +1331,17 @@ func TestValidateAndAddEndpoints(t *testing.T) { } fakeZoneGetter := negtypes.NewFakeZoneGetter() - podLister := negtypes.NewTestContext().PodInformer.GetIndexer() + testContext := negtypes.NewTestContext() + podLister := testContext.PodInformer.GetIndexer() addPodsToLister(podLister) + nodeLister := testContext.NodeInformer.GetIndexer() + nodeLister.Add(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: instance1, + }, + }) + testCases := []struct { desc string ep negtypes.AddressData @@ -1347,7 +1365,7 @@ func TestValidateAndAddEndpoints(t *testing.T) { expectedPodMap: podMap, }, { - desc: "endpoint without nodeName", + desc: "endpoint without nodeName, nodeName should be filled", ep: negtypes.AddressData{ Addresses: []string{"10.100.1.1"}, NodeName: nil, @@ -1362,7 +1380,7 @@ func TestValidateAndAddEndpoints(t *testing.T) { expectedPodMap: podMap, }, { - desc: "endpoint with empty nodeName", + desc: "endpoint with empty nodeName, nodeName should be filled", ep: negtypes.AddressData{ Addresses: []string{"10.100.1.1"}, NodeName: &emptyNodeName, @@ -1401,7 +1419,7 @@ func TestValidateAndAddEndpoints(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { targetMap := map[string]negtypes.NetworkEndpointSet{} endpointPodMap := negtypes.EndpointPodMap{} - validateAndAddEndpoints(tc.ep, fakeZoneGetter, podLister, matchPort, tc.endpointType, targetMap, endpointPodMap) + validateAndAddEndpoints(tc.ep, fakeZoneGetter, podLister, nodeLister, matchPort, tc.endpointType, targetMap, endpointPodMap) if !reflect.DeepEqual(targetMap, tc.expectedEndpointMap) { t.Errorf("degraded mode endpoint set is not calculated correctly:\ngot %+v,\n expected %+v", targetMap, tc.expectedEndpointMap) @@ -1416,6 +1434,15 @@ func TestValidateAndAddEndpoints(t *testing.T) { func TestValidatePod(t *testing.T) { t.Parallel() + instance1 := negtypes.TestInstance1 + testNodeNonExistent := "node-non-existent" + testContext := negtypes.NewTestContext() + nodeLister := testContext.NodeInformer.GetIndexer() + nodeLister.Add(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: instance1, + }, + }) testCases := []struct { desc string pod *v1.Pod @@ -1431,6 +1458,9 @@ func TestValidatePod(t *testing.T) { Status: v1.PodStatus{ Phase: v1.PodRunning, }, + Spec: corev1.PodSpec{ + NodeName: instance1, + }, }, expect: true, }, @@ -1444,6 +1474,9 @@ func TestValidatePod(t *testing.T) { Status: v1.PodStatus{ Phase: v1.PodFailed, }, + Spec: corev1.PodSpec{ + NodeName: instance1, + }, }, expect: false, }, @@ -1457,13 +1490,32 @@ func TestValidatePod(t *testing.T) { Status: v1.PodStatus{ Phase: v1.PodSucceeded, }, + Spec: corev1.PodSpec{ + NodeName: instance1, + }, + }, + expect: false, + }, + { + desc: "a pod from non-existent node", + pod: &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testNamespace, + Name: "pod4", + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + }, + Spec: corev1.PodSpec{ + NodeName: testNodeNonExistent, + }, }, expect: false, }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - if got := validatePod(tc.pod); got != tc.expect { + if got := validatePod(tc.pod, nodeLister); got != tc.expect { t.Errorf("validatePod() = %t, expected %t", got, tc.expect) } })