From 8e1c34e222d6c194302c649270737c516fe33edf Mon Sep 17 00:00:00 2001 From: "Alex Ellis (OpenFaaS Ltd)" Date: Tue, 17 Jan 2023 21:01:36 +0000 Subject: [PATCH] Set default max scale to 5 replicas and a 10% increment Sets a new default maximum scale limit of 5 replicas out of the box for CE users, CE meaning "Community" rather than "Commercial". The increment factor of 10 vs 25 should not make a difference to genuine community and hobbyist users. Tested and verified with unit tests and hey with a CE cluster where the maximum limit was reached over several minutes, finally going back to 1 replica. Signed-off-by: Alex Ellis (OpenFaaS Ltd) --- gateway/handlers/alerthandler.go | 1 + gateway/handlers/alerthandler_test.go | 107 ++++++++++++++------------ gateway/plugin/external.go | 12 +-- gateway/plugin/external_test.go | 1 - gateway/scaling/ranges.go | 13 +--- gateway/scaling/service_query.go | 1 - 6 files changed, 64 insertions(+), 71 deletions(-) diff --git a/gateway/handlers/alerthandler.go b/gateway/handlers/alerthandler.go index 0cabdec5..442e878d 100644 --- a/gateway/handlers/alerthandler.go +++ b/gateway/handlers/alerthandler.go @@ -102,6 +102,7 @@ func scaleService(alert requests.PrometheusInnerAlert, service scaling.ServiceQu func CalculateReplicas(status string, currentReplicas uint64, maxReplicas uint64, minReplicas uint64, scalingFactor uint64) uint64 { var newReplicas uint64 + maxReplicas = uint64(math.Min(float64(maxReplicas), float64(scaling.DefaultMaxReplicas))) step := uint64(math.Ceil(float64(maxReplicas) / 100 * float64(scalingFactor))) if status == "firing" && step > 0 { diff --git a/gateway/handlers/alerthandler_test.go b/gateway/handlers/alerthandler_test.go index 0d89e3a9..c9250d27 100644 --- a/gateway/handlers/alerthandler_test.go +++ b/gateway/handlers/alerthandler_test.go @@ -1,4 +1,4 @@ -// Copyright (c) Alex Ellis 2017. All rights reserved. +// Copyright (c) Alex Ellis 1017. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. package handlers @@ -12,9 +12,9 @@ import ( func TestDisabledScale(t *testing.T) { minReplicas := uint64(1) scalingFactor := uint64(0) - newReplicas := CalculateReplicas("firing", scaling.DefaultMinReplicas, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) - if newReplicas != minReplicas { - t.Logf("Expected not to scale, but replicas were: %d", newReplicas) + got := CalculateReplicas("firing", scaling.DefaultMinReplicas, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) + if got != minReplicas { + t.Logf("Expected not to scale, but replicas were: %d", got) t.Fail() } } @@ -22,20 +22,23 @@ func TestDisabledScale(t *testing.T) { func TestParameterEdge(t *testing.T) { minReplicas := uint64(0) scalingFactor := uint64(0) - newReplicas := CalculateReplicas("firing", scaling.DefaultMinReplicas, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) - if newReplicas != 0 { + got := CalculateReplicas("firing", scaling.DefaultMinReplicas, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) + if got != 0 { t.Log("Expected not to scale") t.Fail() } } -func TestScalingWithSameUpperLowerLimit(t *testing.T) { - minReplicas := uint64(1) - scalingFactor := uint64(20) - // status string, currentReplicas uint64, maxReplicas uint64, minReplicas uint64, scalingFactor uint64) - newReplicas := CalculateReplicas("firing", minReplicas, minReplicas, minReplicas, scalingFactor) - if newReplicas != 1 { - t.Logf("Replicas - want: %d, got: %d", minReplicas, newReplicas) +func TestScaling_SameUpperLowerLimit(t *testing.T) { + minReplicas := uint64(5) + maxReplicas := uint64(5) + scalingFactor := uint64(10) + + got := CalculateReplicas("firing", minReplicas, minReplicas, maxReplicas, scalingFactor) + + want := minReplicas + if want != got { + t.Logf("Replicas - want: %d, got: %d", want, got) t.Fail() } } @@ -43,58 +46,62 @@ func TestScalingWithSameUpperLowerLimit(t *testing.T) { func TestMaxScale(t *testing.T) { minReplicas := uint64(1) scalingFactor := uint64(100) - newReplicas := CalculateReplicas("firing", scaling.DefaultMinReplicas, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) - if newReplicas != 20 { - t.Log("Expected ceiling of 20 replicas") - t.Fail() + got := CalculateReplicas("firing", scaling.DefaultMinReplicas, scaling.DefaultMaxReplicas*2, minReplicas, scalingFactor) + if got != scaling.DefaultMaxReplicas { + t.Fatalf("want ceiling: %d, but got: %d", scaling.DefaultMaxReplicas, got) } } -func TestInitialScale(t *testing.T) { +func TestInitialScale_From1_Factor10(t *testing.T) { minReplicas := uint64(1) - scalingFactor := uint64(20) - newReplicas := CalculateReplicas("firing", scaling.DefaultMinReplicas, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) - if newReplicas != 5 { - t.Log("Expected the increment to equal 5") - t.Fail() + scalingFactor := uint64(10) + got := CalculateReplicas("firing", scaling.DefaultMinReplicas, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) + want := uint64(2) + + if got != want { + t.Fatalf("want: %d, but got: %d", want, got) } } -func TestScale(t *testing.T) { +func TestScale_midrange_factor25(t *testing.T) { minReplicas := uint64(1) - scalingFactor := uint64(20) - newReplicas := CalculateReplicas("firing", 4, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) - if newReplicas != 8 { - t.Log("Expected newReplicas to equal 8") - t.Fail() + scalingFactor := uint64(25) + current := uint64(4) + maxReplicas := uint64(scaling.DefaultMaxReplicas) + + got := CalculateReplicas("firing", current, maxReplicas, minReplicas, scalingFactor) + want := uint64(5) + if want != got { + t.Fatalf("want: %d, but got: %d", want, got) } } -func TestScaleCeiling(t *testing.T) { +func TestScale_Ceiling_IsDefaultMaxReplicas(t *testing.T) { minReplicas := uint64(1) - scalingFactor := uint64(20) - newReplicas := CalculateReplicas("firing", 20, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) - if newReplicas != 20 { - t.Log("Expected ceiling of 20 replicas") - t.Fail() + scalingFactor := uint64(10) + current := uint64(scaling.DefaultMaxReplicas) + + got := CalculateReplicas("firing", current, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) + if got != scaling.DefaultMaxReplicas { + t.Fatalf("want: %d, but got: %d", scaling.DefaultMaxReplicas, got) } } -func TestScaleCeilingEdge(t *testing.T) { +func TestScaleCeilingReplicasOver(t *testing.T) { minReplicas := uint64(1) - scalingFactor := uint64(20) - newReplicas := CalculateReplicas("firing", 19, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) - if newReplicas != 20 { - t.Log("Expected ceiling of 20 replicas") - t.Fail() + scalingFactor := uint64(10) + got := CalculateReplicas("firing", 19, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) + + if got != scaling.DefaultMaxReplicas { + t.Fatalf("want: %d, but got: %d", scaling.DefaultMaxReplicas, got) } } func TestBackingOff(t *testing.T) { minReplicas := uint64(1) - scalingFactor := uint64(20) - newReplicas := CalculateReplicas("resolved", 8, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) - if newReplicas != 1 { + scalingFactor := uint64(10) + got := CalculateReplicas("resolved", 8, scaling.DefaultMaxReplicas, minReplicas, scalingFactor) + if got != 1 { t.Log("Expected backing off to 1 replica") t.Fail() } @@ -104,9 +111,9 @@ func TestScaledUpFrom1(t *testing.T) { currentReplicas := uint64(1) maxReplicas := uint64(5) scalingFactor := uint64(30) - newReplicas := CalculateReplicas("firing", currentReplicas, maxReplicas, scaling.DefaultMinReplicas, scalingFactor) - if newReplicas <= currentReplicas { - t.Log("Expected newReplicas > currentReplica") + got := CalculateReplicas("firing", currentReplicas, maxReplicas, scaling.DefaultMinReplicas, scalingFactor) + if got <= currentReplicas { + t.Log("Expected got > currentReplica") t.Fail() } } @@ -115,9 +122,9 @@ func TestScaledUpWithSmallParam(t *testing.T) { currentReplicas := uint64(1) maxReplicas := uint64(4) scalingFactor := uint64(1) - newReplicas := CalculateReplicas("firing", currentReplicas, maxReplicas, scaling.DefaultMinReplicas, scalingFactor) - if newReplicas <= currentReplicas { - t.Log("Expected newReplicas > currentReplica") + got := CalculateReplicas("firing", currentReplicas, maxReplicas, scaling.DefaultMinReplicas, scalingFactor) + if got <= currentReplicas { + t.Log("Expected got > currentReplica") t.Fail() } } diff --git a/gateway/plugin/external.go b/gateway/plugin/external.go index 571971d1..336f7c34 100644 --- a/gateway/plugin/external.go +++ b/gateway/plugin/external.go @@ -7,7 +7,7 @@ import ( "bytes" "encoding/json" "fmt" - "io/ioutil" + "io" "log" "net" "net/http" @@ -89,7 +89,7 @@ func (s ExternalServiceQuery) GetReplicas(serviceName, serviceNamespace string) var bytesOut []byte if res.Body != nil { - bytesOut, _ = ioutil.ReadAll(res.Body) + bytesOut, _ = io.ReadAll(res.Body) defer res.Body.Close() } @@ -111,20 +111,17 @@ func (s ExternalServiceQuery) GetReplicas(serviceName, serviceNamespace string) scalingFactor := uint64(scaling.DefaultScalingFactor) availableReplicas := function.AvailableReplicas - targetLoad := uint64(scaling.DefaultTargetLoad) - if function.Labels != nil { labels := *function.Labels minReplicas = extractLabelValue(labels[scaling.MinScaleLabel], minReplicas) maxReplicas = extractLabelValue(labels[scaling.MaxScaleLabel], maxReplicas) extractedScalingFactor := extractLabelValue(labels[scaling.ScalingFactorLabel], scalingFactor) - targetLoad = extractLabelValue(labels[scaling.TargetLoadLabel], targetLoad) - if extractedScalingFactor >= 0 && extractedScalingFactor <= 100 { + if extractedScalingFactor > 0 && extractedScalingFactor <= 100 { scalingFactor = extractedScalingFactor } else { - log.Printf("Bad Scaling Factor: %d, is not in range of [0 - 100]. Will fallback to %d", extractedScalingFactor, scalingFactor) + return scaling.ServiceQueryResponse{}, fmt.Errorf("bad scaling factor: %d, is not in range of [0 - 100]", extractedScalingFactor) } } @@ -135,7 +132,6 @@ func (s ExternalServiceQuery) GetReplicas(serviceName, serviceNamespace string) ScalingFactor: scalingFactor, AvailableReplicas: availableReplicas, Annotations: function.Annotations, - TargetLoad: targetLoad, }, err } diff --git a/gateway/plugin/external_test.go b/gateway/plugin/external_test.go index 157559de..dd53e014 100644 --- a/gateway/plugin/external_test.go +++ b/gateway/plugin/external_test.go @@ -75,7 +75,6 @@ func TestGetReplicasExistentFn(t *testing.T) { MinReplicas: uint64(scaling.DefaultMinReplicas), ScalingFactor: uint64(scaling.DefaultScalingFactor), AvailableReplicas: 0, - TargetLoad: 10, } var injector middleware.AuthInjector diff --git a/gateway/scaling/ranges.go b/gateway/scaling/ranges.go index 4d86d5bc..4e6cab2c 100644 --- a/gateway/scaling/ranges.go +++ b/gateway/scaling/ranges.go @@ -5,13 +5,10 @@ const ( DefaultMinReplicas = 1 // DefaultMaxReplicas is the amount of replicas a service will auto-scale up to. - DefaultMaxReplicas = 20 + DefaultMaxReplicas = 5 // DefaultScalingFactor is the defining proportion for the scaling increments. - DefaultScalingFactor = 20 - - // DefaultTargetLoad - DefaultTargetLoad = 10 + DefaultScalingFactor = 10 DefaultTypeScale = "rps" @@ -23,10 +20,4 @@ const ( // ScalingFactorLabel label indicates the scaling factor for a function ScalingFactorLabel = "com.openfaas.scale.factor" - - // TargetLoadLabel see also DefaultTargetScale - TargetLoadLabel = "com.openfaas.scale.target" - - // ScaleTypeLabel see also DefaultScaleType - ScaleTypeLabel = "com.openfaas.scale.type" ) diff --git a/gateway/scaling/service_query.go b/gateway/scaling/service_query.go index 0c9873c7..7d60e61b 100644 --- a/gateway/scaling/service_query.go +++ b/gateway/scaling/service_query.go @@ -17,5 +17,4 @@ type ServiceQueryResponse struct { ScalingFactor uint64 AvailableReplicas uint64 Annotations *map[string]string - TargetLoad uint64 }