diff --git a/pkg/neg/syncers/endpoints_calculator.go b/pkg/neg/syncers/endpoints_calculator.go index 9754db894c..0352a41311 100644 --- a/pkg/neg/syncers/endpoints_calculator.go +++ b/pkg/neg/syncers/endpoints_calculator.go @@ -208,13 +208,13 @@ func nodeMapToString(nodeMap map[string][]*v1.Node) string { return strings.Join(str, ",") } -// ValidateEndpoints checks if endpoint information is correct for L7 Endpoint Calculator. -// It returns error if one of the two checks fails: +// ValidateEndpoints checks if endpoint information is correct. // -// 1. The endpoint count from endpointData doesn't equal to the one from endpointPodMap: -// endpiontPodMap removes the duplicated endpoints, and dupCount stores the number of duplicated it removed -// and we compare the endpoint counts with duplicates -// 2. The endpoint count from endpointData or the one from endpointPodMap is 0 +// For L7 Endpoint Calculator, it returns error if one of the two checks fails: +// 1. The endpoint count from endpointData doesn't equal to the one from endpointPodMap: +// endpiontPodMap removes the duplicated endpoints, and dupCount stores the number of duplicated it removed +// and we compare the endpoint counts with duplicates +// 2. The endpoint count from endpointData or the one from endpointPodMap is 0 func (l *L7EndpointsCalculator) ValidateEndpoints(endpointData []types.EndpointsData, endpointPodMap types.EndpointPodMap, dupCount int) error { // Endpoint count from EndpointPodMap countFromPodMap := len(endpointPodMap) + dupCount diff --git a/pkg/neg/syncers/transaction.go b/pkg/neg/syncers/transaction.go index 814341ae3e..a73d5b962c 100644 --- a/pkg/neg/syncers/transaction.go +++ b/pkg/neg/syncers/transaction.go @@ -187,7 +187,9 @@ func (s *transactionSyncer) syncInternal() error { start := time.Now() err := s.syncInternalImpl() - + if err != nil { + s.setErrorState(s.getErrorStateReason(err)) + } s.updateStatus(err) metrics.PublishNegSyncMetrics(string(s.NegSyncerKey.NegType), string(s.endpointsCalculator.Mode()), err, start) return err @@ -259,16 +261,15 @@ func (s *transactionSyncer) syncInternalImpl() error { } endpointsData := negtypes.EndpointsDataFromEndpointSlices(endpointSlices) targetMap, endpointPodMap, dupCount, err = s.endpointsCalculator.CalculateEndpoints(endpointsData, currentMap) - if valid, reason := s.isValidEPField(err); !valid { - s.setErrorState(reason) - } - if valid, reason := s.isValidEndpointInfo(endpointsData, endpointPodMap, dupCount); !valid { - s.setErrorState(reason) - } if err != nil { return fmt.Errorf("endpoints calculation error in mode %q, err: %w", s.endpointsCalculator.Mode(), err) } - + err = s.endpointsCalculator.ValidateEndpoints(endpointsData, endpointPodMap, dupCount) + if err != nil { + // TODO(cheungdavid): return error from ValidateEndpoint after degraded mode is implemented + // for now we don't return error so it won't break the sync + s.setErrorState(s.getErrorStateReason(err)) + } s.logStats(targetMap, "desired NEG endpoints") // Calculate the endpoints to add and delete to transform the current state to desire state @@ -354,69 +355,55 @@ func (s *transactionSyncer) ensureNetworkEndpointGroups() error { return utilerrors.NewAggregate(errList) } -// isValidEndpointInfo checks if endpoint information is correct. -// It returns false and the corresponding reason if one of the two checks fails: -// -// 1. The endpoint count from endpointData doesn't equal to the one from endpointPodMap: -// endpiontPodMap removes the duplicated endpoints, and dupCount stores the number of duplicated it removed -// and we compare the endpoint counts with duplicates -// 2. The endpoint count from endpointData or the one from endpointPodMap is 0 -func (s *transactionSyncer) isValidEndpointInfo(eds []negtypes.EndpointsData, endpointPodMap negtypes.EndpointPodMap, dupCount int) (bool, string) { - // Endpoint count from EndpointPodMap - countFromPodMap := len(endpointPodMap) + dupCount - if countFromPodMap == 0 { - s.logger.Info("Detected endpoint count from endpointPodMap going to zero", "endpointPodMap", endpointPodMap) - return false, negtypes.ResultEPCalculationCountZero - } - - // Endpoint count from EndpointData - countFromEndpointData := 0 - for _, ed := range eds { - countFromEndpointData += len(ed.Addresses) - } - if countFromEndpointData == 0 { - s.logger.Info("Detected endpoint count from endpointData going to zero", "endpointData", eds) - return false, negtypes.ResultEPSEndpointCountZero - } - - if countFromEndpointData != countFromPodMap { - s.logger.Info("Detected error when comparing endpoint counts", "endpointData", eds, "endpointPodMap", endpointPodMap, "dupCount", dupCount) - return false, negtypes.ResultEPCountsDiffer - } - return true, negtypes.ResultSuccess -} - -// isValidEPField returns false and the corresponding reason if there is endpoint with missing zone or nodeName -func (s *transactionSyncer) isValidEPField(err error) (bool, string) { +// getErrorStateReason returns the reason of the error state based returned error +func (s *transactionSyncer) getErrorStateReason(err error) string { if errors.Is(err, negtypes.ErrEPMissingNodeName) { s.logger.Info("Detected unexpected error when checking missing nodeName", "error", err) - return false, negtypes.ResultEPMissingNodeName + return negtypes.ResultEPMissingNodeName } if errors.Is(err, negtypes.ErrEPMissingZone) { s.logger.Info("Detected unexpected error when checking missing zone", "error", err) - return false, negtypes.ResultEPMissingZone + return negtypes.ResultEPMissingZone + } + if errors.Is(err, negtypes.ErrEPCalculationCountZero) { + return negtypes.ResultEPCalculationCountZero + } + if errors.Is(err, negtypes.ErrEPSEndpointCountZero) { + return negtypes.ResultEPSEndpointCountZero } - return true, negtypes.ResultSuccess + if errors.Is(err, negtypes.ErrEPCountsDiffer) { + return negtypes.ResultEPCountsDiffer + } + if errors.Is(err, negtypes.ErrInvalidAPIResponse) { + return negtypes.ResultInvalidAPIResponse + } + if errors.Is(err, negtypes.ErrInvalidEPAttach) { + return negtypes.ResultInvalidEPAttach + } + if errors.Is(err, negtypes.ErrInvalidEPDetach) { + return negtypes.ResultInvalidEPDetach + } + return "" } -// isValidEPBatch returns false and the corresponding reason if the error from endpoint batch response is due to bad request -func (s *transactionSyncer) isValidEPBatch(err error, operation transactionOp, networkEndpoints []*composite.NetworkEndpoint) (bool, string) { +// ValidEndpointBatch parse the error from endpoint batch response to NEG sync errors +func (s *transactionSyncer) ValidateEndpointBatch(err error, operation transactionOp) error { apiErr, ok := err.(*googleapi.Error) if !ok { s.logger.Info("Detected error when parsing batch response error", "operation", operation, "error", err) - return false, negtypes.ResultInvalidAPIResponse + return negtypes.ErrInvalidAPIResponse } errCode := apiErr.Code if errCode == http.StatusBadRequest { s.logger.Info("Detected error when sending endpoint batch information", "operation", operation, "errorCode", errCode) if operation == attachOp { - return false, negtypes.ResultInvalidEPAttach + return negtypes.ErrInvalidEPAttach } if operation == detachOp { - return false, negtypes.ResultInvalidEPDetach + return negtypes.ErrInvalidEPDetach } } - return true, negtypes.ResultSuccess + return nil } // syncNetworkEndpoints spins off go routines to execute NEG operations @@ -526,10 +513,11 @@ func (s *transactionSyncer) operationInternal(operation transactionOp, zone stri s.recordEvent(apiv1.EventTypeNormal, operation.String(), fmt.Sprintf("%s %d network endpoint(s) (NEG %q in zone %q)", operation.String(), len(networkEndpointMap), s.NegSyncerKey.NegName, zone)) } else { s.recordEvent(apiv1.EventTypeWarning, operation.String()+"Failed", fmt.Sprintf("Failed to %s %d network endpoint(s) (NEG %q in zone %q): %v", operation.String(), len(networkEndpointMap), s.NegSyncerKey.NegName, zone, err)) - if valid, reason := s.isValidEPBatch(err, operation, networkEndpoints); !valid { + endpointBatchErr := s.ValidateEndpointBatch(err, operation) + if endpointBatchErr != nil { s.syncLock.Lock() defer s.syncLock.Unlock() - s.setErrorState(reason) + s.setErrorState(s.getErrorStateReason(endpointBatchErr)) } } diff --git a/pkg/neg/syncers/transaction_test.go b/pkg/neg/syncers/transaction_test.go index b146e8cacd..c35b79783a 100644 --- a/pkg/neg/syncers/transaction_test.go +++ b/pkg/neg/syncers/transaction_test.go @@ -18,10 +18,8 @@ package syncers import ( context2 "context" - "errors" "fmt" "net" - "net/http" "reflect" "strconv" "testing" @@ -29,8 +27,6 @@ import ( "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" - "google.golang.org/api/compute/v1" - "google.golang.org/api/googleapi" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1392,967 +1388,6 @@ func TestUnknownNodes(t *testing.T) { } } -func TestIsValidEndpointInfo(t *testing.T) { - t.Parallel() - _, transactionSyncer := newTestTransactionSyncer(negtypes.NewAdapter(gce.NewFakeGCECloud(gce.DefaultTestClusterValues())), negtypes.VmIpPortEndpointType, false) - - instance1 := testInstance1 - instance2 := testInstance2 - instance4 := testInstance4 - testPortName := "" - testServiceNamespace := "namespace" - port80 := int32(80) - port81 := int32(81) - testIP1 := "10.100.1.1" - testIP2 := "10.100.1.2" - testIP3 := "10.100.2.2" - testIP4 := "10.100.4.1" - testPodName1 := "pod1" - testPodName2 := "pod2" - testPodName3 := "pod3" - testPodName4 := "pod4" - - testEndpointPodMap := map[negtypes.NetworkEndpoint]types.NamespacedName{ - { - IP: testIP1, - Port: "80", - Node: instance1, - }: { - Namespace: testServiceNamespace, - Name: testPodName1, - }, - { - IP: testIP2, - Port: "80", - Node: instance1, - }: { - Namespace: testServiceNamespace, - Name: testPodName2, - }, - { - IP: testIP3, - Port: "81", - Node: instance2, - }: { - Namespace: testServiceNamespace, - Name: testPodName3, - }, - { - IP: testIP4, - Port: "81", - Node: instance4, - }: { - Namespace: testServiceNamespace, - Name: testPodName4, - }, - } - - testCases := []struct { - desc string - endpointsData []negtypes.EndpointsData - endpointPodMap map[negtypes.NetworkEndpoint]types.NamespacedName - dupCount int - expect bool - expectedReason string - }{ - { - desc: "counts equal, endpointData has no duplicated endpoints", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName1, - }, - NodeName: &instance1, - Addresses: []string{testIP1}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - endpointPodMap: testEndpointPodMap, - dupCount: 0, - expect: true, - expectedReason: "", - }, - { - desc: "counts equal, endpointData has duplicated endpoints", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName1, - }, - NodeName: &instance1, - Addresses: []string{testIP1}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, // this is a duplicated endpoint - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - endpointPodMap: testEndpointPodMap, - dupCount: 1, - expect: true, - expectedReason: "", - }, - { - desc: "counts not equal, endpointData has no duplicated endpoints", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - endpointPodMap: testEndpointPodMap, - dupCount: 0, - expect: false, - expectedReason: negtypes.ResultEPCountsDiffer, - }, - { - desc: "counts not equal, endpointData has duplicated endpoints", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, // this is a duplicated endpoint - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - endpointPodMap: testEndpointPodMap, - dupCount: 1, - expect: false, - expectedReason: negtypes.ResultEPCountsDiffer, - }, - { - desc: "endpointData has zero endpoint", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{}, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{}, - }, - }, - endpointPodMap: testEndpointPodMap, - dupCount: 0, - expect: false, - expectedReason: negtypes.ResultEPSEndpointCountZero, - }, - { - desc: "endpointPodMap has zero endpoint", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName1, - }, - NodeName: &instance1, - Addresses: []string{testIP1}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - endpointPodMap: map[negtypes.NetworkEndpoint]types.NamespacedName{}, - dupCount: 0, - expect: false, - expectedReason: negtypes.ResultEPCalculationCountZero, - }, - { - desc: "endpointData and endpointPodMap both have zero endpoint", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{}, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{}, - }, - }, - endpointPodMap: map[negtypes.NetworkEndpoint]types.NamespacedName{}, - dupCount: 0, - expect: false, - expectedReason: negtypes.ResultEPCalculationCountZero, // PodMap count is check and returned first, - }, - { - desc: "endpointData and endpointPodMap both have non-zero endpoints", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName1, - }, - NodeName: &instance1, - Addresses: []string{testIP1}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - endpointPodMap: testEndpointPodMap, - dupCount: 0, - expect: true, - expectedReason: negtypes.ResultSuccess, - }, - } - - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - if got, reason := transactionSyncer.isValidEndpointInfo(tc.endpointsData, tc.endpointPodMap, tc.dupCount); got != tc.expect && reason != tc.expectedReason { - t.Errorf("invalidEndpointInfo() = %t, expected %t", got, tc.expect) - } - }) - } -} - -func TestIsValidEPField(t *testing.T) { - t.Parallel() - _, transactionSyncer := newTestTransactionSyncer(negtypes.NewAdapter(gce.NewFakeGCECloud(gce.DefaultTestClusterValues())), negtypes.VmIpPortEndpointType, false) - - instance1 := testInstance1 - instance2 := testInstance2 - instance4 := testInstance4 - testPortName := "" - testServiceNamespace := "namespace" - port80 := int32(80) - port81 := int32(81) - testIP1 := "10.100.1.1" - testIP2 := "10.100.1.2" - testIP3 := "10.100.2.2" - testIP4 := "10.100.4.1" - testPodName1 := "pod1" - testPodName2 := "pod2" - testPodName3 := "pod3" - testPodName4 := "pod4" - emptyZoneNodeName := "empty-zone-node" - emptyNodeName := "" - fakeZoneGetter := transactionSyncer.zoneGetter.(*negtypes.FakeZoneGetter) - if err := fakeZoneGetter.AddZone("", emptyZoneNodeName); err != nil { - t.Errorf("error when adding zone, expected nil, err: %v", err) - } - - testCases := []struct { - desc string - endpointsData []negtypes.EndpointsData - expect bool - expectedReason string - }{ - { - desc: "no missing zone or nodeName", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName1, - }, - NodeName: &instance1, - Addresses: []string{testIP1}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - expect: true, - expectedReason: negtypes.ResultSuccess, - }, - { - desc: "contains one missing nodeName", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName1, - }, - NodeName: nil, - Addresses: []string{testIP1}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - expect: false, - expectedReason: negtypes.ResultEPMissingNodeName, - }, - { - desc: "contains one empty nodeName", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName1, - }, - NodeName: &emptyNodeName, - Addresses: []string{testIP1}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - expect: false, - expectedReason: negtypes.ResultEPMissingNodeName, - }, - { - desc: "contains one missing zone", - endpointsData: []negtypes.EndpointsData{ - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-1", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port80, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName1, - }, - NodeName: &emptyZoneNodeName, - Addresses: []string{testIP1}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName2, - }, - NodeName: &instance1, - Addresses: []string{testIP2}, - Ready: true, - }, - }, - }, - { - Meta: &metav1.ObjectMeta{ - Name: testServiceName + "-2", - Namespace: testServiceNamespace, - }, - Ports: []negtypes.PortData{ - { - Name: testPortName, - Port: port81, - }, - }, - Addresses: []negtypes.AddressData{ - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName3, - }, - NodeName: &instance2, - Addresses: []string{testIP3}, - Ready: true, - }, - { - TargetRef: &corev1.ObjectReference{ - Namespace: testServiceNamespace, - Name: testPodName4, - }, - NodeName: &instance4, - Addresses: []string{testIP4}, - Ready: true, - }, - }, - }, - }, - expect: false, - expectedReason: negtypes.ResultEPMissingZone, - }, - } - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - _, _, _, err := transactionSyncer.endpointsCalculator.CalculateEndpoints(tc.endpointsData, nil) - if got, reason := transactionSyncer.isValidEPField(err); got != tc.expect && reason != tc.expectedReason { - t.Errorf("isValidEPField() = %t, expected %t, err: %v, ", got, tc.expect, err) - } - }) - } -} - -func TestCheckValidEPBatch(t *testing.T) { - fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) - fakeCloud := negtypes.NewAdapter(fakeGCE) - zone := "us-central1-a" - networkEndpoints := []*composite.NetworkEndpoint{} - - testCases := []struct { - desc string - APIResponse error - expect bool - expectedReason string - }{ - { - desc: "NEG API call server error, status code 500", - APIResponse: &googleapi.Error{ - Code: http.StatusOK, - }, - expect: true, - expectedReason: negtypes.ResultSuccess, - }, - { - desc: "NEG API call request error, status code 400", - APIResponse: &googleapi.Error{ - Code: http.StatusBadRequest, - }, - expect: false, - expectedReason: negtypes.ResultInvalidEPAttach, - }, - { - desc: "NEG API call invalid response", - APIResponse: errors.New("non googleapi error"), - expect: false, - expectedReason: negtypes.ResultInvalidAPIResponse, - }, - } - - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - mockGCE := fakeGCE.Compute().(*cloud.MockGCE) - mockGCE.MockNetworkEndpointGroups.AttachNetworkEndpointsHook = func(ctx context2.Context, key *meta.Key, arg0 *compute.NetworkEndpointGroupsAttachEndpointsRequest, neg *cloud.MockNetworkEndpointGroups) error { - return tc.APIResponse - } - _, transactionSyncer := newTestTransactionSyncer(fakeCloud, negtypes.VmIpPortEndpointType, false) - - err := transactionSyncer.cloud.AttachNetworkEndpoints(transactionSyncer.NegSyncerKey.NegName, zone, networkEndpoints, transactionSyncer.NegSyncerKey.GetAPIVersion()) - if got, reason := transactionSyncer.isValidEPBatch(err, attachOp, networkEndpoints); got != tc.expect || reason != tc.expectedReason { - t.Errorf("isInvalidEPBatch() = %t, expected %t", got, tc.expect) - } - }) - } -} - func newL4ILBTestTransactionSyncer(fakeGCE negtypes.NetworkEndpointGroupCloud, mode negtypes.EndpointsCalculatorMode) (negtypes.NegSyncer, *transactionSyncer) { negsyncer, ts := newTestTransactionSyncer(fakeGCE, negtypes.VmIpEndpointType, false) ts.endpointsCalculator = GetEndpointsCalculator(ts.nodeLister, ts.podLister, ts.zoneGetter, ts.NegSyncerKey, mode, klog.TODO())