fix: GCE_VM_IP NEGs should be synced on each Node change.

This reverts the behaviour to what existed prior to kubernetes#1754
gauravkghildiyal · Aug 14, 2024 · 83df5b6 · 83df5b6
1 parent d5825ac
commit 83df5b6
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 9 deletions.
diff --git a/pkg/neg/manager.go b/pkg/neg/manager.go
@@ -110,7 +110,6 @@ type syncerManager struct {
 
 	// zone maps keep track of the last set of zones the neg controller has seen
 	// for their respective NEG types. zone maps are protected by the mu mutex.
-	vmIpZoneMap     map[string]struct{}
 	vmIpPortZoneMap map[string]struct{}
 
 	// lpConfig configures the pod label to be propagated to NEG endpoints.
@@ -135,8 +134,7 @@ func newSyncerManager(namer negtypes.NetworkEndpointGroupNamer,
 	lpConfig podlabels.PodLabelPropagationConfig,
 	logger klog.Logger) *syncerManager {
 
-	var vmIpZoneMap, vmIpPortZoneMap map[string]struct{}
-	updateZoneMap(&vmIpZoneMap, negtypes.NodeFilterForNetworkEndpointType(negtypes.VmIpEndpointType), zoneGetter, logger)
+	var vmIpPortZoneMap map[string]struct{}
 	updateZoneMap(&vmIpPortZoneMap, negtypes.NodeFilterForNetworkEndpointType(negtypes.VmIpPortEndpointType), zoneGetter, logger)
 
 	return &syncerManager{
@@ -158,7 +156,6 @@ func newSyncerManager(namer negtypes.NetworkEndpointGroupNamer,
 		enableDualStackNEG:  enableDualStackNEG,
 		numGCWorkers:        numGCWorkers,
 		logger:              logger,
-		vmIpZoneMap:         vmIpZoneMap,
 		vmIpPortZoneMap:     vmIpPortZoneMap,
 		lpConfig:            lpConfig,
 	}
@@ -318,7 +315,6 @@ func (manager *syncerManager) SyncNodes() {
 	defer manager.mu.Unlock()
 
 	// When a zone change occurs (new zone is added or deleted), a sync should be triggered
-	isVmIpZoneChange := updateZoneMap(&manager.vmIpZoneMap, negtypes.NodeFilterForNetworkEndpointType(negtypes.VmIpEndpointType), manager.zoneGetter, manager.logger)
 	isVmIpPortZoneChange := updateZoneMap(&manager.vmIpPortZoneMap, negtypes.NodeFilterForNetworkEndpointType(negtypes.VmIpPortEndpointType), manager.zoneGetter, manager.logger)
 
 	for key, syncer := range manager.syncerMap {
@@ -329,9 +325,7 @@ func (manager *syncerManager) SyncNodes() {
 		switch key.NegType {
 
 		case negtypes.VmIpEndpointType:
-			if isVmIpZoneChange {
-				syncer.Sync()
-			}
+			syncer.Sync()
 
 		case negtypes.VmIpPortEndpointType, negtypes.NonGCPPrivateEndpointType:
 			if isVmIpPortZoneChange {

diff --git a/pkg/neg/manager_test.go b/pkg/neg/manager_test.go
@@ -1442,6 +1442,11 @@ func TestSyncNodesConditions(t *testing.T) {
 			expectSync: false,
 			negType:    negtypes.VmIpPortEndpointType,
 		},
+		{
+			desc:       "vm ip neg, should always sync on any node change",
+			expectSync: true,
+			negType:    negtypes.VmIpEndpointType,
+		},
 		{
 			desc:       "vm ip neg, zones added",
 			expectSync: true,
@@ -1458,7 +1463,7 @@ func TestSyncNodesConditions(t *testing.T) {
 		},
 		{
 			desc:       "vm ip neg, zones are the same",
-			expectSync: false,
+			expectSync: true,
 			negType:    negtypes.VmIpEndpointType,
 		},
 		{