Skip to content

Commit

Permalink
Merge pull request #1157 from axbarsan/fix-amp-spot-maxprice-not-seri…
Browse files Browse the repository at this point in the history
…alizing

Fix AzureMachinePool/AzureMachine spot instances max price failing to serialize when using the client
  • Loading branch information
k8s-ci-robot authored Feb 2, 2021
2 parents 9f1dd0c + 3985d63 commit 8df2692
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 8 deletions.
4 changes: 2 additions & 2 deletions api/v1alpha3/azuremachine_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1alpha3

import (
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3"
"sigs.k8s.io/cluster-api/errors"
Expand Down Expand Up @@ -119,8 +120,7 @@ type AzureMachineSpec struct {
type SpotVMOptions struct {
// MaxPrice defines the maximum price the user is willing to pay for Spot VM instances
// +optional
// +kubebuilder:validation:Type=number
MaxPrice *string `json:"maxPrice,omitempty"`
MaxPrice *resource.Quantity `json:"maxPrice,omitempty"`
}

// AzureMachineStatus defines the observed state of AzureMachine
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha3/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cloud/converters/spotinstances.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func GetSpotVMOptions(spotVMOptions *infrav1.SpotVMOptions) (compute.VirtualMach
}
var billingProfile *compute.BillingProfile
if spotVMOptions.MaxPrice != nil {
maxPrice, err := strconv.ParseFloat(*spotVMOptions.MaxPrice, 64)
maxPrice, err := strconv.ParseFloat(spotVMOptions.MaxPrice.AsDec().String(), 64)
if err != nil {
return "", "", nil, err
}
Expand Down
24 changes: 24 additions & 0 deletions cloud/services/scalesets/scalesets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/Azure/go-autorest/autorest/to"
"github.com/golang/mock/gomock"
. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/klog/klogr"
Expand Down Expand Up @@ -331,6 +332,29 @@ func TestReconcileVMSS(t *testing.T) {
setupCreatingSucceededExpectations(s, m, putFuture)
},
},
{
name: "should start creating a vmss with spot vm and a maximum price",
expectedError: "failed to get VMSS my-vmss after create or update: failed to get result from future: operation type PUT on Azure resource my-rg/my-vmss is not done",
expect: func(g *WithT, s *mock_scalesets.MockScaleSetScopeMockRecorder, m *mock_scalesets.MockClientMockRecorder) {
spec := newDefaultVMSSSpec()
maxPrice := resource.MustParse("0.001")
spec.SpotVMOptions = &infrav1.SpotVMOptions{
MaxPrice: &maxPrice,
}
s.ScaleSetSpec().Return(spec).AnyTimes()
setupDefaultVMSSStartCreatingExpectations(s, m)
vmss := newDefaultVMSS()
vmss.VirtualMachineScaleSetProperties.VirtualMachineProfile.Priority = compute.Spot
vmss.VirtualMachineScaleSetProperties.VirtualMachineProfile.BillingProfile = &compute.BillingProfile{
MaxPrice: to.Float64Ptr(0.001),
}
vmss.VirtualMachineScaleSetProperties.VirtualMachineProfile.EvictionPolicy = compute.Deallocate
vmss = setHashOnVMSS(g, vmss)
m.CreateOrUpdateAsync(gomockinternal.AContext(), defaultResourceGroup, defaultVMSSName, gomockinternal.DiffEq(vmss)).
Return(putFuture, nil)
setupCreatingSucceededExpectations(s, m, putFuture)
},
},
{
name: "should start creating a vmss with encryption",
expectedError: "failed to get VMSS my-vmss after create or update: failed to get result from future: operation type PUT on Azure resource my-rg/my-vmss is not done",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,9 +312,13 @@ spec:
should use a Spot VM
properties:
maxPrice:
anyOf:
- type: integer
- type: string
description: MaxPrice defines the maximum price the user is
willing to pay for Spot VM instances
type: number
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
type: object
sshPublicKey:
description: SSHPublicKey is the SSH public key string base64
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,13 @@ spec:
should use a Spot VM
properties:
maxPrice:
anyOf:
- type: integer
- type: string
description: MaxPrice defines the maximum price the user is willing
to pay for Spot VM instances
type: number
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
type: object
sshPublicKey:
type: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -427,9 +427,13 @@ spec:
Machine should use a Spot VM
properties:
maxPrice:
anyOf:
- type: integer
- type: string
description: MaxPrice defines the maximum price the user
is willing to pay for Spot VM instances
type: number
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
type: object
sshPublicKey:
type: string
Expand Down

0 comments on commit 8df2692

Please sign in to comment.