test: align managed mp webhook tests

Signed-off-by: Carlos Salas <[email protected]>

exp/api/v1beta1/gcpmanagedmachinepool_webhook_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2023 The Kubernetes Authors.
+Copyright 2024 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -18,149 +18,187 @@ package v1beta1
 
 import (
 	"strings"
+	"testing"
 
-	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	infrav1 "sigs.k8s.io/cluster-api-provider-gcp/api/v1beta1"
 )
 
-var gcpmmp *GCPManagedMachinePool
-
-var _ = Describe("Test GCPManagedMachinePool Webhooks", func() {
-	BeforeEach(func() {
-		machineType := "e2-medium"
-		diskSizeGb := int32(100)
+var (
+	minCount          = int32(1)
+	maxCount          = int32(3)
+	invalidMinCount   = int32(-1)
+	enableAutoscaling = false
+	diskSizeGb        = int32(200)
+	maxPods           = int64(10)
+	localSsds         = int32(0)
+	invalidDiskSizeGb = int32(-200)
+	invalidMaxPods    = int64(-10)
+	invalidLocalSsds  = int32(-0)
+)
 
-		gcpmmp = &GCPManagedMachinePool{
-			Spec: GCPManagedMachinePoolSpec{
-				NodePoolName: "test-gke-pool",
-				MachineType:  &machineType,
-				DiskSizeGb:   &diskSizeGb,
+func TestGCPManagedMachinePoolValidatingWebhookCreate(t *testing.T) {
+	tests := []struct {
+		name        string
+		spec        GCPManagedMachinePoolSpec
+		expectError bool
+	}{
+		{
+			name: "valid node pool name",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName: "nodepool1",
 			},
-		}
-	})
-
-	Context("Test validateSpec", func() {
-		It("should error when node pool name is too long", func() {
-			gcpmmp.Spec.NodePoolName = strings.Repeat("A", maxNodePoolNameLength+1)
-			errs := gcpmmp.validateSpec()
-			Expect(errs).ToNot(BeEmpty())
-		})
-		It("should pass when node pool name is within limit", func() {
-			gcpmmp.Spec.NodePoolName = strings.Repeat("A", maxNodePoolNameLength)
-			errs := gcpmmp.validateSpec()
-			Expect(errs).To(BeEmpty())
-		})
-	})
+			expectError: false,
+		},
+		{
+			name: "node pool name is too long",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName: strings.Repeat("A", maxNodePoolNameLength+1),
+			},
+			expectError: true,
+		},
+		{
+			name: "scaling with valid min/max count",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName: "nodepool1",
+				Scaling: &NodePoolAutoScaling{
+					MinCount: &minCount,
+					MaxCount: &maxCount,
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "scaling with invalid min/max count",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName: "nodepool1",
+				Scaling: &NodePoolAutoScaling{
+					MinCount: &invalidMinCount,
+					MaxCount: &maxCount,
+				},
+			},
+			expectError: true,
+		},
+		{
+			name: "scaling with max < min count",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName: "nodepool1",
+				Scaling: &NodePoolAutoScaling{
+					MinCount: &maxCount,
+					MaxCount: &minCount,
+				},
+			},
+			expectError: true,
+		},
+		{
+			name: "autoscaling disabled and min/max provided",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName: "nodepool1",
+				Scaling: &NodePoolAutoScaling{
+					EnableAutoscaling: &enableAutoscaling,
+					MinCount:          &minCount,
+					MaxCount:          &maxCount,
+				},
+			},
+			expectError: true,
+		},
+		{
+			name: "valid non-negative values",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName:   "nodepool1",
+				DiskSizeGb:     &diskSizeGb,
+				MaxPodsPerNode: &maxPods,
+				LocalSsdCount:  &localSsds,
+			},
+			expectError: false,
+		},
+		{
+			name: "invalid negative values",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName:   "nodepool1",
+				DiskSizeGb:     &invalidDiskSizeGb,
+				MaxPodsPerNode: &invalidMaxPods,
+				LocalSsdCount:  &invalidLocalSsds,
+			},
+			expectError: true,
+		},
+	}
 
