diff --git a/internal/controllers/machinedeployment/machinedeployment_rolling.go b/internal/controllers/machinedeployment/machinedeployment_rolling.go index 31bd697d8b24..250e389ff6b4 100644 --- a/internal/controllers/machinedeployment/machinedeployment_rolling.go +++ b/internal/controllers/machinedeployment/machinedeployment_rolling.go @@ -21,11 +21,13 @@ import ( "sort" "github.com/pkg/errors" + "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil" + "sigs.k8s.io/cluster-api/util/patch" ) // rolloutRolling implements the logic for rolling a new MachineSet. @@ -72,6 +74,10 @@ func (r *Reconciler) rolloutRolling(ctx context.Context, md *clusterv1.MachineDe } func (r *Reconciler) reconcileNewMachineSet(ctx context.Context, allMSs []*clusterv1.MachineSet, newMS *clusterv1.MachineSet, deployment *clusterv1.MachineDeployment) error { + if err := r.cleanupDisableMachineCreateAnnotation(ctx, newMS); err != nil { + return err + } + if deployment.Spec.Replicas == nil { return errors.Errorf("spec.replicas for MachineDeployment %v is nil, this is unexpected", client.ObjectKeyFromObject(deployment)) } @@ -293,3 +299,25 @@ func (r *Reconciler) scaleDownOldMachineSetsForRollingUpdate(ctx context.Context return totalScaledDown, nil } + +// cleanupDisableMachineCreateAnnotation will remove the disable machine create annotation from new MachineSets that were created during reconcileOldMachineSetsOnDelete. +func (r *Reconciler) cleanupDisableMachineCreateAnnotation(ctx context.Context, newMS *clusterv1.MachineSet) error { + log := ctrl.LoggerFrom(ctx, "MachineSet", klog.KObj(newMS)) + + if newMS.Annotations != nil { + if _, ok := newMS.Annotations[clusterv1.DisableMachineCreateAnnotation]; ok { + log.V(4).Info("removing annotation on latest MachineSet to enable machine creation") + patchHelper, err := patch.NewHelper(newMS, r.Client) + if err != nil { + return err + } + delete(newMS.Annotations, clusterv1.DisableMachineCreateAnnotation) + err = patchHelper.Patch(ctx, newMS) + if err != nil { + return err + } + } + } + + return nil +} diff --git a/internal/controllers/machinedeployment/machinedeployment_rolling_test.go b/internal/controllers/machinedeployment/machinedeployment_rolling_test.go index 745e122b72b3..a586d9a90de2 100644 --- a/internal/controllers/machinedeployment/machinedeployment_rolling_test.go +++ b/internal/controllers/machinedeployment/machinedeployment_rolling_test.go @@ -217,6 +217,41 @@ func TestReconcileNewMachineSet(t *testing.T) { }, error: nil, }, + { + name: "Rolling Updated Cleanup disable machine create annotation", + machineDeployment: &clusterv1.MachineDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "bar", + }, + Spec: clusterv1.MachineDeploymentSpec{ + Strategy: &clusterv1.MachineDeploymentStrategy{ + Type: clusterv1.RollingUpdateMachineDeploymentStrategyType, + RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{ + MaxUnavailable: intOrStrPtr(0), + MaxSurge: intOrStrPtr(0), + }, + }, + Replicas: ptr.To[int32](2), + }, + }, + newMachineSet: &clusterv1.MachineSet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "foo", + Name: "bar", + Annotations: map[string]string{ + clusterv1.DisableMachineCreateAnnotation: "true", + clusterv1.DesiredReplicasAnnotation: "2", + clusterv1.MaxReplicasAnnotation: "2", + }, + }, + Spec: clusterv1.MachineSetSpec{ + Replicas: ptr.To[int32](2), + }, + }, + expectedNewMachineSetReplicas: 2, + error: nil, + }, } for _, tc := range testCases { @@ -252,6 +287,9 @@ func TestReconcileNewMachineSet(t *testing.T) { g.Expect(*freshNewMachineSet.Spec.Replicas).To(BeEquivalentTo(tc.expectedNewMachineSetReplicas)) + _, ok := freshNewMachineSet.GetAnnotations()[clusterv1.DisableMachineCreateAnnotation] + g.Expect(ok).To(BeFalse()) + desiredReplicasAnnotation, ok := freshNewMachineSet.GetAnnotations()[clusterv1.DesiredReplicasAnnotation] g.Expect(ok).To(BeTrue()) g.Expect(strconv.Atoi(desiredReplicasAnnotation)).To(BeEquivalentTo(*tc.machineDeployment.Spec.Replicas)) diff --git a/internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go b/internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go index b8e7fcd75297..866d2614f358 100644 --- a/internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go +++ b/internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go @@ -165,22 +165,9 @@ func (r *Reconciler) reconcileOldMachineSetsOnDelete(ctx context.Context, oldMSs // reconcileNewMachineSetOnDelete handles reconciliation of the latest MachineSet associated with the MachineDeployment in the OnDelete MachineDeploymentStrategyType. func (r *Reconciler) reconcileNewMachineSetOnDelete(ctx context.Context, allMSs []*clusterv1.MachineSet, newMS *clusterv1.MachineSet, deployment *clusterv1.MachineDeployment) error { - // logic same as reconcile logic for RollingUpdate - log := ctrl.LoggerFrom(ctx, "MachineSet", klog.KObj(newMS)) - - if newMS.Annotations != nil { - if _, ok := newMS.Annotations[clusterv1.DisableMachineCreateAnnotation]; ok { - log.V(4).Info("removing annotation on latest MachineSet to enable machine creation") - patchHelper, err := patch.NewHelper(newMS, r.Client) - if err != nil { - return err - } - delete(newMS.Annotations, clusterv1.DisableMachineCreateAnnotation) - err = patchHelper.Patch(ctx, newMS) - if err != nil { - return err - } - } + if err := r.cleanupDisableMachineCreateAnnotation(ctx, newMS); err != nil { + return err } + return r.reconcileNewMachineSet(ctx, allMSs, newMS, deployment) }