internal lb do not remove nodes with empty string in the zone field

kubernetes · Sep 3, 2024 · 6563a59 · 6563a59
1 parent 1de8477
commit 6563a59
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 3 deletions.
diff --git a/providers/gce/gce_loadbalancer_internal.go b/providers/gce/gce_loadbalancer_internal.go
@@ -603,7 +603,7 @@ func (g *Cloud) ensureInternalHealthCheck(name string, svcName types.NamespacedN
 	return hc, nil
 }
 
-func (g *Cloud) ensureInternalInstanceGroup(name, zone string, nodes []*v1.Node) (string, error) {
+func (g *Cloud) ensureInternalInstanceGroup(name, zone string, nodes []*v1.Node, emptyZoneNodesNames sets.String) (string, error) {
 	klog.V(2).Infof("ensureInternalInstanceGroup(%v, %v): checking group that it contains %v nodes [node names limited, total number of nodes: %d]", name, zone, loggableNodeNames(nodes), len(nodes))
 	ig, err := g.GetInstanceGroup(name, zone)
 	if err != nil && !isNotFound(err) {
@@ -650,7 +650,7 @@ func (g *Cloud) ensureInternalInstanceGroup(name, zone string, nodes []*v1.Node)
 		}
 	}
 
-	removeNodes := gceNodes.Difference(kubeNodes).List()
+	removeNodes := gceNodes.Difference(kubeNodes).Difference(emptyZoneNodesNames).List()
 	addNodes := kubeNodes.Difference(gceNodes).List()
 
 	if len(removeNodes) != 0 {
@@ -689,8 +689,21 @@ func (g *Cloud) ensureInternalInstanceGroups(name string, nodes []*v1.Node) ([]s
 
 	zonedNodes := splitNodesByZone(nodes)
 	klog.V(2).Infof("ensureInternalInstanceGroups(%v): %d nodes over %d zones in region %v", name, len(nodes), len(zonedNodes), g.region)
+
+	emptyZoneNodesNames := sets.NewString()
+	for _, n := range zonedNodes[""] {
+		emptyZoneNodesNames.Insert(n.Name)
+	}
+
+	if len(emptyZoneNodesNames) > 0 {
+		klog.V(2).Infof("%d nodes have empty zone: %v in region %v", len(emptyZoneNodesNames), emptyZoneNodesNames, g.region)
+	}
+
 	var igLinks []string
 	for zone, nodes := range zonedNodes {
+		if zone == "" {
+			continue // skip ensuring nodes with empty zone
+		}
 		if g.AlphaFeatureGate.Enabled(AlphaFeatureSkipIGsManagement) {
 			igs, err := g.FilterInstanceGroupsByNamePrefix(name, zone)
 			if err != nil {
@@ -700,7 +713,7 @@ func (g *Cloud) ensureInternalInstanceGroups(name string, nodes []*v1.Node) ([]s
 				igLinks = append(igLinks, ig.SelfLink)
 			}
 		} else {
-			igLink, err := g.ensureInternalInstanceGroup(name, zone, nodes)
+			igLink, err := g.ensureInternalInstanceGroup(name, zone, nodes, emptyZoneNodesNames)
 			if err != nil {
 				return nil, err
 			}

diff --git a/providers/gce/gce_loadbalancer_internal_test.go b/providers/gce/gce_loadbalancer_internal_test.go
@@ -705,6 +705,31 @@ func TestUpdateInternalLoadBalancerNodes(t *testing.T) {
 	)
 }
 
+func TestUpdateInternalLoadBalancerNodesWithEmptyZone(t *testing.T) {
+	t.Parallel()
+
+	vals := DefaultTestClusterValues()
+	gce, err := fakeGCECloud(vals)
+	require.NoError(t, err)
+	node1Name := []string{"test-node-1"}
+
+	svc := fakeLoadbalancerService(string(LBTypeInternal))
+	svc, err = gce.client.CoreV1().Services(svc.Namespace).Create(context.TODO(), svc, metav1.CreateOptions{})
+	require.NoError(t, err)
+	nodes, err := createAndInsertNodes(gce, node1Name, vals.ZoneName)
+	nodes[0].Labels[v1.LabelTopologyZone] = "" // empty zone
+	require.NoError(t, err)
+
+	_, err = gce.ensureInternalLoadBalancer(vals.ClusterName, vals.ClusterID, svc, nil, nodes)
+	assert.NoError(t, err)
+
+	// Expect load balancer to not have deleted node test-node-1
+	node := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: node1Name}}
+	exist, err := gce.InstanceExists(context.TODO(), node)
+	require.NoError(t, err)
+	assert.Equal(t, exist, true)
+}
+
 func TestEnsureInternalLoadBalancerDeleted(t *testing.T) {
 	t.Parallel()