Skip to content

Commit fda7d56

Browse files
lubronzhanlubronzhan
committed
Two places
Signed-off-by: Lubron Zhan <[email protected]>
1 parent a261193 commit fda7d56

File tree

5 files changed

+118
-9
lines changed

5 files changed

+118
-9
lines changed

‎internal/controllers/machinedeployment/machinedeployment_controller.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -309,18 +309,17 @@ func (r *Reconciler) reconcile(ctx context.Context, s *scope) error {
309309
return errors.Errorf("missing MachineDeployment strategy")
310310
}
311311

312-
if md.Spec.Strategy.Type == clusterv1.RollingUpdateMachineDeploymentStrategyType {
312+
switch md.Spec.Strategy.Type {
313+
case clusterv1.RollingUpdateMachineDeploymentStrategyType:
313314
if md.Spec.Strategy.RollingUpdate == nil {
314315
return errors.Errorf("missing MachineDeployment settings for strategy type: %s", md.Spec.Strategy.Type)
315316
}
316317
return r.rolloutRolling(ctx, md, s.machineSets, templateExists)
317-
}
318-
319-
if md.Spec.Strategy.Type == clusterv1.OnDeleteMachineDeploymentStrategyType {
318+
case clusterv1.OnDeleteMachineDeploymentStrategyType:
320319
return r.rolloutOnDelete(ctx, md, s.machineSets, templateExists)
320+
default:
321+
return errors.Errorf("unexpected deployment strategy type: %s", md.Spec.Strategy.Type)
321322
}
322-
323-
return errors.Errorf("unexpected deployment strategy type: %s", md.Spec.Strategy.Type)
324323
}
325324

