Expose Horizontal Pod Autoscaler Behavior and add hpa scaledown test #1077
Conversation
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
| // for the OpenTelemetryCollector workload. | ||
| // | ||
| // +optional | ||
| Autoscaler *AutoscalerSpec `json:"autoscaler,omitempty"` |
There was a problem hiding this comment.
question: did we consider embedding the HPA spec in here? Or at least embed the autoscaling behavior spec here?
There was a problem hiding this comment.
I was just adding enough code to be able to get an e2e test to work within the time allocated, which means we need to scale down much quicker that the default 300 seconds.
@pavolloffay what do you think? Do I need to add the other values of PA scaling rules here?
There was a problem hiding this comment.
For my use case, I know that I would like to be able to specify policies and not just StabilizationWindowSeconds. Embedding only StabilizationWindowSeconds is going to make it so we need to add in each feature on request, making a code change for each one.
| @@ -129,6 +129,15 @@ func (r *OpenTelemetryCollector) validateCRDSpec() error { | |||
| return fmt.Errorf("the OpenTelemetry Spec autoscale configuration is incorrect, minReplicas should be one or more") | |||
| } | |||
|
|
|||
| if r.Spec.Autoscaler != nil { | |||
There was a problem hiding this comment.
We should also check that min or max replicas is set in order to use this feature
There was a problem hiding this comment.
That's inside an if statement that's checking whether maxReplicas is set.
Signed-off-by: Kevin Earls <[email protected]>
| @@ -13,4 +13,7 @@ metadata: | |||
| spec: | |||
| minReplicas: 1 | |||
| maxReplicas: 2 | |||
| # This is not neccesarily exact. We really just want to wait until this is no longer <unknown> | |||
There was a problem hiding this comment.
This is a really good "hack"
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
|
Does this resolve #942 ? |
|
@pavolloffay Yes |
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
kevinearls
changed the title
Signed-off-by: Kevin Earls <[email protected]>
Signed-off-by: Kevin Earls <[email protected]>
…pen-telemetry#1077) * Add scaledown test for autoscaling Signed-off-by: Kevin Earls <[email protected]> * Fix nits Signed-off-by: Kevin Earls <[email protected]> * Appease the linter Signed-off-by: Kevin Earls <[email protected]> * Don't use a default, only set scaleUp/scaleDown if they are in the CR Signed-off-by: Kevin Earls <[email protected]> * Removed commented out code Signed-off-by: Kevin Earls <[email protected]> * Change defaults for scaleUp/scaleDown Signed-off-by: Kevin Earls <[email protected]> * Update autoscaling scaleup/scaledown Signed-off-by: Kevin Earls <[email protected]> * Run generate Signed-off-by: Kevin Earls <[email protected]> * Ran make api-docs Signed-off-by: Kevin Earls <[email protected]> * Update HPA implementation to embed HorizontalPodAutoscalerBehavior Signed-off-by: Kevin Earls <[email protected]> * Only set behavior if it exists in the collector CR Signed-off-by: Kevin Earls <[email protected]> * Aded some unit tests Signed-off-by: Kevin Earls <[email protected]> * Add kuttl assertion that hpa scaled down Signed-off-by: Kevin Earls <[email protected]> * added whitespace to rerun tests Signed-off-by: Kevin Earls <[email protected]> Signed-off-by: Kevin Earls <[email protected]>
No description provided.