Merge pull request #1155 from freehan/neg-log

add logging in NEG syncer

pkg/neg/syncers/transaction.go

@@ -155,18 +155,18 @@ func (s *transactionSyncer) syncInternal() error {
 	if err != nil {
 		return err
 	}
+	s.logStats(currentMap, "current NEG endpoints")
+
 	// Merge the current state from cloud with the transaction table together
 	// The combined state represents the eventual result when all transactions completed
 	mergeTransactionIntoZoneEndpointMap(currentMap, s.transactions)
+	s.logStats(currentMap, "after in-progress operations have completed, NEG endpoints")
 
 	targetMap, endpointPodMap, err := s.endpointsCalculator.CalculateEndpoints(ep.(*apiv1.Endpoints), currentMap)
+	s.logStats(targetMap, "desired NEG endpoints")
 
 	// Calculate the endpoints to add and delete to transform the current state to desire state
 	addEndpoints, removeEndpoints := calculateNetworkEndpointDifference(targetMap, currentMap)
-	if s.NegType == negtypes.VmIpEndpointType && len(removeEndpoints) > 0 {
-		// Make removals minimum since the traffic will be abruptly stopped. Log removals
-		klog.V(3).Infof("Removing endpoints %+v from GCE_VM_IP NEG %s", removeEndpoints, s.negName)
-	}
 	// Calculate Pods that are already in the NEG
 	_, committedEndpoints := calculateNetworkEndpointDifference(addEndpoints, targetMap)
 	// Filter out the endpoints with existing transaction
@@ -185,7 +185,8 @@ func (s *transactionSyncer) syncInternal() error {
 		klog.V(4).Infof("No endpoint change for %s/%s, skip syncing NEG. ", s.Namespace, s.Name)
 		return nil
 	}
-
+	s.logEndpoints(addEndpoints, "adding endpoint")
+	s.logEndpoints(removeEndpoints, "removing endpoint")
 	return s.syncNetworkEndpoints(addEndpoints, removeEndpoints)
 }
 
@@ -391,3 +392,17 @@ func mergeTransactionIntoZoneEndpointMap(endpointMap map[string]negtypes.Network
 	}
 	return
 }
+
+// logStats logs aggregated stats of the input endpointMap
+func (s *transactionSyncer) logStats(endpointMap map[string]negtypes.NetworkEndpointSet, desc string) {
+	stats := []string{}
+	for zone, endpointSet := range endpointMap {
+		stats = append(stats, fmt.Sprintf("%d endpoints in zone %q", endpointSet.Len(), zone))
+	}
+	klog.V(3).Infof("For NEG %q, %s: %s.", s.negName, desc, strings.Join(stats, ","))
+}
+
+// logEndpoints logs individual endpoint in the input endpointMap
+func (s *transactionSyncer) logEndpoints(endpointMap map[string]negtypes.NetworkEndpointSet, desc string) {
+	klog.V(3).Infof("For NEG %q, %s: %+v", s.negName, desc, endpointMap)
+}

pkg/neg/syncers/utils.go

@@ -177,7 +177,7 @@ func toZoneNetworkEndpointMap(endpoints *apiv1.Endpoints, zoneGetter negtypes.Zo
 		klog.Errorf("Endpoint object is nil")
 		return zoneNetworkEndpointMap, networkEndpointPodMap, nil
 	}
-
+	var foundMatchingPort bool
 	for _, subset := range endpoints.Subsets {
 		matchPort := ""
 		// service spec allows target Port to be a named Port.
@@ -193,6 +193,7 @@ func toZoneNetworkEndpointMap(endpoints *apiv1.Endpoints, zoneGetter negtypes.Zo
 		if len(matchPort) == 0 {
 			continue
 		}
+		foundMatchingPort = true
 
 		// processAddressFunc adds the qualified endpoints from the input list into the endpointSet group by zone
 		processAddressFunc := func(addresses []v1.EndpointAddress, includeAllEndpoints bool) error {
@@ -244,6 +245,13 @@ func toZoneNetworkEndpointMap(endpoints *apiv1.Endpoints, zoneGetter negtypes.Zo
 			return nil, nil, err
 		}
 	}
+	if !foundMatchingPort {
+		klog.Errorf("Service port name %q was not found in the endpoints object %+v", servicePortName, endpoints)
+	}
+
+	if len(zoneNetworkEndpointMap) == 0 || len(networkEndpointPodMap) == 0 {
+		klog.V(3).Infof("Generated empty endpoint maps (zoneNetworkEndpointMap: %+v, networkEndpointPodMap: %v) from Endpoints object: %+v", zoneNetworkEndpointMap, networkEndpointPodMap, endpoints)
+	}
 	return zoneNetworkEndpointMap, networkEndpointPodMap, nil
 }