Skip to content

🌱 Add rollout planner #12804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
k8s-ci-robot merged 5 commits into from
Oct 1, 2025
Merged

🌱 Add rollout planner #12804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ba93b67
Add rollout planner
fabriziopandini Sep 26, 2025
4bf2661
Minor improvements
fabriziopandini Sep 29, 2025
9988c42
Address feedback
fabriziopandini Sep 30, 2025
4565d00
More feedback
fabriziopandini Sep 30, 2025
4659076
More feedback
fabriziopandini Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 132 additions & 75 deletions internal/controllers/machinedeployment/machinedeployment_rolling.go
View file
Open in desktop

Large diffs are not rendered by default.

146 changes: 32 additions & 114 deletions internal/controllers/machinedeployment/machinedeployment_rolling_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,13 @@ limitations under the License.
package machinedeployment

import (
"strconv"
"testing"

. "github.com/onsi/gomega"
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/tools/record"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
"sigs.k8s.io/cluster-api/internal/controllers/machinedeployment/mdutil"
)

func TestReconcileNewMachineSet(t *testing.T) {
Expand All @@ -41,36 +35,6 @@ func TestReconcileNewMachineSet(t *testing.T) {
expectedNewMachineSetReplicas int
error error
}{
{
name: "It fails when machineDeployment has no replicas",
machineDeployment: &clusterv1.MachineDeployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "bar",
},
},
newMachineSet: &clusterv1.MachineSet{
Spec: clusterv1.MachineSetSpec{
Replicas: ptr.To[int32](2),
},
},
error: errors.Errorf("spec.replicas for MachineDeployment foo/bar is nil, this is unexpected"),
},
{
name: "It fails when new machineSet has no replicas",
machineDeployment: &clusterv1.MachineDeployment{
Spec: clusterv1.MachineDeploymentSpec{
Replicas: ptr.To[int32](2),
},
},
newMachineSet: &clusterv1.MachineSet{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "bar",
},
},
error: errors.Errorf("spec.replicas for MachineSet foo/bar is nil, this is unexpected"),
},
{
name: "RollingUpdate strategy: Scale up: 0 -> 2",
machineDeployment: &clusterv1.MachineDeployment{
Expand Down Expand Up @@ -268,21 +232,12 @@ func TestReconcileNewMachineSet(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)

resources := []client.Object{
tc.machineDeployment,
}

allMachineSets := append(tc.oldMachineSets, tc.newMachineSet)
for key := range allMachineSets {
resources = append(resources, allMachineSets[key])
}

r := &Reconciler{
Client: fake.NewClientBuilder().WithObjects(resources...).Build(),
recorder: record.NewFakeRecorder(32),
}
planner := newRolloutPlanner()
planner.md = tc.machineDeployment
planner.newMS = tc.newMachineSet
planner.oldMSs = tc.oldMachineSets

err := r.reconcileNewMachineSet(ctx, allMachineSets, tc.newMachineSet, tc.machineDeployment)
err := planner.reconcileNewMachineSet(ctx)
if tc.error != nil {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(BeEquivalentTo(tc.error.Error()))
Expand All @@ -291,22 +246,23 @@ func TestReconcileNewMachineSet(t *testing.T) {

g.Expect(err).ToNot(HaveOccurred())

freshNewMachineSet := &clusterv1.MachineSet{}
err = r.Client.Get(ctx, client.ObjectKeyFromObject(tc.newMachineSet), freshNewMachineSet)
g.Expect(err).ToNot(HaveOccurred())

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))
scaleIntent := ptr.Deref(tc.newMachineSet.Spec.Replicas, 0)
if v, ok := planner.scaleIntents[tc.newMachineSet.Name]; ok {
scaleIntent = v
}
g.Expect(scaleIntent).To(BeEquivalentTo(tc.expectedNewMachineSetReplicas))

maxReplicasAnnotation, ok := freshNewMachineSet.GetAnnotations()[clusterv1.MaxReplicasAnnotation]
g.Expect(ok).To(BeTrue())
g.Expect(strconv.Atoi(maxReplicasAnnotation)).To(BeEquivalentTo(*tc.machineDeployment.Spec.Replicas + mdutil.MaxSurge(*tc.machineDeployment)))
// TODO(in-place): Restore tests on DisableMachineCreateAnnotation and MaxReplicasAnnotation as soon as handling those annotation is moved into the rollout planner
// _, 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))
//
// maxReplicasAnnotation, ok := freshNewMachineSet.GetAnnotations()[clusterv1.MaxReplicasAnnotation]
// g.Expect(ok).To(BeTrue())
// g.Expect(strconv.Atoi(maxReplicasAnnotation)).To(BeEquivalentTo(*tc.machineDeployment.Spec.Replicas + mdutil.MaxSurge(*tc.machineDeployment)))
})
}
}
Expand All @@ -320,36 +276,6 @@ func TestReconcileOldMachineSets(t *testing.T) {
expectedOldMachineSetsReplicas int
error error
}{
{
name: "It fails when machineDeployment has no replicas",
machineDeployment: &clusterv1.MachineDeployment{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "bar",
},
},
newMachineSet: &clusterv1.MachineSet{
Spec: clusterv1.MachineSetSpec{
Replicas: ptr.To[int32](2),
},
},
error: errors.Errorf("spec.replicas for MachineDeployment foo/bar is nil, this is unexpected"),
},
{
name: "It fails when new machineSet has no replicas",
machineDeployment: &clusterv1.MachineDeployment{
Spec: clusterv1.MachineDeploymentSpec{
Replicas: ptr.To[int32](2),
},
},
newMachineSet: &clusterv1.MachineSet{
ObjectMeta: metav1.ObjectMeta{
Namespace: "foo",
Name: "bar",
},
},
error: errors.Errorf("spec.replicas for MachineSet foo/bar is nil, this is unexpected"),
},
{
name: "RollingUpdate strategy: Scale down old MachineSets when all new replicas are available",
machineDeployment: &clusterv1.MachineDeployment{
Expand Down Expand Up @@ -465,33 +391,25 @@ func TestReconcileOldMachineSets(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
g := NewWithT(t)

resources := []client.Object{
tc.machineDeployment,
}

allMachineSets := append(tc.oldMachineSets, tc.newMachineSet)
for key := range allMachineSets {
resources = append(resources, allMachineSets[key])
}

r := &Reconciler{
Client: fake.NewClientBuilder().WithObjects(resources...).Build(),
recorder: record.NewFakeRecorder(32),
}
planner := newRolloutPlanner()
planner.md = tc.machineDeployment
planner.newMS = tc.newMachineSet
planner.oldMSs = tc.oldMachineSets

err := r.reconcileOldMachineSets(ctx, allMachineSets, tc.oldMachineSets, tc.newMachineSet, tc.machineDeployment)
err := planner.reconcileOldMachineSets(ctx)
if tc.error != nil {
g.Expect(err).To(HaveOccurred())
g.Expect(err.Error()).To(BeEquivalentTo(tc.error.Error()))
return
}

g.Expect(err).ToNot(HaveOccurred())
for key := range tc.oldMachineSets {
freshOldMachineSet := &clusterv1.MachineSet{}
err = r.Client.Get(ctx, client.ObjectKeyFromObject(tc.oldMachineSets[key]), freshOldMachineSet)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(*freshOldMachineSet.Spec.Replicas).To(BeEquivalentTo(tc.expectedOldMachineSetsReplicas))
for i := range tc.oldMachineSets {
scaleIntent := ptr.Deref(tc.oldMachineSets[i].Spec.Replicas, 0)
if v, ok := planner.scaleIntents[tc.oldMachineSets[i].Name]; ok {
scaleIntent = v
}
g.Expect(scaleIntent).To(BeEquivalentTo(tc.expectedOldMachineSetsReplicas))
}
})
}
Expand Down
22 changes: 19 additions & 3 deletions internal/controllers/machinedeployment/machinedeployment_rollout_ondelete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand All @@ -48,7 +49,7 @@ func (r *Reconciler) rolloutOnDelete(ctx context.Context, md *clusterv1.MachineD
allMSs := append(oldMSs, newMS)

// Scale up, if we can.
if err := r.reconcileNewMachineSetOnDelete(ctx, allMSs, newMS, md); err != nil {
if err := r.reconcileNewMachineSetOnDelete(ctx, md, oldMSs, newMS); err != nil {
return err
}

Expand Down Expand Up @@ -164,10 +165,25 @@ func (r *Reconciler) reconcileOldMachineSetsOnDelete(ctx context.Context, oldMSs
}

// reconcileNewMachineSetOnDelete handles reconciliation of the latest MachineSet associated with the MachineDeployment in the OnDelete rollout strategy.
func (r *Reconciler) reconcileNewMachineSetOnDelete(ctx context.Context, allMSs []*clusterv1.MachineSet, newMS *clusterv1.MachineSet, deployment *clusterv1.MachineDeployment) error {
func (r *Reconciler) reconcileNewMachineSetOnDelete(ctx context.Context, md *clusterv1.MachineDeployment, oldMSs []*clusterv1.MachineSet, newMS *clusterv1.MachineSet) error {
// TODO(in-place): also apply/remove labels should go into rolloutPlanner
if err := r.cleanupDisableMachineCreateAnnotation(ctx, newMS); err != nil {
return err
}

return r.reconcileNewMachineSet(ctx, allMSs, newMS, deployment)
planner := newRolloutPlanner()
planner.md = md
planner.newMS = newMS
planner.oldMSs = oldMSs

if err := planner.reconcileNewMachineSet(ctx); err != nil {
return err
}

// TODO(in-place): this should be changed as soon as rolloutPlanner support MS creation and adding/removing labels from MS
scaleIntent := ptr.Deref(newMS.Spec.Replicas, 0)
if v, ok := planner.scaleIntents[newMS.Name]; ok {
scaleIntent = v
}
return r.scaleMachineSet(ctx, newMS, scaleIntent, md)
}
Loading