Merge pull request #922 from freehan/neg-type

move neg type into syncerKey
kubernetes · Oct 30, 2019 · a4da6e0 · a4da6e0
2 parents 212e82a + b30ed3f
commit a4da6e0
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 31 deletions.
diff --git a/pkg/neg/manager.go b/pkg/neg/manager.go
@@ -20,7 +20,7 @@ import (
 	"fmt"
 	"sync"
 
-	v1 "k8s.io/api/core/v1"
+	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -114,15 +114,9 @@ func (manager *syncerManager) EnsureSyncers(namespace, name string, newPorts neg
 		syncerKey := getSyncerKey(namespace, name, svcPort, portInfo)
 		syncer, ok := manager.syncerMap[syncerKey]
 		if !ok {
-			networkEndpointType := negtypes.VMNetworkEndpointType
-			if flags.F.EnableNonGCPMode {
-				networkEndpointType = negtypes.NonGCPPrivateEndpointType
-			}
-
 			syncer = negsyncer.NewTransactionSyncer(
 				syncerKey,
 				portInfo.NegName,
-				networkEndpointType,
 				manager.recorder,
 				manager.cloud,
 				manager.zoneGetter,
@@ -311,13 +305,19 @@ func (manager *syncerManager) ensureDeleteNetworkEndpointGroup(name, zone string
 
 // getSyncerKey encodes a service namespace, name, service port and targetPort into a string key
 func getSyncerKey(namespace, name string, servicePortKey negtypes.PortInfoMapKey, portInfo negtypes.PortInfo) negtypes.NegSyncerKey {
+	networkEndpointType := negtypes.VmIpPortEndpointType
+	if flags.F.EnableNonGCPMode {
+		networkEndpointType = negtypes.NonGCPPrivateEndpointType
+	}
+
 	return negtypes.NegSyncerKey{
 		Namespace:    namespace,
 		Name:         name,
 		Port:         servicePortKey.ServicePort,
 		TargetPort:   portInfo.TargetPort,
 		Subset:       servicePortKey.Subset,
 		SubsetLabels: portInfo.SubsetLabels,
+		NegType:      networkEndpointType,
 	}
 }
 

diff --git a/pkg/neg/manager_test.go b/pkg/neg/manager_test.go
@@ -314,7 +314,7 @@ func TestGarbageCollectionNEG(t *testing.T) {
 		t.Fatalf("Failed to ensure syncer: %v", err)
 	}
 
-	for _, networkEndpointType := range []negtypes.NetworkEndpointType{negtypes.VMNetworkEndpointType, negtypes.NonGCPPrivateEndpointType} {
+	for _, networkEndpointType := range []negtypes.NetworkEndpointType{negtypes.VmIpPortEndpointType, negtypes.NonGCPPrivateEndpointType} {
 		negName := manager.namer.NEG("test", "test", 80)
 		manager.cloud.CreateNetworkEndpointGroup(&compute.NetworkEndpointGroup{
 			Name:                negName,

diff --git a/pkg/neg/syncers/transaction.go b/pkg/neg/syncers/transaction.go
@@ -36,8 +36,6 @@ type transactionSyncer struct {
 	// metadata
 	negtypes.NegSyncerKey
 	negName string
-	// The type of the network endpoints in this NEG.
-	networkEndpointType negtypes.NetworkEndpointType
 
 	// syncer provides syncer life cycle interfaces
 	syncer negtypes.NegSyncer
@@ -67,21 +65,20 @@ type transactionSyncer struct {
 	reflector readiness.Reflector
 }
 
-func NewTransactionSyncer(negSyncerKey negtypes.NegSyncerKey, networkEndpointGroupName string, networkEndpointType negtypes.NetworkEndpointType, recorder record.EventRecorder, cloud negtypes.NetworkEndpointGroupCloud, zoneGetter negtypes.ZoneGetter, podLister cache.Indexer, serviceLister cache.Indexer, endpointLister cache.Indexer, reflector readiness.Reflector) negtypes.NegSyncer {
+func NewTransactionSyncer(negSyncerKey negtypes.NegSyncerKey, networkEndpointGroupName string, recorder record.EventRecorder, cloud negtypes.NetworkEndpointGroupCloud, zoneGetter negtypes.ZoneGetter, podLister cache.Indexer, serviceLister cache.Indexer, endpointLister cache.Indexer, reflector readiness.Reflector) negtypes.NegSyncer {
 	// TransactionSyncer implements the syncer core
 	ts := &transactionSyncer{
-		NegSyncerKey:        negSyncerKey,
-		negName:             networkEndpointGroupName,
-		networkEndpointType: networkEndpointType,
-		needInit:            true,
-		transactions:        NewTransactionTable(),
-		podLister:           podLister,
-		serviceLister:       serviceLister,
-		endpointLister:      endpointLister,
-		recorder:            recorder,
-		cloud:               cloud,
-		zoneGetter:          zoneGetter,
-		reflector:           reflector,
+		NegSyncerKey:   negSyncerKey,
+		negName:        networkEndpointGroupName,
+		needInit:       true,
+		transactions:   NewTransactionTable(),
+		podLister:      podLister,
+		serviceLister:  serviceLister,
+		endpointLister: endpointLister,
+		recorder:       recorder,
+		cloud:          cloud,
+		zoneGetter:     zoneGetter,
+		reflector:      reflector,
 	}
 	// Syncer implements life cycle logic
 	syncer := newSyncer(negSyncerKey, networkEndpointGroupName, serviceLister, recorder, ts)
@@ -133,7 +130,7 @@ func (s *transactionSyncer) syncInternal() error {
 		return nil
 	}
 
-	targetMap, endpointPodMap, err := toZoneNetworkEndpointMap(ep.(*apiv1.Endpoints), s.zoneGetter, s.TargetPort, s.podLister, s.NegSyncerKey.SubsetLabels, s.networkEndpointType)
+	targetMap, endpointPodMap, err := toZoneNetworkEndpointMap(ep.(*apiv1.Endpoints), s.zoneGetter, s.TargetPort, s.podLister, s.NegSyncerKey.SubsetLabels, s.NegSyncerKey.NegType)
 	if err != nil {
 		return err
 	}
@@ -181,7 +178,7 @@ func (s *transactionSyncer) ensureNetworkEndpointGroups() error {
 
 	var errList []error
 	for _, zone := range zones {
-		if err := ensureNetworkEndpointGroup(s.Namespace, s.Name, s.negName, zone, s.NegSyncerKey.String(), s.networkEndpointType, s.cloud, s.serviceLister, s.recorder); err != nil {
+		if err := ensureNetworkEndpointGroup(s.Namespace, s.Name, s.negName, zone, s.NegSyncerKey.String(), s.NegSyncerKey.NegType, s.cloud, s.serviceLister, s.recorder); err != nil {
 			errList = append(errList, err)
 		}
 	}

diff --git a/pkg/neg/syncers/transaction_test.go b/pkg/neg/syncers/transaction_test.go
@@ -1082,6 +1082,7 @@ func newTestTransactionSyncer(fakeGCE negtypes.NetworkEndpointGroupCloud) (negty
 	svcPort := negtypes.NegSyncerKey{
 		Namespace:  testNamespace,
 		Name:       testService,
+		NegType:    negtypes.VmIpPortEndpointType,
 		Port:       80,
 		TargetPort: "8080",
 	}
@@ -1091,7 +1092,6 @@ func newTestTransactionSyncer(fakeGCE negtypes.NetworkEndpointGroupCloud) (negty
 
 	negsyncer := NewTransactionSyncer(svcPort,
 		testNegName,
-		negtypes.VMNetworkEndpointType,
 		record.NewFakeRecorder(100),
 		fakeGCE,
 		negtypes.NewFakeZoneGetter(),

diff --git a/pkg/neg/syncers/utils_test.go b/pkg/neg/syncers/utils_test.go
@@ -318,7 +318,7 @@ func TestEnsureNetworkEndpointGroup(t *testing.T) {
 			description:         "Create NEG of type GCE_VM_IP_PORT",
 			negName:             "gcp-neg",
 			enableNonGCPMode:    false,
-			networkEndpointType: negtypes.VMNetworkEndpointType,
+			networkEndpointType: negtypes.VmIpPortEndpointType,
 			expectedSubnetwork:  testSubnetwork,
 		},
 		{
@@ -418,7 +418,7 @@ func TestToZoneNetworkEndpointMapUtil(t *testing.T) {
 			targetPort:          "8888",
 			endpointSets:        map[string]negtypes.NetworkEndpointSet{},
 			expectMap:           negtypes.EndpointPodMap{},
-			networkEndpointType: negtypes.VMNetworkEndpointType,
+			networkEndpointType: negtypes.VmIpPortEndpointType,
 		},
 		{
 			desc:       "target port number",
@@ -439,7 +439,7 @@ func TestToZoneNetworkEndpointMapUtil(t *testing.T) {
 				networkEndpointFromEncodedEndpoint("10.100.3.1||instance3||80"): types.NamespacedName{Namespace: testServiceNamespace, Name: "pod4"},
 				networkEndpointFromEncodedEndpoint("10.100.1.3||instance1||80"): types.NamespacedName{Namespace: testServiceNamespace, Name: "pod5"},
 			},
-			networkEndpointType: negtypes.VMNetworkEndpointType,
+			networkEndpointType: negtypes.VmIpPortEndpointType,
 		},
 		{
 			desc:       "named target port",
@@ -460,7 +460,7 @@ func TestToZoneNetworkEndpointMapUtil(t *testing.T) {
 				networkEndpointFromEncodedEndpoint("10.100.3.2||instance3||8081"): types.NamespacedName{Namespace: testServiceNamespace, Name: "pod10"},
 				networkEndpointFromEncodedEndpoint("10.100.4.2||instance4||8081"): types.NamespacedName{Namespace: testServiceNamespace, Name: "pod11"},
 			},
-			networkEndpointType: negtypes.VMNetworkEndpointType,
+			networkEndpointType: negtypes.VmIpPortEndpointType,
 		},
 		{
 			desc:       "Non-GCP network endpoints",

diff --git a/pkg/neg/types/types.go b/pkg/neg/types/types.go
@@ -32,7 +32,7 @@ import (
 type NetworkEndpointType string
 
 const (
-	VMNetworkEndpointType     = NetworkEndpointType("GCE_VM_IP_PORT")
+	VmIpPortEndpointType      = NetworkEndpointType("GCE_VM_IP_PORT")
 	NonGCPPrivateEndpointType = NetworkEndpointType("NON_GCP_PRIVATE_IP_PORT")
 )
 
@@ -204,6 +204,9 @@ type NegSyncerKey struct {
 
 	// Subset label, should set together with Subset.
 	SubsetLabels string
+
+	// NegType is the type of the network endpoints in this NEG.
+	NegType NetworkEndpointType
 }
 
 func (key NegSyncerKey) String() string {