326325
func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) error {

‎internal/controllers/machinedeployment/machinedeployment_rolling.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,12 @@ func (r *Reconciler) reconcileOldMachineSets(ctx context.Context, allMSs []*clus
179179

180180
log.V(4).Info("Cleaned up unhealthy replicas from old MachineSets", "count", cleanupCount)
181181

182+
log.V(4).Info("Ensure old MachineSet has deletion related timeouts propagated from MachineDeployment")
183+
if err := r.patchOldMSDeletionTimeoutsFromMD(ctx, oldMSs, deployment); err != nil {
184+
return err
185+
}
186+
log.V(4).Info("Ensured old MachineSet has deletion related timeouts propagated from MachineDeployment")
187+
182188
// Scale down old MachineSets, need check maxUnavailable to ensure we can scale down
183189
allMSs = oldMSs
184190
allMSs = append(allMSs, newMS)

‎internal/controllers/machinedeployment/machinedeployment_rolling_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package machinedeployment
1919
import (
2020
"strconv"
2121
"testing"
22+
"time"
2223

2324
. "github.com/onsi/gomega"
2425
"github.com/pkg/errors"
@@ -302,6 +303,7 @@ func TestReconcileNewMachineSet(t *testing.T) {
302303
}
303304

304305
func TestReconcileOldMachineSets(t *testing.T) {
306+
duration10m := &metav1.Duration{Duration: 10 * time.Minute}
305307
testCases := []struct {
306308
name string
307309
machineDeployment *clusterv1.MachineDeployment
@@ -468,6 +470,83 @@ func TestReconcileOldMachineSets(t *testing.T) {
468470
},
469471
expectedOldMachineSetsReplicas: 8,
470472
},
473+
{
474+
name: "RollingUpdate strategy: Scale down old MachineSets and propagate the node deletion related timeouts to old MachineSets",
475+
machineDeployment: &clusterv1.MachineDeployment{
476+
ObjectMeta: metav1.ObjectMeta{
477+
Namespace: "foo",
478+
Name: "bar",
479+
},
480+
Spec: clusterv1.MachineDeploymentSpec{
481+
Strategy: &clusterv1.MachineDeploymentStrategy{
482+
Type: clusterv1.RollingUpdateMachineDeploymentStrategyType,
483+
RollingUpdate: &clusterv1.MachineRollingUpdateDeployment{
484+
MaxUnavailable: intOrStrPtr(1),
485+
MaxSurge: intOrStrPtr(3),
486+
},
487+
},
488+
Replicas: ptr.To[int32](2),
489+
Template: clusterv1.MachineTemplateSpec{
490+
Spec: clusterv1.MachineSpec{
491+
NodeDrainTimeout: duration10m,
492+
NodeDeletionTimeout: duration10m,
493+
NodeVolumeDetachTimeout: duration10m,
494+
},
495+
},
496+
},
497+
},
498+
newMachineSet: &clusterv1.MachineSet{
499+
ObjectMeta: metav1.ObjectMeta{
500+
Namespace: "foo",
501+
Name: "bar",
502+
},
503+
Spec: clusterv1.MachineSetSpec{
504+
Replicas: ptr.To[int32](0),
505+
},
506+
Status: clusterv1.MachineSetStatus{
507+
Deprecated: &clusterv1.MachineSetDeprecatedStatus{
508+
V1Beta1: &clusterv1.MachineSetV1Beta1DeprecatedStatus{
509+
AvailableReplicas: 2,
510+
},
511+
},
512+
},
513+
},
514+
oldMachineSets: []*clusterv1.MachineSet{
515+
{
516+
ObjectMeta: metav1.ObjectMeta{
517+
Namespace: "foo",
518+
Name: "2replicas",
519+
},
520+
Spec: clusterv1.MachineSetSpec{
521+
Replicas: ptr.To[int32](2),
522+
},
523+
Status: clusterv1.MachineSetStatus{
524+
Deprecated: &clusterv1.MachineSetDeprecatedStatus{
525+
V1Beta1: &clusterv1.MachineSetV1Beta1DeprecatedStatus{
526+
AvailableReplicas: 2,
527+
},
528+
},
529+
},
530+
},
531+
{
532+
ObjectMeta: metav1.ObjectMeta{
533+
Namespace: "foo",
534+
Name: "1replicas",
535+
},
536+
Spec: clusterv1.MachineSetSpec{
537+
Replicas: ptr.To[int32](1),
538+
},
539+
Status: clusterv1.MachineSetStatus{
540+
Deprecated: &clusterv1.MachineSetDeprecatedStatus{
541+
V1Beta1: &clusterv1.MachineSetV1Beta1DeprecatedStatus{
542+
AvailableReplicas: 1,
543+
},
544+
},
545+
},
546+
},
547+
},
548+
expectedOldMachineSetsReplicas: 0,
549+
},
471550
}
472551
for _, tc := range testCases {
473552
t.Run(tc.name, func(t *testing.T) {
@@ -500,6 +579,9 @@ func TestReconcileOldMachineSets(t *testing.T) {
500579
err = r.Client.Get(ctx, client.ObjectKeyFromObject(tc.oldMachineSets[key]), freshOldMachineSet)
501580
g.Expect(err).ToNot(HaveOccurred())
502581
g.Expect(*freshOldMachineSet.Spec.Replicas).To(BeEquivalentTo(tc.expectedOldMachineSetsReplicas))
582+
g.Expect(freshOldMachineSet.Spec.Template.Spec.NodeDrainTimeout).To(BeEquivalentTo(tc.machineDeployment.Spec.Template.Spec.NodeDrainTimeout))
583+
g.Expect(freshOldMachineSet.Spec.Template.Spec.NodeDeletionTimeout).To(BeEquivalentTo(tc.machineDeployment.Spec.Template.Spec.NodeDeletionTimeout))
584+
g.Expect(freshOldMachineSet.Spec.Template.Spec.NodeVolumeDetachTimeout).To(BeEquivalentTo(tc.machineDeployment.Spec.Template.Spec.NodeVolumeDetachTimeout))
503585
}
504586
})
505587
}

‎internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,9 @@ func (r *Reconciler) reconcileOldMachineSetsOnDelete(ctx context.Context, oldMSs
110110
continue
111111
}
112112
log.V(4).Info("Ensure old MachineSet has deletion related timeouts propagated from MachineDeployment")
113-
oldMS.Spec.Template.Spec.NodeDrainTimeout = deployment.Spec.Template.Spec.NodeDrainTimeout
114-
oldMS.Spec.Template.Spec.NodeDeletionTimeout = deployment.Spec.Template.Spec.NodeDeletionTimeout
115-
oldMS.Spec.Template.Spec.NodeVolumeDetachTimeout = deployment.Spec.Template.Spec.NodeVolumeDetachTimeout
113+
if err := r.patchOldMSDeletionTimeoutsFromMD(ctx, oldMSs, deployment); err != nil {
114+
return err
115+
}
116116
log.V(4).Info("Fetching Machines associated with MachineSet")
117117
// Get all Machines linked to this MachineSet.
118118
allMachinesInOldMS := &clusterv1.MachineList{}

‎internal/controllers/machinedeployment/machinedeployment_sync.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -669,3 +669,25 @@ func (r *Reconciler) cleanupDeployment(ctx context.Context, oldMSs []*clusterv1.
669669

670670
return nil
671671
}
672+
673+
func (r *Reconciler) patchOldMSDeletionTimeoutsFromMD(ctx context.Context, oldMSs []*clusterv1.MachineSet, deployment *clusterv1.MachineDeployment) error {
674+
log := ctrl.LoggerFrom(ctx)
675+
for _, oldMS := range oldMSs {
676+
patchHelper, err := patch.NewHelper(oldMS, r.Client)
677+
if err != nil {
678+
return errors.Wrapf(err, "failed to generate patch for MachineSet %q", klog.KObj(oldMS))
679+
}
680+
681+
// Set all other in-place mutable fields that impact the ability to tear down existing machines.
682+
oldMS.Spec.Template.Spec.NodeDrainTimeout = deployment.Spec.Template.Spec.NodeDrainTimeout
683+
oldMS.Spec.Template.Spec.NodeDeletionTimeout = deployment.Spec.Template.Spec.NodeDeletionTimeout
684+
oldMS.Spec.Template.Spec.NodeVolumeDetachTimeout = deployment.Spec.Template.Spec.NodeVolumeDetachTimeout
685+
686+
err = patchHelper.Patch(ctx, oldMS)
687+
if err != nil {
688+
log.Error(err, "Failed to update MachineSet", "MachineSet", klog.KObj(oldMS))
689+
return errors.Wrapf(err, "failed to update MachineSet %q", klog.KObj(oldMS))
690+
}
691+
}
692+
return nil
693+
}

0 commit comments

Comments
 (0)