Skip to content

Commit

Permalink
remove disableMachineCreate annotation from new machinesets during ro…
Browse files Browse the repository at this point in the history
…lling machine deployment reconciliation
  • Loading branch information
Patryk-Stefanski committed Nov 28, 2024
1 parent c54a51b commit 052ff50
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

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.
Expand Down Expand Up @@ -72,6 +73,11 @@ 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 {
newMS, err := r.cleanupDisableMachineCreateAnnotation(ctx, newMS)
if 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))
}
Expand Down Expand Up @@ -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) (*clusterv1.MachineSet, error) {
log := ctrl.LoggerFrom(ctx)

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 nil, err
}
delete(newMS.Annotations, clusterv1.DisableMachineCreateAnnotation)
err = patchHelper.Patch(ctx, newMS)
if err != nil {
return nil, err
}
}
}

return newMS, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func TestReconcileNewMachineSet(t *testing.T) {
expectedNewMachineSetReplicas int
error error
}{
// Existing test cases...
{
name: "It fails when machineDeployment has no replicas",
machineDeployment: &clusterv1.MachineDeployment{
Expand Down Expand Up @@ -217,6 +218,32 @@ func TestReconcileNewMachineSet(t *testing.T) {
},
error: nil,
},
{
name: "Cleanup disable machine create annotation",
machineDeployment: &clusterv1.MachineDeployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "bar",
},
Spec: clusterv1.MachineDeploymentSpec{
Replicas: ptr.To[int32](2),
},
},
newMachineSet: &clusterv1.MachineSet{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "bar",
Annotations: map[string]string{
clusterv1.DisableMachineCreateAnnotation: "true",
},
},
Spec: clusterv1.MachineSetSpec{
Replicas: ptr.To[int32](2),
},
},
expectedNewMachineSetReplicas: 2,
error: nil,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -252,6 +279,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))
Expand Down

0 comments on commit 052ff50

Please sign in to comment.