Merge pull request #12419 from sbueringer/pr-use-machinsetdeletepolicy-enum

k8s-ci-robot · web-flow · commit 07db0a3e2162 · 2025-06-30T09:54:37.000-07:00
⚠️ Use MachineSetDeletePolicy enum in MD & MS API
diff --git a/api/core/v1beta1/zz_generated.conversion.go b/api/core/v1beta1/zz_generated.conversion.go
diff --git 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
@@ -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
diff --git a/internal/api/core/v1alpha3/zz_generated.conversion.go b/internal/api/core/v1alpha3/zz_generated.conversion.go
diff --git a/internal/api/core/v1alpha4/zz_generated.conversion.go b/internal/api/core/v1alpha4/zz_generated.conversion.go
diff --git 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,18 +379,18 @@ 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) {
 			g.Expect(env.List(ctx, machineSets, msListOpts...)).Should(Succeed())
 			// 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -104,8 +104,7 @@ func (webhook *MachineSet) Default(ctx context.Context, obj runtime.Object) erro`
`104`	`104`	`m.Spec.Replicas = ptr.To[int32](replicas)`
`105`	`105`
`106`	`106`	`if m.Spec.DeletePolicy == "" {`
`107`		`- randomPolicy := string(clusterv1.RandomMachineSetDeletePolicy)`
`108`		`- m.Spec.DeletePolicy = randomPolicy`
	`107`	`+ m.Spec.DeletePolicy = clusterv1.RandomMachineSetDeletePolicy`
`109`	`108`	`}`
`110`	`109`
`111`	`110`	`if m.Spec.Selector.MatchLabels == nil {`