Merge pull request #4656 from giantswarm/validate-spot-mixed-instances

✨ Prevent users setting values that are invalid configuration on AWS
kubernetes-sigs · Nov 28, 2023 · b4506f8 · b4506f8
2 parents a1b6f35 + 99ddc99
commit b4506f8
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 0 deletions.
diff --git a/exp/api/v1beta2/awsmachinepool_webhook.go b/exp/api/v1beta2/awsmachinepool_webhook.go
@@ -108,6 +108,13 @@ func (r *AWSMachinePool) validateAdditionalSecurityGroups() field.ErrorList {
 	}
 	return allErrs
 }
+func (r *AWSMachinePool) validateSpotInstances() field.ErrorList {
+	var allErrs field.ErrorList
+	if r.Spec.AWSLaunchTemplate.SpotMarketOptions != nil && r.Spec.MixedInstancesPolicy != nil {
+		allErrs = append(allErrs, field.Forbidden(field.NewPath("spec.awsLaunchTemplate.spotMarketOptions"), "either spec.awsLaunchTemplate.spotMarketOptions or spec.mixedInstancesPolicy should be used"))
+	}
+	return allErrs
+}
 
 // ValidateCreate will do any extra validation when creating a AWSMachinePool.
 func (r *AWSMachinePool) ValidateCreate() (admission.Warnings, error) {
@@ -120,6 +127,7 @@ func (r *AWSMachinePool) ValidateCreate() (admission.Warnings, error) {
 	allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
 	allErrs = append(allErrs, r.validateSubnets()...)
 	allErrs = append(allErrs, r.validateAdditionalSecurityGroups()...)
+	allErrs = append(allErrs, r.validateSpotInstances()...)
 
 	if len(allErrs) == 0 {
 		return nil, nil
@@ -140,6 +148,7 @@ func (r *AWSMachinePool) ValidateUpdate(old runtime.Object) (admission.Warnings,
 	allErrs = append(allErrs, r.Spec.AdditionalTags.Validate()...)
 	allErrs = append(allErrs, r.validateSubnets()...)
 	allErrs = append(allErrs, r.validateAdditionalSecurityGroups()...)
+	allErrs = append(allErrs, r.validateSpotInstances()...)
 
 	if len(allErrs) == 0 {
 		return nil, nil

diff --git a/exp/api/v1beta2/awsmachinepool_webhook_test.go b/exp/api/v1beta2/awsmachinepool_webhook_test.go
@@ -139,6 +139,20 @@ func TestAWSMachinePoolValidateCreate(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "Should fail if both spot market options or mixed instances policy are set",
+			pool: &AWSMachinePool{
+				Spec: AWSMachinePoolSpec{
+					MixedInstancesPolicy: &MixedInstancesPolicy{
+						Overrides: []Overrides{{InstanceType: "t3.medium"}},
+					},
+					AWSLaunchTemplate: AWSLaunchTemplate{
+						SpotMarketOptions: &infrav1.SpotMarketOptions{MaxPrice: aws.String("0.1")},
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -252,6 +266,27 @@ func TestAWSMachinePoolValidateUpdate(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "Should fail update if both spec.awsLaunchTemplate.SpotMarketOptions and spec.MixedInstancesPolicy are passed in AWSMachinePool spec",
+			old: &AWSMachinePool{
+				Spec: AWSMachinePoolSpec{
+					MixedInstancesPolicy: &MixedInstancesPolicy{
+						Overrides: []Overrides{{InstanceType: "t3.medium"}},
+					},
+				},
+			},
+			new: &AWSMachinePool{
+				Spec: AWSMachinePoolSpec{
+					MixedInstancesPolicy: &MixedInstancesPolicy{
+						Overrides: []Overrides{{InstanceType: "t3.medium"}},
+					},
+					AWSLaunchTemplate: AWSLaunchTemplate{
+						SpotMarketOptions: &infrav1.SpotMarketOptions{MaxPrice: pointer.String("0.1")},
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {