Exclude nodes from load balancers upon cordoning

pkg/instancegroups/instancegroups.go

@@ -344,6 +344,38 @@ func (c *RollingUpdateCluster) patchTaint(node *corev1.Node) error {
 	return err
 }
 
+func (c *RollingUpdateCluster) patchExcludeFromLB(node *corev1.Node) error {
+	oldData, err := json.Marshal(node)
+	if err != nil {
+		return err
+	}
+
+	if node.Labels == nil {
+		node.Labels = map[string]string{}
+	}
+
+	if _, ok := node.Labels[corev1.LabelNodeExcludeBalancers]; ok {
+		return nil
+	}
+	node.Labels[corev1.LabelNodeExcludeBalancers] = ""
+
+	newData, err := json.Marshal(node)
+	if err != nil {
+		return err
+	}
+
+	patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, node)
+	if err != nil {
+		return err
+	}
+
+	_, err = c.K8sClient.CoreV1().Nodes().Patch(c.Ctx, node.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{})
+	if apierrors.IsNotFound(err) {
+		return nil
+	}
+	return err
+}
+
 func (c *RollingUpdateCluster) drainTerminateAndWait(u *cloudinstances.CloudInstance, sleepAfterTerminate time.Duration) error {
 	instanceID := u.ID
 
@@ -608,6 +640,13 @@ func (c *RollingUpdateCluster) drainNode(u *cloudinstances.CloudInstance) error
 		return fmt.Errorf("error cordoning node: %v", err)
 	}
 
