Merge pull request #4126 from nojnhuh/aks-spot-diff

default spec.spotMaxPrice in AzureManagedMachinePool
kubernetes-sigs · Oct 13, 2023 · 8b3a1cf · 8b3a1cf
2 parents 30d08b2 + 6856444
commit 8b3a1cf
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 1 deletion.
diff --git a/api/v1beta1/azuremanagedmachinepool_webhook.go b/api/v1beta1/azuremanagedmachinepool_webhook.go
@@ -25,8 +25,10 @@ import (
 	"strings"
 	"unicode"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
 	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -80,6 +82,10 @@ func (mw *azureManagedMachinePoolWebhook) Default(ctx context.Context, obj runti
 		m.Spec.OSType = ptr.To(DefaultOSType)
 	}
 
+	if ptr.Deref(m.Spec.ScaleSetPriority, "") == string(armcontainerservice.ScaleSetPrioritySpot) && m.Spec.SpotMaxPrice == nil {
+		m.Spec.SpotMaxPrice = ptr.To(resource.MustParse("-1"))
+	}
+
 	return nil
 }
 

diff --git a/api/v1beta1/azuremanagedmachinepool_webhook_test.go b/api/v1beta1/azuremanagedmachinepool_webhook_test.go
@@ -22,6 +22,7 @@ import (
 
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
 	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilfeature "k8s.io/component-base/featuregate/testing"
@@ -59,6 +60,7 @@ func TestAzureManagedMachinePoolDefaultingWebhook(t *testing.T) {
 	g.Expect(val).To(Equal("System"))
 	g.Expect(*ammp.Spec.Name).To(Equal("fooname"))
 	g.Expect(*ammp.Spec.OSType).To(Equal(LinuxOS))
+	g.Expect(ammp.Spec.SpotMaxPrice).To(BeNil())
 
 	t.Logf("Testing ammp defaulting webhook with empty string name specified in Spec")
 	emptyName := ""
@@ -80,6 +82,18 @@ func TestAzureManagedMachinePoolDefaultingWebhook(t *testing.T) {
 	err = mw.Default(context.Background(), ammp)
 	g.Expect(err).NotTo(HaveOccurred())
 	g.Expect(*ammp.Spec.OsDiskType).To(Equal("Ephemeral"))
+
+	t.Logf("Testing ammp defaulting webhook with scaleSetPriority Spot")
+	ammp.Spec.ScaleSetPriority = ptr.To(string(armcontainerservice.ScaleSetPrioritySpot))
+	err = mw.Default(context.Background(), ammp)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(ammp.Spec.SpotMaxPrice.Equal(resource.MustParse("-1"))).To(BeTrue())
+
+	t.Logf("Testing ammp defaulting webhook with scaleSetPriority Spot with max price set")
+	ammp.Spec.SpotMaxPrice = ptr.To(resource.MustParse("100"))
+	err = mw.Default(context.Background(), ammp)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(ammp.Spec.SpotMaxPrice.Equal(resource.MustParse("100"))).To(BeTrue())
 }
 
 func TestAzureManagedMachinePoolUpdatingWebhook(t *testing.T) {

diff --git a/test/e2e/aks_spot.go b/test/e2e/aks_spot.go
@@ -22,6 +22,7 @@ package e2e
 import (
 	"context"
 
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerservice/armcontainerservice/v4"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
@@ -67,7 +68,7 @@ func AKSSpotSpec(ctx context.Context, inputGetter func() AKSSpotSpecInput) {
 		Spec: infrav1.AzureManagedMachinePoolSpec{
 			Mode:             "User",
 			SKU:              "Standard_D2s_v3",
-			ScaleSetPriority: ptr.To("Spot"),
+			ScaleSetPriority: ptr.To(string(armcontainerservice.ScaleSetPrioritySpot)),
 			Scaling:          &scaling,
 			SpotMaxPrice:     &spotMaxPrice,
 			ScaleDownMode:    ptr.To("Deallocate"),