Support smaller ScaledownStepSize (less than 1000) (#92)

* Automatic linting Signed-off-by: rustyclock <rustyclock@protonmail.com> * Support smaller `ScaledownStepSize` Currently `ScaledownStepSize` can only be in multiple of 1000 PUs. This can lead to rapid downscaling for smaller instances (<= 2000 PUs). Therefore, allow for setting a smaller `ScaledownStepSize` so that smaller instances can also be downscaled smoothly. Signed-off-by: rustyclock <rustyclock@protonmail.com> * Add validation checks for `ScaledownStepSize` Signed-off-by: rustyclock <rustyclock@protonmail.com> Signed-off-by: rustyclock <rustyclock@protonmail.com>
mercari · Dec 2, 2022 · 91d26f3 · 91d26f3
1 parent 6385820
commit 91d26f3
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 20 deletions.
diff --git a/Makefile b/Makefile
@@ -131,7 +131,7 @@ envtest: ## Download envtest-setup locally if necessary.
 
 KIND = $(shell pwd)/bin/kind
 kind: ## Downlaod 'kind' locally if necessary
-	$(call go-install-tool,$(KIND),sigs.k8s.io/kind@v0.11.1)
+	$(call go-install-tool,$(KIND),sigs.k8s.io/kind@v0.17.0)
 
 KPT = $(shell pwd)/bin/kpt
 kpt: ## Downlaod 'kpt' locally if necessary

diff --git a/api/v1beta1/groupversion_info.go b/api/v1beta1/groupversion_info.go
@@ -15,8 +15,8 @@ limitations under the License.
 */
 
 // Package v1beta1 contains API Schema definitions for the spanner v1beta1 API group
-//+kubebuilder:object:generate=true
-//+groupName=spanner.mercari.com
+// +kubebuilder:object:generate=true
+// +groupName=spanner.mercari.com
 package v1beta1
 
 import (

diff --git a/api/v1beta1/spannerautoscaler_types.go b/api/v1beta1/spannerautoscaler_types.go
@@ -100,9 +100,8 @@ type ScaleConfig struct {
 	// ProcessingUnits for scaling of the Spanner instance. Ref: [Spanner Compute Capacity](https://cloud.google.com/spanner/docs/compute-capacity#compute_capacity)
 	ProcessingUnits ScaleConfigPUs `json:"processingUnits,omitempty"`
 
-	// The maximum number of processing units which can be deleted in one scale-down operation. Should be a multiple of 1000.
+	// The maximum number of processing units which can be deleted in one scale-down operation. It can be a multiple of 100 for values < 1000, or a multiple of 1000 otherwise.
 	// +kubebuilder:default=2000
-	// +kubebuilder:validation:MultipleOf=1000
 	ScaledownStepSize int `json:"scaledownStepSize,omitempty"`
 
 	// How often autoscaler is reevaluated for scale down.

diff --git a/api/v1beta1/spannerautoscaler_webhook.go b/api/v1beta1/spannerautoscaler_webhook.go
@@ -188,6 +188,18 @@ func (r *SpannerAutoscaler) validateScaleConfig() *field.Error {
 		}
 	}
 
+	if sc.ScaledownStepSize > 1000 && sc.ScaledownStepSize%1000 != 0 {
+		return field.Invalid(
+			field.NewPath("spec").Child("scaleConfig").Child("scaledownStepSize"),
+			sc.ScaledownStepSize,
+			"must be a multiple of 1000 for values which are greater than 1000")
+	} else if sc.ScaledownStepSize < 1000 && sc.ScaledownStepSize%100 != 0 {
+		return field.Invalid(
+			field.NewPath("spec").Child("scaleConfig").Child("scaledownStepSize"),
+			sc.ScaledownStepSize,
+			"must be a multiple of 100 for values which are less than 1000")
+	}
+
 	return nil
 }
 

diff --git a/api/v1beta1/spannerautoscaleschedule_types.go b/api/v1beta1/spannerautoscaleschedule_types.go
@@ -44,11 +44,11 @@ type SpannerAutoscaleScheduleSpec struct {
 // SpannerAutoscaleScheduleStatus defines the observed state of SpannerAutoscaleSchedule
 type SpannerAutoscaleScheduleStatus struct{}
 
-//+kubebuilder:object:root=true
-//+kubebuilder:subresource:status
-//+kubebuilder:printcolumn:name="Cron",type="string",JSONPath=".spec.schedule.cron"
-//+kubebuilder:printcolumn:name="Duration",type="string",JSONPath=".spec.schedule.duration"
-//+kubebuilder:printcolumn:name="Additional PU",type="integer",JSONPath=".spec.additionalProcessingUnits"
+// +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
+// +kubebuilder:printcolumn:name="Cron",type="string",JSONPath=".spec.schedule.cron"
+// +kubebuilder:printcolumn:name="Duration",type="string",JSONPath=".spec.schedule.duration"
+// +kubebuilder:printcolumn:name="Additional PU",type="integer",JSONPath=".spec.additionalProcessingUnits"
 // SpannerAutoscaleSchedule is the Schema for the spannerautoscaleschedules API
 type SpannerAutoscaleSchedule struct {
 	metav1.TypeMeta   `json:",inline"`

diff --git a/config/crd/bases/spanner.mercari.com_spannerautoscalers.yaml b/config/crd/bases/spanner.mercari.com_spannerautoscalers.yaml
@@ -360,9 +360,8 @@ spec:
                   scaledownStepSize:
                     default: 2000
                     description: The maximum number of processing units which can
-                      be deleted in one scale-down operation. Should be a multiple
-                      of 1000.
-                    multipleOf: 1000
+                      be deleted in one scale-down operation. It can be a multiple
+                      of 100 for values < 1000, or a multiple of 1000 otherwise.
                     type: integer
                   targetCPUUtilization:
                     description: 'The CPU utilization which the autoscaling will try

diff --git a/controllers/spannerautoscaler_controller.go b/controllers/spannerautoscaler_controller.go
@@ -531,15 +531,24 @@ func calcDesiredProcessingUnits(sa spannerv1beta1.SpannerAutoscaler) int {
 		desiredPU = ((requiredPU / 1000) + 1) * 1000
 	}
 
+	sdStepSize := sa.Spec.ScaleConfig.ScaledownStepSize
+
+	// round up the scaledownStepSize to avoid intermediate values
+	// for example: 8000 -> 7000 instead of 8000 -> 7400
+	if sdStepSize < 1000 && sa.Status.CurrentProcessingUnits > 1000 {
+		sdStepSize = 1000
+	}
+
 	// in case of scaling down, check that we don't scale down beyond the ScaledownStepSize
-	if scaledDownPU := (sa.Status.CurrentProcessingUnits - sa.Spec.ScaleConfig.ScaledownStepSize); desiredPU < scaledDownPU {
+	if scaledDownPU := (sa.Status.CurrentProcessingUnits - sdStepSize); desiredPU < scaledDownPU {
 		desiredPU = scaledDownPU
 	}
 
 	// keep the scaling between the specified min/max range
 	minPU := sa.Spec.ScaleConfig.ProcessingUnits.Min
 	maxPU := sa.Spec.ScaleConfig.ProcessingUnits.Max
 
+	// fetch min/max range from status, in case any schedules have updated the range
 	if sa.Status.DesiredMinPUs > 0 {
 		minPU = sa.Status.DesiredMinPUs
 	}

diff --git a/controllers/spannerautoscaler_controller_test.go b/controllers/spannerautoscaler_controller_test.go
@@ -185,17 +185,33 @@ var _ = DescribeTable("Calculate Desired Processing Units",
 		Expect(got).To(Equal(want))
 	},
 	Entry("should not scale", 25, 200, 30, 100, 1000, 2000, 200),
-	Entry("should scale up", 50, 300, 30, 100, 1000, 2000, 600),
+	Entry("should scale up 1", 50, 300, 30, 100, 1000, 2000, 600),
 	Entry("should scale up 2", 50, 3000, 30, 1000, 10000, 2000, 6000),
 	Entry("should scale up 3", 50, 900, 40, 100, 5000, 2000, 2000),
-	Entry("should scale down", 30, 500, 50, 100, 10000, 2000, 400),
+	Entry("should scale down 1", 30, 500, 50, 100, 10000, 2000, 400),
 	Entry("should scale down 2", 30, 5000, 50, 1000, 10000, 2000, 4000),
-	Entry("should scale up to max PUs", 50, 300, 30, 100, 400, 2000, 400),
+	Entry("should scale down 3", 25, 1000, 65, 300, 10000, 800, 400),
+	Entry("should scale down 4", 25, 800, 65, 300, 10000, 800, 400),
+	Entry("should scale down 5", 25, 700, 65, 300, 10000, 800, 300),
+	Entry("should scale up to max PUs 1", 50, 300, 30, 100, 400, 2000, 400),
 	Entry("should scale up to max PUs 2", 50, 3000, 30, 1000, 4000, 2000, 4000),
-	Entry("should scale down to min PUs", 0, 500, 50, 100, 1000, 2000, 100),
+	Entry("should scale down to min PUs 1", 0, 500, 50, 100, 1000, 2000, 100),
 	Entry("should scale down to min PUs 2", 0, 5000, 50, 1000, 10000, 5000, 1000),
 	Entry("should scale down to min PUs 3", 0, 5000, 50, 100, 10000, 5000, 100),
-	Entry("should scale down with MaxScaleDownNodes", 30, 10000, 50, 5000, 10000, 2000, 8000),
+	Entry("should scale down with ScaledownStepSize 1", 30, 10000, 50, 5000, 10000, 2000, 8000),
+	Entry("should scale down with ScaledownStepSize 2", 30, 10000, 50, 5000, 12000, 200, 9000),
+	Entry("should scale down with ScaledownStepSize 3", 30, 10000, 50, 5000, 12000, 100, 9000),
+	Entry("should scale down with ScaledownStepSize 4", 30, 10000, 50, 5000, 12000, 900, 9000),
+	Entry("should scale down with ScaledownStepSize 5", 25, 2000, 65, 300, 10000, 500, 1000),
+	Entry("should scale down with ScaledownStepSize 6", 25, 2000, 65, 300, 10000, 800, 1000),
+	Entry("should scale down with ScaledownStepSize 7", 25, 1000, 65, 300, 10000, 500, 500),
+	Entry("should scale down with ScaledownStepSize 8", 20, 800, 75, 300, 10000, 200, 600),
+	Entry("should scale down with ScaledownStepSize 9", 25, 2000, 50, 300, 10000, 500, 2000),
+	Entry("should scale down with ScaledownStepSize 10", 25, 2000, 50, 300, 10000, 1000, 2000),
+	Entry("should scale down with ScaledownStepSize 11", 25, 2000, 70, 300, 10000, 400, 1000),
+	Entry("should scale down with ScaledownStepSize 12", 25, 1000, 70, 300, 10000, 400, 600),
+	Entry("should scale down with ScaledownStepSize 13", 20, 2000, 50, 300, 10000, 200, 1000),
+	Entry("should scale down with ScaledownStepSize 14", 20, 2000, 75, 300, 10000, 200, 1000),
 )
 
 var _ = Describe("Fetch Credentials", func() {

diff --git a/docs/crd-reference.md b/docs/crd-reference.md
@@ -91,7 +91,7 @@ _Appears in:_
 | `computeType` _ComputeType_ | Whether to use `nodes` or `processing-units` for scaling. This is only used at the time of CustomResource creation. If compute capacity is provided in `nodes`, then it is automatically converted to `processing-units` at the time of resource creation, and internally, only `ProcessingUnits` are used for computations and scaling. |
 | `nodes` _[ScaleConfigNodes](#scaleconfignodes)_ | If `nodes` are provided at the time of resource creation, then they are automatically converted to `processing-units`. So it is recommended to use only the processing units. Ref: [Spanner Compute Capacity](https://cloud.google.com/spanner/docs/compute-capacity#compute_capacity) |
 | `processingUnits` _[ScaleConfigPUs](#scaleconfigpus)_ | ProcessingUnits for scaling of the Spanner instance. Ref: [Spanner Compute Capacity](https://cloud.google.com/spanner/docs/compute-capacity#compute_capacity) |
-| `scaledownStepSize` _integer_ | The maximum number of processing units which can be deleted in one scale-down operation. Should be a multiple of 1000. |
+| `scaledownStepSize` _integer_ | The maximum number of processing units which can be deleted in one scale-down operation. It can be a multiple of 100 for values < 1000, or a multiple of 1000 otherwise. |
 | `scaledownInterval` _[Duration](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.22/#duration-v1-meta)_ | How often autoscaler is reevaluated for scale down. The cool down period between two consecutive scaledown operations. If this option is omitted, the value of the `--scale-down-interval` command line option is taken as the default value. |
 | `targetCPUUtilization` _[TargetCPUUtilization](#targetcpuutilization)_ | The CPU utilization which the autoscaling will try to achieve. Ref: [Spanner CPU utilization](https://cloud.google.com/spanner/docs/cpu-utilization#task-priority) |