From d46ee9c883b39d05d9d5f1dfe909effd504c8b73 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Tue, 20 Apr 2021 17:58:26 -0700 Subject: [PATCH] Exclude nodes from load balancers upon cordoning --- pkg/instancegroups/instancegroups.go | 39 ++++++++++++++++++++++++ pkg/instancegroups/rollingupdate_test.go | 33 ++++++++++++++++++-- 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/pkg/instancegroups/instancegroups.go b/pkg/instancegroups/instancegroups.go index 84c2e5f0e8..bf07a12bf4 100644 --- a/pkg/instancegroups/instancegroups.go +++ b/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 diff --git a/pkg/instancegroups/rollingupdate_test.go b/pkg/instancegroups/rollingupdate_test.go index ebe8c6b59c..37d8166873 100644 --- a/pkg/instancegroups/rollingupdate_test.go +++ b/pkg/instancegroups/rollingupdate_test.go @@ -45,8 +45,9 @@ import ( ) const ( - cordonPatch = "{\"spec\":{\"unschedulable\":true}}" - taintPatch = "{\"spec\":{\"taints\":[{\"effect\":\"PreferNoSchedule\",\"key\":\"kops.k8s.io/scheduled-for-update\"}]}}" + 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\"}]}}" ) func getTestSetup() (*RollingUpdateCluster, *awsup.MockAWSCloud) { @@ -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()))