diff --git a/exp/api/v1beta1/gcpmanagedmachinepool_webhook_test.go b/exp/api/v1beta1/gcpmanagedmachinepool_webhook_test.go index 3dacba740..410e76937 100644 --- a/exp/api/v1beta1/gcpmanagedmachinepool_webhook_test.go +++ b/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()) }) - }) -}) + } +}