From 9cc19102f3d1591631bf4c4c474ff3c363f9f29d Mon Sep 17 00:00:00 2001 From: kargakis Date: Wed, 24 Feb 2016 13:21:13 +0100 Subject: [PATCH] kubectl: preserve availability when maxUnavailability is not 100% --- pkg/kubectl/rolling_updater.go | 28 +------- pkg/kubectl/rolling_updater_test.go | 102 +++++++--------------------- 2 files changed, 27 insertions(+), 103 deletions(-) diff --git a/pkg/kubectl/rolling_updater.go b/pkg/kubectl/rolling_updater.go index 6b332d0685a6c..625ea65d7b006 100644 --- a/pkg/kubectl/rolling_updater.go +++ b/pkg/kubectl/rolling_updater.go @@ -20,7 +20,6 @@ import ( goerrors "errors" "fmt" "io" - "math" "strconv" "strings" "time" @@ -198,12 +197,12 @@ func (r *RollingUpdater) Update(config *RollingUpdaterConfig) error { oldRc.Name, originalReplicasAnnotation, oldRc.Annotations[originalReplicasAnnotation]) } // The maximum pods which can go unavailable during the update. - maxUnavailable, err := extractMaxValue(config.MaxUnavailable, "maxUnavailable", desired) + maxUnavailable, err := intstr.GetValueFromIntOrPercent(&config.MaxUnavailable, desired, false) if err != nil { return err } // The maximum scaling increment. - maxSurge, err := extractMaxValue(config.MaxSurge, "maxSurge", desired) + maxSurge, err := intstr.GetValueFromIntOrPercent(&config.MaxSurge, desired, true) if err != nil { return err } @@ -486,29 +485,6 @@ func (r *RollingUpdater) cleanupWithClients(oldRc, newRc *api.ReplicationControl } } -// func extractMaxValue is a helper to extract config max values as either -// absolute numbers or based on percentages of the given value. -func extractMaxValue(field intstr.IntOrString, name string, value int) (int, error) { - switch field.Type { - case intstr.Int: - if field.IntVal < 0 { - return 0, fmt.Errorf("%s must be >= 0", name) - } - return field.IntValue(), nil - case intstr.String: - s := strings.Replace(field.StrVal, "%", "", -1) - v, err := strconv.Atoi(s) - if err != nil { - return 0, fmt.Errorf("invalid %s value %q: %v", name, field.StrVal, err) - } - if v < 0 { - return 0, fmt.Errorf("%s must be >= 0", name) - } - return int(math.Ceil(float64(value) * (float64(v)) / 100)), nil - } - return 0, fmt.Errorf("invalid kind %q for %s", field.Type, name) -} - func Rename(c client.ReplicationControllersNamespacer, rc *api.ReplicationController, newName string) error { oldName := rc.Name rc.Name = newName diff --git a/pkg/kubectl/rolling_updater_test.go b/pkg/kubectl/rolling_updater_test.go index 350a524ba3d92..882267accb4a4 100644 --- a/pkg/kubectl/rolling_updater_test.go +++ b/pkg/kubectl/rolling_updater_test.go @@ -430,20 +430,21 @@ Scaling foo-v1 down to 0 Scaling foo-v2 up to 10 `, }, { - name: "1->1 10/0 fast readiness", + name: "1->1 25/25 maintain minimum availability", oldRc: oldRc(1, 1), newRc: newRc(0, 1), newRcExists: false, - maxUnavail: intstr.FromString("10%"), - maxSurge: intstr.FromString("0%"), + maxUnavail: intstr.FromString("25%"), + maxSurge: intstr.FromString("25%"), expected: []interface{}{ - down{oldReady: 1, newReady: 0, to: 0}, up{1}, + down{oldReady: 1, newReady: 0, noop: true}, + down{oldReady: 1, newReady: 1, to: 0}, }, output: `Created foo-v2 -Scaling up foo-v2 from 0 to 1, scaling down foo-v1 from 1 to 0 (keep 0 pods available, don't exceed 1 pods) -Scaling foo-v1 down to 0 +Scaling up foo-v2 from 0 to 1, scaling down foo-v1 from 1 to 0 (keep 1 pods available, don't exceed 2 pods) Scaling foo-v2 up to 1 +Scaling foo-v1 down to 0 `, }, { name: "1->1 0/10 delayed readiness", @@ -475,7 +476,7 @@ Scaling foo-v1 down to 0 down{oldReady: 1, newReady: 1, to: 0}, }, output: `Created foo-v2 -Scaling up foo-v2 from 0 to 1, scaling down foo-v1 from 1 to 0 (keep 0 pods available, don't exceed 2 pods) +Scaling up foo-v2 from 0 to 1, scaling down foo-v1 from 1 to 0 (keep 1 pods available, don't exceed 2 pods) Scaling foo-v2 up to 1 Scaling foo-v1 down to 0 `, @@ -655,6 +656,23 @@ Scaling foo-v1 down to 1 Scaling foo-v2 up to 1 Scaling foo-v1 down to 0 Scaling foo-v2 up to 2 +`, + }, + { + name: "1->1 100%/0 allow maxUnavailability", + oldRc: oldRc(1, 1), + newRc: newRc(0, 1), + newRcExists: false, + maxUnavail: intstr.FromString("100%"), + maxSurge: intstr.FromInt(0), + expected: []interface{}{ + down{oldReady: 1, newReady: 0, to: 0}, + up{1}, + }, + output: `Created foo-v2 +Scaling up foo-v2 from 0 to 1, scaling down foo-v1 from 1 to 0 (keep 0 pods available, don't exceed 1 pods) +Scaling foo-v1 down to 0 +Scaling foo-v2 up to 1 `, }, } @@ -1571,73 +1589,3 @@ func TestRollingUpdater_readyPods(t *testing.T) { } } } - -func TestRollingUpdater_extractMaxValue(t *testing.T) { - tests := []struct { - field intstr.IntOrString - original int - expected int - valid bool - }{ - { - field: intstr.FromInt(1), - original: 100, - expected: 1, - valid: true, - }, - { - field: intstr.FromInt(0), - original: 100, - expected: 0, - valid: true, - }, - { - field: intstr.FromInt(-1), - original: 100, - valid: false, - }, - { - field: intstr.FromString("10%"), - original: 100, - expected: 10, - valid: true, - }, - { - field: intstr.FromString("100%"), - original: 100, - expected: 100, - valid: true, - }, - { - field: intstr.FromString("200%"), - original: 100, - expected: 200, - valid: true, - }, - { - field: intstr.FromString("0%"), - original: 100, - expected: 0, - valid: true, - }, - { - field: intstr.FromString("-1%"), - original: 100, - valid: false, - }, - } - - for i, test := range tests { - t.Logf("evaluating test %d", i) - max, err := extractMaxValue(test.field, "field", test.original) - if test.valid && err != nil { - t.Fatalf("unexpected error: %v", err) - } - if !test.valid && err == nil { - t.Fatalf("expected an error") - } - if e, a := test.expected, max; e != a { - t.Fatalf("expected max %d, got %d", e, a) - } - } -}