Skip to content

Commit 005a765

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #548 from athiruma/OCPBUGS-54567
OCPBUGS-54567: UPSTREAM: 5374: add marketType field validation
2 parents 2d79247 + 78b7f66 commit 005a765

9 files changed

+144
-0
lines changed

‎api/v1beta2/awsmachine_types.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ const (
6464
)
6565

6666
// AWSMachineSpec defines the desired state of an Amazon EC2 instance.
67+
// +kubebuilder:validation:XValidation:rule="!has(self.capacityReservationId) || !has(self.marketType) || self.marketType != 'Spot'",message="capacityReservationId may not be set when marketType is Spot"
68+
// +kubebuilder:validation:XValidation:rule="!has(self.capacityReservationId) || !has(self.spotMarketOptions)",message="capacityReservationId cannot be set when spotMarketOptions is specified"
6769
type AWSMachineSpec struct {
6870
// ProviderID is the unique identifier as specified by the cloud provider.
6971
ProviderID *string `json:"providerID,omitempty"`

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,6 +1104,13 @@ spec:
11041104
required:
11051105
- instanceType
11061106
type: object
1107+
x-kubernetes-validations:
1108+
- message: capacityReservationId may not be set when marketType is Spot
1109+
rule: '!has(self.capacityReservationId) || !has(self.marketType) ||
1110+
self.marketType != ''Spot'''
1111+
- message: capacityReservationId cannot be set when spotMarketOptions
1112+
is specified
1113+
rule: '!has(self.capacityReservationId) || !has(self.spotMarketOptions)'
11071114
status:
11081115
description: AWSMachineStatus defines the observed state of AWSMachine.
11091116
properties:

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,14 @@ spec:
10451045
required:
10461046
- instanceType
10471047
type: object
1048+
x-kubernetes-validations:
1049+
- message: capacityReservationId may not be set when marketType
1050+
is Spot
1051+
rule: '!has(self.capacityReservationId) || !has(self.marketType)
1052+
|| self.marketType != ''Spot'''
1053+
- message: capacityReservationId cannot be set when spotMarketOptions
1054+
is specified
1055+
rule: '!has(self.capacityReservationId) || !has(self.spotMarketOptions)'
10481056
required:
10491057
- spec
10501058
type: object

‎exp/api/v1beta2/awsmachinepool_webhook.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ limitations under the License.
1717
package v1beta2
1818

1919
import (
20+
"fmt"
2021
"time"
2122

2223
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -201,6 +202,18 @@ func (r *AWSMachinePool) validateInstanceMarketType() field.ErrorList {
201202
if r.Spec.AWSLaunchTemplate.MarketType == v1beta2.MarketTypeCapacityBlock && r.Spec.AWSLaunchTemplate.CapacityReservationID == nil {
202203
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec.awsLaunchTemplate.capacityReservationID"), "is required when CapacityBlock is provided"))
203204
}
205+
switch r.Spec.AWSLaunchTemplate.MarketType {
206+
case "", v1beta2.MarketTypeOnDemand, v1beta2.MarketTypeSpot, v1beta2.MarketTypeCapacityBlock:
207+
default:
208+
allErrs = append(allErrs, field.Invalid(field.NewPath("spec.awsLaunchTemplate.MarketType"), r.Spec.AWSLaunchTemplate.MarketType, fmt.Sprintf("Valid values are: %s, %s, %s and omitted", v1beta2.MarketTypeOnDemand, v1beta2.MarketTypeSpot, v1beta2.MarketTypeCapacityBlock)))
209+
}
210+
if r.Spec.AWSLaunchTemplate.MarketType == v1beta2.MarketTypeSpot && r.Spec.AWSLaunchTemplate.CapacityReservationID != nil {
211+
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec.awsLaunchTemplate.marketType"), "cannot be set to 'Spot' when CapacityReservationID is specified"))
212+
}
213+
214+
if r.Spec.AWSLaunchTemplate.CapacityReservationID != nil && r.Spec.AWSLaunchTemplate.SpotMarketOptions != nil {
215+
allErrs = append(allErrs, field.Forbidden(field.NewPath("spec.awsLaunchTemplate.spotMarketOptions"), "cannot be set to when CapacityReservationID is specified"))
216+
}
204217

205218
return allErrs
206219
}

‎exp/api/v1beta2/awsmachinepool_webhook_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,53 @@ func TestAWSMachinePoolValidateCreate(t *testing.T) {
174174
},
175175
wantErr: true,
176176
},
177+
{
178+
name: "with invalid MarketType provided",
179+
pool: &AWSMachinePool{
180+
Spec: AWSMachinePoolSpec{
181+
AWSLaunchTemplate: AWSLaunchTemplate{
182+
MarketType: "invalid",
183+
},
184+
},
185+
},
186+
wantErr: true,
187+
},
188+
{
189+
name: "with MarketType empty value provided",
190+
pool: &AWSMachinePool{
191+
Spec: AWSMachinePoolSpec{
192+
AWSLaunchTemplate: AWSLaunchTemplate{
193+
MarketType: "",
194+
},
195+
},
196+
},
197+
wantErr: false,
198+
},
199+
{
200+
name: "with MarketType Spot and CapacityReservationID value provided",
201+
pool: &AWSMachinePool{
202+
Spec: AWSMachinePoolSpec{
203+
AWSLaunchTemplate: AWSLaunchTemplate{
204+
MarketType: infrav1.MarketTypeSpot,
205+
CapacityReservationID: aws.String("cr-123"),
206+
},
207+
},
208+
},
209+
wantErr: true,
210+
},
211+
{
212+
name: "with CapacityReservationID and SpotMarketOptions value provided",
213+
pool: &AWSMachinePool{
214+
Spec: AWSMachinePoolSpec{
215+
AWSLaunchTemplate: AWSLaunchTemplate{
216+
SpotMarketOptions: &infrav1.SpotMarketOptions{},
217+
CapacityReservationID: aws.String("cr-123"),
218+
},
219+
},
220+
},
221+
wantErr: true,
222+
},
223+
177224
{
178225
name: "invalid, MarketType set to MarketTypeCapacityBlock and spotMarketOptions are specified",
179226
pool: &AWSMachinePool{

‎openshift/infrastructure-components-openshift.yaml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,6 +1586,13 @@ spec:
15861586
required:
15871587
- instanceType
15881588
type: object
1589+
x-kubernetes-validations:
1590+
- message: capacityReservationId may not be set when marketType is Spot
1591+
rule: '!has(self.capacityReservationId) || !has(self.marketType) ||
1592+
self.marketType != ''Spot'''
1593+
- message: capacityReservationId cannot be set when spotMarketOptions
1594+
is specified
1595+
rule: '!has(self.capacityReservationId) || !has(self.spotMarketOptions)'
15891596
status:
15901597
description: AWSMachineStatus defines the observed state of AWSMachine.
15911598
properties:
@@ -6189,6 +6196,14 @@ spec:
61896196
required:
61906197
- instanceType
61916198
type: object
6199+
x-kubernetes-validations:
6200+
- message: capacityReservationId may not be set when marketType
6201+
is Spot
6202+
rule: '!has(self.capacityReservationId) || !has(self.marketType)
6203+
|| self.marketType != ''Spot'''
6204+
- message: capacityReservationId cannot be set when spotMarketOptions
6205+
is specified
6206+
rule: '!has(self.capacityReservationId) || !has(self.spotMarketOptions)'
61926207
required:
61936208
- spec
61946209
type: object

‎openshift/manifests/0000_30_cluster-api_04_cm.infrastructure-aws.yaml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1588,6 +1588,13 @@ data:
15881588
required:
15891589
- instanceType
15901590
type: object
1591+
x-kubernetes-validations:
1592+
- message: capacityReservationId may not be set when marketType is Spot
1593+
rule: '!has(self.capacityReservationId) || !has(self.marketType) ||
1594+
self.marketType != ''Spot'''
1595+
- message: capacityReservationId cannot be set when spotMarketOptions
1596+
is specified
1597+
rule: '!has(self.capacityReservationId) || !has(self.spotMarketOptions)'
15911598
status:
15921599
description: AWSMachineStatus defines the observed state of AWSMachine.
15931600
properties:
@@ -6191,6 +6198,14 @@ data:
61916198
required:
61926199
- instanceType
61936200
type: object
6201+
x-kubernetes-validations:
6202+
- message: capacityReservationId may not be set when marketType
6203+
is Spot
6204+
rule: '!has(self.capacityReservationId) || !has(self.marketType)
6205+
|| self.marketType != ''Spot'''
6206+
- message: capacityReservationId cannot be set when spotMarketOptions
6207+
is specified
6208+
rule: '!has(self.capacityReservationId) || !has(self.spotMarketOptions)'
61946209
required:
61956210
- spec
61966211
type: object

‎pkg/cloud/services/ec2/instances.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,6 +1163,10 @@ func getInstanceMarketOptionsRequest(i *infrav1.Instance) (*ec2.InstanceMarketOp
11631163
return nil, errors.New("can't create spot capacity-blocks, remove spot market request")
11641164
}
11651165

1166+
if (i.MarketType == infrav1.MarketTypeSpot || i.SpotMarketOptions != nil) && i.CapacityReservationID != nil {
1167+
return nil, errors.New("unable to generate marketOptions for spot instance, capacityReservationID is incompatible with marketType spot and spotMarketOptions")
1168+
}
1169+
11661170
// Infer MarketType if not explicitly set
11671171
if i.SpotMarketOptions != nil && i.MarketType == "" {
11681172
i.MarketType = infrav1.MarketTypeSpot
@@ -1172,6 +1176,10 @@ func getInstanceMarketOptionsRequest(i *infrav1.Instance) (*ec2.InstanceMarketOp
11721176
i.MarketType = infrav1.MarketTypeOnDemand
11731177
}
11741178

1179+
if i.MarketType == infrav1.MarketTypeSpot && i.SpotMarketOptions == nil {
1180+
i.SpotMarketOptions = &infrav1.SpotMarketOptions{}
1181+
}
1182+
11751183
switch i.MarketType {
11761184
case infrav1.MarketTypeCapacityBlock:
11771185
if i.CapacityReservationID == nil {

‎pkg/cloud/services/ec2/instances_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5863,6 +5863,35 @@ func TestGetInstanceMarketOptionsRequest(t *testing.T) {
58635863
},
58645864
expectedError: nil,
58655865
},
5866+
{
5867+
name: "with marketType Spot specified",
5868+
instance: &infrav1.Instance{
5869+
MarketType: infrav1.MarketTypeSpot,
5870+
},
5871+
expectedRequest: &ec2.InstanceMarketOptionsRequest{
5872+
MarketType: aws.String(ec2.MarketTypeSpot),
5873+
SpotOptions: &ec2.SpotMarketOptions{
5874+
InstanceInterruptionBehavior: aws.String(ec2.InstanceInterruptionBehaviorTerminate),
5875+
SpotInstanceType: aws.String(ec2.SpotInstanceTypeOneTime),
5876+
},
5877+
},
5878+
},
5879+
{
5880+
name: "with marketType Spot and capacityRerservationID specified",
5881+
instance: &infrav1.Instance{
5882+
MarketType: infrav1.MarketTypeSpot,
5883+
CapacityReservationID: mockCapacityReservationID,
5884+
},
5885+
expectedError: errors.Errorf("unable to generate marketOptions for spot instance, capacityReservationID is incompatible with marketType spot and spotMarketOptions"),
5886+
},
5887+
{
5888+
name: "with spotMarketOptions and capacityRerservationID specified",
5889+
instance: &infrav1.Instance{
5890+
SpotMarketOptions: &infrav1.SpotMarketOptions{},
5891+
CapacityReservationID: mockCapacityReservationID,
5892+
},
5893+
expectedError: errors.Errorf("unable to generate marketOptions for spot instance, capacityReservationID is incompatible with marketType spot and spotMarketOptions"),
5894+
},
58665895
{
58675896
name: "with an empty MaxPrice specified",
58685897
instance: &infrav1.Instance{

0 commit comments

Comments
 (0)