diff --git a/pkg/neg/controller.go b/pkg/neg/controller.go index c1666c4c6c..41997ea33b 100644 --- a/pkg/neg/controller.go +++ b/pkg/neg/controller.go @@ -340,6 +340,7 @@ func (c *Controller) Run(stopCh <-chan struct{}) { wait.Until(c.gc, c.gcPeriod, stopCh) }() go c.reflector.Run(stopCh) + go c.syncerMetrics.Run(stopCh) <-stopCh } diff --git a/pkg/neg/metrics/neg_metrics_collector.go b/pkg/neg/metrics/neg_metrics_collector.go index 7cec9491c9..285c971122 100644 --- a/pkg/neg/metrics/neg_metrics_collector.go +++ b/pkg/neg/metrics/neg_metrics_collector.go @@ -20,11 +20,27 @@ import ( "sync" "time" + "github.com/prometheus/client_golang/prometheus" "k8s.io/apimachinery/pkg/util/wait" negtypes "k8s.io/ingress-gce/pkg/neg/types" "k8s.io/klog/v2" ) +var ( + syncResultLabel = "result" + syncResultKey = "sync_result" + + // syncerSyncResult tracks the count for each sync result + syncerSyncResult = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Subsystem: negControllerSubsystem, + Name: syncResultKey, + Help: "Current count for each sync result", + }, + []string{syncResultLabel}, + ) +) + type SyncerMetricsCollector interface { UpdateSyncer(key negtypes.NegSyncerKey, result *negtypes.NegSyncResult) } @@ -56,6 +72,7 @@ func FakeSyncerMetrics() *SyncerMetrics { // RegisterSyncerMetrics registers syncer related metrics func RegisterSyncerMetrics() { + prometheus.MustRegister(syncerSyncResult) } func (sm *SyncerMetrics) Run(stopCh <-chan struct{}) { @@ -74,6 +91,11 @@ func (sm *SyncerMetrics) export() { // UpdateSyncer update the status of corresponding syncer based on the syncResult. func (sm *SyncerMetrics) UpdateSyncer(key negtypes.NegSyncerKey, syncResult *negtypes.NegSyncResult) { + if syncResult.Result == negtypes.ResultInProgress { + return + } + syncerSyncResult.WithLabelValues(syncResult.Result).Inc() + sm.mu.Lock() defer sm.mu.Unlock() if sm.syncerStatusMap == nil { diff --git a/pkg/neg/syncers/transaction.go b/pkg/neg/syncers/transaction.go index fbc1078927..028aea3ffd 100644 --- a/pkg/neg/syncers/transaction.go +++ b/pkg/neg/syncers/transaction.go @@ -193,14 +193,17 @@ func (s *transactionSyncer) syncInternal() error { defer s.syncLock.Unlock() start := time.Now() - err := s.syncInternalImpl() + result := s.syncInternalImpl() - s.updateStatus(err) - metrics.PublishNegSyncMetrics(string(s.NegSyncerKey.NegType), string(s.endpointsCalculator.Mode()), err, start) - return err + s.updateStatus(result.Error) + metrics.PublishNegSyncMetrics(string(s.NegSyncerKey.NegType), string(s.endpointsCalculator.Mode()), result.Error, start) + if s.enableEndpointSlices && result.Error != nil { + s.syncCollector.UpdateSyncer(s.NegSyncerKey, result) + } + return result.Error } -func (s *transactionSyncer) syncInternalImpl() error { +func (s *transactionSyncer) syncInternalImpl() *negtypes.NegSyncResult { // TODO(cheungdavid): for now we reset the boolean so it is a no-op, but // in the future, it will be used to trigger degraded mode if the syncer is in error state. if s.inErrorState() { @@ -209,20 +212,20 @@ func (s *transactionSyncer) syncInternalImpl() error { if s.needInit || s.isZoneChange() { if err := s.ensureNetworkEndpointGroups(); err != nil { - return err + return negtypes.NewNegSyncResult(err, negtypes.ResultNegNotFound) } s.needInit = false } if s.syncer.IsStopped() || s.syncer.IsShuttingDown() { s.logger.V(3).Info("Skip syncing NEG", "negSyncerKey", s.NegSyncerKey.String()) - return nil + return negtypes.NewNegSyncResult(nil, negtypes.ResultSuccess) } s.logger.V(2).Info("Sync NEG", "negSyncerKey", s.NegSyncerKey.String(), "endpointsCalculatorMode", s.endpointsCalculator.Mode()) currentMap, err := retrieveExistingZoneNetworkEndpointMap(s.NegSyncerKey.NegName, s.zoneGetter, s.cloud, s.NegSyncerKey.GetAPIVersion(), s.endpointsCalculator.Mode()) if err != nil { - return err + return negtypes.NewNegSyncResult(err, negtypes.ResultCurrentEPNotFound) } s.logStats(currentMap, "current NEG endpoints") @@ -238,11 +241,11 @@ func (s *transactionSyncer) syncInternalImpl() error { if s.enableEndpointSlices { slices, err := s.endpointSliceLister.ByIndex(endpointslices.EndpointSlicesByServiceIndex, endpointslices.FormatEndpointSlicesServiceKey(s.Namespace, s.Name)) if err != nil { - return err + return negtypes.NewNegSyncResult(err, negtypes.ResultEPSNotFound) } if len(slices) < 1 { s.logger.Error(nil, "Endpoint slices for the service doesn't exist. Skipping NEG sync") - return nil + return negtypes.NewNegSyncResult(nil, negtypes.ResultSuccess) } endpointSlices := make([]*discovery.EndpointSlice, len(slices)) negCR, err := getNegFromStore(s.svcNegLister, s.Namespace, s.NegSyncerKey.NegName) @@ -267,14 +270,16 @@ 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, result := s.CheckEPField(err); !valid { + s.setErrorState(result.Result) + return result } - if valid, reason := s.isValidEndpointInfo(endpointsData, endpointPodMap, dupCount); !valid { - s.setErrorState(reason) + if valid, result := s.CheckEndpointInfo(endpointsData, endpointPodMap, dupCount); !valid { + s.setErrorState(result.Result) + return result } if err != nil { - return fmt.Errorf("endpoints calculation error in mode %q, err: %w", s.endpointsCalculator.Mode(), err) + return negtypes.NewNegSyncResult(fmt.Errorf("endpoints calculation error in mode %q, err: %w", s.endpointsCalculator.Mode(), err), negtypes.ResultOtherError) } } else { ep, exists, err := s.endpointLister.Get( @@ -286,16 +291,16 @@ func (s *transactionSyncer) syncInternalImpl() error { }, ) if err != nil { - return err + return negtypes.NewNegSyncResult(err, negtypes.ResultOtherError) } if !exists { s.logger.Info("Endpoint does not exist. Skipping NEG sync", "endpoint", klog.KRef(s.Namespace, s.Name)) - return nil + return negtypes.NewNegSyncResult(nil, negtypes.ResultSuccess) } endpointsData := negtypes.EndpointsDataFromEndpoints(ep.(*apiv1.Endpoints)) targetMap, endpointPodMap, _, err = s.endpointsCalculator.CalculateEndpoints(endpointsData, currentMap) if err != nil { - return fmt.Errorf("endpoints calculation error in mode %q, err: %w", s.endpointsCalculator.Mode(), err) + return negtypes.NewNegSyncResult(fmt.Errorf("endpoints calculation error in mode %q, err: %w", s.endpointsCalculator.Mode(), err), negtypes.ResultOtherError) } } @@ -320,7 +325,7 @@ func (s *transactionSyncer) syncInternalImpl() error { if len(addEndpoints) == 0 && len(removeEndpoints) == 0 { s.logger.V(3).Info("No endpoint change. Skip syncing NEG. ", s.Namespace, s.Name) - return nil + return negtypes.NewNegSyncResult(nil, negtypes.ResultSuccess) } s.logEndpoints(addEndpoints, "adding endpoint") s.logEndpoints(removeEndpoints, "removing endpoint") @@ -384,19 +389,19 @@ 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: +// CheckEndpointInfo checks if endpoint information is correct. +// It returns false with 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) { +func (s *transactionSyncer) CheckEndpointInfo(eds []negtypes.EndpointsData, endpointPodMap negtypes.EndpointPodMap, dupCount int) (bool, *negtypes.NegSyncResult) { // 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 + return false, negtypes.NewNegSyncResult(negtypes.ErrEPCalculationCountZero, negtypes.ResultEPCalculationCountZero) } // Endpoint count from EndpointData @@ -406,27 +411,35 @@ func (s *transactionSyncer) isValidEndpointInfo(eds []negtypes.EndpointsData, en } if countFromEndpointData == 0 { s.logger.Info("Detected endpoint count from endpointData going to zero", "endpointData", eds) - return false, negtypes.ResultEPSEndpointCountZero + return false, negtypes.NewNegSyncResult(negtypes.ErrEPSEndpointCountZero, 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 false, negtypes.NewNegSyncResult(negtypes.ErrEPCountsDiffer, negtypes.ResultEPCountsDiffer) } - return true, negtypes.ResultSuccess + return true, negtypes.NewNegSyncResult(nil, 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) { - if errors.Is(err, ErrEPMissingNodeName) { +// CheckEPField checks if endpoints have valid field +// It return the result boolean with the corresponding reason +func (s *transactionSyncer) CheckEPField(err error) (bool, *negtypes.NegSyncResult) { + if errors.Is(err, negtypes.ErrEPMissingNodeName) { s.logger.Info("Detected unexpected error when checking missing nodeName", "error", err) - return false, negtypes.ResultEPMissingNodeName + return false, negtypes.NewNegSyncResult(err, negtypes.ResultEPMissingNodeName) } - if errors.Is(err, ErrEPMissingZone) { + if errors.Is(err, negtypes.ErrEPMissingZone) { s.logger.Info("Detected unexpected error when checking missing zone", "error", err) - return false, negtypes.ResultEPMissingZone + return false, negtypes.NewNegSyncResult(err, negtypes.ResultEPMissingZone) + } + if errors.Is(err, negtypes.ErrNodeNotFound) { + return true, negtypes.NewNegSyncResult(err, negtypes.ResultNodeNotFound) + } + if err != nil { + return true, negtypes.NewNegSyncResult(nil, negtypes.ResultOtherError) + } else { + return true, negtypes.NewNegSyncResult(nil, negtypes.ResultSuccess) } - return true, negtypes.ResultSuccess } // isValidEPBatch returns false and the corresponding reason if the error from endpoint batch response is due to bad request @@ -450,7 +463,7 @@ func (s *transactionSyncer) isValidEPBatch(err error, operation transactionOp, n } // syncNetworkEndpoints spins off go routines to execute NEG operations -func (s *transactionSyncer) syncNetworkEndpoints(addEndpoints, removeEndpoints map[string]negtypes.NetworkEndpointSet) error { +func (s *transactionSyncer) syncNetworkEndpoints(addEndpoints, removeEndpoints map[string]negtypes.NetworkEndpointSet) *negtypes.NegSyncResult { var wg sync.WaitGroup syncFunc := func(endpointMap map[string]negtypes.NetworkEndpointSet, operation transactionOp) error { @@ -486,19 +499,37 @@ func (s *transactionSyncer) syncNetworkEndpoints(addEndpoints, removeEndpoints m } if err := syncFunc(addEndpoints, attachOp); err != nil { - return err + return negtypes.NewNegSyncResult(err, negtypes.ResultOtherError) } if err := syncFunc(removeEndpoints, detachOp); err != nil { - return err + return negtypes.NewNegSyncResult(err, negtypes.ResultOtherError) } go s.collectSyncResult(&wg) - return nil + return negtypes.NewNegSyncResult(nil, negtypes.ResultInProgress) } // collectSyncResult collects the result of the sync and emits the metrics for sync result func (s *transactionSyncer) collectSyncResult(wg *sync.WaitGroup) { wg.Wait() + s.syncLock.Lock() + defer s.syncLock.Unlock() + + var syncResult *negtypes.NegSyncResult + switch s.errorState { + case "": + syncResult = negtypes.NewNegSyncResult(nil, negtypes.ResultSuccess) + case negtypes.ResultInvalidAPIResponse: + syncResult = negtypes.NewNegSyncResult(negtypes.ErrInvalidAPIResponse, negtypes.ResultInvalidAPIResponse) + case negtypes.ResultInvalidEPAttach: + syncResult = negtypes.NewNegSyncResult(negtypes.ErrInvalidEPAttach, negtypes.ResultInvalidEPAttach) + case negtypes.ResultInvalidEPDetach: + syncResult = negtypes.NewNegSyncResult(negtypes.ErrInvalidEPDetach, negtypes.ResultInvalidEPDetach) + default: + syncResult = negtypes.NewNegSyncResult(errors.New("Unknown error state value"), negtypes.ResultOtherError) + } + + s.syncCollector.UpdateSyncer(s.NegSyncerKey, syncResult) } // attachNetworkEndpoints creates go routine to run operations for attaching network endpoints diff --git a/pkg/neg/syncers/transaction_test.go b/pkg/neg/syncers/transaction_test.go index bf58e56ce9..b8cf0d118b 100644 --- a/pkg/neg/syncers/transaction_test.go +++ b/pkg/neg/syncers/transaction_test.go @@ -206,9 +206,9 @@ func TestTransactionSyncNetworkEndpoints(t *testing.T) { } for _, tc := range testCases { - err := transactionSyncer.syncNetworkEndpoints(tc.addEndpoints, tc.removeEndpoints) - if err != nil { - t.Errorf("For case %q, syncNetworkEndpoints() got %v, want nil", tc.desc, err) + syncResult := transactionSyncer.syncNetworkEndpoints(tc.addEndpoints, tc.removeEndpoints) + if syncResult.Error != nil { + t.Errorf("For case %q, syncNetworkEndpoints() got %v, want nil", tc.desc, syncResult.Error) } if err := waitForTransactions(transactionSyncer); err != nil { @@ -1414,7 +1414,7 @@ func TestUnknownNodes(t *testing.T) { } } -func TestIsValidEndpointInfo(t *testing.T) { +func TestCheckEndpointInfo(t *testing.T) { t.Parallel() _, transactionSyncer := newTestTransactionSyncer(negtypes.NewAdapter(gce.NewFakeGCECloud(gce.DefaultTestClusterValues())), negtypes.VmIpPortEndpointType, false, true) @@ -1475,7 +1475,7 @@ func TestIsValidEndpointInfo(t *testing.T) { endpointPodMap map[negtypes.NetworkEndpoint]types.NamespacedName dupCount int expect bool - expectedReason string + expectedResult *negtypes.NegSyncResult }{ { desc: "counts equal, endpointData has no duplicated endpoints", @@ -1548,7 +1548,7 @@ func TestIsValidEndpointInfo(t *testing.T) { endpointPodMap: testEndpointPodMap, dupCount: 0, expect: true, - expectedReason: negtypes.ResultSuccess, + expectedResult: negtypes.NewNegSyncResult(nil, negtypes.ResultSuccess), }, { desc: "counts equal, endpointData has duplicated endpoints", @@ -1630,7 +1630,7 @@ func TestIsValidEndpointInfo(t *testing.T) { endpointPodMap: testEndpointPodMap, dupCount: 1, expect: true, - expectedReason: negtypes.ResultSuccess, + expectedResult: negtypes.NewNegSyncResult(nil, negtypes.ResultSuccess), }, { desc: "counts not equal, endpointData has no duplicated endpoints", @@ -1694,7 +1694,7 @@ func TestIsValidEndpointInfo(t *testing.T) { endpointPodMap: testEndpointPodMap, dupCount: 0, expect: false, - expectedReason: negtypes.ResultEPCountsDiffer, + expectedResult: negtypes.NewNegSyncResult(negtypes.ErrEPCountsDiffer, negtypes.ResultEPCountsDiffer), }, { desc: "counts not equal, endpointData has duplicated endpoints", @@ -1767,7 +1767,7 @@ func TestIsValidEndpointInfo(t *testing.T) { endpointPodMap: testEndpointPodMap, dupCount: 1, expect: false, - expectedReason: negtypes.ResultEPCountsDiffer, + expectedResult: negtypes.NewNegSyncResult(negtypes.ErrEPCountsDiffer, negtypes.ResultEPCountsDiffer), }, { desc: "endpointData has zero endpoint", @@ -1802,7 +1802,7 @@ func TestIsValidEndpointInfo(t *testing.T) { endpointPodMap: testEndpointPodMap, dupCount: 0, expect: false, - expectedReason: negtypes.ResultEPSEndpointCountZero, + expectedResult: negtypes.NewNegSyncResult(negtypes.ErrEPSEndpointCountZero, negtypes.ResultEPSEndpointCountZero), }, { desc: "endpointPodMap has zero endpoint", @@ -1875,7 +1875,7 @@ func TestIsValidEndpointInfo(t *testing.T) { endpointPodMap: map[negtypes.NetworkEndpoint]types.NamespacedName{}, dupCount: 0, expect: false, - expectedReason: negtypes.ResultEPCalculationCountZero, + expectedResult: negtypes.NewNegSyncResult(negtypes.ErrEPCalculationCountZero, negtypes.ResultEPCalculationCountZero), }, { desc: "endpointData and endpointPodMap both have zero endpoint", @@ -1910,7 +1910,7 @@ func TestIsValidEndpointInfo(t *testing.T) { endpointPodMap: map[negtypes.NetworkEndpoint]types.NamespacedName{}, dupCount: 0, expect: false, - expectedReason: negtypes.ResultEPCalculationCountZero, + expectedResult: negtypes.NewNegSyncResult(negtypes.ErrEPCalculationCountZero, negtypes.ResultEPCalculationCountZero), // PodMap count is check and returned first, }, { desc: "endpointData and endpointPodMap both have non-zero endpoints", @@ -1983,20 +1983,27 @@ func TestIsValidEndpointInfo(t *testing.T) { endpointPodMap: testEndpointPodMap, dupCount: 0, expect: true, - expectedReason: negtypes.ResultSuccess, + expectedResult: negtypes.NewNegSyncResult(nil, 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) + got, result := transactionSyncer.CheckEndpointInfo(tc.endpointsData, tc.endpointPodMap, tc.dupCount) + if got != tc.expect { + t.Errorf("CheckEndpointInfo() got valid = %t, expected %t", got, tc.expect) + } + if result.Result != tc.expectedResult.Result { + t.Errorf("CheckEndpointInfo() got result = %s, expected %s", result.Result, tc.expectedResult.Result) + } + if !errors.Is(result.Error, tc.expectedResult.Error) { + t.Errorf("CheckEndpointInfo() got error = %t, expected %t", result.Error, tc.expectedResult.Error) } }) } } -func TestIsValidEPField(t *testing.T) { +func TestCheckEPField(t *testing.T) { t.Parallel() _, transactionSyncer := newTestTransactionSyncer(negtypes.NewAdapter(gce.NewFakeGCECloud(gce.DefaultTestClusterValues())), negtypes.VmIpPortEndpointType, false, true) @@ -2026,7 +2033,7 @@ func TestIsValidEPField(t *testing.T) { desc string endpointsData []negtypes.EndpointsData expect bool - expectedReason string + expectedResult *negtypes.NegSyncResult }{ { desc: "no missing zone or nodeName", @@ -2097,7 +2104,7 @@ func TestIsValidEPField(t *testing.T) { }, }, expect: true, - expectedReason: negtypes.ResultSuccess, + expectedResult: negtypes.NewNegSyncResult(nil, negtypes.ResultSuccess), }, { desc: "contains one missing nodeName", @@ -2168,7 +2175,7 @@ func TestIsValidEPField(t *testing.T) { }, }, expect: false, - expectedReason: negtypes.ResultEPMissingNodeName, + expectedResult: negtypes.NewNegSyncResult(negtypes.ErrEPMissingNodeName, negtypes.ResultEPMissingNodeName), }, { desc: "contains one empty nodeName", @@ -2239,7 +2246,7 @@ func TestIsValidEPField(t *testing.T) { }, }, expect: false, - expectedReason: negtypes.ResultEPMissingNodeName, + expectedResult: negtypes.NewNegSyncResult(negtypes.ErrEPMissingNodeName, negtypes.ResultEPMissingNodeName), }, { desc: "contains one missing zone", @@ -2310,20 +2317,27 @@ func TestIsValidEPField(t *testing.T) { }, }, expect: false, - expectedReason: negtypes.ResultEPMissingZone, + expectedResult: negtypes.NewNegSyncResult(negtypes.ErrEPMissingZone, 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) + got, result := transactionSyncer.CheckEPField(err) + if got != tc.expect { + t.Errorf("CheckEPField() got valid = %t, expected %t", got, tc.expect) + } + if result.Result != tc.expectedResult.Result { + t.Errorf("CheckEPField() got result = %s, expected %s", result.Result, tc.expectedResult.Result) + } + if !errors.Is(result.Error, tc.expectedResult.Error) { + t.Errorf("CheckEPField() got error = %t, expected %t", result.Error, tc.expectedResult.Error) } }) } } -func TestIsValidEPBatch(t *testing.T) { +func TestCheckValidEPBatch(t *testing.T) { fakeGCE := gce.NewFakeGCECloud(gce.DefaultTestClusterValues()) fakeCloud := negtypes.NewAdapter(fakeGCE) zone := "us-central1-a" @@ -2368,7 +2382,7 @@ func TestIsValidEPBatch(t *testing.T) { _, transactionSyncer := newTestTransactionSyncer(fakeCloud, negtypes.VmIpPortEndpointType, false, true) 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 { + if got, reason := transactionSyncer.isValidEPBatch(err, attachOp, networkEndpoints); got != tc.expect || reason != tc.expectedReason { t.Errorf("isInvalidEPBatch() = %t, expected %t", got, tc.expect) } }) diff --git a/pkg/neg/syncers/utils.go b/pkg/neg/syncers/utils.go index 4c2b0b2dc3..5c65a4c108 100644 --- a/pkg/neg/syncers/utils.go +++ b/pkg/neg/syncers/utils.go @@ -17,7 +17,6 @@ limitations under the License. package syncers import ( - "errors" "fmt" "strconv" "strings" @@ -47,12 +46,6 @@ const ( separator = "||" ) -var ( - ErrEPMissingNodeName = errors.New("endpoint has empty nodeName field") - ErrNodeNotFound = errors.New("failed to retrieve associated zone of node") - ErrEPMissingZone = errors.New("endpoint has empty zone field") -) - // encodeEndpoint encodes ip and instance into a single string func encodeEndpoint(ip, instance, port string) string { return strings.Join([]string{ip, instance, port}, separator) @@ -254,7 +247,7 @@ func toZoneNetworkEndpointMap(eds []negtypes.EndpointsData, zoneGetter negtypes. for _, endpointAddress := range ed.Addresses { if endpointAddress.NodeName == nil || len(*endpointAddress.NodeName) == 0 { klog.V(2).Infof("Endpoint %q in Endpoints %s/%s does not have an associated node. Skipping", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name) - return nil, nil, dupCount, ErrEPMissingNodeName + return nil, nil, dupCount, negtypes.ErrEPMissingNodeName } if endpointAddress.TargetRef == nil { 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) @@ -262,10 +255,10 @@ func toZoneNetworkEndpointMap(eds []negtypes.EndpointsData, zoneGetter negtypes. } zone, err := zoneGetter.GetZoneForNode(*endpointAddress.NodeName) if err != nil { - return nil, nil, dupCount, ErrNodeNotFound + return nil, nil, dupCount, negtypes.ErrNodeNotFound } if zone == "" { - return nil, nil, dupCount, ErrEPMissingZone + return nil, nil, dupCount, negtypes.ErrEPMissingZone } if zoneNetworkEndpointMap[zone] == nil { zoneNetworkEndpointMap[zone] = negtypes.NewNetworkEndpointSet() diff --git a/pkg/neg/types/sync_results.go b/pkg/neg/types/sync_results.go index 5253fa4bfe..6610869879 100644 --- a/pkg/neg/types/sync_results.go +++ b/pkg/neg/types/sync_results.go @@ -34,16 +34,22 @@ var ( ResultEPCalculationCountZero = "EPCalculationCountZero" ErrEPCalculationCountZero = errors.New("endpoint count from endpointPodMap cannot be zero") - // these results have their own errors ResultInvalidAPIResponse = "InvalidAPIResponse" - ResultInvalidEPAttach = "InvalidEPAttach" - ResultInvalidEPDetach = "InvalidEPDetach" - ResultNegNotFound = "NegNotFound" - ResultCurrentEPNotFound = "CurrentEPNotFound" - ResultEPSNotFound = "EPSNotFound" - ResultOtherError = "OtherError" - ResultInProgress = "InProgress" - ResultSuccess = "Success" + ErrInvalidAPIResponse = errors.New("received response error doesn't match googleapi.Error type") + + ResultInvalidEPAttach = "InvalidEPAttach" + ErrInvalidEPAttach = errors.New("endpoint information for attach operation is incorrect") + + ResultInvalidEPDetach = "InvalidEPDetach" + ErrInvalidEPDetach = errors.New("endpoint information for detach operation is incorrect") + + // these results have their own errors + ResultNegNotFound = "NegNotFound" + ResultCurrentEPNotFound = "CurrentEPNotFound" + ResultEPSNotFound = "EPSNotFound" + ResultOtherError = "OtherError" + ResultInProgress = "InProgress" + ResultSuccess = "Success" ) type NegSyncResult struct {