diff --git a/.chloggen/fix_validation-stabilizationWindowSeconds.yaml b/.chloggen/fix_validation-stabilizationWindowSeconds.yaml new file mode 100755 index 0000000000..b90f0ecbd9 --- /dev/null +++ b/.chloggen/fix_validation-stabilizationWindowSeconds.yaml @@ -0,0 +1,18 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action) +component: collector-webhook + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Fixed validation of `stabilizationWindowSeconds` in autoscaler behaviour" + +# One or more tracking issues related to the change +issues: [3345] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + The validation of `stabilizationWindowSeconds` in the `autoscaler.behaviour.scale[Up|Down]` incorrectly rejected 0 as an invalid value. + This has been fixed to ensure that the value is validated correctly (should be >=0 and <=3600) and the error messsage has been updated to reflect this. diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go index e79754b4bd..5a6b80a3b9 100644 --- a/apis/v1beta1/collector_webhook.go +++ b/apis/v1beta1/collector_webhook.go @@ -385,13 +385,13 @@ func ValidatePorts(ports []PortsSpec) error { func checkAutoscalerSpec(autoscaler *AutoscalerSpec) error { if autoscaler.Behavior != nil { if autoscaler.Behavior.ScaleDown != nil && autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds != nil && - *autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds < int32(1) { - return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleDown should be one or more") + (*autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds < int32(0) || *autoscaler.Behavior.ScaleDown.StabilizationWindowSeconds > 3600) { + return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleDown.stabilizationWindowSeconds should be >=0 and <=3600") } if autoscaler.Behavior.ScaleUp != nil && autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds != nil && - *autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds < int32(1) { - return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleUp should be one or more") + (*autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds < int32(0) || *autoscaler.Behavior.ScaleUp.StabilizationWindowSeconds > 3600) { + return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, scaleUp.stabilizationWindowSeconds should be >=0 and <=3600") } } if autoscaler.TargetCPUUtilization != nil && *autoscaler.TargetCPUUtilization < int32(1) { diff --git a/apis/v1beta1/collector_webhook_test.go b/apis/v1beta1/collector_webhook_test.go index 0b6b915486..d6145543f3 100644 --- a/apis/v1beta1/collector_webhook_test.go +++ b/apis/v1beta1/collector_webhook_test.go @@ -17,6 +17,7 @@ package v1beta1_test import ( "context" "fmt" + "math" "os" "testing" @@ -582,6 +583,7 @@ func TestOTELColValidatingWebhook(t *testing.T) { one := int32(1) three := int32(3) five := int32(5) + maxInt := int32(math.MaxInt32) cfg := v1beta1.Config{} err := yaml.Unmarshal([]byte(cfgYaml), &cfg) @@ -913,36 +915,68 @@ func TestOTELColValidatingWebhook(t *testing.T) { expectedErr: "minReplicas should be one or more", }, { - name: "invalid autoscaler scale down", + name: "invalid autoscaler scale down stablization window - <0", otelcol: v1beta1.OpenTelemetryCollector{ Spec: v1beta1.OpenTelemetryCollectorSpec{ Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{ ScaleDown: &autoscalingv2.HPAScalingRules{ - StabilizationWindowSeconds: &zero, + StabilizationWindowSeconds: &minusOne, }, }, }, }, }, - expectedErr: "scaleDown should be one or more", + expectedErr: "scaleDown.stabilizationWindowSeconds should be >=0 and <=3600", }, { - name: "invalid autoscaler scale up", + name: "invalid autoscaler scale down stablization window - >3600", + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ + MaxReplicas: &three, + Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{ + ScaleDown: &autoscalingv2.HPAScalingRules{ + StabilizationWindowSeconds: &maxInt, + }, + }, + }, + }, + }, + expectedErr: "scaleDown.stabilizationWindowSeconds should be >=0 and <=3600", + }, + { + name: "invalid autoscaler scale up stablization window - <0", + otelcol: v1beta1.OpenTelemetryCollector{ + Spec: v1beta1.OpenTelemetryCollectorSpec{ + Autoscaler: &v1beta1.AutoscalerSpec{ + MaxReplicas: &three, + Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{ + ScaleUp: &autoscalingv2.HPAScalingRules{ + StabilizationWindowSeconds: &minusOne, + }, + }, + }, + }, + }, + expectedErr: "scaleUp.stabilizationWindowSeconds should be >=0 and <=3600", + }, + { + name: "invalid autoscaler scale up stablization window - >3600", otelcol: v1beta1.OpenTelemetryCollector{ Spec: v1beta1.OpenTelemetryCollectorSpec{ Autoscaler: &v1beta1.AutoscalerSpec{ MaxReplicas: &three, Behavior: &autoscalingv2.HorizontalPodAutoscalerBehavior{ ScaleUp: &autoscalingv2.HPAScalingRules{ - StabilizationWindowSeconds: &zero, + StabilizationWindowSeconds: &maxInt, }, }, }, }, }, - expectedErr: "scaleUp should be one or more", + expectedErr: "scaleUp.stabilizationWindowSeconds should be >=0 and <=3600", }, { name: "invalid autoscaler target cpu utilization",