From c72af29e846ac5adce0a2965670f95e80b68c782 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Mon, 30 Jun 2025 15:44:00 +0200 Subject: [PATCH] Use MachineSetDeletePolicy enum in MD & MS API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stefan Büringer buringerst@vmware.com --- api/core/v1beta1/zz_generated.conversion.go | 6 +++--- api/core/v1beta2/machinedeployment_types.go | 3 +-- api/core/v1beta2/machineset_types.go | 4 ++-- api/core/v1beta2/zz_generated.deepcopy.go | 2 +- .../api/core/v1alpha3/zz_generated.conversion.go | 4 ++-- .../api/core/v1alpha4/zz_generated.conversion.go | 6 +++--- .../machinedeployment_controller_test.go | 12 ++++++------ .../machinedeployment/machinedeployment_sync_test.go | 12 ++++++------ .../machineset/machineset_delete_policy.go | 4 ++-- internal/webhooks/machineset.go | 3 +-- internal/webhooks/machineset_test.go | 2 +- test/e2e/clusterclass_rollout.go | 2 +- 12 files changed, 29 insertions(+), 31 deletions(-) diff --git a/api/core/v1beta1/zz_generated.conversion.go b/api/core/v1beta1/zz_generated.conversion.go index 24a279a41eca..8e2b740f216e 100644 --- a/api/core/v1beta1/zz_generated.conversion.go +++ b/api/core/v1beta1/zz_generated.conversion.go @@ -3307,7 +3307,7 @@ func Convert_v1beta2_MachineReadinessGate_To_v1beta1_MachineReadinessGate(in *v1 func autoConvert_v1beta1_MachineRollingUpdateDeployment_To_v1beta2_MachineRollingUpdateDeployment(in *MachineRollingUpdateDeployment, out *v1beta2.MachineRollingUpdateDeployment, s conversion.Scope) error { out.MaxUnavailable = (*intstr.IntOrString)(unsafe.Pointer(in.MaxUnavailable)) out.MaxSurge = (*intstr.IntOrString)(unsafe.Pointer(in.MaxSurge)) - out.DeletePolicy = (*string)(unsafe.Pointer(in.DeletePolicy)) + out.DeletePolicy = (*v1beta2.MachineSetDeletePolicy)(unsafe.Pointer(in.DeletePolicy)) return nil } @@ -3406,7 +3406,7 @@ func autoConvert_v1beta1_MachineSetSpec_To_v1beta2_MachineSetSpec(in *MachineSet out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) // WARNING: in.MinReadySeconds requires manual conversion: does not exist in peer-type - out.DeletePolicy = in.DeletePolicy + out.DeletePolicy = v1beta2.MachineSetDeletePolicy(in.DeletePolicy) out.Selector = in.Selector if err := Convert_v1beta1_MachineTemplateSpec_To_v1beta2_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err @@ -3418,7 +3418,7 @@ func autoConvert_v1beta1_MachineSetSpec_To_v1beta2_MachineSetSpec(in *MachineSet func autoConvert_v1beta2_MachineSetSpec_To_v1beta1_MachineSetSpec(in *v1beta2.MachineSetSpec, out *MachineSetSpec, s conversion.Scope) error { out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) - out.DeletePolicy = in.DeletePolicy + out.DeletePolicy = string(in.DeletePolicy) out.Selector = in.Selector if err := Convert_v1beta2_MachineTemplateSpec_To_v1beta1_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err diff --git a/api/core/v1beta2/machinedeployment_types.go b/api/core/v1beta2/machinedeployment_types.go index 5aecd24682d4..b53194a123b7 100644 --- a/api/core/v1beta2/machinedeployment_types.go +++ b/api/core/v1beta2/machinedeployment_types.go @@ -359,9 +359,8 @@ type MachineRollingUpdateDeployment struct { // deletePolicy defines the policy used by the MachineDeployment to identify nodes to delete when downscaling. // Valid values are "Random, "Newest", "Oldest" // When no value is supplied, the default DeletePolicy of MachineSet is used - // +kubebuilder:validation:Enum=Random;Newest;Oldest // +optional - DeletePolicy *string `json:"deletePolicy,omitempty"` + DeletePolicy *MachineSetDeletePolicy `json:"deletePolicy,omitempty"` } // ANCHOR_END: MachineRollingUpdateDeployment diff --git a/api/core/v1beta2/machineset_types.go b/api/core/v1beta2/machineset_types.go index 3e5bf0cf3df8..8b2c62f46f42 100644 --- a/api/core/v1beta2/machineset_types.go +++ b/api/core/v1beta2/machineset_types.go @@ -67,9 +67,8 @@ type MachineSetSpec struct { // deletePolicy defines the policy used to identify nodes to delete when downscaling. // Defaults to "Random". Valid values are "Random, "Newest", "Oldest" - // +kubebuilder:validation:Enum=Random;Newest;Oldest // +optional - DeletePolicy string `json:"deletePolicy,omitempty"` + DeletePolicy MachineSetDeletePolicy `json:"deletePolicy,omitempty"` // selector is a label query over machines that should match the replica count. // Label keys and values that must match in order to be controlled by this MachineSet. @@ -249,6 +248,7 @@ type MachineTemplateSpec struct { // MachineSetDeletePolicy defines how priority is assigned to nodes to delete when // downscaling a MachineSet. Defaults to "Random". +// +kubebuilder:validation:Enum=Random;Newest;Oldest type MachineSetDeletePolicy string const ( diff --git a/api/core/v1beta2/zz_generated.deepcopy.go b/api/core/v1beta2/zz_generated.deepcopy.go index f267696437b3..de4f8d0fb9ff 100644 --- a/api/core/v1beta2/zz_generated.deepcopy.go +++ b/api/core/v1beta2/zz_generated.deepcopy.go @@ -2588,7 +2588,7 @@ func (in *MachineRollingUpdateDeployment) DeepCopyInto(out *MachineRollingUpdate } if in.DeletePolicy != nil { in, out := &in.DeletePolicy, &out.DeletePolicy - *out = new(string) + *out = new(MachineSetDeletePolicy) **out = **in } } diff --git a/internal/api/core/v1alpha3/zz_generated.conversion.go b/internal/api/core/v1alpha3/zz_generated.conversion.go index 44cbe543bc73..7b2823cea39b 100644 --- a/internal/api/core/v1alpha3/zz_generated.conversion.go +++ b/internal/api/core/v1alpha3/zz_generated.conversion.go @@ -1379,7 +1379,7 @@ func autoConvert_v1alpha3_MachineSetSpec_To_v1beta2_MachineSetSpec(in *MachineSe out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) // WARNING: in.MinReadySeconds requires manual conversion: does not exist in peer-type - out.DeletePolicy = in.DeletePolicy + out.DeletePolicy = v1beta2.MachineSetDeletePolicy(in.DeletePolicy) out.Selector = in.Selector if err := Convert_v1alpha3_MachineTemplateSpec_To_v1beta2_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err @@ -1390,7 +1390,7 @@ func autoConvert_v1alpha3_MachineSetSpec_To_v1beta2_MachineSetSpec(in *MachineSe func autoConvert_v1beta2_MachineSetSpec_To_v1alpha3_MachineSetSpec(in *v1beta2.MachineSetSpec, out *MachineSetSpec, s conversion.Scope) error { out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) - out.DeletePolicy = in.DeletePolicy + out.DeletePolicy = string(in.DeletePolicy) out.Selector = in.Selector if err := Convert_v1beta2_MachineTemplateSpec_To_v1alpha3_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err diff --git a/internal/api/core/v1alpha4/zz_generated.conversion.go b/internal/api/core/v1alpha4/zz_generated.conversion.go index 3a3b470ef101..b43d1dbda20f 100644 --- a/internal/api/core/v1alpha4/zz_generated.conversion.go +++ b/internal/api/core/v1alpha4/zz_generated.conversion.go @@ -1716,7 +1716,7 @@ func autoConvert_v1beta2_MachinePoolStatus_To_v1alpha4_MachinePoolStatus(in *v1b func autoConvert_v1alpha4_MachineRollingUpdateDeployment_To_v1beta2_MachineRollingUpdateDeployment(in *MachineRollingUpdateDeployment, out *v1beta2.MachineRollingUpdateDeployment, s conversion.Scope) error { out.MaxUnavailable = (*intstr.IntOrString)(unsafe.Pointer(in.MaxUnavailable)) out.MaxSurge = (*intstr.IntOrString)(unsafe.Pointer(in.MaxSurge)) - out.DeletePolicy = (*string)(unsafe.Pointer(in.DeletePolicy)) + out.DeletePolicy = (*v1beta2.MachineSetDeletePolicy)(unsafe.Pointer(in.DeletePolicy)) return nil } @@ -1815,7 +1815,7 @@ func autoConvert_v1alpha4_MachineSetSpec_To_v1beta2_MachineSetSpec(in *MachineSe out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) // WARNING: in.MinReadySeconds requires manual conversion: does not exist in peer-type - out.DeletePolicy = in.DeletePolicy + out.DeletePolicy = v1beta2.MachineSetDeletePolicy(in.DeletePolicy) out.Selector = in.Selector if err := Convert_v1alpha4_MachineTemplateSpec_To_v1beta2_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err @@ -1826,7 +1826,7 @@ func autoConvert_v1alpha4_MachineSetSpec_To_v1beta2_MachineSetSpec(in *MachineSe func autoConvert_v1beta2_MachineSetSpec_To_v1alpha4_MachineSetSpec(in *v1beta2.MachineSetSpec, out *MachineSetSpec, s conversion.Scope) error { out.ClusterName = in.ClusterName out.Replicas = (*int32)(unsafe.Pointer(in.Replicas)) - out.DeletePolicy = in.DeletePolicy + out.DeletePolicy = string(in.DeletePolicy) out.Selector = in.Selector if err := Convert_v1beta2_MachineTemplateSpec_To_v1alpha4_MachineTemplateSpec(&in.Template, &out.Template, s); err != nil { return err diff --git a/internal/controllers/machinedeployment/machinedeployment_controller_test.go b/internal/controllers/machinedeployment/machinedeployment_controller_test.go index 02e2c7a65015..4159d249d8e9 100644 --- a/internal/controllers/machinedeployment/machinedeployment_controller_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_controller_test.go @@ -118,7 +118,7 @@ func TestMachineDeploymentReconciler(t *testing.T) { RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{ MaxUnavailable: intOrStrPtr(0), MaxSurge: intOrStrPtr(1), - DeletePolicy: ptr.To("Oldest"), + DeletePolicy: ptr.To(clusterv1.OldestMachineSetDeletePolicy), }, }, Template: clusterv1.MachineTemplateSpec{ @@ -204,7 +204,7 @@ func TestMachineDeploymentReconciler(t *testing.T) { }, timeout).Should(BeEquivalentTo(1)) t.Log("Verifying that the deployment's deletePolicy was propagated to the machineset") - g.Expect(machineSets.Items[0].Spec.DeletePolicy).To(Equal("Oldest")) + g.Expect(machineSets.Items[0].Spec.DeletePolicy).To(Equal(clusterv1.OldestMachineSetDeletePolicy)) t.Log("Verifying the linked infrastructure template has a cluster owner reference") g.Eventually(func() bool { @@ -379,7 +379,7 @@ func TestMachineDeploymentReconciler(t *testing.T) { // expect the Reconcile to be called and the MachineSet to be updated in-place. t.Log("Updating deletePolicy on the MachineDeployment") modifyFunc = func(d *clusterv1.MachineDeployment) { - d.Spec.Strategy.RollingUpdate.DeletePolicy = ptr.To("Newest") + d.Spec.Strategy.RollingUpdate.DeletePolicy = ptr.To(clusterv1.NewestMachineSetDeletePolicy) } g.Expect(updateMachineDeployment(ctx, env, deployment, modifyFunc)).To(Succeed()) g.Eventually(func(g Gomega) { @@ -387,10 +387,10 @@ func TestMachineDeploymentReconciler(t *testing.T) { // Verify we still only have 2 MachineSets. g.Expect(machineSets.Items).To(HaveLen(2)) // Verify the DeletePolicy value is updated - g.Expect(machineSets.Items[0].Spec.DeletePolicy).Should(Equal("Newest")) + g.Expect(machineSets.Items[0].Spec.DeletePolicy).Should(Equal(clusterv1.NewestMachineSetDeletePolicy)) // Verify that the old machine set retains its delete policy - g.Expect(machineSets.Items[1].Spec.DeletePolicy).To(Equal("Oldest")) + g.Expect(machineSets.Items[1].Spec.DeletePolicy).To(Equal(clusterv1.OldestMachineSetDeletePolicy)) }).Should(Succeed()) // Verify that all the MachineSets have the expected OwnerRef. @@ -541,7 +541,7 @@ func TestMachineDeploymentReconciler_CleanUpManagedFieldsForSSAAdoption(t *testi RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{ MaxUnavailable: intOrStrPtr(0), MaxSurge: intOrStrPtr(1), - DeletePolicy: ptr.To("Oldest"), + DeletePolicy: ptr.To(clusterv1.OldestMachineSetDeletePolicy), }, }, Template: clusterv1.MachineTemplateSpec{ diff --git a/internal/controllers/machinedeployment/machinedeployment_sync_test.go b/internal/controllers/machinedeployment/machinedeployment_sync_test.go index 3ed7a3957c1e..e9c5d2e15a3b 100644 --- a/internal/controllers/machinedeployment/machinedeployment_sync_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_sync_test.go @@ -393,7 +393,7 @@ func newTestMachineDeployment(replicas, statusReplicas, upToDateReplicas, availa RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{ MaxUnavailable: intOrStrPtr(0), MaxSurge: intOrStrPtr(1), - DeletePolicy: ptr.To("Oldest"), + DeletePolicy: ptr.To(clusterv1.OldestMachineSetDeletePolicy), }, }, }, @@ -544,7 +544,7 @@ func TestComputeDesiredMachineSet(t *testing.T) { Type: clusterv1.RollingUpdateMachineDeploymentStrategyType, RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{ MaxSurge: intOrStrPtr(1), - DeletePolicy: ptr.To("Random"), + DeletePolicy: ptr.To(clusterv1.RandomMachineSetDeletePolicy), MaxUnavailable: intOrStrPtr(0), }, }, @@ -584,7 +584,7 @@ func TestComputeDesiredMachineSet(t *testing.T) { Spec: clusterv1.MachineSetSpec{ ClusterName: "test-cluster", Replicas: ptr.To[int32](3), - DeletePolicy: string(clusterv1.RandomMachineSetDeletePolicy), + DeletePolicy: clusterv1.RandomMachineSetDeletePolicy, Selector: metav1.LabelSelector{MatchLabels: map[string]string{"k1": "v1"}}, Template: *deployment.Spec.Template.DeepCopy(), MachineNamingStrategy: &clusterv1.MachineNamingStrategy{ @@ -640,7 +640,7 @@ func TestComputeDesiredMachineSet(t *testing.T) { existingMS.Spec.Template.Spec.NodeDrainTimeoutSeconds = duration5s existingMS.Spec.Template.Spec.NodeDeletionTimeoutSeconds = duration5s existingMS.Spec.Template.Spec.NodeVolumeDetachTimeoutSeconds = duration5s - existingMS.Spec.DeletePolicy = string(clusterv1.NewestMachineSetDeletePolicy) + existingMS.Spec.DeletePolicy = clusterv1.NewestMachineSetDeletePolicy existingMS.Spec.Template.Spec.MinReadySeconds = ptr.To[int32](0) expectedMS := skeletonMSBasedOnMD.DeepCopy() @@ -680,7 +680,7 @@ func TestComputeDesiredMachineSet(t *testing.T) { existingMS.Spec.Template.Spec.NodeDrainTimeoutSeconds = duration5s existingMS.Spec.Template.Spec.NodeDeletionTimeoutSeconds = duration5s existingMS.Spec.Template.Spec.NodeVolumeDetachTimeoutSeconds = duration5s - existingMS.Spec.DeletePolicy = string(clusterv1.NewestMachineSetDeletePolicy) + existingMS.Spec.DeletePolicy = clusterv1.NewestMachineSetDeletePolicy existingMS.Spec.Template.Spec.MinReadySeconds = ptr.To[int32](0) oldMS := skeletonMSBasedOnMD.DeepCopy() @@ -734,7 +734,7 @@ func TestComputeDesiredMachineSet(t *testing.T) { existingMS.Spec.Template.Spec.NodeDrainTimeoutSeconds = duration5s existingMS.Spec.Template.Spec.NodeDeletionTimeoutSeconds = duration5s existingMS.Spec.Template.Spec.NodeVolumeDetachTimeoutSeconds = duration5s - existingMS.Spec.DeletePolicy = string(clusterv1.NewestMachineSetDeletePolicy) + existingMS.Spec.DeletePolicy = clusterv1.NewestMachineSetDeletePolicy existingMS.Spec.Template.Spec.MinReadySeconds = ptr.To[int32](0) expectedMS := skeletonMSBasedOnMD.DeepCopy() diff --git a/internal/controllers/machineset/machineset_delete_policy.go b/internal/controllers/machineset/machineset_delete_policy.go index 5c3eba3777b7..21d57f696a4b 100644 --- a/internal/controllers/machineset/machineset_delete_policy.go +++ b/internal/controllers/machineset/machineset_delete_policy.go @@ -124,7 +124,7 @@ func getMachinesToDeletePrioritized(filteredMachines []*clusterv1.Machine, diff func getDeletePriorityFunc(ms *clusterv1.MachineSet) (deletePriorityFunc, error) { // Map the Spec.DeletePolicy value to the appropriate delete priority function - switch msdp := clusterv1.MachineSetDeletePolicy(ms.Spec.DeletePolicy); msdp { + switch ms.Spec.DeletePolicy { case clusterv1.RandomMachineSetDeletePolicy: return randomDeletePolicy, nil case clusterv1.NewestMachineSetDeletePolicy: @@ -134,7 +134,7 @@ func getDeletePriorityFunc(ms *clusterv1.MachineSet) (deletePriorityFunc, error) case "": return randomDeletePolicy, nil default: - return nil, errors.Errorf("Unsupported delete policy %s. Must be one of 'Random', 'Newest', or 'Oldest'", msdp) + return nil, errors.Errorf("Unsupported delete policy %s. Must be one of 'Random', 'Newest', or 'Oldest'", ms.Spec.DeletePolicy) } } diff --git a/internal/webhooks/machineset.go b/internal/webhooks/machineset.go index eea3813af4a3..0b11579abb5f 100644 --- a/internal/webhooks/machineset.go +++ b/internal/webhooks/machineset.go @@ -104,8 +104,7 @@ func (webhook *MachineSet) Default(ctx context.Context, obj runtime.Object) erro m.Spec.Replicas = ptr.To[int32](replicas) if m.Spec.DeletePolicy == "" { - randomPolicy := string(clusterv1.RandomMachineSetDeletePolicy) - m.Spec.DeletePolicy = randomPolicy + m.Spec.DeletePolicy = clusterv1.RandomMachineSetDeletePolicy } if m.Spec.Selector.MatchLabels == nil { diff --git a/internal/webhooks/machineset_test.go b/internal/webhooks/machineset_test.go index 8f545b16db0f..8da51aeb91d5 100644 --- a/internal/webhooks/machineset_test.go +++ b/internal/webhooks/machineset_test.go @@ -57,7 +57,7 @@ func TestMachineSetDefault(t *testing.T) { g.Expect(webhook.Default(reqCtx, ms)).To(Succeed()) g.Expect(ms.Labels[clusterv1.ClusterNameLabel]).To(Equal(ms.Spec.ClusterName)) - g.Expect(ms.Spec.DeletePolicy).To(Equal(string(clusterv1.RandomMachineSetDeletePolicy))) + g.Expect(ms.Spec.DeletePolicy).To(Equal(clusterv1.RandomMachineSetDeletePolicy)) g.Expect(ms.Spec.Selector.MatchLabels).To(HaveKeyWithValue(clusterv1.MachineSetNameLabel, "test-ms")) g.Expect(ms.Spec.Template.Labels).To(HaveKeyWithValue(clusterv1.MachineSetNameLabel, "test-ms")) g.Expect(*ms.Spec.Template.Spec.Version).To(Equal("v1.19.10")) diff --git a/test/e2e/clusterclass_rollout.go b/test/e2e/clusterclass_rollout.go index 5b8adaaf63c3..cd6f5c101bd7 100644 --- a/test/e2e/clusterclass_rollout.go +++ b/test/e2e/clusterclass_rollout.go @@ -198,7 +198,7 @@ func ClusterClassRolloutSpec(ctx context.Context, inputGetter func() ClusterClas RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{ MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: 0}, MaxSurge: &intstr.IntOrString{Type: intstr.Int, IntVal: 5 + rand.Int31n(20)}, //nolint:gosec - DeletePolicy: ptr.To(string(clusterv1.NewestMachineSetDeletePolicy)), + DeletePolicy: ptr.To(clusterv1.NewestMachineSetDeletePolicy), }, Remediation: &clusterv1.RemediationStrategy{ MaxInFlight: &intstr.IntOrString{Type: intstr.Int, IntVal: 2 + rand.Int31n(20)}, //nolint:gosec