diff --git a/pkg/instancegroups/instancegroups.go b/pkg/instancegroups/instancegroups.go index 5e5556608b..ae9b1317e7 100644 --- a/pkg/instancegroups/instancegroups.go +++ b/pkg/instancegroups/instancegroups.go @@ -152,14 +152,15 @@ func (r *RollingUpdateInstanceGroup) RollingUpdate(rollingUpdateData *RollingUpd settings := resolveSettings(cluster, r.CloudGroup.InstanceGroup, numInstances) concurrency := 0 - maxConcurrency := 1 + maxConcurrency := settings.MaxUnavailable.IntValue() - if r.CloudGroup.InstanceGroup.Spec.Role == api.InstanceGroupRoleNode && !rollingUpdateData.Interactive { - maxConcurrency = settings.MaxUnavailable.IntValue() - if maxConcurrency == 0 { - klog.Infof("Rolling updates for InstanceGroup %s are disabled", r.CloudGroup.InstanceGroup.Name) - return nil - } + if maxConcurrency == 0 { + klog.Infof("Rolling updates for InstanceGroup %s are disabled", r.CloudGroup.InstanceGroup.Name) + return nil + } + + if rollingUpdateData.Interactive { + maxConcurrency = 1 } terminateChan := make(chan error, maxConcurrency) diff --git a/pkg/instancegroups/rollingupdate_test.go b/pkg/instancegroups/rollingupdate_test.go index ad894abe28..7084c40f51 100644 --- a/pkg/instancegroups/rollingupdate_test.go +++ b/pkg/instancegroups/rollingupdate_test.go @@ -605,52 +605,6 @@ func TestRollingUpdateTaintAllButOneNeedUpdate(t *testing.T) { assertGroupInstanceCount(t, cloud, "node-1", 1) } -func TestRollingUpdateSettingsIgnoredForMaster(t *testing.T) { - c, cloud, cluster := getTestSetup() - - two := intstr.FromInt(2) - cluster.Spec.RollingUpdate = &kopsapi.RollingUpdate{ - MaxUnavailable: &two, - } - - groups := make(map[string]*cloudinstances.CloudInstanceGroup) - makeGroup(groups, c.K8sClient, cloud, "master-1", kopsapi.InstanceGroupRoleMaster, 3, 2) - err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{}) - assert.NoError(t, err, "rolling update") - - cordoned := "" - tainted := map[string]bool{} - deleted := map[string]bool{} - for _, action := range c.K8sClient.(*fake.Clientset).Actions() { - switch a := action.(type) { - case testingclient.PatchAction: - if string(a.GetPatch()) == cordonPatch { - assertCordon(t, a) - 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 { - assertTaint(t, a) - assert.Equal(t, "", cordoned, "not tainting while node cordoned") - assert.False(t, tainted[a.GetName()], "node", a.GetName(), "already tainted") - tainted[a.GetName()] = true - } - case testingclient.DeleteAction: - assert.Equal(t, "nodes", a.GetResource().Resource) - assert.Equal(t, cordoned, a.GetName(), "node was cordoned before delete") - assert.False(t, deleted[a.GetName()], "node", a.GetName(), "already deleted") - deleted[a.GetName()] = true - cordoned = "" - case testingclient.ListAction: - // Don't care - default: - t.Errorf("unexpected action %v", a) - } - } - - assertGroupInstanceCount(t, cloud, "master-1", 1) -} - func TestRollingUpdateDisabled(t *testing.T) { c, cloud, cluster := getTestSetup() @@ -665,8 +619,8 @@ func TestRollingUpdateDisabled(t *testing.T) { assertGroupInstanceCount(t, cloud, "node-1", 3) assertGroupInstanceCount(t, cloud, "node-2", 3) - assertGroupInstanceCount(t, cloud, "master-1", 0) - assertGroupInstanceCount(t, cloud, "bastion-1", 0) + assertGroupInstanceCount(t, cloud, "master-1", 2) + assertGroupInstanceCount(t, cloud, "bastion-1", 1) } func TestRollingUpdateDisabledCloudonly(t *testing.T) { @@ -684,8 +638,8 @@ func TestRollingUpdateDisabledCloudonly(t *testing.T) { assertGroupInstanceCount(t, cloud, "node-1", 3) assertGroupInstanceCount(t, cloud, "node-2", 3) - assertGroupInstanceCount(t, cloud, "master-1", 0) - assertGroupInstanceCount(t, cloud, "bastion-1", 0) + assertGroupInstanceCount(t, cloud, "master-1", 2) + assertGroupInstanceCount(t, cloud, "bastion-1", 1) } // The concurrent update tests attempt to induce the following expected update sequence: @@ -861,6 +815,29 @@ func TestRollingUpdateMaxUnavailableAllButOneNeedUpdate(t *testing.T) { concurrentTest.AssertComplete() } +func TestRollingUpdateMaxUnavailableAllNeedUpdateMaster(t *testing.T) { + c, cloud, cluster := getTestSetup() + + concurrentTest := newConcurrentTest(t, cloud, true) + c.ValidateSuccessDuration = 0 + c.ClusterValidator = concurrentTest + cloud.MockAutoscaling = concurrentTest + + two := intstr.FromInt(2) + cluster.Spec.RollingUpdate = &kopsapi.RollingUpdate{ + MaxUnavailable: &two, + } + + groups := make(map[string]*cloudinstances.CloudInstanceGroup) + makeGroup(groups, c.K8sClient, cloud, "master-1", kopsapi.InstanceGroupRoleMaster, 7, 7) + + err := c.RollingUpdate(groups, cluster, &kopsapi.InstanceGroupList{}) + assert.NoError(t, err, "rolling update") + + assertGroupInstanceCount(t, cloud, "master-1", 0) + concurrentTest.AssertComplete() +} + func assertCordon(t *testing.T, action testingclient.PatchAction) { assert.Equal(t, "nodes", action.GetResource().Resource) assert.Equal(t, cordonPatch, string(action.GetPatch()))