From 3985d63c9f370e8a87f757f79515d989c23b0ff1 Mon Sep 17 00:00:00 2001 From: Adrian Barsan Date: Tue, 2 Feb 2021 14:12:04 +0100 Subject: [PATCH] Use apimachinery 'resource.Quantity' type for the AzureMachinePool.SpotVMOptions.MaxPrice property By using a quantity type, which is a fixed-point representation of a number, we are able to not lose precision when serializing floats. Also, when applying CRs directly, we have the liberty to use stringified floats, as well as integers or the quantity serialization format. --- api/v1alpha3/azuremachine_types.go | 4 ++-- api/v1alpha3/zz_generated.deepcopy.go | 4 ++-- cloud/converters/spotinstances.go | 2 +- cloud/services/scalesets/scalesets_test.go | 24 +++++++++++++++++++ ...re.cluster.x-k8s.io_azuremachinepools.yaml | 6 ++++- ...ucture.cluster.x-k8s.io_azuremachines.yaml | 6 ++++- ...luster.x-k8s.io_azuremachinetemplates.yaml | 6 ++++- 7 files changed, 44 insertions(+), 8 deletions(-) diff --git a/api/v1alpha3/azuremachine_types.go b/api/v1alpha3/azuremachine_types.go index cd058dd4d34..7d207d97466 100644 --- a/api/v1alpha3/azuremachine_types.go +++ b/api/v1alpha3/azuremachine_types.go @@ -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" @@ -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 diff --git a/api/v1alpha3/zz_generated.deepcopy.go b/api/v1alpha3/zz_generated.deepcopy.go index 09d710aef90..a67b1432764 100644 --- a/api/v1alpha3/zz_generated.deepcopy.go +++ b/api/v1alpha3/zz_generated.deepcopy.go @@ -959,8 +959,8 @@ func (in *SpotVMOptions) DeepCopyInto(out *SpotVMOptions) { *out = *in if in.MaxPrice != nil { in, out := &in.MaxPrice, &out.MaxPrice - *out = new(string) - **out = **in + x := (*in).DeepCopy() + *out = &x } } diff --git a/cloud/converters/spotinstances.go b/cloud/converters/spotinstances.go index e8d505354b3..e8b6ea65786 100644 --- a/cloud/converters/spotinstances.go +++ b/cloud/converters/spotinstances.go @@ -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 } diff --git a/cloud/services/scalesets/scalesets_test.go b/cloud/services/scalesets/scalesets_test.go index f77df20843e..e5a1fe2023c 100644 --- a/cloud/services/scalesets/scalesets_test.go +++ b/cloud/services/scalesets/scalesets_test.go @@ -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" @@ -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", diff --git a/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml b/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml index 951fcb1f8bf..db6c9636354 100644 --- a/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml +++ b/config/crd/bases/exp.infrastructure.cluster.x-k8s.io_azuremachinepools.yaml @@ -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 diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml index 6cf678d9645..85bf797cd4d 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml @@ -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 diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml index c5c60304c2b..3aa180924d1 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml @@ -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