+	if err := c.patchExcludeFromLB(u.Node); err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil
+		}
+		return fmt.Errorf("error excluding node from load balancer: %v", err)
+	}
+
 	if err := drain.RunNodeDrain(helper, u.Node.Name); err != nil {
 		if apierrors.IsNotFound(err) {
 			return nil

pkg/instancegroups/rollingupdate_test.go

@@ -46,6 +46,7 @@ import (
 
 const (
 	cordonPatch    = "{\"spec\":{\"unschedulable\":true}}"
+	excludeLBPatch = "{\"metadata\":{\"labels\":{\"node.kubernetes.io/exclude-from-external-load-balancers\":\"\"}}}"
 	taintPatch     = "{\"spec\":{\"taints\":[{\"effect\":\"PreferNoSchedule\",\"key\":\"kops.k8s.io/scheduled-for-update\"}]}}"
 )
 
@@ -213,6 +214,7 @@ func TestRollingUpdateAllNeedUpdate(t *testing.T) {
 	assert.NoError(t, err, "rolling update")
 
 	cordoned := ""
+	excluded := ""
 	tainted := map[string]bool{}
 	deleted := map[string]bool{}
 	for _, action := range c.K8sClient.(*fake.Clientset).Actions() {
@@ -223,6 +225,11 @@ func TestRollingUpdateAllNeedUpdate(t *testing.T) {
 				assert.Equal(t, "", cordoned, "at most one node cordoned at a time")
 				assert.True(t, tainted[a.GetName()], "node", a.GetName(), "tainted")
 				cordoned = a.GetName()
+			} else if string(a.GetPatch()) == excludeLBPatch {
+				assertExclude(t, a)
+				assert.Equal(t, "", excluded, "at most one node excluded at a time")
+				assert.True(t, tainted[a.GetName()], "node", a.GetName(), "tainted")
+				excluded = a.GetName()
 			} else {
 				assertTaint(t, a)
 				assert.Equal(t, "", cordoned, "not tainting while node cordoned")
@@ -232,6 +239,7 @@ func TestRollingUpdateAllNeedUpdate(t *testing.T) {
 		case testingclient.DeleteAction:
 			assert.Equal(t, "nodes", a.GetResource().Resource)
 			assert.Equal(t, cordoned, a.GetName(), "node was cordoned before delete")
+			assert.Equal(t, excluded, a.GetName(), "node was excluded before delete")
 			assert.False(t, deleted[a.GetName()], "node", a.GetName(), "already deleted")
 			if !strings.HasPrefix(a.GetName(), "master-") {
 				assert.True(t, deleted["master-1a.local"], "master-1a was deleted before node", a.GetName())
@@ -239,6 +247,7 @@ func TestRollingUpdateAllNeedUpdate(t *testing.T) {
 			}
 			deleted[a.GetName()] = true
 			cordoned = ""
+			excluded = ""
 		case testingclient.ListAction:
 			// Don't care
 		default:
@@ -813,6 +822,7 @@ func TestRollingUpdateTaintAllButOneNeedUpdate(t *testing.T) {
 	assert.NoError(t, err, "rolling update")
 
 	cordoned := ""
+	excluded := ""
 	tainted := map[string]bool{}
 	deleted := map[string]bool{}
 	for _, action := range c.K8sClient.(*fake.Clientset).Actions() {
@@ -822,6 +832,10 @@ func TestRollingUpdateTaintAllButOneNeedUpdate(t *testing.T) {
 				assertCordon(t, a)
 				assert.Equal(t, "", cordoned, "at most one node cordoned at a time")
 				cordoned = a.GetName()
+			} else if string(a.GetPatch()) == excludeLBPatch {
+				assertExclude(t, a)
+				assert.Equal(t, "", excluded, "at most one node excluded at a time")
+				excluded = a.GetName()
 			} else {
 				assertTaint(t, a)
 				assert.False(t, tainted[a.GetName()], "node", a.GetName(), "already tainted")
@@ -830,10 +844,12 @@ func TestRollingUpdateTaintAllButOneNeedUpdate(t *testing.T) {
 		case testingclient.DeleteAction:
 			assert.Equal(t, "nodes", a.GetResource().Resource)
 			assert.Equal(t, cordoned, a.GetName(), "node was cordoned before delete")
+			assert.Equal(t, excluded, a.GetName(), "node was excluded before delete")
 			assert.Len(t, tainted, 2, "all nodes tainted before any delete")
 			assert.False(t, deleted[a.GetName()], "node", a.GetName(), "already deleted")
 			deleted[a.GetName()] = true
 			cordoned = ""
+			excluded = ""
 		case testingclient.ListAction:
 			// Don't care
 		default:
@@ -859,6 +875,7 @@ func TestRollingUpdateMaxSurgeIgnoredForMaster(t *testing.T) {
 	assert.NoError(t, err, "rolling update")
 
 	cordoned := ""
+	excluded := ""
 	tainted := map[string]bool{}
 	deleted := map[string]bool{}
 	for _, action := range c.K8sClient.(*fake.Clientset).Actions() {
@@ -869,6 +886,11 @@ func TestRollingUpdateMaxSurgeIgnoredForMaster(t *testing.T) {
 				assert.Equal(t, "", cordoned, "at most one node cordoned at a time")
 				assert.True(t, tainted[a.GetName()], "node", a.GetName(), "tainted")
 				cordoned = a.GetName()
+			} else if string(a.GetPatch()) == excludeLBPatch {
+				assertExclude(t, a)
+				assert.Equal(t, "", excluded, "at most one node excluded at a time")
+				assert.True(t, tainted[a.GetName()], "node", a.GetName(), "tainted")
+				excluded = a.GetName()
 			} else {
 				assertTaint(t, a)
 				assert.Equal(t, "", cordoned, "not tainting while node cordoned")
@@ -878,9 +900,11 @@ func TestRollingUpdateMaxSurgeIgnoredForMaster(t *testing.T) {
 		case testingclient.DeleteAction:
 			assert.Equal(t, "nodes", a.GetResource().Resource)
 			assert.Equal(t, cordoned, a.GetName(), "node was cordoned before delete")
+			assert.Equal(t, excluded, a.GetName(), "node was excluded before delete")
 			assert.False(t, deleted[a.GetName()], "node", a.GetName(), "already deleted")
 			deleted[a.GetName()] = true
 			cordoned = ""
+			excluded = ""
 		case testingclient.ListAction:
 			// Don't care
 		default:
@@ -1484,6 +1508,11 @@ func assertCordon(t *testing.T, action testingclient.PatchAction) {
 	assert.Equal(t, cordonPatch, string(action.GetPatch()))
 }
 
+func assertExclude(t *testing.T, action testingclient.PatchAction) {
+	assert.Equal(t, "nodes", action.GetResource().Resource)
+	assert.Equal(t, excludeLBPatch, string(action.GetPatch()))
+}
+
 func assertTaint(t *testing.T, action testingclient.PatchAction) {
 	assert.Equal(t, "nodes", action.GetResource().Resource)
 	assert.Equal(t, taintPatch, string(action.GetPatch()))