diff --git a/docs/instance_groups.md b/docs/instance_groups.md index 129f315e44..55bacf6c89 100644 --- a/docs/instance_groups.md +++ b/docs/instance_groups.md @@ -257,6 +257,9 @@ spec: maxSize: 10 ``` +You can also specify defaults for all instance groups of type Node or APIServer by setting the `warmPool` field in the cluster spec. +If warm pools are enabled at the cluster spec level, you can disable them at the instance group level by setting `maxSize: 0`. + ### Lifecycle hook By default AWS does not guarantee that the kOps configuration will run to completion. Nor that the instance will timely shut down after completion if the instance is allowed to run that long. In order to guarantee this, a lifecycle hook is needed. diff --git a/k8s/crds/kops.k8s.io_clusters.yaml b/k8s/crds/kops.k8s.io_clusters.yaml index 5d9cb01dcc..1f29a7b909 100644 --- a/k8s/crds/kops.k8s.io_clusters.yaml +++ b/k8s/crds/kops.k8s.io_clusters.yaml @@ -4141,6 +4141,29 @@ spec: needed containers. This is needed if some APIs do have self-signed certs type: boolean + warmPool: + description: WarmPool defines the default warm pool settings for instance + groups (AWS only). + properties: + enableLifecycleHook: + description: EnableLifecycleHook determines if an ASG lifecycle + hook will be added ensuring that nodeup runs to completion. + Note that the metadata API must be protected from arbitrary + Pods when this is enabled. + type: boolean + maxSize: + description: MaxSize is the maximum size of the warm pool. The + desired size of the instance group is subtracted from this number + to determine the desired size of the warm pool (unless the resulting + number is smaller than MinSize). The default is the instance + group's MaxSize. + format: int64 + type: integer + minSize: + description: MinSize is the minimum size of the pool + format: int64 + type: integer + type: object type: object type: object served: true diff --git a/pkg/apis/kops/BUILD.bazel b/pkg/apis/kops/BUILD.bazel index 7e07d37a43..c93f695471 100644 --- a/pkg/apis/kops/BUILD.bazel +++ b/pkg/apis/kops/BUILD.bazel @@ -42,6 +42,7 @@ go_library( go_test( name = "go_default_test", srcs = [ + "cluster_test.go", "parse_test.go", "semver_test.go", ], @@ -49,6 +50,7 @@ go_test( deps = [ "//upup/pkg/fi/utils:go_default_library", "//vendor/github.com/blang/semver/v4:go_default_library", + "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/gopkg.in/inf.v0:go_default_library", ], ) diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go index 4519f745e6..39e30f33bd 100644 --- a/pkg/apis/kops/cluster.go +++ b/pkg/apis/kops/cluster.go @@ -201,10 +201,12 @@ type ClusterSpec struct { // specified, each parameter must follow the form variable=value, the way // it would appear in sysctl.conf. SysctlParameters []string `json:"sysctlParameters,omitempty"` - // RollingUpdate defines the default rolling-update settings for instance groups + // RollingUpdate defines the default rolling-update settings for instance groups. RollingUpdate *RollingUpdate `json:"rollingUpdate,omitempty"` // ClusterAutoscaler defines the cluster autoscaler configuration. ClusterAutoscaler *ClusterAutoscalerConfig `json:"clusterAutoscaler,omitempty"` + // WarmPool defines the default warm pool settings for instance groups (AWS only). + WarmPool *WarmPoolSpec `json:"warmPool,omitempty"` } // NodeAuthorizationSpec is used to node authorization @@ -843,3 +845,49 @@ type PackagesConfig struct { // UrlArm64 overrides the URL for the ARM64 package. UrlArm64 *string `json:"urlArm64,omitempty"` } + +type WarmPoolSpec struct { + // MinSize is the minimum size of the warm pool. + MinSize int64 `json:"minSize,omitempty"` + // MaxSize is the maximum size of the warm pool. The desired size of the instance group + // is subtracted from this number to determine the desired size of the warm pool + // (unless the resulting number is smaller than MinSize). + // The default is the instance group's MaxSize. + MaxSize *int64 `json:"maxSize,omitempty"` + // EnableLifecyleHook determines if an ASG lifecycle hook will be added ensuring that nodeup runs to completion. + // Note that the metadata API must be protected from arbitrary Pods when this is enabled. + EnableLifecycleHook bool `json:"enableLifecycleHook,omitempty"` +} + +func (in *WarmPoolSpec) IsEnabled() bool { + return in != nil && (in.MaxSize == nil || *in.MaxSize != 0) +} + +func (in *WarmPoolSpec) ResolveDefaults(ig *InstanceGroup) *WarmPoolSpec { + igWarmPool := ig.Spec.WarmPool + if igWarmPool == nil { + if in == nil || (ig.Spec.Role == InstanceGroupRoleMaster || ig.Spec.Role == InstanceGroupRoleBastion) { + var zero int64 + return &WarmPoolSpec{ + MaxSize: &zero, + } + } + return in + } + + if in == nil || (ig.Spec.Role == InstanceGroupRoleMaster || ig.Spec.Role == InstanceGroupRoleBastion) { + return igWarmPool + } + + spec := *igWarmPool + if spec.MaxSize == nil { + spec.MaxSize = in.MaxSize + } + if spec.MinSize == 0 { + spec.MinSize = in.MinSize + } + if !spec.EnableLifecycleHook { + spec.EnableLifecycleHook = in.EnableLifecycleHook + } + return &spec +} diff --git a/pkg/apis/kops/cluster_test.go b/pkg/apis/kops/cluster_test.go new file mode 100644 index 0000000000..6880eafe41 --- /dev/null +++ b/pkg/apis/kops/cluster_test.go @@ -0,0 +1,202 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kops + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestWarmPoolSpec_IsEnabled(t *testing.T) { + tests := []struct { + name string + spec *WarmPoolSpec + expected bool + }{ + { + name: "nil", + spec: nil, + expected: false, + }, + { + name: "empty", + spec: &WarmPoolSpec{}, + expected: true, + }, + { + name: "1", + spec: &WarmPoolSpec{ + MaxSize: int64ptr(1), + }, + expected: true, + }, + { + name: "0", + spec: &WarmPoolSpec{ + MaxSize: int64ptr(0), + }, + expected: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if actual := tc.spec.IsEnabled(); actual != tc.expected { + t.Errorf("IsEnabled() = %v, want %v", actual, tc.expected) + } + }) + } +} + +func int64ptr(v int64) *int64 { + return &v +} + +func TestWarmPoolSpec_ResolveDefaults(t *testing.T) { + for _, tc := range []struct { + name string + defaultValue interface{} + nonDefaultValue interface{} + }{ + { + name: "MinSize", + defaultValue: int64(0), + nonDefaultValue: int64(1), + }, + { + name: "MaxSize", + defaultValue: nil, + nonDefaultValue: int64(1), + }, + { + name: "EnableLifecycleHook", + defaultValue: false, + nonDefaultValue: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + defaultCluster := &WarmPoolSpec{} + setFieldValue(defaultCluster, tc.name, tc.defaultValue) + + nonDefaultCluster := &WarmPoolSpec{} + setFieldValue(nonDefaultCluster, tc.name, tc.nonDefaultValue) + + defaultGroup := &WarmPoolSpec{} + setFieldValue(defaultGroup, tc.name, tc.defaultValue) + + nonDefaultGroup := &WarmPoolSpec{} + setFieldValue(nonDefaultGroup, tc.name, tc.nonDefaultValue) + + expectedDefaultValue := tc.defaultValue + if expectedDefaultValue == nil { + expectedDefaultValue = reflect.Zero(reflect.ValueOf(*defaultGroup).FieldByName(tc.name).Type().Elem()).Interface() + } + + assertResolvesValue(t, tc.name, expectedDefaultValue, nil, nil, InstanceGroupRoleNode, "nil nil node") + assertResolvesValue(t, tc.name, tc.defaultValue, &WarmPoolSpec{}, nil, InstanceGroupRoleNode, "{nil} nil node") + assertResolvesValue(t, tc.name, tc.defaultValue, defaultCluster, nil, InstanceGroupRoleNode, "{default} nil node") + assertResolvesValue(t, tc.name, tc.nonDefaultValue, nonDefaultCluster, nil, InstanceGroupRoleNode, "{nonDefault} nil node") + + assertResolvesValue(t, tc.name, tc.defaultValue, nil, &WarmPoolSpec{}, InstanceGroupRoleNode, "nil {nil} node") + assertResolvesValue(t, tc.name, tc.defaultValue, &WarmPoolSpec{}, &WarmPoolSpec{}, InstanceGroupRoleNode, "{nil} {nil} node") + assertResolvesValue(t, tc.name, tc.defaultValue, defaultCluster, &WarmPoolSpec{}, InstanceGroupRoleNode, "{default} {nil} node") + assertResolvesValue(t, tc.name, tc.nonDefaultValue, nonDefaultCluster, &WarmPoolSpec{}, InstanceGroupRoleNode, "{nonDefault} {nil} node") + + assertResolvesValue(t, tc.name, tc.defaultValue, nil, defaultGroup, InstanceGroupRoleNode, "nil {default} node") + assertResolvesValue(t, tc.name, tc.defaultValue, &WarmPoolSpec{}, defaultGroup, InstanceGroupRoleNode, "{nil} {default} node") + assertResolvesValue(t, tc.name, tc.defaultValue, defaultCluster, defaultGroup, InstanceGroupRoleNode, "{default} {default} node") + if reflect.ValueOf(*defaultGroup).FieldByName(tc.name).Type().Kind() == reflect.Ptr && tc.defaultValue != nil { + assertResolvesValue(t, tc.name, tc.defaultValue, nonDefaultCluster, defaultGroup, InstanceGroupRoleNode, "{nonDefault} {default} node") + } else { + assertResolvesValue(t, tc.name, tc.nonDefaultValue, nonDefaultCluster, defaultGroup, InstanceGroupRoleNode, "{nonDefault} {default} node") + } + + assertResolvesValue(t, tc.name, tc.nonDefaultValue, nil, nonDefaultGroup, InstanceGroupRoleNode, "nil {nonDefault} node") + assertResolvesValue(t, tc.name, tc.nonDefaultValue, &WarmPoolSpec{}, nonDefaultGroup, InstanceGroupRoleNode, "{nil} {nonDefault} node") + assertResolvesValue(t, tc.name, tc.nonDefaultValue, defaultCluster, nonDefaultGroup, InstanceGroupRoleNode, "{default} {nonDefault} node") + assertResolvesValue(t, tc.name, tc.nonDefaultValue, nonDefaultCluster, nonDefaultGroup, InstanceGroupRoleNode, "{nonDefault} {nonDefault} node") + + assertResolvesValue(t, tc.name, expectedDefaultValue, nil, nil, InstanceGroupRoleMaster, "nil nil master") + assertResolvesValue(t, tc.name, expectedDefaultValue, &WarmPoolSpec{}, nil, InstanceGroupRoleMaster, "{nil} nil master") + assertResolvesValue(t, tc.name, expectedDefaultValue, defaultCluster, nil, InstanceGroupRoleMaster, "{default} nil master") + assertResolvesValue(t, tc.name, expectedDefaultValue, nonDefaultCluster, nil, InstanceGroupRoleMaster, "{nonDefault} nil master") + + assertResolvesValue(t, tc.name, tc.defaultValue, nil, &WarmPoolSpec{}, InstanceGroupRoleMaster, "nil {nil} master") + assertResolvesValue(t, tc.name, tc.defaultValue, &WarmPoolSpec{}, &WarmPoolSpec{}, InstanceGroupRoleMaster, "{nil} {nil} master") + assertResolvesValue(t, tc.name, tc.defaultValue, defaultCluster, &WarmPoolSpec{}, InstanceGroupRoleMaster, "{default} {nil} master") + assertResolvesValue(t, tc.name, tc.defaultValue, nonDefaultCluster, &WarmPoolSpec{}, InstanceGroupRoleMaster, "{nonDefault} {nil} master") + + assertResolvesValue(t, tc.name, tc.defaultValue, nil, defaultGroup, InstanceGroupRoleMaster, "nil {default} master") + assertResolvesValue(t, tc.name, tc.defaultValue, &WarmPoolSpec{}, defaultGroup, InstanceGroupRoleMaster, "{nil} {default} master") + assertResolvesValue(t, tc.name, tc.defaultValue, defaultCluster, defaultGroup, InstanceGroupRoleMaster, "{default} {default} master") + assertResolvesValue(t, tc.name, tc.defaultValue, nonDefaultCluster, defaultGroup, InstanceGroupRoleMaster, "{nonDefault} {default} master") + + assertResolvesValue(t, tc.name, tc.nonDefaultValue, nil, nonDefaultGroup, InstanceGroupRoleMaster, "nil {nonDefault} master") + assertResolvesValue(t, tc.name, tc.nonDefaultValue, &WarmPoolSpec{}, nonDefaultGroup, InstanceGroupRoleMaster, "{nil} {nonDefault} master") + assertResolvesValue(t, tc.name, tc.nonDefaultValue, defaultCluster, nonDefaultGroup, InstanceGroupRoleMaster, "{default} {nonDefault} master") + assertResolvesValue(t, tc.name, tc.nonDefaultValue, nonDefaultCluster, nonDefaultGroup, InstanceGroupRoleMaster, "{nonDefault} {nonDefault} master") + }) + } +} + +func setFieldValue(aStruct interface{}, fieldName string, fieldValue interface{}) { + field := reflect.ValueOf(aStruct).Elem().FieldByName(fieldName) + fieldType := field.Type() + if fieldType.Kind() == reflect.Ptr { + if fieldValue != nil { + value := reflect.New(fieldType.Elem()) + value.Elem().Set(reflect.ValueOf(fieldValue)) + field.Set(value) + } + } else { + field.Set(reflect.ValueOf(fieldValue)) + } +} + +func assertResolvesValue(t *testing.T, name string, expected interface{}, warmPoolSpecDefault *WarmPoolSpec, ig *WarmPoolSpec, role InstanceGroupRole, msg interface{}) bool { + cluster := Cluster{ + Spec: ClusterSpec{ + WarmPool: warmPoolSpecDefault, + }, + } + instanceGroup := InstanceGroup{ + Spec: InstanceGroupSpec{ + Role: role, + WarmPool: ig, + }, + } + warmPoolSpecDefaultCopy := warmPoolSpecDefault.DeepCopy() + warmPoolSpecCopy := ig.DeepCopy() + + resolved := cluster.Spec.WarmPool.ResolveDefaults(&instanceGroup) + value := reflect.ValueOf(*resolved).FieldByName(name) + + assert.Equal(t, warmPoolSpecDefault, cluster.Spec.WarmPool, "cluster not modified") + assert.True(t, reflect.DeepEqual(warmPoolSpecDefault, warmPoolSpecDefaultCopy), "WarmPoolSpec not modified") + assert.Equal(t, ig, instanceGroup.Spec.WarmPool, "instancegroup not modified") + assert.True(t, reflect.DeepEqual(ig, warmPoolSpecCopy), "WarmPoolSpec not modified") + + if expected == nil { + return assert.Nil(t, value.Interface(), msg) + } else if value.Type().Kind() == reflect.Ptr { + return assert.NotNil(t, value.Interface(), msg) && + assert.Equal(t, expected, value.Elem().Interface(), msg) + } else { + return assert.Equal(t, expected, value.Interface(), msg) + } +} diff --git a/pkg/apis/kops/instancegroup.go b/pkg/apis/kops/instancegroup.go index 8f5d892408..c3a0a84aec 100644 --- a/pkg/apis/kops/instancegroup.go +++ b/pkg/apis/kops/instancegroup.go @@ -186,23 +186,6 @@ type InstanceGroupSpec struct { WarmPool *WarmPoolSpec `json:"warmPool,omitempty"` } -type WarmPoolSpec struct { - // MinSize is the minimum size of the warm pool. - MinSize int64 `json:"minSize,omitempty"` - // MaxSize is the maximum size of the warm pool. The desired size of the instance group - // is subtracted from this number to determine the desired size of the warm pool - // (unless the resulting number is smaller than MinSize). - // The default is the instance group's MaxSize. - MaxSize *int64 `json:"maxSize,omitempty"` - // EnableLifecycleHook determines if an ASG lifecycle hook will be added ensuring that nodeup runs to completion. - // Note that the metadata API must be protected from arbitrary Pods when this is enabled. - EnableLifecycleHook bool `json:"enableLifecycleHook,omitempty"` -} - -func (in *WarmPoolSpec) IsEnabled() bool { - return in != nil && (in.MaxSize == nil || *in.MaxSize != 0) -} - const ( // SpotAllocationStrategyLowestPrices indicates a lowest-price strategy SpotAllocationStrategyLowestPrices = "lowest-price" diff --git a/pkg/apis/kops/v1alpha2/cluster.go b/pkg/apis/kops/v1alpha2/cluster.go index dd5c2098ed..d8df986fc3 100644 --- a/pkg/apis/kops/v1alpha2/cluster.go +++ b/pkg/apis/kops/v1alpha2/cluster.go @@ -204,6 +204,8 @@ type ClusterSpec struct { RollingUpdate *RollingUpdate `json:"rollingUpdate,omitempty"` // ClusterAutoscaler defines the cluaster autoscaler configuration. ClusterAutoscaler *ClusterAutoscalerConfig `json:"clusterAutoscaler,omitempty"` + // WarmPool defines the default warm pool settings for instance groups (AWS only). + WarmPool *WarmPoolSpec `json:"warmPool,omitempty"` } // NodeAuthorizationSpec is used to node authorization @@ -707,3 +709,16 @@ type PackagesConfig struct { // UrlArm64 overrides the URL for the ARM64 package. UrlArm64 *string `json:"urlArm64,omitempty"` } + +type WarmPoolSpec struct { + // MinSize is the minimum size of the pool + MinSize int64 `json:"minSize,omitempty"` + // MaxSize is the maximum size of the warm pool. The desired size of the instance group + // is subtracted from this number to determine the desired size of the warm pool + // (unless the resulting number is smaller than MinSize). + // The default is the instance group's MaxSize. + MaxSize *int64 `json:"maxSize,omitempty"` + // EnableLifecycleHook determines if an ASG lifecycle hook will be added ensuring that nodeup runs to completion. + // Note that the metadata API must be protected from arbitrary Pods when this is enabled. + EnableLifecycleHook bool `json:"enableLifecycleHook,omitempty"` +} diff --git a/pkg/apis/kops/v1alpha2/instancegroup.go b/pkg/apis/kops/v1alpha2/instancegroup.go index d73848116f..33ddeea08f 100644 --- a/pkg/apis/kops/v1alpha2/instancegroup.go +++ b/pkg/apis/kops/v1alpha2/instancegroup.go @@ -151,18 +151,6 @@ type InstanceGroupSpec struct { // WarmPool configures an ASG warm pool for the instance group WarmPool *WarmPoolSpec `json:"warmPool,omitempty"` } -type WarmPoolSpec struct { - // MinSize is the minimum size of the pool - MinSize int64 `json:"minSize,omitempty"` - // MaxSize is the maximum size of the warm pool. The desired size of the instance group - // is subtracted from this number to determine the desired size of the warm pool - // (unless the resulting number is smaller than MinSize). - // The default is the instance group's MaxSize. - MaxSize *int64 `json:"maxSize,omitempty"` - // EnableLifecycleHook determines if an ASG lifecycle hook will be added ensuring that nodeup runs to completion. - // Note that the metadata API must be protected from arbitrary Pods when this is enabled. - EnableLifecycleHook bool `json:"enableLifecycleHook,omitempty"` -} // InstanceMetadataOptions defines the EC2 instance metadata service options (AWS Only) type InstanceMetadataOptions struct { diff --git a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go index 2557934c69..cc6aaf1746 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.conversion.go +++ b/pkg/apis/kops/v1alpha2/zz_generated.conversion.go @@ -2503,6 +2503,15 @@ func autoConvert_v1alpha2_ClusterSpec_To_kops_ClusterSpec(in *ClusterSpec, out * } else { out.ClusterAutoscaler = nil } + if in.WarmPool != nil { + in, out := &in.WarmPool, &out.WarmPool + *out = new(kops.WarmPoolSpec) + if err := Convert_v1alpha2_WarmPoolSpec_To_kops_WarmPoolSpec(*in, *out, s); err != nil { + return err + } + } else { + out.WarmPool = nil + } return nil } @@ -2880,6 +2889,15 @@ func autoConvert_kops_ClusterSpec_To_v1alpha2_ClusterSpec(in *kops.ClusterSpec, } else { out.ClusterAutoscaler = nil } + if in.WarmPool != nil { + in, out := &in.WarmPool, &out.WarmPool + *out = new(WarmPoolSpec) + if err := Convert_kops_WarmPoolSpec_To_v1alpha2_WarmPoolSpec(*in, *out, s); err != nil { + return err + } + } else { + out.WarmPool = nil + } return nil } diff --git a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go index c9e6b5fea3..ddc2d37678 100644 --- a/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go +++ b/pkg/apis/kops/v1alpha2/zz_generated.deepcopy.go @@ -1093,6 +1093,11 @@ func (in *ClusterSpec) DeepCopyInto(out *ClusterSpec) { *out = new(ClusterAutoscalerConfig) (*in).DeepCopyInto(*out) } + if in.WarmPool != nil { + in, out := &in.WarmPool, &out.WarmPool + *out = new(WarmPoolSpec) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/apis/kops/validation/instancegroup.go b/pkg/apis/kops/validation/instancegroup.go index b130ed884f..6a3b3cbf7d 100644 --- a/pkg/apis/kops/validation/instancegroup.go +++ b/pkg/apis/kops/validation/instancegroup.go @@ -143,29 +143,6 @@ func ValidateInstanceGroup(g *kops.InstanceGroup, cloud fi.Cloud) field.ErrorLis allErrs = append(allErrs, IsValidValue(field.NewPath("spec", "updatePolicy"), g.Spec.UpdatePolicy, []string{kops.UpdatePolicyAutomatic, kops.UpdatePolicyExternal})...) - warmPool := g.Spec.WarmPool - if warmPool != nil { - if g.Spec.Role != kops.InstanceGroupRoleNode && g.Spec.Role != kops.InstanceGroupRoleAPIServer { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "warmPool"), "warm pool only allowed on instance groups with role Node or APIServer")) - } - if g.Spec.MixedInstancesPolicy != nil { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "warmPool"), "warm pool cannot be combined with a mixed instances policy")) - } - if g.Spec.MaxPrice != nil { - allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "warmPool"), "warm pool cannot be used with spot instances")) - } - if warmPool.MaxSize != nil { - if *warmPool.MaxSize < 0 { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "warmPool", "maxSize"), *warmPool.MaxSize, "warm pool maxSize cannot be negative")) - } else if warmPool.MinSize > *warmPool.MaxSize { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "warmPool", "maxSize"), fi.Int64Value(warmPool.MaxSize), "warm pool maxSize cannot be set to lower than minSize")) - - } - } - if warmPool.MinSize < 0 { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "warmPool", "minSize"), warmPool.MinSize, "warm pool minSize cannot be negative")) - } - } return allErrs } @@ -238,6 +215,33 @@ func CrossValidateInstanceGroup(g *kops.InstanceGroup, cluster *kops.Cluster, cl allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "warmPool"), "warm pool only supported on AWS")) } } + + { + warmPool := cluster.Spec.WarmPool.ResolveDefaults(g) + if warmPool.MaxSize == nil || *warmPool.MaxSize != 0 { + if g.Spec.Role != kops.InstanceGroupRoleNode && g.Spec.Role != kops.InstanceGroupRoleAPIServer { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "warmPool"), "warm pool only allowed on instance groups with role Node or APIServer")) + } + if g.Spec.MixedInstancesPolicy != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "warmPool"), "warm pool cannot be combined with a mixed instances policy")) + } + if g.Spec.MaxPrice != nil { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "warmPool"), "warm pool cannot be used with spot instances")) + } + } + if warmPool.MaxSize != nil { + if *warmPool.MaxSize < 0 { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "warmPool", "maxSize"), *warmPool.MaxSize, "warm pool maxSize cannot be negative")) + } else if warmPool.MinSize > *warmPool.MaxSize { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "warmPool", "maxSize"), *warmPool.MaxSize, "warm pool maxSize cannot be set to lower than minSize")) + + } + } + if warmPool.MinSize < 0 { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "warmPool", "minSize"), warmPool.MinSize, "warm pool minSize cannot be negative")) + } + } + return allErrs } diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go index fad13d653d..c52e799790 100644 --- a/pkg/apis/kops/validation/validation.go +++ b/pkg/apis/kops/validation/validation.go @@ -246,6 +246,14 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie allErrs = append(allErrs, validateCloudConfiguration(spec.CloudConfig, fieldPath.Child("cloudConfig"))...) } + if spec.WarmPool != nil { + if kops.CloudProviderID(spec.CloudProvider) != kops.CloudProviderAWS { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "warmPool"), "warm pool only supported on AWS")) + } else { + allErrs = append(allErrs, validateWarmPool(spec.WarmPool, fieldPath.Child("warmPool"))...) + } + } + return allErrs } @@ -1342,3 +1350,17 @@ func validateCloudConfiguration(cloudConfig *kops.CloudConfiguration, fldPath *f } return allErrs } + +func validateWarmPool(warmPool *kops.WarmPoolSpec, fldPath *field.Path) (allErrs field.ErrorList) { + if warmPool.MaxSize != nil { + if *warmPool.MaxSize < 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("maxSize"), *warmPool.MaxSize, "warm pool maxSize cannot be negative")) + } else if warmPool.MinSize > *warmPool.MaxSize { + allErrs = append(allErrs, field.Invalid(fldPath.Child("maxSize"), *warmPool.MaxSize, "warm pool maxSize cannot be set to lower than minSize")) + } + } + if warmPool.MinSize < 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("minSize"), warmPool.MinSize, "warm pool minSize cannot be negative")) + } + return allErrs +} diff --git a/pkg/apis/kops/zz_generated.deepcopy.go b/pkg/apis/kops/zz_generated.deepcopy.go index b0aa060f9e..16a716ff18 100644 --- a/pkg/apis/kops/zz_generated.deepcopy.go +++ b/pkg/apis/kops/zz_generated.deepcopy.go @@ -1193,6 +1193,11 @@ func (in *ClusterSpec) DeepCopyInto(out *ClusterSpec) { *out = new(ClusterAutoscalerConfig) (*in).DeepCopyInto(*out) } + if in.WarmPool != nil { + in, out := &in.WarmPool, &out.WarmPool + *out = new(WarmPoolSpec) + (*in).DeepCopyInto(*out) + } return } diff --git a/pkg/model/awsmodel/autoscalinggroup.go b/pkg/model/awsmodel/autoscalinggroup.go index 85c2c3bdda..7ec1562b38 100644 --- a/pkg/model/awsmodel/autoscalinggroup.go +++ b/pkg/model/awsmodel/autoscalinggroup.go @@ -57,6 +57,7 @@ type AutoscalingGroupModelBuilder struct { BootstrapScriptBuilder *model.BootstrapScriptBuilder Lifecycle *fi.Lifecycle SecurityLifecycle *fi.Lifecycle + Cluster *kops.Cluster } var _ fi.ModelBuilder = &AutoscalingGroupModelBuilder{} @@ -87,7 +88,7 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error { tsk.LaunchTemplate = task c.AddTask(tsk) - warmPool := ig.Spec.WarmPool + warmPool := b.Cluster.Spec.WarmPool.ResolveDefaults(ig) { enabled := fi.Bool(warmPool.IsEnabled()) warmPoolTask := &awstasks.WarmPool{ diff --git a/pkg/model/awsmodel/autoscalinggroup_test.go b/pkg/model/awsmodel/autoscalinggroup_test.go index 57d9a926ab..32369b1b19 100644 --- a/pkg/model/awsmodel/autoscalinggroup_test.go +++ b/pkg/model/awsmodel/autoscalinggroup_test.go @@ -68,6 +68,7 @@ func TestRootVolumeOptimizationFlag(t *testing.T) { InstanceGroups: igs, }, }, + Cluster: cluster, } c := &fi.ModelBuilderContext{ @@ -153,6 +154,7 @@ func TestAPIServerAdditionalSecurityGroupsWithNLB(t *testing.T) { InstanceGroups: igs, }, }, + Cluster: cluster, } c := &fi.ModelBuilderContext{ diff --git a/pkg/model/iam.go b/pkg/model/iam.go index 1b6dd43a20..f40511e9dc 100644 --- a/pkg/model/iam.go +++ b/pkg/model/iam.go @@ -36,6 +36,7 @@ type IAMModelBuilder struct { *KopsModelContext Lifecycle *fi.Lifecycle + Cluster *kops.Cluster } var _ fi.ModelBuilder = &IAMModelBuilder{} @@ -76,8 +77,10 @@ func (b *IAMModelBuilder) Build(c *fi.ModelBuilderContext) error { // Generate IAM tasks for each shared role for profileARN, igRole := range sharedProfileARNsToIGRole { lchPermissions := false + defaultWarmPool := b.Cluster.Spec.WarmPool for _, ig := range b.InstanceGroups { - if ig.Spec.Role == igRole && ig.Spec.WarmPool.IsEnabled() && ig.Spec.WarmPool.EnableLifecycleHook { + warmPool := defaultWarmPool.ResolveDefaults(ig) + if ig.Spec.Role == igRole && warmPool.IsEnabled() && warmPool.EnableLifecycleHook { lchPermissions = true break @@ -99,16 +102,18 @@ func (b *IAMModelBuilder) Build(c *fi.ModelBuilderContext) error { } // Generate IAM tasks for each managed role + defaultWarmPool := b.Cluster.Spec.WarmPool for igRole := range managedRoles { - warmPool := false + haveWarmPool := false for _, ig := range b.InstanceGroups { - if ig.Spec.Role == igRole && ig.Spec.WarmPool.IsEnabled() && ig.Spec.WarmPool.EnableLifecycleHook { - warmPool = true + warmPool := defaultWarmPool.ResolveDefaults(ig) + if ig.Spec.Role == igRole && warmPool.IsEnabled() && warmPool.EnableLifecycleHook { + haveWarmPool = true break } } - role, err := iam.BuildNodeRoleSubject(igRole, warmPool) + role, err := iam.BuildNodeRoleSubject(igRole, haveWarmPool) if err != nil { return err } diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index f39345be0b..0add59a5e8 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/upup/pkg/fi/cloudup/apply_cluster.go @@ -554,7 +554,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error { &model.FirewallModelBuilder{KopsModelContext: modelContext, Lifecycle: &securityLifecycle}, &model.SSHKeyModelBuilder{KopsModelContext: modelContext, Lifecycle: &securityLifecycle}, &model.NetworkModelBuilder{KopsModelContext: modelContext, Lifecycle: &networkLifecycle}, - &model.IAMModelBuilder{KopsModelContext: modelContext, Lifecycle: &securityLifecycle}, + &model.IAMModelBuilder{KopsModelContext: modelContext, Lifecycle: &securityLifecycle, Cluster: cluster}, &awsmodel.OIDCProviderBuilder{KopsModelContext: modelContext, Lifecycle: &securityLifecycle, KeyStore: keyStore}, ) @@ -563,6 +563,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error { BootstrapScriptBuilder: bootstrapScriptBuilder, Lifecycle: &clusterLifecycle, SecurityLifecycle: &securityLifecycle, + Cluster: cluster, } if featureflag.Spotinst.Enabled() { diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go index 69cccdd49c..b0eb3a388f 100644 --- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go +++ b/upup/pkg/fi/cloudup/bootstrapchannelbuilder/bootstrapchannelbuilder.go @@ -424,7 +424,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann if b.UseServiceAccountIAM() { serviceAccountRoles := []iam.Subject{&dnscontroller.ServiceAccount{}} for _, serviceAccountRole := range serviceAccountRoles { - iamModelBuilder := &model.IAMModelBuilder{KopsModelContext: b.KopsModelContext, Lifecycle: b.Lifecycle} + iamModelBuilder := &model.IAMModelBuilder{KopsModelContext: b.KopsModelContext, Lifecycle: b.Lifecycle, Cluster: b.Cluster} err := iamModelBuilder.BuildServiceAccountRoleTasks(serviceAccountRole, c) if err != nil { @@ -582,7 +582,7 @@ func (b *BootstrapChannelBuilder) buildAddons(c *fi.ModelBuilderContext) (*chann if b.UseServiceAccountIAM() { serviceAccountRoles := []iam.Subject{&awsloadbalancercontroller.ServiceAccount{}} for _, serviceAccountRole := range serviceAccountRoles { - iamModelBuilder := &model.IAMModelBuilder{KopsModelContext: b.KopsModelContext, Lifecycle: b.Lifecycle} + iamModelBuilder := &model.IAMModelBuilder{KopsModelContext: b.KopsModelContext, Lifecycle: b.Lifecycle, Cluster: b.Cluster} err := iamModelBuilder.BuildServiceAccountRoleTasks(serviceAccountRole, c) if err != nil { diff --git a/upup/pkg/fi/nodeup/command.go b/upup/pkg/fi/nodeup/command.go index 20fb5df37f..33ab90a7c6 100644 --- a/upup/pkg/fi/nodeup/command.go +++ b/upup/pkg/fi/nodeup/command.go @@ -362,7 +362,8 @@ func (c *NodeUpCommand) Run(out io.Writer) error { klog.Exitf("error closing target: %v", err) } - if modelContext.InstanceGroup.Spec.WarmPool.IsEnabled() && modelContext.InstanceGroup.Spec.WarmPool.EnableLifecycleHook { + warmPool := c.cluster.Spec.WarmPool.ResolveDefaults(modelContext.InstanceGroup) + if warmPool.IsEnabled() && warmPool.EnableLifecycleHook { if api.CloudProviderID(c.cluster.Spec.CloudProvider) == api.CloudProviderAWS { err := completeWarmingLifecycleAction(cloud.(awsup.AWSCloud), modelContext) if err != nil {