-
Notifications
You must be signed in to change notification settings - Fork 455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added autoscaler options to worker groups #9245
Added autoscaler options to worker groups #9245
Conversation
/assign |
/retest |
37d1ff2
to
594bd8c
Compare
/retest |
/test pull-gardener-e2e-kind-ha-multi-zone-upgrade |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just a few more nits before we are ready :)
docs/extensions/worker.md
Outdated
@@ -115,6 +121,8 @@ The `spec.pools[].nodeTemplate.capacity` field contains the resource information | |||
|
|||
The `spec.pools[].machineControllerManager` field allows to configure the settings for machine-controller-manager component. Providers must populate these settings on worker-pool to the related [fields](https://github.com/gardener/machine-controller-manager/blob/master/kubernetes/machine_objects/machine-deployment.yaml#L30-L34) in MachineDeployment. | |||
|
|||
The `spec.pools[].clusterAutoscaler` field contains `cluster-autoscaler` settings that are to be applied only to specific worker group. `Cluster-autoscaler` expects to find these settings as annotations on the `MachineDeployment`, and so providers must pass these values to the corresponding `MachineDeployment` via annotations. The keys for these annotations can be found [here](https://github.com/gardener/gardener/blob/master/pkg/apis/extensions/v1alpha1/types_worker.go) and the values for the corresponding annotations should be the same as what is passed into the field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `spec.pools[].clusterAutoscaler` field contains `cluster-autoscaler` settings that are to be applied only to specific worker group. `Cluster-autoscaler` expects to find these settings as annotations on the `MachineDeployment`, and so providers must pass these values to the corresponding `MachineDeployment` via annotations. The keys for these annotations can be found [here](https://github.com/gardener/gardener/blob/master/pkg/apis/extensions/v1alpha1/types_worker.go) and the values for the corresponding annotations should be the same as what is passed into the field. | |
The `spec.pools[].clusterAutoscaler` field contains `cluster-autoscaler` settings that are to be applied only to specific worker group. `cluster-autoscaler` expects to find these settings as annotations on the `MachineDeployment`, and so providers must pass these values to the corresponding `MachineDeployment` via annotations. The keys for these annotations can be found [here](https://github.com/gardener/gardener/blob/master/pkg/apis/extensions/v1alpha1/types_worker.go) and the values for the corresponding annotations should be the same as what is passed into the field. |
Do you want to refer to your helper function (extensionsv1alpha1helper.GetMachineDeploymentClusterAutoscalerAnnotations
)?
@@ -1388,6 +1388,64 @@ var _ = Describe("Shoot Validation Tests", func() { | |||
Expect(errorList).To(BeEmpty()) | |||
}) | |||
}) | |||
|
|||
Describe("clusterAutoscaler options validation", func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Describe("clusterAutoscaler options validation", func() { | |
Describe("ClusterAutoscaler options validation", func() { |
annotations := map[string]string{} | ||
if caOptions != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
annotations := map[string]string{} | |
if caOptions != nil { | |
var annotations map[string]string | |
if caOptions != nil { | |
annotations := map[string]string{} |
@@ -68,3 +68,27 @@ func FilePathsFrom(files []extensionsv1alpha1.File) []string { | |||
|
|||
return out | |||
} | |||
|
|||
// GetMachineDeploymentClusterAutoscalerAnnotations returns a map of annotations with values intended to be used as cluster autoscaler options for the worker group |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// GetMachineDeploymentClusterAutoscalerAnnotations returns a map of annotations with values intended to be used as cluster autoscaler options for the worker group | |
// GetMachineDeploymentClusterAutoscalerAnnotations returns a map of annotations with values intended to be used as cluster-autoscaler options for the worker group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
LGTM label has been added. Git tree hash: e768e1a87c8bb033a7a9e8eadaa37fafa3049e33
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rfranzke The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
How to categorize this PR?
/area auto-scaling
/kind api-change
What this PR does / why we need it:
This PR adds some
autoscaler
options to workers.With this change, users can configure certain worker specific autoscaler options instead of one single value for all worker groups
Which issue(s) this PR fixes:
Fixes partially gardener/autoscaler#240
Special notes for your reviewer:
Autoscaler
will read these values from annotations in themachineDeployment
(ref: gardener/autoscaler#257), so the values fromspec.provider.worker.autoscaler
are added as annotations to themachineDeployment
Release note: