Merge pull request #2857 from gauravkghildiyal/rename-endpointgroupinfo-to-neglocation

Rename epGroupInfo to negLocation for a better representation of the concept

Author: k8s-ci-robot

pkg/neg/syncers/dualstack/migrator_test.go

@@ -299,16 +299,16 @@ func TestFilter_FunctionalTest(t *testing.T) {
 			removeEndpoints := make(map[types.NEGLocation]types.NetworkEndpointSet)    // Contains single-stack endpoints which are to be removed from the NEG.
 			committedEndpoints := make(map[types.NEGLocation]types.NetworkEndpointSet) // Initially empty.
 			for i := 0; i < tc.initialNEGEndpointsCount; i++ {
-				epGroupInfo := types.NEGLocation{Zone: fmt.Sprintf("zone-%v", i%tc.zonesCount), Subnet: defaultTestSubnet}
+				negLocation := types.NEGLocation{Zone: fmt.Sprintf("zone-%v", i%tc.zonesCount), Subnet: defaultTestSubnet}
 				ipv4 := fmt.Sprintf("ipv4-%v", 2*i+1)
 				ipv6 := fmt.Sprintf("ipv6-%v", 2*i+2)
-				if addEndpoints[epGroupInfo] == nil {
-					addEndpoints[epGroupInfo] = types.NewNetworkEndpointSet()
-					removeEndpoints[epGroupInfo] = types.NewNetworkEndpointSet()
-					committedEndpoints[epGroupInfo] = types.NewNetworkEndpointSet()
+				if addEndpoints[negLocation] == nil {
+					addEndpoints[negLocation] = types.NewNetworkEndpointSet()
+					removeEndpoints[negLocation] = types.NewNetworkEndpointSet()
+					committedEndpoints[negLocation] = types.NewNetworkEndpointSet()
 				}
-				addEndpoints[epGroupInfo].Insert(types.NetworkEndpoint{IP: ipv4, IPv6: ipv6})
-				removeEndpoints[epGroupInfo].Insert(types.NetworkEndpoint{IP: ipv4})
+				addEndpoints[negLocation].Insert(types.NetworkEndpoint{IP: ipv4, IPv6: ipv6})
+				removeEndpoints[negLocation].Insert(types.NetworkEndpoint{IP: ipv4})
 			}
 
 			tc.migrator.errorStateChecker.(*fakeErrorStateChecker).errorState = tc.errorState

pkg/neg/syncers/transaction.go

@@ -629,18 +629,18 @@ func (s *transactionSyncer) syncNetworkEndpoints(addEndpoints, removeEndpoints m
 }
 
 // attachNetworkEndpoints runs operation for attaching network endpoints.
-func (s *transactionSyncer) attachNetworkEndpoints(epGroupInfo negtypes.NEGLocation, networkEndpointMap map[negtypes.NetworkEndpoint]*composite.NetworkEndpoint) {
-	s.logger.V(2).Info("Attaching endpoints to NEG.", "countOfEndpointsBeingAttached", len(networkEndpointMap), "negSyncerKey", s.NegSyncerKey.String(), "zone", epGroupInfo.Zone, "subnet", epGroupInfo.Subnet)
-	err := s.operationInternal(attachOp, epGroupInfo, networkEndpointMap, s.logger)
+func (s *transactionSyncer) attachNetworkEndpoints(negLocation negtypes.NEGLocation, networkEndpointMap map[negtypes.NetworkEndpoint]*composite.NetworkEndpoint) {
+	s.logger.V(2).Info("Attaching endpoints to NEG.", "countOfEndpointsBeingAttached", len(networkEndpointMap), "negSyncerKey", s.NegSyncerKey.String(), "zone", negLocation.Zone, "subnet", negLocation.Subnet)
+	err := s.operationInternal(attachOp, negLocation, networkEndpointMap, s.logger)
 
 	// WARNING: commitTransaction must be called at last for analyzing the operation result
 	s.commitTransaction(err, networkEndpointMap)
 }
 
 // detachNetworkEndpoints runs operation for detaching network endpoints.
-func (s *transactionSyncer) detachNetworkEndpoints(epGroupInfo negtypes.NEGLocation, networkEndpointMap map[negtypes.NetworkEndpoint]*composite.NetworkEndpoint, hasMigrationDetachments bool) {
-	s.logger.V(2).Info("Detaching endpoints from NEG.", "countOfEndpointsBeingDetached", len(networkEndpointMap), "negSyncerKey", s.NegSyncerKey.String(), "zone", epGroupInfo.Zone, "subnet", epGroupInfo.Subnet)
-	err := s.operationInternal(detachOp, epGroupInfo, networkEndpointMap, s.logger)
+func (s *transactionSyncer) detachNetworkEndpoints(negLocation negtypes.NEGLocation, networkEndpointMap map[negtypes.NetworkEndpoint]*composite.NetworkEndpoint, hasMigrationDetachments bool) {
+	s.logger.V(2).Info("Detaching endpoints from NEG.", "countOfEndpointsBeingDetached", len(networkEndpointMap), "negSyncerKey", s.NegSyncerKey.String(), "zone", negLocation.Zone, "subnet", negLocation.Subnet)
+	err := s.operationInternal(detachOp, negLocation, networkEndpointMap, s.logger)
 
 	if hasMigrationDetachments {
 		// Unpause the migration since the ongoing migration-detachments have
@@ -655,14 +655,14 @@ func (s *transactionSyncer) detachNetworkEndpoints(epGroupInfo negtypes.NEGLocat
 // operationInternal executes NEG API call and commits the transactions
 // It will record events when operations are completed
 // If error occurs or any transaction entry requires reconciliation, it will trigger resync
-func (s *transactionSyncer) operationInternal(operation transactionOp, epGroupInfo negtypes.NEGLocation, networkEndpointMap map[negtypes.NetworkEndpoint]*composite.NetworkEndpoint, logger klog.Logger) error {
+func (s *transactionSyncer) operationInternal(operation transactionOp, negLocation negtypes.NEGLocation, networkEndpointMap map[negtypes.NetworkEndpoint]*composite.NetworkEndpoint, logger klog.Logger) error {
 	var err error
 	start := time.Now()
 	networkEndpoints := []*composite.NetworkEndpoint{}
 	for _, ne := range networkEndpointMap {
 		networkEndpoints = append(networkEndpoints, ne)
 	}
-	zone := epGroupInfo.Zone
+	zone := negLocation.Zone
 	negName := s.NegSyncerKey.NegName
 	if flags.F.EnableMultiSubnetClusterPhase1 {
 		defaultSubnet, err := utils.KeyName(s.networkInfo.SubnetworkURL)
@@ -676,8 +676,8 @@ func (s *transactionSyncer) operationInternal(operation transactionOp, epGroupIn
 		// (epGroupInfo.Subnet) is not the defaultSubnet, we are dealing with
 		// the Multi-Subnet Cluster use case, wherein the name of the NEG would
 		// need to be different.
-		if s.networkInfo.IsDefault && epGroupInfo.Subnet != defaultSubnet {
-			negName, err = s.getNonDefaultSubnetNEGName(epGroupInfo.Subnet)
+		if s.networkInfo.IsDefault && negLocation.Subnet != defaultSubnet {
+			negName, err = s.getNonDefaultSubnetNEGName(negLocation.Subnet)
 			if err != nil {
 				s.logger.Error(err, "Errored getting non-default subnet NEG name when updating NEG endpoints")
 				return err

pkg/neg/syncers/utils.go

@@ -288,16 +288,16 @@ func toZoneNetworkEndpointMap(eds []negtypes.EndpointsData, zoneGetter *zonegett
 				continue
 			}
 			globalEPCount[negtypes.Total] += 1
-			epGroupInfo, _, getEpGroupInfoErr := getEndpointZoneSubnet(endpointAddress, zoneGetter, logger)
-			if getEpGroupInfoErr != nil {
-				metrics.PublishNegControllerErrorCountMetrics(getEpGroupInfoErr, true)
-				if enableMultiSubnetCluster && errors.Is(getEpGroupInfoErr, zonegetter.ErrNodeNotInDefaultSubnet) {
-					epLogger.Error(getEpGroupInfoErr, "Detected endpoint not from default subnet. Skipping")
+			negLocation, _, getNegLocationErr := getEndpointZoneSubnet(endpointAddress, zoneGetter, logger)
+			if getNegLocationErr != nil {
+				metrics.PublishNegControllerErrorCountMetrics(getNegLocationErr, true)
+				if enableMultiSubnetCluster && errors.Is(getNegLocationErr, zonegetter.ErrNodeNotInDefaultSubnet) {
+					epLogger.Error(getNegLocationErr, "Detected endpoint not from default subnet. Skipping")
 					localEPCount[negtypes.NodeInNonDefaultSubnet]++
 					continue
 				}
-				epLogger.Error(getEpGroupInfoErr, "Detected unexpected error when getting zone for endpoint")
-				return ZoneNetworkEndpointMapResult{}, fmt.Errorf("unexpected error when getting zone for endpoint %q in endpoint slice %s/%s: %w", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name, getEpGroupInfoErr)
+				epLogger.Error(getNegLocationErr, "Detected unexpected error when getting zone for endpoint")
+				return ZoneNetworkEndpointMapResult{}, fmt.Errorf("unexpected error when getting zone for endpoint %q in endpoint slice %s/%s: %w", endpointAddress.Addresses, ed.Meta.Namespace, ed.Meta.Name, getNegLocationErr)
 			}
 
 			_, _, getPodErr := getEndpointPod(endpointAddress, podLister)
@@ -311,8 +311,8 @@ func toZoneNetworkEndpointMap(eds []negtypes.EndpointsData, zoneGetter *zonegett
 				epLogger.V(2).Info("Endpoint does not have an associated pod. Skipping")
 				continue
 			}
-			if zoneNetworkEndpointMap[epGroupInfo] == nil {
-				zoneNetworkEndpointMap[epGroupInfo] = negtypes.NewNetworkEndpointSet()
+			if zoneNetworkEndpointMap[negLocation] == nil {
+				zoneNetworkEndpointMap[negLocation] = negtypes.NewNetworkEndpointSet()
 			}
 
 			podIPs := ipsForPod[types.NamespacedName{Namespace: endpointAddress.TargetRef.Namespace, Name: endpointAddress.TargetRef.Name}]
@@ -331,7 +331,7 @@ func toZoneNetworkEndpointMap(eds []negtypes.EndpointsData, zoneGetter *zonegett
 				// Non-GCP network endpoints don't have associated nodes.
 				networkEndpoint.Node = ""
 			}
-			zoneNetworkEndpointMap[epGroupInfo].Insert(networkEndpoint)
+			zoneNetworkEndpointMap[negLocation].Insert(networkEndpoint)
 
 			// if existing name is alphabetically lower than current one, continue and don't replace
 			if existingPod, contains := networkEndpointPodMap[networkEndpoint]; contains {
@@ -484,9 +484,9 @@ func toZoneNetworkEndpointMapDegradedMode(eds []negtypes.EndpointsData, zoneGett
 				localEPCount[negtypes.NodeNotFound]++
 				continue
 			}
-			epGroupInfo := negtypes.NEGLocation{Zone: zone, Subnet: subnet}
-			if zoneNetworkEndpointMap[epGroupInfo] == nil {
-				zoneNetworkEndpointMap[epGroupInfo] = negtypes.NewNetworkEndpointSet()
+			negLocation := negtypes.NEGLocation{Zone: zone, Subnet: subnet}
+			if zoneNetworkEndpointMap[negLocation] == nil {
+				zoneNetworkEndpointMap[negLocation] = negtypes.NewNetworkEndpointSet()
 			}
 
 			podIPs := ipsForPod[types.NamespacedName{Namespace: endpointAddress.TargetRef.Namespace, Name: endpointAddress.TargetRef.Name}]
@@ -529,7 +529,7 @@ func toZoneNetworkEndpointMapDegradedMode(eds []negtypes.EndpointsData, zoneGett
 				// Non-GCP network endpoints don't have associated nodes.
 				networkEndpoint.Node = ""
 			}
-			zoneNetworkEndpointMap[epGroupInfo].Insert(networkEndpoint)
+			zoneNetworkEndpointMap[negLocation].Insert(networkEndpoint)
 
 			// if existing name is alphabetically lower than current one, continue and don't replace
 			if existingPod, contains := networkEndpointPodMap[networkEndpoint]; contains {