diff --git a/pkg/neg/syncers/endpoints_calculator.go b/pkg/neg/syncers/endpoints_calculator.go index 2b6d8a13cb..3f6b3a67ab 100644 --- a/pkg/neg/syncers/endpoints_calculator.go +++ b/pkg/neg/syncers/endpoints_calculator.go @@ -219,7 +219,7 @@ func (l *L7EndpointsCalculator) CalculateEndpoints(eds []types.EndpointsData, _ // CalculateEndpoints determines the endpoints in the NEGs based on the current service endpoints and the current NEGs. func (l *L7EndpointsCalculator) CalculateEndpointsDegradedMode(eds []types.EndpointsData, _ map[string]types.NetworkEndpointSet) (map[string]types.NetworkEndpointSet, types.EndpointPodMap, error) { - result := toZoneNetworkEndpointMapDegradedMode(eds, l.zoneGetter, l.podLister, l.nodeLister, l.servicePortName, l.networkEndpointType) + result := toZoneNetworkEndpointMapDegradedMode(eds, l.zoneGetter, l.podLister, l.nodeLister, l.servicePortName, l.networkEndpointType, l.enableDualStackNEG) return result.NetworkEndpointSet, result.EndpointPodMap, nil } diff --git a/pkg/neg/syncers/transaction_test.go b/pkg/neg/syncers/transaction_test.go index 34c44d369d..73695c55ab 100644 --- a/pkg/neg/syncers/transaction_test.go +++ b/pkg/neg/syncers/transaction_test.go @@ -1803,12 +1803,16 @@ func TestEnableDegradedMode(t *testing.T) { _, s := newTestTransactionSyncer(fakeCloud, negtypes.VmIpPortEndpointType, false) s.NegSyncerKey.NegName = tc.negName s.needInit = false - addPodsToLister(s.podLister) + addPodsToLister(s.podLister, getDefaultEndpointSlices()) for i := 1; i <= 4; i++ { s.nodeLister.Add(&corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("instance%v", i), }, + Spec: corev1.NodeSpec{ + PodCIDR: fmt.Sprintf("10.100.%v.0/24", i), + PodCIDRs: []string{fmt.Sprintf("200%v:db8::/48", i), fmt.Sprintf("10.100.%v.0/24", i)}, + }, }) } for _, eps := range tc.testEndpointSlices { diff --git a/pkg/neg/syncers/utils.go b/pkg/neg/syncers/utils.go index d5985e428a..48700299d5 100644 --- a/pkg/neg/syncers/utils.go +++ b/pkg/neg/syncers/utils.go @@ -371,15 +371,11 @@ func getEndpointPod( // toZoneNetworkEndpointMap translates addresses in endpoints object into zone and endpoints map, and also return the count for duplicated endpoints // we will not raise error in degraded mode for misconfigured endpoints, instead they will be filtered directly -func toZoneNetworkEndpointMapDegradedMode( - eds []negtypes.EndpointsData, - zoneGetter negtypes.ZoneGetter, - podLister, nodeLister cache.Indexer, - servicePortName string, - networkEndpointType negtypes.NetworkEndpointType, -) ZoneNetworkEndpointMapResult { +func toZoneNetworkEndpointMapDegradedMode(eds []negtypes.EndpointsData, zoneGetter negtypes.ZoneGetter, podLister, nodeLister cache.Indexer, servicePortName string, networkEndpointType negtypes.NetworkEndpointType, enableDualStackNEG bool) ZoneNetworkEndpointMapResult { zoneNetworkEndpointMap := map[string]negtypes.NetworkEndpointSet{} networkEndpointPodMap := negtypes.EndpointPodMap{} + dupCount := 0 + ipsForPod := ipsForPod(eds) for _, ed := range eds { matchPort := "" for _, port := range ed.Ports { @@ -392,7 +388,7 @@ func toZoneNetworkEndpointMapDegradedMode( continue } for _, endpointAddress := range ed.Addresses { - if endpointAddress.AddressType != discovery.AddressTypeIPv4 { + if !enableDualStackNEG && endpointAddress.AddressType != discovery.AddressTypeIPv4 { klog.Infof("Skipping non IPv4 address in degraded mode: %q, in endpoint slice %s/%s", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name) continue } @@ -401,10 +397,6 @@ func toZoneNetworkEndpointMapDegradedMode( klog.Errorf("Endpoint %q in Endpoints %s/%s receives error when getting pod, err: %v, skipping", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name, getPodErr) continue } - if err := validatePod(pod, nodeLister); err != nil { - klog.Errorf("Endpoint %q in Endpoints %s/%s correponds to an invalid pod: %v, skipping", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name, err) - continue - } nodeName := pod.Spec.NodeName zone, err := zoneGetter.GetZoneForNode(nodeName) if err != nil { @@ -414,25 +406,46 @@ func toZoneNetworkEndpointMapDegradedMode( if zoneNetworkEndpointMap[zone] == nil { zoneNetworkEndpointMap[zone] = negtypes.NewNetworkEndpointSet() } - for _, address := range endpointAddress.Addresses { - networkEndpoint := negtypes.NetworkEndpoint{IP: address, Port: matchPort, Node: nodeName} - if networkEndpointType == negtypes.NonGCPPrivateEndpointType { - // Non-GCP network endpoints don't have associated nodes. - networkEndpoint.Node = "" - } - zoneNetworkEndpointMap[zone].Insert(networkEndpoint) - - if existingPod, contains := networkEndpointPodMap[networkEndpoint]; contains { - // if existing name is alphabetically lower than current one, continue and don't replace - if existingPod.Name < endpointAddress.TargetRef.Name { - klog.Infof("Found duplicate endpoints for %q, save the pod information from the alphabetically higher pod", address) - continue - } + + podIPs := ipsForPod[types.NamespacedName{Namespace: endpointAddress.TargetRef.Namespace, Name: endpointAddress.TargetRef.Name}] + // TODO(cheungdavid): Remove this validation when single stack ipv6 endpoint is supported + if parseIPAddress(podIPs.IP) == "" { + klog.Errorf("For endpoint %q in pod %q, it has an invalid IPv4 address, err: %v, skipping", endpointAddress.Addresses, pod.ObjectMeta.Name, negtypes.ErrEPIPNotFromPod) + continue + } + networkEndpoint := negtypes.NetworkEndpoint{IP: podIPs.IP, Port: matchPort, Node: nodeName} + if enableDualStackNEG { + // Convert all addresses to a standard form as per rfc5952 to prevent + // accidental diffs resulting from different formats. + networkEndpoint.IPv6 = parseIPAddress(podIPs.IPv6) + } + // endpoint address should match to the IP of its pod + if err = podContainsEndpointAddress(networkEndpoint, pod); err != nil { + klog.Errorf("Endpoint %q in Endpoints %s/%s has IP(s) not match to its pod %s: %w, skipping", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name, pod.Name, err) + continue + } + if err := validatePod(pod, nodeLister, networkEndpoint); err != nil { + klog.Errorf("Endpoint %q in Endpoints %s/%s correponds to an invalid pod: %v, skipping", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name, err) + continue + } + if networkEndpointType == negtypes.NonGCPPrivateEndpointType { + // Non-GCP network endpoints don't have associated nodes. + networkEndpoint.Node = "" + } + zoneNetworkEndpointMap[zone].Insert(networkEndpoint) + + // if existing name is alphabetically lower than current one, continue and don't replace + if existingPod, contains := networkEndpointPodMap[networkEndpoint]; contains { + dupCount += 1 + if existingPod.Name < endpointAddress.TargetRef.Name { + klog.Infof("Found duplicate endpoints for %v, save the pod information from the alphabetically higher pod", networkEndpoint) + continue // if existing name is alphabetically lower than current one, continue and don't replace } - networkEndpointPodMap[networkEndpoint] = types.NamespacedName{Namespace: endpointAddress.TargetRef.Namespace, Name: endpointAddress.TargetRef.Name} } + networkEndpointPodMap[networkEndpoint] = types.NamespacedName{Namespace: endpointAddress.TargetRef.Namespace, Name: endpointAddress.TargetRef.Name} } } + return ZoneNetworkEndpointMapResult{ NetworkEndpointSet: zoneNetworkEndpointMap, EndpointPodMap: networkEndpointPodMap, @@ -443,7 +456,8 @@ func toZoneNetworkEndpointMapDegradedMode( // it returns error if the pod: // 1. is in terminal state // 2. corresponds to a non-existent node -func validatePod(pod *apiv1.Pod, nodeLister cache.Indexer) error { +// 3. have an IP that matches to a podIP, but is outside of the node's allocated IP range +func validatePod(pod *apiv1.Pod, nodeLister cache.Indexer, networkEndpoint negtypes.NetworkEndpoint) error { // Terminal Pod means a pod is in PodFailed or PodSucceeded phase phase := pod.Status.Phase if phase == apiv1.PodFailed || phase == apiv1.PodSucceeded { @@ -453,10 +467,13 @@ func validatePod(pod *apiv1.Pod, nodeLister cache.Indexer) error { if err != nil || !exists { return negtypes.ErrEPNodeNotFound } - _, isNode := obj.(*apiv1.Node) - if !isNode { + node, ok := obj.(*apiv1.Node) + if !ok { return negtypes.ErrEPNodeTypeAssertionFailed } + if err = nodeContainsPodIP(node, networkEndpoint); err != nil { + return err + } return nil } @@ -486,6 +503,66 @@ func ipsForPod(eds []negtypes.EndpointsData) map[types.NamespacedName]negtypes.N return result } +// podContainsEndpointAddress checks the pod's existing PodIP(s) +// and return error if the given endpoint's IP address does not any of them. +// If this is a dual stack endpoint, we would validate both IPs +func podContainsEndpointAddress(networkEndpoint negtypes.NetworkEndpoint, pod *apiv1.Pod) error { + // TODO(cheungdavid): update ipv4 check when single stack ipv6 is supported + endpointIPs := []string{networkEndpoint.IP} + if networkEndpoint.IPv6 != "" { + endpointIPs = append(endpointIPs, networkEndpoint.IPv6) + } + + matching := 0 + for _, endpointIP := range endpointIPs { + // a pod can have at most two PodIPs, one for ipv4 and one for ipv6 + for _, podIP := range pod.Status.PodIPs { + if endpointIP == podIP.IP { + matching += 1 + } + } + } + if matching != len(endpointIPs) { + return fmt.Errorf("%w: endpoint %v has IP(s) not match to its pod's IP(s)", negtypes.ErrEPIPNotFromPod, endpointIPs) + } + return nil +} + +// nodeContainsPodIP checks the node's existing PodCIDR(s), +// and return error if the pod IP used by the endpoint is not within one of the podCIDR ranges. +// If this is a dual stack endpoint, we would validate both pod IPs +func nodeContainsPodIP(node *apiv1.Node, networkEndpoint negtypes.NetworkEndpoint) error { + ipnets := []*net.IPNet{} + // a node can have at most two PodCIDRs, one for ipv4 and one for ipv6 + for _, podCIDR := range node.Spec.PodCIDRs { + podCIDR = strings.TrimSpace(podCIDR) + _, ipnet, err := net.ParseCIDR(podCIDR) + if err != nil { + // swallow errors for CIDRs that are invalid + continue + } + ipnets = append(ipnets, ipnet) + } + podIPs := []net.IP{net.ParseIP(networkEndpoint.IP)} + if networkEndpoint.IPv6 != "" { + podIPs = append(podIPs, net.ParseIP(networkEndpoint.IPv6)) + } + + matching := 0 + for _, podIP := range podIPs { + for _, net := range ipnets { + if net.Contains(podIP) { + matching += 1 + break + } + } + } + if matching != len(podIPs) { + return fmt.Errorf("%w: podIP(s) used by endpoint %v not match to the node's PodCIDR range(s)", negtypes.ErrEPIPOutOfPodCIDR, podIPs) + } + return nil +} + // retrieveExistingZoneNetworkEndpointMap lists existing network endpoints in the neg and return the zone and endpoints map func retrieveExistingZoneNetworkEndpointMap(negName string, zoneGetter negtypes.ZoneGetter, cloud negtypes.NetworkEndpointGroupCloud, version meta.Version, mode negtypes.EndpointsCalculatorMode) (map[string]negtypes.NetworkEndpointSet, labels.EndpointPodLabelMap, error) { // Include zones that have non-candidate nodes currently. It is possible that NEGs were created in those zones previously and the endpoints now became non-candidates. diff --git a/pkg/neg/syncers/utils_test.go b/pkg/neg/syncers/utils_test.go index f15e4df8ea..6596956418 100644 --- a/pkg/neg/syncers/utils_test.go +++ b/pkg/neg/syncers/utils_test.go @@ -22,12 +22,12 @@ import ( "net" "reflect" "strconv" + "strings" "testing" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" "github.com/google/go-cmp/cmp" - corev1 "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" discovery "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -482,7 +482,8 @@ func TestToZoneNetworkEndpointMap(t *testing.T) { t.Parallel() zoneGetter := negtypes.NewFakeZoneGetter() podLister := negtypes.NewTestContext().PodInformer.GetIndexer() - addPodsToLister(podLister) + testEndpointSlice := getDefaultEndpointSlices() + addPodsToLister(podLister, testEndpointSlice) testCases := []struct { desc string portName string @@ -1543,14 +1544,18 @@ func TestToZoneNetworkEndpointMapDegradedMode(t *testing.T) { fakeZoneGetter := negtypes.NewFakeZoneGetter() testContext := negtypes.NewTestContext() podLister := testContext.PodInformer.GetIndexer() - addPodsToLister(podLister) + addPodsToLister(podLister, getDefaultEndpointSlices()) nodeLister := testContext.NodeInformer.GetIndexer() for i := 1; i <= 4; i++ { - nodeLister.Add(&corev1.Node{ + nodeLister.Add(&v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("instance%v", i), }, + Spec: v1.NodeSpec{ + PodCIDR: fmt.Sprintf("10.100.%v.0/24", i), + PodCIDRs: []string{fmt.Sprintf("200%v:db8::/48", i), fmt.Sprintf("10.100.%v.0/24", i)}, + }, }) } @@ -1648,7 +1653,7 @@ func TestToZoneNetworkEndpointMapDegradedMode(t *testing.T) { } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - result := toZoneNetworkEndpointMapDegradedMode(negtypes.EndpointsDataFromEndpointSlices(tc.testEndpointSlices), fakeZoneGetter, podLister, nodeLister, tc.portName, tc.networkEndpointType) + result := toZoneNetworkEndpointMapDegradedMode(negtypes.EndpointsDataFromEndpointSlices(tc.testEndpointSlices), fakeZoneGetter, podLister, nodeLister, tc.portName, tc.networkEndpointType, false) if !reflect.DeepEqual(result.NetworkEndpointSet, tc.expectedEndpointMap) { t.Errorf("degraded mode endpoint set is not calculated correctly:\ngot %+v,\n expected %+v", result.NetworkEndpointSet, tc.expectedEndpointMap) } @@ -1666,27 +1671,52 @@ func TestDegradedModeValidateEndpointInfo(t *testing.T) { port80 := int32(80) protocolTCP := v1.ProtocolTCP instance1 := negtypes.TestInstance1 + instance2 := negtypes.TestInstance2 + instance3 := negtypes.TestInstance3 + instance4 := negtypes.TestInstance4 fakeZoneGetter := negtypes.NewFakeZoneGetter() testContext := negtypes.NewTestContext() podLister := testContext.PodInformer.GetIndexer() - addPodsToLister(podLister) + addPodsToLister(podLister, getDefaultEndpointSlices()) nodeLister := testContext.NodeInformer.GetIndexer() - nodeLister.Add(&corev1.Node{ + nodeLister.Add(&v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: instance1, }, + Spec: v1.NodeSpec{ + PodCIDR: "10.100.1.0/24", + PodCIDRs: []string{"2001:db8::/48", "10.100.1.0/24"}, + }, + }) + nodeLister.Add(&v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: instance3, + }, + Spec: v1.NodeSpec{ + PodCIDR: "10.100.3.0/24", + PodCIDRs: []string{"a:b::/48", "10.100.3.0/24"}, + }, + }) + nodeLister.Add(&v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: instance4, + }, + Spec: v1.NodeSpec{ + PodCIDR: "10.100.4.0/24", + PodCIDRs: []string{"a:b::/48", "10.100.4.0/24"}, + }, }) endpointMap := map[string]negtypes.NetworkEndpointSet{ negtypes.TestZone1: negtypes.NewNetworkEndpointSet( - networkEndpointFromEncodedEndpoint("10.100.1.1||instance1||80"), - networkEndpointFromEncodedEndpoint("10.100.1.2||instance1||80"), + negtypes.NetworkEndpoint{IP: "10.100.1.1", Node: instance1, Port: "80"}, + negtypes.NetworkEndpoint{IP: "10.100.1.2", Node: instance1, Port: "80"}, ), } podMap := negtypes.EndpointPodMap{ - networkEndpointFromEncodedEndpoint("10.100.1.1||instance1||80"): types.NamespacedName{Namespace: testServiceNamespace, Name: "pod1"}, - networkEndpointFromEncodedEndpoint("10.100.1.2||instance1||80"): types.NamespacedName{Namespace: testServiceNamespace, Name: "pod2"}, + negtypes.NetworkEndpoint{IP: "10.100.1.1", Node: instance1, Port: "80"}: types.NamespacedName{Namespace: testServiceNamespace, Name: "pod1"}, + negtypes.NetworkEndpoint{IP: "10.100.1.2", Node: instance1, Port: "80"}: types.NamespacedName{Namespace: testServiceNamespace, Name: "pod2"}, } testCases := []struct { @@ -1697,7 +1727,7 @@ func TestDegradedModeValidateEndpointInfo(t *testing.T) { expectedPodMap negtypes.EndpointPodMap }{ { - desc: "endpoint without nodeName, nodeName should be filled", + desc: "contains one endpoint without nodeName, nodeName should be filled", testEndpointSlices: []*discovery.EndpointSlice{ { ObjectMeta: metav1.ObjectMeta{ @@ -1740,7 +1770,7 @@ func TestDegradedModeValidateEndpointInfo(t *testing.T) { expectedPodMap: podMap, }, { - desc: "endpoint with empty nodeName, nodeName should be filled", + desc: "contains one endpoint with empty nodeName, nodeName should be filled", testEndpointSlices: []*discovery.EndpointSlice{ { ObjectMeta: metav1.ObjectMeta{ @@ -1782,10 +1812,163 @@ func TestDegradedModeValidateEndpointInfo(t *testing.T) { expectedEndpointMap: endpointMap, expectedPodMap: podMap, }, + { + desc: "single stack ipv4 endpoints, contains one endpoint with IPv4 address not matching to its pod, endpoint should be removed", + testEndpointSlices: []*discovery.EndpointSlice{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: testServiceName + "-1", + Namespace: testServiceNamespace, + Labels: map[string]string{ + discovery.LabelServiceName: testServiceName, + }, + }, + AddressType: "IPv4", + Endpoints: []discovery.Endpoint{ + { + Addresses: []string{"10.100.1.1"}, + NodeName: &instance1, + TargetRef: &v1.ObjectReference{ + Namespace: testServiceNamespace, + Name: "pod1", + }, + }, + { + Addresses: []string{"10.100.1.2"}, + NodeName: &instance1, + TargetRef: &v1.ObjectReference{ + Namespace: testServiceNamespace, + Name: "pod2", + }, + }, + { + Addresses: []string{"10.100.2.2"}, // the IPv4 address of this pod is 10.100.2.1 + NodeName: &instance2, + TargetRef: &v1.ObjectReference{ + Namespace: testServiceNamespace, + Name: "pod3", + }, + }, + }, + Ports: []discovery.EndpointPort{ + { + Name: &emptyNamedPort, + Port: &port80, + Protocol: &protocolTCP, + }, + }, + }, + }, + endpointType: negtypes.VmIpPortEndpointType, + expectedEndpointMap: endpointMap, + expectedPodMap: podMap, + }, + { + desc: "dual stack endpoints, contains one endpoint with IPv6 address not matching to its pod, endpoint should be removed", + testEndpointSlices: []*discovery.EndpointSlice{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: testServiceName + "-3", + Namespace: testServiceNamespace, + Labels: map[string]string{ + discovery.LabelServiceName: testServiceName, + }, + }, + AddressType: "IPv4", + Endpoints: []discovery.Endpoint{ + { + Addresses: []string{"10.100.3.2"}, + NodeName: &instance3, + TargetRef: &v1.ObjectReference{ + Namespace: testServiceNamespace, + Name: "pod10", + }, + }, + { + Addresses: []string{"10.100.4.2"}, + NodeName: &instance4, + TargetRef: &v1.ObjectReference{ + Namespace: testServiceNamespace, + Name: "pod11", + }, + }, + { + Addresses: []string{"10.100.4.4"}, + NodeName: &instance4, + TargetRef: &v1.ObjectReference{ + Namespace: testServiceNamespace, + Name: "pod12", + }, + }, + }, + Ports: []discovery.EndpointPort{ + { + Name: &emptyNamedPort, + Port: &port80, + Protocol: &protocolTCP, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: testServiceName + "-4", + Namespace: testServiceNamespace, + Labels: map[string]string{ + discovery.LabelServiceName: testServiceName, + }, + }, + AddressType: "IPv6", + Endpoints: []discovery.Endpoint{ + { + Addresses: []string{"a:b::1"}, + NodeName: &instance3, + TargetRef: &v1.ObjectReference{ + Namespace: testServiceNamespace, + Name: "pod10", + }, + }, + { + Addresses: []string{"a:b::2"}, + NodeName: &instance4, + TargetRef: &v1.ObjectReference{ + Namespace: testServiceNamespace, + Name: "pod11", + }, + }, + { + Addresses: []string{"a:b::4"}, // the IPv6 address of this pod is a:b::3 + NodeName: &instance4, + TargetRef: &v1.ObjectReference{ + Namespace: testServiceNamespace, + Name: "pod12", + }, + }, + }, + Ports: []discovery.EndpointPort{ + { + Name: &emptyNamedPort, + Port: &port80, + Protocol: &protocolTCP, + }, + }, + }, + }, + endpointType: negtypes.VmIpPortEndpointType, + expectedEndpointMap: map[string]negtypes.NetworkEndpointSet{ + negtypes.TestZone2: negtypes.NewNetworkEndpointSet( + negtypes.NetworkEndpoint{IP: "10.100.3.2", IPv6: "a:b::1", Node: instance3, Port: "80"}, + negtypes.NetworkEndpoint{IP: "10.100.4.2", IPv6: "a:b::2", Node: instance4, Port: "80"}, + ), + }, + expectedPodMap: negtypes.EndpointPodMap{ + negtypes.NetworkEndpoint{IP: "10.100.3.2", IPv6: "a:b::1", Node: instance3, Port: "80"}: types.NamespacedName{Namespace: testServiceNamespace, Name: "pod10"}, + negtypes.NetworkEndpoint{IP: "10.100.4.2", IPv6: "a:b::2", Node: instance4, Port: "80"}: types.NamespacedName{Namespace: testServiceNamespace, Name: "pod11"}, + }, + }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - result := toZoneNetworkEndpointMapDegradedMode(negtypes.EndpointsDataFromEndpointSlices(tc.testEndpointSlices), fakeZoneGetter, podLister, nodeLister, emptyNamedPort, tc.endpointType) + result := toZoneNetworkEndpointMapDegradedMode(negtypes.EndpointsDataFromEndpointSlices(tc.testEndpointSlices), fakeZoneGetter, podLister, nodeLister, emptyNamedPort, tc.endpointType, true) if !reflect.DeepEqual(result.NetworkEndpointSet, tc.expectedEndpointMap) { t.Errorf("degraded mode endpoint set is not calculated correctly:\ngot %+v,\n expected %+v", result.NetworkEndpointSet, tc.expectedEndpointMap) } @@ -1803,18 +1986,28 @@ func TestValidatePod(t *testing.T) { testNodeNonExistent := "node-non-existent" testContext := negtypes.NewTestContext() nodeLister := testContext.NodeInformer.GetIndexer() - nodeLister.Add(&corev1.Node{ + nodeLister.Add(&v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: instance1, }, + Spec: v1.NodeSpec{ + PodCIDR: "10.100.1.0/24", + PodCIDRs: []string{"2001:db8::/48", "10.100.1.0/24"}, + }, }) + testPodIPv4 := "10.100.1.1" + testPodIPv4OutOfRange := "10.101.1.1" + testPodIPv6 := "2001:db8::2:1" + testPodIPv6OutOfRange := "2001:db9::2:1" + testCases := []struct { - desc string - pod *v1.Pod - expectErr error + desc string + pod *v1.Pod + networkEndpoint negtypes.NetworkEndpoint + expectErr error }{ { - desc: "a valid pod with phase running", + desc: "a valid pod with single stack IPv4 address and phase running", pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: testServiceNamespace, @@ -1823,64 +2016,188 @@ func TestValidatePod(t *testing.T) { Status: v1.PodStatus{ Phase: v1.PodRunning, }, - Spec: corev1.PodSpec{ + Spec: v1.PodSpec{ NodeName: instance1, }, }, - expectErr: nil, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv4}, + expectErr: nil, }, { - desc: "a terminal pod with phase failed", + desc: "a valid pod with single stack IPv6 address and phase running", pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: testServiceNamespace, Name: "pod2", }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + Spec: v1.PodSpec{ + NodeName: instance1, + }, + }, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv6}, + expectErr: nil, + }, + { + desc: "a terminal pod with phase failed", + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testServiceNamespace, + Name: "pod3", + }, Status: v1.PodStatus{ Phase: v1.PodFailed, }, - Spec: corev1.PodSpec{ + Spec: v1.PodSpec{ NodeName: instance1, }, }, - expectErr: negtypes.ErrEPPodTerminal, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv4}, + expectErr: negtypes.ErrEPPodTerminal, }, { desc: "a terminal pod with phase succeeded", pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: testServiceNamespace, - Name: "pod3", + Name: "pod4", }, Status: v1.PodStatus{ Phase: v1.PodSucceeded, }, - Spec: corev1.PodSpec{ + Spec: v1.PodSpec{ NodeName: instance1, }, }, - expectErr: negtypes.ErrEPPodTerminal, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv4}, + expectErr: negtypes.ErrEPPodTerminal, }, { desc: "a pod from non-existent node", - pod: &corev1.Pod{ + pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Namespace: testServiceNamespace, - Name: "pod4", + Name: "pod5", }, - Status: corev1.PodStatus{ - Phase: corev1.PodRunning, + Status: v1.PodStatus{ + Phase: v1.PodRunning, }, - Spec: corev1.PodSpec{ + Spec: v1.PodSpec{ NodeName: testNodeNonExistent, }, }, - expectErr: negtypes.ErrEPNodeNotFound, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv4}, + expectErr: negtypes.ErrEPNodeNotFound, + }, + { + desc: "a pod with single stack IPv4 IP address outside of the node's allocated pod range", + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testServiceNamespace, + Name: "pod6", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + Spec: v1.PodSpec{ + NodeName: instance1, + }, + }, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv4OutOfRange}, + expectErr: negtypes.ErrEPIPOutOfPodCIDR, + }, + { + desc: "a pod with single stack IPv6 IP address outside of the node's allocated pod range", + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testServiceNamespace, + Name: "pod7", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + Spec: v1.PodSpec{ + NodeName: instance1, + }, + }, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv6OutOfRange}, + expectErr: negtypes.ErrEPIPOutOfPodCIDR, + }, + { + desc: "a pod with dual stack, and both of its ips are within the range", + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testServiceNamespace, + Name: "pod8", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + Spec: v1.PodSpec{ + NodeName: instance1, + }, + }, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv4, IPv6: testPodIPv6}, + expectErr: nil, + }, + { + desc: "a pod with dual stack, and its IPv4 IP address is outside of the node's allocated pod range", + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testServiceNamespace, + Name: "pod9", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + Spec: v1.PodSpec{ + NodeName: instance1, + }, + }, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv4OutOfRange, IPv6: testPodIPv6}, + expectErr: negtypes.ErrEPIPOutOfPodCIDR, + }, + { + desc: "a pod with dual stack, and its IPv6 IP address is outside of the node's allocated pod range", + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testServiceNamespace, + Name: "pod10", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + Spec: v1.PodSpec{ + NodeName: instance1, + }, + }, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv4, IPv6: testPodIPv6OutOfRange}, + expectErr: negtypes.ErrEPIPOutOfPodCIDR, + }, + { + desc: "a pod with single stack IPv4, and its IPv6 IP address is empty", + pod: &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: testServiceNamespace, + Name: "pod11", + }, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + }, + Spec: v1.PodSpec{ + NodeName: instance1, + }, + }, + networkEndpoint: negtypes.NetworkEndpoint{IP: testPodIPv4, IPv6: ""}, + expectErr: nil, }, } for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { - if got := validatePod(tc.pod, nodeLister); !errors.Is(got, tc.expectErr) { + + if got := validatePod(tc.pod, nodeLister, tc.networkEndpoint); !errors.Is(got, tc.expectErr) { t.Errorf("validatePod() = %t, expected %t\n", got, tc.expectErr) } }) @@ -1927,85 +2244,40 @@ func TestParseIPAddress(t *testing.T) { } } -func addPodsToLister(podLister cache.Indexer) { - // add all pods in default endpoint into podLister - for i := 1; i <= 6; i++ { - podLister.Add(&corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testServiceNamespace, - Name: fmt.Sprintf("pod%v", i), - }, - Status: corev1.PodStatus{ - Phase: v1.PodRunning, - }, - Spec: corev1.PodSpec{ - NodeName: testInstance1, - }, - }) +// addPodsToLister takes endpoints from endpointSlices +// and add pods to podLister based on endpoints' IPs and pod mapping. +func addPodsToLister(podLister cache.Indexer, endpointSlices []*discovery.EndpointSlice) { + // collect both ipv4 and ipv6 IP address for pods + podToIPs := make(map[string][]v1.PodIP) + podToNodeName := make(map[string]string) + for _, eps := range endpointSlices { + for _, ep := range eps.Endpoints { + pod := fmt.Sprintf("%s/%s", ep.TargetRef.Namespace, ep.TargetRef.Name) + podToNodeName[pod] = *ep.NodeName + for _, addr := range ep.Addresses { + podToIPs[pod] = append(podToIPs[pod], v1.PodIP{IP: addr}) + } + } } - for i := 7; i <= 12; i++ { - podLister.Add(&corev1.Pod{ + for pod, IPs := range podToIPs { + strs := strings.Split(pod, "/") + podNamespace := strs[0] + podName := strs[1] + podLister.Add(&v1.Pod{ ObjectMeta: metav1.ObjectMeta{ - Namespace: testServiceNamespace, - Name: fmt.Sprintf("pod%v", i), + Namespace: podNamespace, + Name: podName, }, - Status: corev1.PodStatus{ - Phase: v1.PodRunning, + Spec: v1.PodSpec{ + NodeName: podToNodeName[pod], }, - Spec: corev1.PodSpec{ - NodeName: testInstance4, + Status: v1.PodStatus{ + Phase: v1.PodRunning, + PodIP: IPs[0].IP, + PodIPs: IPs, }, }) } - - podLister.Update(&corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testServiceNamespace, - Name: "pod3", - }, - Status: corev1.PodStatus{ - Phase: v1.PodRunning, - }, - Spec: corev1.PodSpec{ - NodeName: testInstance2, - }, - }) - podLister.Update(&corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testServiceNamespace, - Name: "pod4", - }, - Status: corev1.PodStatus{ - Phase: v1.PodRunning, - }, - Spec: corev1.PodSpec{ - NodeName: testInstance3, - }, - }) - podLister.Update(&corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testServiceNamespace, - Name: "pod7", - }, - Status: corev1.PodStatus{ - Phase: v1.PodRunning, - }, - Spec: corev1.PodSpec{ - NodeName: testInstance2, - }, - }) - podLister.Update(&corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testServiceNamespace, - Name: "pod10", - }, - Status: corev1.PodStatus{ - Phase: v1.PodRunning, - }, - Spec: corev1.PodSpec{ - NodeName: testInstance3, - }, - }) } // numToIP converts the given number to an IP address. diff --git a/pkg/neg/types/sync_errors.go b/pkg/neg/types/sync_errors.go index c7743d358e..641183ef93 100644 --- a/pkg/neg/types/sync_errors.go +++ b/pkg/neg/types/sync_errors.go @@ -32,6 +32,8 @@ const ( ReasonInvalidAPIResponse = Reason("InvalidAPIResponse") ReasonInvalidEPAttach = Reason("InvalidEPAttach") ReasonInvalidEPDetach = Reason("InvalidEPDetach") + ReasonEPIPNotFromPod = Reason("EPIPNotFromPod") + ReasonEPIPOutOfPodCIDR = Reason("EPIPOutOfPodCIDR") // these are for non error-state error ReasonNegNotFound = Reason("NegNotFound") @@ -112,6 +114,17 @@ var ( Reason: ReasonInvalidEPDetach, IsErrorState: true, } + ErrEPIPNotFromPod = NegSyncError{ + Err: errors.New("endpoint has an IP that does not correspond to its pod"), + Reason: ReasonEPIPNotFromPod, + IsErrorState: true, + } + ErrEPIPOutOfPodCIDR = NegSyncError{ + Err: errors.New("endpoint corresponds to a pod with IP out of PodCIDR range"), + Reason: ReasonEPIPOutOfPodCIDR, + IsErrorState: true, + } + ErrNegNotFound = NegSyncError{ Err: errors.New("failed to get NEG for service"), Reason: ReasonNegNotFound,