Skip to content

Commit 71efb58

Browse files
moko-poimoko-poi
committed
Add deprecation notice and improve documentation for InstanceType field
1 parent e04437e commit 71efb58

File tree

4 files changed

+159
-8
lines changed

4 files changed

+159
-8
lines changed

‎config/crd/bases/infrastructure.cluster.x-k8s.io_awsmanagedmachinepools.yaml‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -931,15 +931,19 @@ spec:
931931
name of the managed machine pool.
932932
type: string
933933
instanceType:
934-
description: InstanceType specifies the AWS instance type
934+
description: |-
935+
InstanceType specifies the AWS instance type.
936+
This field is deprecated. Use InstanceTypes instead.
935937
type: string
936938
instanceTypes:
937939
description: |-
938940
InstanceTypes specifies a list of AWS instance types for the node group.
939-
When specified, this allows using multiple instance types which enhances
940-
the availability of Spot instances.
941+
The order of instance types specified determines priority of picking that instance type.
942+
This is also influenced by the CapacityType. For On-Demand capacity, the allocation strategy
943+
uses the order of instance types to determine which instance type to use first when fulfilling capacity.
944+
See AWS documentation https://docs.aws.amazon.com/eks/latest/userguide/managed-node-groups.html#managed-node-group-capacity-types
945+
for more information.
941946
At most one of InstanceType or InstanceTypes may be specified.
942-
See https://docs.aws.amazon.com/eks/latest/userguide/managed-node-groups.html#managed-node-group-capacity-types
943947
items:
944948
type: string
945949
type: array

