Expose Horizontal Pod Autoscaler Behavior and add hpa scaledown test #1077
Conversation
Signed-off-by: Kevin Earls <kearls@redhat.com>
Signed-off-by: Kevin Earls <kearls@redhat.com>
Signed-off-by: Kevin Earls <kearls@redhat.com>
Signed-off-by: Kevin Earls <kearls@redhat.com>
Signed-off-by: Kevin Earls <kearls@redhat.com>
| // 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 <kearls@redhat.com>
| @@ -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 <kearls@redhat.com>
Signed-off-by: Kevin Earls <kearls@redhat.com>
|
Does this resolve #942 ? |
|
@pavolloffay Yes |
Signed-off-by: Kevin Earls <kearls@redhat.com>
Signed-off-by: Kevin Earls <kearls@redhat.com>
Signed-off-by: Kevin Earls <kearls@redhat.com>
kevinearls
changed the title
Signed-off-by: Kevin Earls <kearls@redhat.com>
Signed-off-by: Kevin Earls <kearls@redhat.com>
…pen-telemetry#1077) * Add scaledown test for autoscaling Signed-off-by: Kevin Earls <kearls@redhat.com> * Fix nits Signed-off-by: Kevin Earls <kearls@redhat.com> * Appease the linter Signed-off-by: Kevin Earls <kearls@redhat.com> * Don't use a default, only set scaleUp/scaleDown if they are in the CR Signed-off-by: Kevin Earls <kearls@redhat.com> * Removed commented out code Signed-off-by: Kevin Earls <kearls@redhat.com> * Change defaults for scaleUp/scaleDown Signed-off-by: Kevin Earls <kearls@redhat.com> * Update autoscaling scaleup/scaledown Signed-off-by: Kevin Earls <kearls@redhat.com> * Run generate Signed-off-by: Kevin Earls <kearls@redhat.com> * Ran make api-docs Signed-off-by: Kevin Earls <kearls@redhat.com> * Update HPA implementation to embed HorizontalPodAutoscalerBehavior Signed-off-by: Kevin Earls <kearls@redhat.com> * Only set behavior if it exists in the collector CR Signed-off-by: Kevin Earls <kearls@redhat.com> * Aded some unit tests Signed-off-by: Kevin Earls <kearls@redhat.com> * Add kuttl assertion that hpa scaled down Signed-off-by: Kevin Earls <kearls@redhat.com> * added whitespace to rerun tests Signed-off-by: Kevin Earls <kearls@redhat.com> Signed-off-by: Kevin Earls <kearls@redhat.com>
No description provided.