From 1d9996b4addba3b7d896ce8301c4b9329c2a0ffb Mon Sep 17 00:00:00 2001 From: Dan Ports Date: Wed, 5 Jan 2022 14:10:06 -0500 Subject: [PATCH 1/6] Support price and priority cluster-autoscaler expanders. --- docs/addons.md | 2 +- k8s/crds/kops.k8s.io_clusters.yaml | 3 ++- pkg/apis/kops/componentconfig.go | 2 +- pkg/apis/kops/v1alpha2/componentconfig.go | 2 +- pkg/apis/kops/v1alpha3/componentconfig.go | 2 +- pkg/apis/kops/validation/validation.go | 2 +- 6 files changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/addons.md b/docs/addons.md index f64249d40e..f76ee08530 100644 --- a/docs/addons.md +++ b/docs/addons.md @@ -46,7 +46,7 @@ spec: memoryRequest: "300Mi" ``` -Read more about cluster autoscaler in the [official documentation](https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler). +Read more about cluster autoscaler in the [official documentation](https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler). The autoscaler supports five different expander strategies; the `priority` expander requires additional configuration in a ConfigMap as described in [its documentation](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/expander/priority/readme.md). ##### Disabling cluster autoscaler for a given instance group {{ kops_feature_table(kops_added_default='1.20') }} diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml index 4ea0f983b1..209ed02716 100644 --- a/k8s/crds/kops.k8s.io_clusters.yaml +++ b/k8s/crds/kops.k8s.io_clusters.yaml @@ -622,7 +622,8 @@ spec: expander: description: 'Expander determines the strategy for which instance group gets expanded. Supported values: least-waste, most-pods, - random. Default: least-waste' + random, price, priority. The priority expander requires additional + configuration via a ConfigMap. Default: least-waste' type: string image: description: 'Image is the docker container used. Default: the diff --git a/pkg/apis/kops/componentconfig.go b/pkg/apis/kops/componentconfig.go index 736d8a3445..27b33c7dd8 100644 --- a/pkg/apis/kops/componentconfig.go +++ b/pkg/apis/kops/componentconfig.go @@ -960,7 +960,7 @@ type ClusterAutoscalerConfig struct { // Default: false Enabled *bool `json:"enabled,omitempty"` // Expander determines the strategy for which instance group gets expanded. - // Supported values: least-waste, most-pods, random. + // Supported values: least-waste, most-pods, random, price, priority. // Default: least-waste Expander *string `json:"expander,omitempty"` // BalanceSimilarNodeGroups makes cluster autoscaler treat similar node groups as one. diff --git a/pkg/apis/kops/v1alpha2/componentconfig.go b/pkg/apis/kops/v1alpha2/componentconfig.go index 857f26080d..2e6f182306 100644 --- a/pkg/apis/kops/v1alpha2/componentconfig.go +++ b/pkg/apis/kops/v1alpha2/componentconfig.go @@ -980,7 +980,7 @@ type ClusterAutoscalerConfig struct { // Default: false Enabled *bool `json:"enabled,omitempty"` // Expander determines the strategy for which instance group gets expanded. - // Supported values: least-waste, most-pods, random. + // Supported values: least-waste, most-pods, random, price, priority. // Default: least-waste Expander *string `json:"expander,omitempty"` // BalanceSimilarNodeGroups makes cluster autoscaler treat similar node groups as one. diff --git a/pkg/apis/kops/v1alpha3/componentconfig.go b/pkg/apis/kops/v1alpha3/componentconfig.go index dbc72716c7..faf1a4dbfc 100644 --- a/pkg/apis/kops/v1alpha3/componentconfig.go +++ b/pkg/apis/kops/v1alpha3/componentconfig.go @@ -957,7 +957,7 @@ type ClusterAutoscalerConfig struct { // Default: false Enabled *bool `json:"enabled,omitempty"` // Expander determines the strategy for which instance group gets expanded. - // Supported values: least-waste, most-pods, random. + // Supported values: least-waste, most-pods, random, price, priority. // Default: least-waste Expander *string `json:"expander,omitempty"` // BalanceSimilarNodeGroups makes cluster autoscaler treat similar node groups as one. diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index cab4ee688f..a8e482d2b5 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -1517,7 +1517,7 @@ func validateNodeLocalDNS(spec *kops.ClusterSpec, fldpath *field.Path) field.Err } func validateClusterAutoscaler(cluster *kops.Cluster, spec *kops.ClusterAutoscalerConfig, fldPath *field.Path) (allErrs field.ErrorList) { - allErrs = append(allErrs, IsValidValue(fldPath.Child("expander"), spec.Expander, []string{"least-waste", "random", "most-pods"})...) + allErrs = append(allErrs, IsValidValue(fldPath.Child("expander"), spec.Expander, []string{"least-waste", "random", "most-pods", "price", "priority"})...) if kops.CloudProviderID(cluster.Spec.CloudProvider) == kops.CloudProviderOpenstack { allErrs = append(allErrs, field.Forbidden(fldPath, "Cluster autoscaler is not supported on OpenStack")) From 8672d9b2194d3bcc83bdfb395fa632cfda7dcecb Mon Sep 17 00:00:00 2001 From: Dan Ports Date: Wed, 5 Jan 2022 22:39:21 -0500 Subject: [PATCH 2/6] Fix CRDs, clarify docs, and add cloud provider check for price expander. --- docs/addons.md | 2 +- k8s/crds/kops.k8s.io_clusters.yaml | 2 +- pkg/apis/kops/componentconfig.go | 1 + pkg/apis/kops/v1alpha2/componentconfig.go | 1 + pkg/apis/kops/v1alpha3/componentconfig.go | 1 + pkg/apis/kops/validation/validation.go | 4 ++++ 6 files changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/addons.md b/docs/addons.md index f76ee08530..d27dfedb64 100644 --- a/docs/addons.md +++ b/docs/addons.md @@ -46,7 +46,7 @@ spec: memoryRequest: "300Mi" ``` -Read more about cluster autoscaler in the [official documentation](https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler). The autoscaler supports five different expander strategies; the `priority` expander requires additional configuration in a ConfigMap as described in [its documentation](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/expander/priority/readme.md). +Read more about cluster autoscaler in the [official documentation](https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler). The autoscaler supports five different expander strategies; the `priority` expander requires additional configuration in a ConfigMap as described in [its documentation](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/expander/priority/readme.md) - please ensure that you have deployed this ConfigMap before enabling the `priority` expander. ##### Disabling cluster autoscaler for a given instance group {{ kops_feature_table(kops_added_default='1.20') }} diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml index 209ed02716..999d9ebec6 100644 --- a/k8s/crds/kops.k8s.io_clusters.yaml +++ b/k8s/crds/kops.k8s.io_clusters.yaml @@ -622,7 +622,7 @@ spec: expander: description: 'Expander determines the strategy for which instance group gets expanded. Supported values: least-waste, most-pods, - random, price, priority. The priority expander requires additional + random, price, priority. The priority expander requires additional configuration via a ConfigMap. Default: least-waste' type: string image: diff --git a/pkg/apis/kops/componentconfig.go b/pkg/apis/kops/componentconfig.go index 27b33c7dd8..80a2c6dff5 100644 --- a/pkg/apis/kops/componentconfig.go +++ b/pkg/apis/kops/componentconfig.go @@ -961,6 +961,7 @@ type ClusterAutoscalerConfig struct { Enabled *bool `json:"enabled,omitempty"` // Expander determines the strategy for which instance group gets expanded. // Supported values: least-waste, most-pods, random, price, priority. + // The priority expander requires additional configuration via a ConfigMap. // Default: least-waste Expander *string `json:"expander,omitempty"` // BalanceSimilarNodeGroups makes cluster autoscaler treat similar node groups as one. diff --git a/pkg/apis/kops/v1alpha2/componentconfig.go b/pkg/apis/kops/v1alpha2/componentconfig.go index 2e6f182306..cb0d179716 100644 --- a/pkg/apis/kops/v1alpha2/componentconfig.go +++ b/pkg/apis/kops/v1alpha2/componentconfig.go @@ -981,6 +981,7 @@ type ClusterAutoscalerConfig struct { Enabled *bool `json:"enabled,omitempty"` // Expander determines the strategy for which instance group gets expanded. // Supported values: least-waste, most-pods, random, price, priority. + // The priority expander requires additional configuration via a ConfigMap. // Default: least-waste Expander *string `json:"expander,omitempty"` // BalanceSimilarNodeGroups makes cluster autoscaler treat similar node groups as one. diff --git a/pkg/apis/kops/v1alpha3/componentconfig.go b/pkg/apis/kops/v1alpha3/componentconfig.go index faf1a4dbfc..45b99e75b5 100644 --- a/pkg/apis/kops/v1alpha3/componentconfig.go +++ b/pkg/apis/kops/v1alpha3/componentconfig.go @@ -958,6 +958,7 @@ type ClusterAutoscalerConfig struct { Enabled *bool `json:"enabled,omitempty"` // Expander determines the strategy for which instance group gets expanded. // Supported values: least-waste, most-pods, random, price, priority. + // The priority expander requires additional configuration via a ConfigMap. // Default: least-waste Expander *string `json:"expander,omitempty"` // BalanceSimilarNodeGroups makes cluster autoscaler treat similar node groups as one. diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index a8e482d2b5..005da7050d 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -1519,6 +1519,10 @@ func validateNodeLocalDNS(spec *kops.ClusterSpec, fldpath *field.Path) field.Err func validateClusterAutoscaler(cluster *kops.Cluster, spec *kops.ClusterAutoscalerConfig, fldPath *field.Path) (allErrs field.ErrorList) { allErrs = append(allErrs, IsValidValue(fldPath.Child("expander"), spec.Expander, []string{"least-waste", "random", "most-pods", "price", "priority"})...) + if *spec.Expander == "price" && kops.CloudProviderID(cluster.Spec.CloudProvider) != kops.CloudProviderGCE { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("expander"), "Cluster autoscaler price expander is only supported on GCE")) + } + if kops.CloudProviderID(cluster.Spec.CloudProvider) == kops.CloudProviderOpenstack { allErrs = append(allErrs, field.Forbidden(fldPath, "Cluster autoscaler is not supported on OpenStack")) } From 7a52896fdc5d316142e0e16c7b9bac2292a08be1 Mon Sep 17 00:00:00 2001 From: Dan Ports Date: Wed, 5 Jan 2022 22:47:34 -0500 Subject: [PATCH 3/6] Warn that the price expander is only supported on GCE in the docs. --- docs/addons.md | 5 ++++- k8s/crds/kops.k8s.io_clusters.yaml | 5 +++-- pkg/apis/kops/componentconfig.go | 1 + pkg/apis/kops/v1alpha2/componentconfig.go | 1 + pkg/apis/kops/v1alpha3/componentconfig.go | 1 + 5 files changed, 10 insertions(+), 3 deletions(-) diff --git a/docs/addons.md b/docs/addons.md index d27dfedb64..8cec591eac 100644 --- a/docs/addons.md +++ b/docs/addons.md @@ -46,7 +46,10 @@ spec: memoryRequest: "300Mi" ``` -Read more about cluster autoscaler in the [official documentation](https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler). The autoscaler supports five different expander strategies; the `priority` expander requires additional configuration in a ConfigMap as described in [its documentation](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/expander/priority/readme.md) - please ensure that you have deployed this ConfigMap before enabling the `priority` expander. +Read more about cluster autoscaler in the [official documentation](https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler). + +##### Expander strategies +The cluster autoscaler supports five different [expander strategies](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/FAQ.md#what-are-expanders). The `price` expander is only supported on GCE. The `priority` expander requires additional configuration in a ConfigMap as described in [its documentation](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/expander/priority/readme.md) - please ensure that you have deployed this ConfigMap before enabling the `priority` expander. ##### Disabling cluster autoscaler for a given instance group {{ kops_feature_table(kops_added_default='1.20') }} diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml index 999d9ebec6..40527af9cd 100644 --- a/k8s/crds/kops.k8s.io_clusters.yaml +++ b/k8s/crds/kops.k8s.io_clusters.yaml @@ -622,8 +622,9 @@ spec: expander: description: 'Expander determines the strategy for which instance group gets expanded. Supported values: least-waste, most-pods, - random, price, priority. The priority expander requires additional - configuration via a ConfigMap. Default: least-waste' + random, price, priority. The price expander is only supported + on GCE. The priority expander requires additional configuration + via a ConfigMap. Default: least-waste' type: string image: description: 'Image is the docker container used. Default: the diff --git a/pkg/apis/kops/componentconfig.go b/pkg/apis/kops/componentconfig.go index 80a2c6dff5..368ab18f7c 100644 --- a/pkg/apis/kops/componentconfig.go +++ b/pkg/apis/kops/componentconfig.go @@ -961,6 +961,7 @@ type ClusterAutoscalerConfig struct { Enabled *bool `json:"enabled,omitempty"` // Expander determines the strategy for which instance group gets expanded. // Supported values: least-waste, most-pods, random, price, priority. + // The price expander is only supported on GCE. // The priority expander requires additional configuration via a ConfigMap. // Default: least-waste Expander *string `json:"expander,omitempty"` diff --git a/pkg/apis/kops/v1alpha2/componentconfig.go b/pkg/apis/kops/v1alpha2/componentconfig.go index cb0d179716..dcc2155a60 100644 --- a/pkg/apis/kops/v1alpha2/componentconfig.go +++ b/pkg/apis/kops/v1alpha2/componentconfig.go @@ -981,6 +981,7 @@ type ClusterAutoscalerConfig struct { Enabled *bool `json:"enabled,omitempty"` // Expander determines the strategy for which instance group gets expanded. // Supported values: least-waste, most-pods, random, price, priority. + // The price expander is only supported on GCE. // The priority expander requires additional configuration via a ConfigMap. // Default: least-waste Expander *string `json:"expander,omitempty"` diff --git a/pkg/apis/kops/v1alpha3/componentconfig.go b/pkg/apis/kops/v1alpha3/componentconfig.go index 45b99e75b5..9ba58c92e3 100644 --- a/pkg/apis/kops/v1alpha3/componentconfig.go +++ b/pkg/apis/kops/v1alpha3/componentconfig.go @@ -958,6 +958,7 @@ type ClusterAutoscalerConfig struct { Enabled *bool `json:"enabled,omitempty"` // Expander determines the strategy for which instance group gets expanded. // Supported values: least-waste, most-pods, random, price, priority. + // The price expander is only supported on GCE. // The priority expander requires additional configuration via a ConfigMap. // Default: least-waste Expander *string `json:"expander,omitempty"` From 2cc26b57cbbb88126224f2d8e86be3fd237a00b8 Mon Sep 17 00:00:00 2001 From: Dan Ports Date: Wed, 5 Jan 2022 22:50:37 -0500 Subject: [PATCH 4/6] Less crashing when validating. --- pkg/apis/kops/validation/validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index 005da7050d..ebfb1cea6b 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -1519,7 +1519,7 @@ func validateNodeLocalDNS(spec *kops.ClusterSpec, fldpath *field.Path) field.Err func validateClusterAutoscaler(cluster *kops.Cluster, spec *kops.ClusterAutoscalerConfig, fldPath *field.Path) (allErrs field.ErrorList) { allErrs = append(allErrs, IsValidValue(fldPath.Child("expander"), spec.Expander, []string{"least-waste", "random", "most-pods", "price", "priority"})...) - if *spec.Expander == "price" && kops.CloudProviderID(cluster.Spec.CloudProvider) != kops.CloudProviderGCE { + if spec.Expander != nil && *spec.Expander == "price" && kops.CloudProviderID(cluster.Spec.CloudProvider) != kops.CloudProviderGCE { allErrs = append(allErrs, field.Forbidden(fldPath.Child("expander"), "Cluster autoscaler price expander is only supported on GCE")) } From 71a2e269835469402fb5e8302a4844477475650e Mon Sep 17 00:00:00 2001 From: Dan Ports Date: Fri, 7 Jan 2022 13:37:45 -0500 Subject: [PATCH 5/6] Fix StringValue nit Co-authored-by: Ole Markus With --- pkg/apis/kops/validation/validation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index ebfb1cea6b..1447cf4766 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -1519,7 +1519,7 @@ func validateNodeLocalDNS(spec *kops.ClusterSpec, fldpath *field.Path) field.Err func validateClusterAutoscaler(cluster *kops.Cluster, spec *kops.ClusterAutoscalerConfig, fldPath *field.Path) (allErrs field.ErrorList) { allErrs = append(allErrs, IsValidValue(fldPath.Child("expander"), spec.Expander, []string{"least-waste", "random", "most-pods", "price", "priority"})...) - if spec.Expander != nil && *spec.Expander == "price" && kops.CloudProviderID(cluster.Spec.CloudProvider) != kops.CloudProviderGCE { + if fi.StringValue(spec.Expander) == "price" && kops.CloudProviderID(cluster.Spec.CloudProvider) != kops.CloudProviderGCE { allErrs = append(allErrs, field.Forbidden(fldPath.Child("expander"), "Cluster autoscaler price expander is only supported on GCE")) } From c413eb2f4bc9a64d98fc5a74415fae20de5ab942 Mon Sep 17 00:00:00 2001 From: Dan Ports Date: Sat, 8 Jan 2022 20:58:31 -0500 Subject: [PATCH 6/6] Keep docs DRY. --- docs/addons.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/addons.md b/docs/addons.md index 8cec591eac..8de04f359f 100644 --- a/docs/addons.md +++ b/docs/addons.md @@ -49,7 +49,9 @@ spec: Read more about cluster autoscaler in the [official documentation](https://github.com/kubernetes/autoscaler/tree/master/cluster-autoscaler). ##### Expander strategies -The cluster autoscaler supports five different [expander strategies](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/FAQ.md#what-are-expanders). The `price` expander is only supported on GCE. The `priority` expander requires additional configuration in a ConfigMap as described in [its documentation](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/expander/priority/readme.md) - please ensure that you have deployed this ConfigMap before enabling the `priority` expander. +Cluster autoscaler supports several different [expander strategies](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/FAQ.md#what-are-expanders). + +Note that the `priority` expander requires additional configuration through a ConfigMap as described in [its documentation](https://github.com/kubernetes/autoscaler/blob/master/cluster-autoscaler/expander/priority/readme.md) - you will need to create this ConfigMap in your cluster before selecting this expander. ##### Disabling cluster autoscaler for a given instance group {{ kops_feature_table(kops_added_default='1.20') }}