‎exp/api/v1beta2/awsmanagedmachinepool_types.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -176,15 +176,18 @@ type AWSManagedMachinePoolSpec struct {
176176
// +optional
177177
DiskSize *int32 `json:"diskSize,omitempty"`
178178

179-
// InstanceType specifies the AWS instance type
179+
// InstanceType specifies the AWS instance type.
180+
// This field is deprecated. Use InstanceTypes instead.
180181
// +optional
181182
InstanceType *string `json:"instanceType,omitempty"`
182183

183184
// InstanceTypes specifies a list of AWS instance types for the node group.
184-
// When specified, this allows using multiple instance types which enhances
185-
// the availability of Spot instances.
185+
// The order of instance types specified determines priority of picking that instance type.
186+
// This is also influenced by the CapacityType. For On-Demand capacity, the allocation strategy
187+
// uses the order of instance types to determine which instance type to use first when fulfilling capacity.
188+
// See AWS documentation https://docs.aws.amazon.com/eks/latest/userguide/managed-node-groups.html#managed-node-group-capacity-types
189+
// for more information.
186190
// At most one of InstanceType or InstanceTypes may be specified.
187-
// See https://docs.aws.amazon.com/eks/latest/userguide/managed-node-groups.html#managed-node-group-capacity-types
188191
// +optional
189192
InstanceTypes []string `json:"instanceTypes,omitempty"`
190193

‎exp/api/v1beta2/awsmanagedmachinepool_webhook.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,11 @@ func (*awsManagedMachinePoolWebhook) ValidateCreate(_ context.Context, obj runti
200200

201201
allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
202202

203+
// Log deprecation warning for InstanceType field
204+
if r.Spec.InstanceType != nil {
205+
mmpLog.Info("spec.instanceType is deprecated, use spec.instanceTypes instead", "managed-machine-pool", klog.KObj(r))
206+
}
207+
203208
if len(allErrs) == 0 {
204209
return nil, nil
205210
}

‎exp/api/v1beta2/awsmanagedmachinepool_webhook_test.go‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,37 @@ func TestAWSManagedMachinePoolValidateCreate(t *testing.T) {
155155
},
156156
wantErr: false,
157157
},
158+
{
159+
name: "both instanceType and instanceTypes are specified",
160+
pool: &AWSManagedMachinePool{
161+
Spec: AWSManagedMachinePoolSpec{
162+
EKSNodegroupName: "eks-node-group-4",
163+
InstanceTypes: []string{"m5.xlarge", "m5.2xlarge"},
164+
InstanceType: ptr.To("m5.xlarge"),
165+
},
166+
},
167+
wantErr: true,
168+
},
169+
{
170+
name: "only instanceTypes is accepted",
171+
pool: &AWSManagedMachinePool{
172+
Spec: AWSManagedMachinePoolSpec{
173+
EKSNodegroupName: "eks-node-group-5",
174+
InstanceTypes: []string{"m5.xlarge", "m5.2xlarge"},
175+
},
176+
},
177+
wantErr: false,
178+
},
179+
{
180+
name: "only instanceType is accepted",
181+
pool: &AWSManagedMachinePool{
182+
Spec: AWSManagedMachinePoolSpec{
183+
EKSNodegroupName: "eks-node-group-6",
184+
InstanceType: ptr.To("m5.xlarge"),
185+
},
186+
},
187+
wantErr: false,
188+
},
158189
}
159190
for _, tt := range tests {
160191
t.Run(tt.name, func(t *testing.T) {
@@ -693,6 +724,114 @@ func TestAWSManagedMachinePoolValidateUpdate(t *testing.T) {
693724
},
694725
wantErr: false,
695726
},
727+
{
728+
name: "adding instanceType is rejected",
729+
old: &AWSManagedMachinePool{
730+
Spec: AWSManagedMachinePoolSpec{
731+
EKSNodegroupName: "eks-node-group-1",
732+
},
733+
},
734+
new: &AWSManagedMachinePool{
735+
Spec: AWSManagedMachinePoolSpec{
736+
EKSNodegroupName: "eks-node-group-1",
737+
InstanceType: ptr.To("m5.xlarge"),
738+
},
739+
},
740+
wantErr: true,
741+
},
742+
{
743+
name: "removing instanceType is rejected",
744+
old: &AWSManagedMachinePool{
745+
Spec: AWSManagedMachinePoolSpec{
746+
EKSNodegroupName: "eks-node-group-1",
747+
InstanceType: ptr.To("m5.xlarge"),
748+
},
749+
},
750+
new: &AWSManagedMachinePool{
751+
Spec: AWSManagedMachinePoolSpec{
752+
EKSNodegroupName: "eks-node-group-1",
753+
},
754+
},
755+
wantErr: true,
756+
},
757+
{
758+
name: "changing instanceType is rejected",
759+
old: &AWSManagedMachinePool{
760+
Spec: AWSManagedMachinePoolSpec{
761+
EKSNodegroupName: "eks-node-group-1",
762+
InstanceType: ptr.To("m5.xlarge"),
763+
},
764+
},
765+
new: &AWSManagedMachinePool{
766+
Spec: AWSManagedMachinePoolSpec{
767+
EKSNodegroupName: "eks-node-group-1",
768+
InstanceType: ptr.To("m5.2xlarge"),
769+
},
770+
},
771+
wantErr: true,
772+
},
773+
{
774+
name: "adding instanceTypes is rejected",
775+
old: &AWSManagedMachinePool{
776+
Spec: AWSManagedMachinePoolSpec{
777+
EKSNodegroupName: "eks-node-group-1",
778+
},
779+
},
780+
new: &AWSManagedMachinePool{
781+
Spec: AWSManagedMachinePoolSpec{
782+
EKSNodegroupName: "eks-node-group-1",
783+
InstanceTypes: []string{"m5.xlarge", "m5.2xlarge"},
784+
},
785+
},
786+
wantErr: true,
787+
},
788+
{
789+
name: "removing instanceTypes is rejected",
790+
old: &AWSManagedMachinePool{
791+
Spec: AWSManagedMachinePoolSpec{
792+
EKSNodegroupName: "eks-node-group-1",
793+
InstanceTypes: []string{"m5.xlarge", "m5.2xlarge"},
794+
},
795+
},
796+
new: &AWSManagedMachinePool{
797+
Spec: AWSManagedMachinePoolSpec{
798+
EKSNodegroupName: "eks-node-group-1",
799+
},
800+
},
801+
wantErr: true,
802+
},
803+
{
804+
name: "changing instanceTypes is rejected",
805+
old: &AWSManagedMachinePool{
806+
Spec: AWSManagedMachinePoolSpec{
807+
EKSNodegroupName: "eks-node-group-1",
808+
InstanceTypes: []string{"m5.xlarge", "m5.2xlarge"},
809+
},
810+
},
811+
new: &AWSManagedMachinePool{
812+
Spec: AWSManagedMachinePoolSpec{
813+
EKSNodegroupName: "eks-node-group-1",
814+
InstanceTypes: []string{"m5.xlarge", "m6.xlarge"},
815+
},
816+
},
817+
wantErr: true,
818+
},
819+
{
820+
name: "adding both instanceType and instanceTypes is rejected",
821+
old: &AWSManagedMachinePool{
822+
Spec: AWSManagedMachinePoolSpec{
823+
EKSNodegroupName: "eks-node-group-1",
824+
},
825+
},
826+
new: &AWSManagedMachinePool{
827+
Spec: AWSManagedMachinePoolSpec{
828+
EKSNodegroupName: "eks-node-group-1",
829+
InstanceType: ptr.To("m5.xlarge"),
830+
InstanceTypes: []string{"m5.xlarge", "m6.xlarge"},
831+
},
832+
},
833+
wantErr: true,
834+
},
696835
}
697836
for _, tt := range tests {
698837
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)