From c409530ab40e576f4f68ed01aba2a4ef1147b978 Mon Sep 17 00:00:00 2001 From: Rishabh Patel <66425093+rishabh-11@users.noreply.github.com> Date: Mon, 5 Jun 2023 15:44:57 +0530 Subject: [PATCH] Keep `DesiredReplicas` annotation up-to-date even if no scaling happens (#821) * keep desired replicas annotation up to date even if no scaling happens * Update pkg/controller/deployment_sync_test.go Co-authored-by: Himanshu Sharma <79965161+himanshu-kun@users.noreply.github.com> --------- Co-authored-by: Himanshu Sharma <79965161+himanshu-kun@users.noreply.github.com> --- pkg/controller/deployment_sync.go | 5 ++++- pkg/controller/deployment_sync_test.go | 20 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/pkg/controller/deployment_sync.go b/pkg/controller/deployment_sync.go index 4c73b1fd5..90e1f1cc0 100644 --- a/pkg/controller/deployment_sync.go +++ b/pkg/controller/deployment_sync.go @@ -404,7 +404,10 @@ func (dc *controller) scale(ctx context.Context, deployment *v1alpha1.MachineDep // deployment. If there is no active machine set, then we should scale up the newest machine set. if activeOrLatest := FindActiveOrLatest(newIS, oldISs); activeOrLatest != nil { if (activeOrLatest.Spec.Replicas) == (deployment.Spec.Replicas) { - return nil + // to deal with the case where the DesiredReplicas annotation is outdated (issue - https://github.com/gardener/machine-controller-manager/issues/815) + klog.V(3).Infof("DesiredReplicas annotation possibly outdated for the machineSet %s, updating if needed...", activeOrLatest.Name) + _, _, err := dc.scaleMachineSet(ctx, activeOrLatest, deployment.Spec.Replicas, deployment, "no-op") + return err } klog.V(3).Infof("Scaling latest/theOnlyActive machineSet %s", activeOrLatest.Name) _, _, err := dc.scaleMachineSetAndRecordEvent(ctx, activeOrLatest, deployment.Spec.Replicas, deployment) diff --git a/pkg/controller/deployment_sync_test.go b/pkg/controller/deployment_sync_test.go index 5045fe062..f8e4b7a6b 100644 --- a/pkg/controller/deployment_sync_test.go +++ b/pkg/controller/deployment_sync_test.go @@ -455,8 +455,9 @@ var _ = Describe("deployment_sync", func() { controlObjects = append(controlObjects, data.setup.oldISs[i]) } - controlObjects = append(controlObjects, data.setup.newIS) - + if data.setup.newIS != nil { + controlObjects = append(controlObjects, data.setup.newIS) + } c, trackers := createController(stop, testNamespace, controlObjects, nil, nil) defer trackers.Stop() waitForCacheSync(stop, c) @@ -840,6 +841,21 @@ var _ = Describe("deployment_sync", func() { err: false, }, }), + Entry("if no scaling needed but DesiredReplicas Annotation for the only active machineSet is outdated, it should be updated with the correct value", &data{ + setup: setup{ + machineDeployment: newMachineDeployment(mDeploymentSpecTemplate, 1, 500, 2, 0, nil, nil, nil, nil), + oldISs: []*machinev1.MachineSet{ + newMachineSet(mSetSpecTemplate, nameMachineSet1, 1, 500, nil, nil, map[string]string{DesiredReplicasAnnotation: "2", MaxReplicasAnnotation: "3"}, nil), + }, + newIS: nil, + }, + expect: expect{ + err: false, + machineSets: []*machinev1.MachineSet{ + newMachineSet(mSetSpecTemplate, nameMachineSet1, 1, 500, nil, nil, map[string]string{DesiredReplicasAnnotation: "1", MaxReplicasAnnotation: "3"}, nil), + }, + }, + }), ) }) })