diff --git a/pkg/neg/controller.go b/pkg/neg/controller.go index da407dcb0c..665a4589d7 100644 --- a/pkg/neg/controller.go +++ b/pkg/neg/controller.go @@ -325,6 +325,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/label_propagation_metrics.go b/pkg/neg/metrics/label_propagation_metrics.go index 9932210649..29794a7fd6 100644 --- a/pkg/neg/metrics/label_propagation_metrics.go +++ b/pkg/neg/metrics/label_propagation_metrics.go @@ -25,6 +25,8 @@ const ( annotationSize = "annotation_size_per_endpoint" labelErrorNumber = "label_propagation_error_count" numberOfEndpoints = "number_of_endpoints" + epWithAnnotation = "with_annotation" + totalEndpoints = "total" ) var ( @@ -33,7 +35,7 @@ var ( } endpointAnnotationLabels = []string{ - "with_annotation", + "feature", } NumberOfEndpoints = prometheus.NewGaugeVec( diff --git a/pkg/neg/metrics/metrics.go b/pkg/neg/metrics/metrics.go index b9b183f554..6b9fdcc129 100644 --- a/pkg/neg/metrics/metrics.go +++ b/pkg/neg/metrics/metrics.go @@ -163,6 +163,10 @@ func RegisterMetrics() { prometheus.MustRegister(InitializationLatency) prometheus.MustRegister(SyncerStaleness) prometheus.MustRegister(EPSStaleness) + prometheus.MustRegister(NumberOfEndpoints) + prometheus.MustRegister(LabelPropagationError) + prometheus.MustRegister(LabelNumber) + prometheus.MustRegister(AnnotationSize) RegisterSyncerMetrics() }) diff --git a/pkg/neg/metrics/metrics_test.go b/pkg/neg/metrics/metrics_test.go index 2c4075281f..10914f667f 100644 --- a/pkg/neg/metrics/metrics_test.go +++ b/pkg/neg/metrics/metrics_test.go @@ -90,7 +90,7 @@ func TestComputeLabelMetrics(t *testing.T) { collector.syncerLabelProagationStats = tc.syncerLabelProagationStats out := collector.computeLabelMetrics() if diff := cmp.Diff(out, tc.expect); diff != "" { - t.Errorf("For test case %s, got %+v, want %+v, diff: %s", tc.desc, out, tc.expect, diff) + t.Errorf("For test case %s, (-want +got):\n%s", tc.desc, diff) } } } diff --git a/pkg/neg/metrics/neg_metrics_collector.go b/pkg/neg/metrics/neg_metrics_collector.go index 2da126a4b8..c927595232 100644 --- a/pkg/neg/metrics/neg_metrics_collector.go +++ b/pkg/neg/metrics/neg_metrics_collector.go @@ -28,6 +28,7 @@ import ( type SyncerMetricsCollector interface { UpdateSyncer(key negtypes.NegSyncerKey, result *negtypes.NegSyncResult) SetSyncerEPMetrics(key negtypes.NegSyncerKey, epState *negtypes.SyncerEPStat) + SetLabelPropagationStats(key negtypes.NegSyncerKey, labelstatLabelPropagationStats LabelPropagationStats) } type SyncerMetrics struct { @@ -50,11 +51,12 @@ type SyncerMetrics struct { // NewNEGMetricsCollector initializes SyncerMetrics and starts a go routine to compute and export metrics periodically. func NewNegMetricsCollector(exportInterval time.Duration, logger klog.Logger) *SyncerMetrics { return &SyncerMetrics{ - syncerStatusMap: make(map[negtypes.NegSyncerKey]string), - syncerEndpointStateMap: make(map[negtypes.NegSyncerKey]negtypes.StateCountMap), - syncerEPSStateMap: make(map[negtypes.NegSyncerKey]negtypes.StateCountMap), - metricsInterval: exportInterval, - logger: logger.WithName("NegMetricsCollector"), + syncerStatusMap: make(map[negtypes.NegSyncerKey]string), + syncerEndpointStateMap: make(map[negtypes.NegSyncerKey]negtypes.StateCountMap), + syncerEPSStateMap: make(map[negtypes.NegSyncerKey]negtypes.StateCountMap), + syncerLabelProagationStats: make(map[negtypes.NegSyncerKey]LabelPropagationStats), + metricsInterval: exportInterval, + logger: logger.WithName("NegMetricsCollector"), } } @@ -79,6 +81,10 @@ func (sm *SyncerMetrics) Run(stopCh <-chan struct{}) { // export exports syncer metrics. func (sm *SyncerMetrics) export() { + lpMetrics := sm.computeLabelMetrics() + NumberOfEndpoints.WithLabelValues(totalEndpoints).Set(float64(lpMetrics.NumberOfEndpoints)) + NumberOfEndpoints.WithLabelValues(epWithAnnotation).Set(float64(lpMetrics.EndpointsWithAnnotation)) + sm.logger.V(3).Info("Exporting syncer related metrics", "Number of Endpoints", lpMetrics.NumberOfEndpoints) } // UpdateSyncer update the status of corresponding syncer based on the syncResult. @@ -87,7 +93,7 @@ func (sm *SyncerMetrics) UpdateSyncer(key negtypes.NegSyncerKey, syncResult *neg defer sm.mu.Unlock() if sm.syncerStatusMap == nil { sm.syncerStatusMap = make(map[negtypes.NegSyncerKey]string) - sm.logger.V(3).Info("Syncer Metrics failed to initialize correctly, reinitializing syncerStatusMap: %v", sm.syncerStatusMap) + sm.logger.V(3).Info("Syncer Metrics failed to initialize correctly, reinitializing syncerStatusMap") } sm.syncerStatusMap[key] = string(syncResult.Result) } @@ -98,17 +104,27 @@ func (sm *SyncerMetrics) SetSyncerEPMetrics(key negtypes.NegSyncerKey, endpointS defer sm.mu.Unlock() if sm.syncerEndpointStateMap == nil { sm.syncerEndpointStateMap = make(map[negtypes.NegSyncerKey]negtypes.StateCountMap) - sm.logger.V(3).Info("Syncer Metrics failed to initialize correctly, reinitializing syncerEPStateMap: %v", sm.syncerEndpointStateMap) + sm.logger.V(3).Info("Syncer Metrics failed to initialize correctly, reinitializing syncerEPStateMap") } sm.syncerEndpointStateMap[key] = endpointStat.EndpointStateCount if sm.syncerEPSStateMap == nil { sm.syncerEPSStateMap = make(map[negtypes.NegSyncerKey]negtypes.StateCountMap) - sm.logger.V(3).Info("Syncer Metrics failed to initialize correctly, reinitializing syncerEPSStateMap: %v", sm.syncerEPSStateMap) + sm.logger.V(3).Info("Syncer Metrics failed to initialize correctly, reinitializing syncerEPSStateMap") } sm.syncerEPSStateMap[key] = endpointStat.EndpointSliceStateCount } +func (sm *SyncerMetrics) SetLabelPropagationStats(key negtypes.NegSyncerKey, labelstatLabelPropagationStats LabelPropagationStats) { + sm.mu.Lock() + defer sm.mu.Unlock() + if sm.syncerLabelProagationStats == nil { + sm.syncerLabelProagationStats = make(map[negtypes.NegSyncerKey]LabelPropagationStats) + sm.logger.V(3).Info("Syncer Metrics failed to initialize correctly, reinitializing syncerLabelProagationStats") + } + sm.syncerLabelProagationStats[key] = labelstatLabelPropagationStats +} + // computeLabelMetrics aggregates label propagation metrics. func (sm *SyncerMetrics) computeLabelMetrics() LabelPropagationMetrics { sm.mu.Lock() diff --git a/pkg/neg/syncers/labels/labels.go b/pkg/neg/syncers/labels/labels.go index f3af999d72..c97559f075 100644 --- a/pkg/neg/syncers/labels/labels.go +++ b/pkg/neg/syncers/labels/labels.go @@ -22,6 +22,7 @@ import ( "fmt" v1 "k8s.io/api/core/v1" + "k8s.io/ingress-gce/pkg/neg/metrics" negtypes "k8s.io/ingress-gce/pkg/neg/types" "k8s.io/ingress-gce/pkg/utils" ) @@ -44,6 +45,12 @@ type PodLabelMap map[string]string // EndpointPodLabelMap is a map of network endpoint, endpoint annotations. type EndpointPodLabelMap map[negtypes.NetworkEndpoint]PodLabelMap +const ( + Truncated = "truncated" + TruncationFailure = "truncation_failed" + OtherError = "other_error" +) + var ( ErrLabelTruncated = errors.New("label is truncated") ErrLabelTruncationFailed = errors.New("failed to truncate label") @@ -68,6 +75,7 @@ func GetPodLabelMap(pod *v1.Pod, lpConfig PodLabelPropagationConfig) (PodLabelMa labelVal, err := truncatePodLabel(lpKey, val, label.MaxLabelSizeBytes) if err != nil { errs = append(errs, err) + publishLabelPropagationTruncationMetrics(err) } // Add the label to the map only if the truncation result is valid @@ -82,6 +90,16 @@ func GetPodLabelMap(pod *v1.Pod, lpConfig PodLabelPropagationConfig) (PodLabelMa return labelMap, nil } +// publishLabelPropagationTruncationMetrics publishes errors occured during +// label truncation. +func publishLabelPropagationTruncationMetrics(err error) { + if errors.Is(err, ErrLabelTruncated) { + metrics.PublishLabelPropagationError(Truncated) + } else if errors.Is(err, ErrLabelTruncationFailed) { + metrics.PublishLabelPropagationError(TruncationFailure) + } +} + // truncatePodLabel calculates the potentially truncated label value to ensure that len(key) + len(label) <= maxTotalSize. // It will return: // @@ -100,3 +118,13 @@ func truncatePodLabel(key, label string, maxTotalSize int) (string, error) { truncatedVal := string(labelBytes[:maxTotalSize-len(keyBytes)]) return truncatedVal, fmt.Errorf("%w: `%s:%s` is truncated to `%s:%s` because the total length exceeded the limit, length: %d, limit: %d", ErrLabelTruncated, key, label, key, truncatedVal, len(key)+len(label), maxTotalSize) } + +// PodLabelMapSize calculates the size of a podLabelMap. +func GetPodLabelMapSize(podLabelMap PodLabelMap) int { + var res int + for key, val := range podLabelMap { + res += len([]byte(key)) + res += len([]byte(val)) + } + return res +} diff --git a/pkg/neg/syncers/transaction.go b/pkg/neg/syncers/transaction.go index d133a9a6b7..30533d67d7 100644 --- a/pkg/neg/syncers/transaction.go +++ b/pkg/neg/syncers/transaction.go @@ -233,7 +233,7 @@ func (s *transactionSyncer) syncInternalImpl() error { } 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()) + currentMap, currentPodLabelMap, err := retrieveExistingZoneNetworkEndpointMap(s.NegSyncerKey.NegName, s.zoneGetter, s.cloud, s.NegSyncerKey.GetAPIVersion(), s.endpointsCalculator.Mode()) if err != nil { return err } @@ -312,8 +312,11 @@ func (s *transactionSyncer) syncInternalImpl() error { // Only fetch label from pod for L7 endpoints if flags.F.EnableNEGLabelPropagation && s.NegType == negtypes.VmIpPortEndpointType { endpointPodLabelMap = getEndpointPodLabelMap(addEndpoints, endpointPodMap, s.podLister, s.podLabelPropagationConfig, s.recorder, s.logger) + publishAnnotationSizeMetrics(addEndpoints, endpointPodLabelMap) } + s.syncCollector.SetLabelPropagationStats(s.NegSyncerKey, collectLabelStats(currentPodLabelMap, endpointPodLabelMap, targetMap)) + if s.needCommit() { s.commitPods(committedEndpoints, endpointPodMap) } @@ -866,11 +869,13 @@ func getEndpointPodLabelMap(endpoints map[string]negtypes.NetworkEndpointSet, en key := fmt.Sprintf("%s/%s", endpointPodMap[endpoint].Namespace, endpointPodMap[endpoint].Name) obj, ok, err := podLister.GetByKey(key) if err != nil || !ok { + metrics.PublishLabelPropagationError(labels.OtherError) logger.Error(err, "getEndpointPodLabelMap: error getting pod", "pod", key, "exist", ok) continue } pod, ok := obj.(*v1.Pod) if !ok { + metrics.PublishLabelPropagationError(labels.OtherError) logger.Error(nil, "expected type *v1.Pod", "pod", key, "type", fmt.Sprintf("%T", obj)) continue } @@ -883,3 +888,28 @@ func getEndpointPodLabelMap(endpoints map[string]negtypes.NetworkEndpointSet, en } return endpointPodLabelMap } + +// publishAnnotationSizeMetrics goes through all the endpoints to be attached +// and publish annotation size metrics. +func publishAnnotationSizeMetrics(endpoints map[string]negtypes.NetworkEndpointSet, endpointPodLabelMap labels.EndpointPodLabelMap) { + for _, endpointSet := range endpoints { + for endpoint := range endpointSet { + labelMap := endpointPodLabelMap[endpoint] + metrics.PublishAnnotationMetrics(labels.GetPodLabelMapSize(labelMap), len(labelMap)) + } + } +} + +// collectLabelStats calculate the number of endpoints and the number of endpoints with annotations. +func collectLabelStats(currentPodLabelMap, addPodLabelMap labels.EndpointPodLabelMap, targetEndpointMap map[string]negtypes.NetworkEndpointSet) metrics.LabelPropagationStats { + labelPropagationStats := metrics.LabelPropagationStats{} + for _, endpointSet := range targetEndpointMap { + for endpoint := range endpointSet { + labelPropagationStats.NumberOfEndpoints += 1 + if currentPodLabelMap[endpoint] != nil || addPodLabelMap[endpoint] != nil { + labelPropagationStats.EndpointsWithAnnotation += 1 + } + } + } + return labelPropagationStats +} diff --git a/pkg/neg/syncers/transaction_test.go b/pkg/neg/syncers/transaction_test.go index ef87b251fb..de9cc35512 100644 --- a/pkg/neg/syncers/transaction_test.go +++ b/pkg/neg/syncers/transaction_test.go @@ -1532,7 +1532,7 @@ func TestUnknownNodes(t *testing.T) { } // Check that unknown zone did not cause endpoints to be removed - out, err := retrieveExistingZoneNetworkEndpointMap(testNegName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode) + out, _, err := retrieveExistingZoneNetworkEndpointMap(testNegName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode) if err != nil { t.Errorf("errored retrieving existing network endpoints") } @@ -1761,7 +1761,7 @@ func TestEnableDegradedMode(t *testing.T) { (s.syncer.(*syncer)).stopped = false tc.modify(s) - out, err := retrieveExistingZoneNetworkEndpointMap(tc.negName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode) + out, _, err := retrieveExistingZoneNetworkEndpointMap(tc.negName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode) if err != nil { t.Errorf("errored retrieving existing network endpoints") } @@ -1777,7 +1777,7 @@ func TestEnableDegradedMode(t *testing.T) { t.Errorf("after syncInternal, error state is %v, expected to be %v", s.inErrorState(), tc.expectedInErrorState) } err = wait.PollImmediate(time.Second, 3*time.Second, func() (bool, error) { - out, err = retrieveExistingZoneNetworkEndpointMap(tc.negName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode) + out, _, err = retrieveExistingZoneNetworkEndpointMap(tc.negName, zoneGetter, fakeCloud, meta.VersionGA, negtypes.L7Mode) if err != nil { return false, err } @@ -1910,6 +1910,109 @@ func TestGetEndpointPodLabelMap(t *testing.T) { } } +func TestCollectLabelStats(t *testing.T) { + t.Parallel() + + testIP1 := "1.2.3.4" + testIP2 := "1.2.3.5" + testIP3 := "1.2.3.6" + testIP4 := "1.2.3.7" + testPort := int64(80) + endpoint1 := negtypes.NetworkEndpoint{IP: testIP1, Node: negtypes.TestInstance1, Port: strconv.Itoa(int(testPort))} + endpoint2 := negtypes.NetworkEndpoint{IP: testIP2, Node: negtypes.TestInstance2, Port: strconv.Itoa(int(testPort))} + endpoint3 := negtypes.NetworkEndpoint{IP: testIP3, Node: negtypes.TestInstance3, Port: strconv.Itoa(int(testPort))} + endpoint4 := negtypes.NetworkEndpoint{IP: testIP4, Node: negtypes.TestInstance4, Port: strconv.Itoa(int(testPort))} + + for _, tc := range []struct { + desc string + curLabelMap labels.EndpointPodLabelMap + addLabelMap labels.EndpointPodLabelMap + targetEndpointMap map[string]negtypes.NetworkEndpointSet + expect metrics.LabelPropagationStats + }{ + { + desc: "Empty inputs", + curLabelMap: labels.EndpointPodLabelMap{}, + addLabelMap: labels.EndpointPodLabelMap{}, + targetEndpointMap: map[string]negtypes.NetworkEndpointSet{}, + expect: metrics.LabelPropagationStats{ + EndpointsWithAnnotation: 0, + NumberOfEndpoints: 0, + }, + }, + { + desc: "No new endpoints to be added", + curLabelMap: labels.EndpointPodLabelMap{ + endpoint1: labels.PodLabelMap{ + "foo": "bar", + }, + }, + addLabelMap: labels.EndpointPodLabelMap{}, + targetEndpointMap: map[string]negtypes.NetworkEndpointSet{ + testZone1: negtypes.NewNetworkEndpointSet( + endpoint1, + endpoint2, + ), + }, + expect: metrics.LabelPropagationStats{ + EndpointsWithAnnotation: 1, + NumberOfEndpoints: 2, + }, + }, + { + desc: "Some endpoints to be added", + curLabelMap: labels.EndpointPodLabelMap{ + endpoint1: labels.PodLabelMap{ + "foo": "bar", + }, + }, + addLabelMap: labels.EndpointPodLabelMap{ + endpoint3: labels.PodLabelMap{ + "foo": "bar", + }, + }, + targetEndpointMap: map[string]negtypes.NetworkEndpointSet{ + testZone1: negtypes.NewNetworkEndpointSet( + endpoint1, + endpoint2, + ), + testZone2: negtypes.NewNetworkEndpointSet( + endpoint3, + endpoint4, + ), + }, + expect: metrics.LabelPropagationStats{ + EndpointsWithAnnotation: 2, + NumberOfEndpoints: 4, + }, + }, + { + desc: "Only newly added endpoints", + curLabelMap: labels.EndpointPodLabelMap{}, + addLabelMap: labels.EndpointPodLabelMap{ + endpoint3: labels.PodLabelMap{ + "foo": "bar", + }, + }, + targetEndpointMap: map[string]negtypes.NetworkEndpointSet{ + testZone2: negtypes.NewNetworkEndpointSet( + endpoint3, + endpoint4, + ), + }, + expect: metrics.LabelPropagationStats{ + EndpointsWithAnnotation: 1, + NumberOfEndpoints: 2, + }, + }, + } { + out := collectLabelStats(tc.curLabelMap, tc.addLabelMap, tc.targetEndpointMap) + if diff := cmp.Diff(out, tc.expect); diff != "" { + t.Errorf("For test case %s: (-want +got): \n%s", tc.desc, diff) + } + } +} + 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(), ts.enableDualStackNEG) diff --git a/pkg/neg/syncers/utils.go b/pkg/neg/syncers/utils.go index 907037047a..d97a3d8c5d 100644 --- a/pkg/neg/syncers/utils.go +++ b/pkg/neg/syncers/utils.go @@ -560,21 +560,22 @@ func ipsForPod(eds []negtypes.EndpointsData) map[types.NamespacedName]negtypes.N } // 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, error) { +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. // Endpoints in those NEGs now need to be removed. This mostly applies to VM_IP_NEGs where the endpoints are nodes. zones, err := zoneGetter.ListZones(utils.AllNodesPredicate) if err != nil { - return nil, err + return nil, nil, err } candidateNodeZones, err := zoneGetter.ListZones(negtypes.NodePredicateForEndpointCalculatorMode(mode)) if err != nil { - return nil, err + return nil, nil, err } candidateZonesMap := sets.NewString(candidateNodeZones...) zoneNetworkEndpointMap := map[string]negtypes.NetworkEndpointSet{} + endpointPodLabelMap := labels.EndpointPodLabelMap{} for _, zone := range zones { networkEndpointsWithHealthStatus, err := cloud.ListNetworkEndpoints(negName, zone, false, version) if err != nil { @@ -584,7 +585,7 @@ func retrieveExistingZoneNetworkEndpointMap(negName string, zoneGetter negtypes. klog.Infof("Ignoring NotFound error for NEG %q in zone %q", negName, zone) continue } - return nil, fmt.Errorf("Failed to lookup NEG in zone %q, candidate zones %v, err - %v", zone, candidateZonesMap, err) + return nil, nil, fmt.Errorf("Failed to lookup NEG in zone %q, candidate zones %v, err - %v", zone, candidateZonesMap, err) } zoneNetworkEndpointMap[zone] = negtypes.NewNetworkEndpointSet() for _, ne := range networkEndpointsWithHealthStatus { @@ -593,9 +594,10 @@ func retrieveExistingZoneNetworkEndpointMap(negName string, zoneGetter negtypes. newNE.Port = strconv.FormatInt(ne.NetworkEndpoint.Port, 10) } zoneNetworkEndpointMap[zone].Insert(newNE) + endpointPodLabelMap[newNE] = ne.NetworkEndpoint.Annotations } } - return zoneNetworkEndpointMap, nil + return zoneNetworkEndpointMap, endpointPodLabelMap, nil } // makeEndpointBatch return a batch of endpoint from the input and remove the endpoints from input set diff --git a/pkg/neg/syncers/utils_test.go b/pkg/neg/syncers/utils_test.go index 4c5896ec19..e69c6ec1a4 100644 --- a/pkg/neg/syncers/utils_test.go +++ b/pkg/neg/syncers/utils_test.go @@ -955,12 +955,21 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { testIP7 := "1.2.3.10" testPort := int64(80) + endpoint1 := negtypes.NetworkEndpoint{IP: testIP1, Node: negtypes.TestInstance1, Port: strconv.Itoa(int(testPort))} + endpoint2 := negtypes.NetworkEndpoint{IP: testIP2, Node: negtypes.TestInstance2, Port: strconv.Itoa(int(testPort))} + endpoint3 := negtypes.NetworkEndpoint{IP: testIP3, Node: negtypes.TestInstance3, Port: strconv.Itoa(int(testPort))} + endpoint4 := negtypes.NetworkEndpoint{IP: testIP4, Node: negtypes.TestInstance4, Port: strconv.Itoa(int(testPort))} + endpoint5 := negtypes.NetworkEndpoint{IP: testIP5, Node: negtypes.TestUnreadyInstance1, Port: strconv.Itoa(int(testPort))} + endpoint6 := negtypes.NetworkEndpoint{IP: testIP6, Node: negtypes.TestUpgradeInstance1, Port: strconv.Itoa(int(testPort))} + endpoint7 := negtypes.NetworkEndpoint{IP: testIP7, Node: negtypes.TestUpgradeInstance2, Port: strconv.Itoa(int(testPort))} + testCases := []struct { - desc string - mutate func(cloud negtypes.NetworkEndpointGroupCloud) - mode negtypes.EndpointsCalculatorMode - expect map[string]negtypes.NetworkEndpointSet - expectErr bool + desc string + mutate func(cloud negtypes.NetworkEndpointGroupCloud) + mode negtypes.EndpointsCalculatorMode + expect map[string]negtypes.NetworkEndpointSet + expectAnnotationMap labels.EndpointPodLabelMap + expectErr bool }{ { desc: "neg does not exist", @@ -992,7 +1001,8 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { negtypes.TestZone2: negtypes.NewNetworkEndpointSet(), negtypes.TestZone4: negtypes.NewNetworkEndpointSet(), }, - expectErr: false, + expectAnnotationMap: labels.EndpointPodLabelMap{}, + expectErr: false, }, { desc: "one empty and two non-empty negs", @@ -1002,14 +1012,22 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { Instance: negtypes.TestInstance1, IpAddress: testIP1, Port: testPort, + Annotations: map[string]string{ + "foo": "bar", + }, }, }, meta.VersionGA) }, expect: map[string]negtypes.NetworkEndpointSet{ - negtypes.TestZone1: negtypes.NewNetworkEndpointSet(negtypes.NetworkEndpoint{IP: testIP1, Node: negtypes.TestInstance1, Port: strconv.Itoa(int(testPort))}), + negtypes.TestZone1: negtypes.NewNetworkEndpointSet(endpoint1), negtypes.TestZone2: negtypes.NewNetworkEndpointSet(), negtypes.TestZone4: negtypes.NewNetworkEndpointSet(), }, + expectAnnotationMap: labels.EndpointPodLabelMap{ + endpoint1: labels.PodLabelMap{ + "foo": "bar", + }, + }, expectErr: false, }, { @@ -1020,17 +1038,28 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { Instance: negtypes.TestInstance2, IpAddress: testIP2, Port: testPort, + Annotations: map[string]string{ + "foo": "bar", + }, }, }, meta.VersionGA) }, expect: map[string]negtypes.NetworkEndpointSet{ negtypes.TestZone1: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP1, Node: negtypes.TestInstance1, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP2, Node: negtypes.TestInstance2, Port: strconv.Itoa(int(testPort))}, + endpoint1, + endpoint2, ), negtypes.TestZone2: negtypes.NewNetworkEndpointSet(), negtypes.TestZone4: negtypes.NewNetworkEndpointSet(), }, + expectAnnotationMap: labels.EndpointPodLabelMap{ + endpoint1: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint2: labels.PodLabelMap{ + "foo": "bar", + }, + }, expectErr: false, }, { @@ -1041,29 +1070,49 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { Instance: negtypes.TestInstance3, IpAddress: testIP3, Port: testPort, + Annotations: map[string]string{ + "foo": "bar", + }, }, { Instance: negtypes.TestInstance4, IpAddress: testIP4, Port: testPort, + Annotations: map[string]string{ + "foo": "bar", + }, }, }, meta.VersionGA) }, expect: map[string]negtypes.NetworkEndpointSet{ negtypes.TestZone1: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP1, Node: negtypes.TestInstance1, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP2, Node: negtypes.TestInstance2, Port: strconv.Itoa(int(testPort))}, + endpoint1, + endpoint2, ), negtypes.TestZone2: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP3, Node: negtypes.TestInstance3, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP4, Node: negtypes.TestInstance4, Port: strconv.Itoa(int(testPort))}, + endpoint3, + endpoint4, ), negtypes.TestZone4: negtypes.NewNetworkEndpointSet(), }, + expectAnnotationMap: labels.EndpointPodLabelMap{ + endpoint1: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint2: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint3: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint4: labels.PodLabelMap{ + "foo": "bar", + }, + }, expectErr: false, }, { - desc: "all 3 negs with multiple endpoints", + desc: "all 3 negs with multiple endpoints, endpoint6 and endpoint7 with no pod label", mutate: func(cloud negtypes.NetworkEndpointGroupCloud) { cloud.AttachNetworkEndpoints(testNegName, negtypes.TestZone4, []*composite.NetworkEndpoint{ { @@ -1080,18 +1129,34 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { }, expect: map[string]negtypes.NetworkEndpointSet{ negtypes.TestZone1: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP1, Node: negtypes.TestInstance1, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP2, Node: negtypes.TestInstance2, Port: strconv.Itoa(int(testPort))}, + endpoint1, + endpoint2, ), negtypes.TestZone2: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP3, Node: negtypes.TestInstance3, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP4, Node: negtypes.TestInstance4, Port: strconv.Itoa(int(testPort))}, + endpoint3, + endpoint4, ), negtypes.TestZone4: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP6, Node: negtypes.TestUpgradeInstance1, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP7, Node: negtypes.TestUpgradeInstance2, Port: strconv.Itoa(int(testPort))}, + endpoint6, + endpoint7, ), }, + expectAnnotationMap: labels.EndpointPodLabelMap{ + endpoint1: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint2: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint3: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint4: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint6: nil, + endpoint7: nil, + }, expectErr: false, }, { @@ -1107,18 +1172,34 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { }, expect: map[string]negtypes.NetworkEndpointSet{ negtypes.TestZone1: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP1, Node: negtypes.TestInstance1, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP2, Node: negtypes.TestInstance2, Port: strconv.Itoa(int(testPort))}, + endpoint1, + endpoint2, ), negtypes.TestZone2: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP3, Node: negtypes.TestInstance3, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP4, Node: negtypes.TestInstance4, Port: strconv.Itoa(int(testPort))}, + endpoint3, + endpoint4, ), negtypes.TestZone4: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP6, Node: negtypes.TestUpgradeInstance1, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP7, Node: negtypes.TestUpgradeInstance2, Port: strconv.Itoa(int(testPort))}, + endpoint6, + endpoint7, ), }, + expectAnnotationMap: labels.EndpointPodLabelMap{ + endpoint1: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint2: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint3: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint4: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint6: nil, + endpoint7: nil, + }, expectErr: false, }, { @@ -1138,21 +1219,38 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { expect: map[string]negtypes.NetworkEndpointSet{ // NEGs in zone1, zone2 and zone4 are created from previous test case. negtypes.TestZone1: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP1, Node: negtypes.TestInstance1, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP2, Node: negtypes.TestInstance2, Port: strconv.Itoa(int(testPort))}, + endpoint1, + endpoint2, ), negtypes.TestZone2: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP3, Node: negtypes.TestInstance3, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP4, Node: negtypes.TestInstance4, Port: strconv.Itoa(int(testPort))}, + endpoint3, + endpoint4, ), negtypes.TestZone3: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP5, Node: negtypes.TestUnreadyInstance1, Port: strconv.Itoa(int(testPort))}, + endpoint5, ), negtypes.TestZone4: negtypes.NewNetworkEndpointSet( - negtypes.NetworkEndpoint{IP: testIP6, Node: negtypes.TestUpgradeInstance1, Port: strconv.Itoa(int(testPort))}, - negtypes.NetworkEndpoint{IP: testIP7, Node: negtypes.TestUpgradeInstance2, Port: strconv.Itoa(int(testPort))}, + endpoint6, + endpoint7, ), }, + expectAnnotationMap: labels.EndpointPodLabelMap{ + endpoint1: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint2: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint3: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint4: labels.PodLabelMap{ + "foo": "bar", + }, + endpoint5: nil, + endpoint6: nil, + endpoint7: nil, + }, expectErr: false, }, { @@ -1167,7 +1265,7 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { for _, tc := range testCases { tc.mutate(negCloud) // tc.mode of "" will result in the default node predicate being selected, which is ok for this test. - out, err := retrieveExistingZoneNetworkEndpointMap(negName, zoneGetter, negCloud, meta.VersionGA, tc.mode) + endpointSets, annotationMap, err := retrieveExistingZoneNetworkEndpointMap(negName, zoneGetter, negCloud, meta.VersionGA, tc.mode) if tc.expectErr { if err == nil { @@ -1180,8 +1278,11 @@ func TestRetrieveExistingZoneNetworkEndpointMap(t *testing.T) { } if !tc.expectErr { - if !reflect.DeepEqual(out, tc.expect) { - t.Errorf("For test case %q, endpointSets output = %+v, but want %+v", tc.desc, tc.expect, out) + if diff := cmp.Diff(endpointSets, tc.expect); diff != "" { + t.Errorf("For test case %q, (-want +got):\n%s", tc.desc, diff) + } + if diff := cmp.Diff(annotationMap, tc.expectAnnotationMap); diff != "" { + t.Errorf("For test case %q, (-want +got):\n%s", tc.desc, diff) } } }