-	Context("Test validateScaling", func() {
-		It("should pass when scaling is not specified", func() {
-			errs := gcpmmp.validateScaling()
-			Expect(errs).To(BeEmpty())
-		})
-		It("should pass when min/max count is valid", func() {
-			minCount := int32(1)
-			maxCount := int32(3)
-			gcpmmp.Spec.Scaling = &NodePoolAutoScaling{
-				MinCount: &minCount,
-				MaxCount: &maxCount,
-			}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
 
-			errs := gcpmmp.validateScaling()
-			Expect(errs).To(BeEmpty())
-		})
-		It("should fail when min is negative", func() {
-			minCount := int32(-1)
-			gcpmmp.Spec.Scaling = &NodePoolAutoScaling{
-				MinCount: &minCount,
+			mmp := &GCPManagedMachinePool{
+				Spec: tc.spec,
 			}
+			warn, err := mmp.ValidateCreate()
 
-			errs := gcpmmp.validateScaling()
-			Expect(errs).ToNot(BeEmpty())
-		})
-		It("should fail when min > max", func() {
-			minCount := int32(3)
-			maxCount := int32(1)
-			gcpmmp.Spec.Scaling = &NodePoolAutoScaling{
-				MinCount: &minCount,
-				MaxCount: &maxCount,
+			if tc.expectError {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).ToNot(HaveOccurred())
 			}
-
-			errs := gcpmmp.validateScaling()
-			Expect(errs).ToNot(BeEmpty())
+			// Nothing emits warnings yet
+			g.Expect(warn).To(BeEmpty())
 		})
-		It("should fail when autoscaling is disabled and min/max is specified", func() {
-			minCount := int32(1)
-			maxCount := int32(3)
-			enabled := false
-			locationPolicy := ManagedNodePoolLocationPolicyAny
-			gcpmmp.Spec.Scaling = &NodePoolAutoScaling{
-				MinCount:          &minCount,
-				MaxCount:          &maxCount,
-				EnableAutoscaling: &enabled,
-				LocationPolicy:    &locationPolicy,
-			}
+	}
+}
+
+func TestGCPManagedMachinePoolValidatingWebhookUpdate(t *testing.T) {
+	tests := []struct {
+		name        string
+		spec        GCPManagedMachinePoolSpec
+		expectError bool
+	}{
+		{
+			name: "node pool is not mutated",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName: "nodepool1",
+			},
+			expectError: false,
+		},
+		{
+			name: "mutable fields are mutated",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName: "nodepool1",
+				AdditionalLabels: infrav1.Labels{
+					"testKey": "testVal",
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "immutable field disk size is mutated",
+			spec: GCPManagedMachinePoolSpec{
+				NodePoolName: "nodepool1",
+				DiskSizeGb:   &diskSizeGb,
+			},
+			expectError: true,
+		},
+	}
 
-			errs := gcpmmp.validateScaling()
-			Expect(errs).To(HaveLen(3))
-		})
-	})
-	Context("Test validateImmutable", func() {
-		It("should pass when node pool is not mutated", func() {
-			old := gcpmmp.DeepCopy()
-			errs := gcpmmp.validateImmutable(old)
-			Expect(errs).To(BeEmpty())
-		})
-		It("should pass when mutable fields are mutated", func() {
-			old := gcpmmp.DeepCopy()
-			gcpmmp.Spec.AdditionalLabels = infrav1.Labels{
-				"testKey": "testVal",
-			}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
 
-			errs := gcpmmp.validateImmutable(old)
-			Expect(errs).To(BeEmpty())
-		})
-		It("should fail when immutable fields are mutated", func() {
-			old := gcpmmp.DeepCopy()
-			diskSizeGb := int32(200)
-			gcpmmp.Spec.DiskSizeGb = &diskSizeGb
-			gcpmmp.Spec.NodePoolName = "new-name"
-			gcpmmp.Spec.Management = &NodePoolManagement{
-				AutoUpgrade: false,
-				AutoRepair:  false,
+			newMMP := &GCPManagedMachinePool{
+				Spec: tc.spec,
+			}
+			oldMMP := &GCPManagedMachinePool{
+				Spec: GCPManagedMachinePoolSpec{
+					NodePoolName: "nodepool1",
+				},
 			}
 
-			errs := gcpmmp.validateImmutable(old)
-			Expect(errs).To(HaveLen(3))
-		})
-	})
+			warn, err := newMMP.ValidateUpdate(oldMMP)
 
-	Context("Test validateNonNegative", func() {
-		It("should pass when number fields are not specified", func() {
-			errs := gcpmmp.validateNonNegative()
-			Expect(errs).To(BeEmpty())
-		})
-		It("should pass when number fields are non-negative", func() {
-			maxPods := int64(10)
-			localSsds := int32(0)
-			diskSize := int32(200)
-			gcpmmp.Spec.MaxPodsPerNode = &maxPods
-			gcpmmp.Spec.LocalSsdCount = &localSsds
-			gcpmmp.Spec.DiskSizeGb = &diskSize
-
-			errs := gcpmmp.validateNonNegative()
-			Expect(errs).To(BeEmpty())
-		})
-		It("should pass when some number fields are negative", func() {
-			maxPods := int64(-1)
-			localSsds := int32(0)
-			diskSize := int32(-100)
-			gcpmmp.Spec.MaxPodsPerNode = &maxPods
-			gcpmmp.Spec.LocalSsdCount = &localSsds
-			gcpmmp.Spec.DiskSizeGb = &diskSize
-
-			errs := gcpmmp.validateNonNegative()
-			Expect(errs).To(HaveLen(2))
+			if tc.expectError {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).ToNot(HaveOccurred())
+			}
+			// Nothing emits warnings yet
+			g.Expect(warn).To(BeEmpty())
 		})
-	})
-})
+	}
+}