From 4a34a724a1b07d66747c0eee646191d61db6d498 Mon Sep 17 00:00:00 2001 From: Sonu Kumar Singh Date: Tue, 22 Jul 2025 20:40:05 +0530 Subject: [PATCH 1/3] Minor logging improvement --- pkg/controller/deployment_inplace.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/controller/deployment_inplace.go b/pkg/controller/deployment_inplace.go index 914f707df..02f0f3aad 100644 --- a/pkg/controller/deployment_inplace.go +++ b/pkg/controller/deployment_inplace.go @@ -10,6 +10,7 @@ import ( "maps" "slices" "sort" + "strings" "github.com/gardener/machine-controller-manager/pkg/apis/machine/v1alpha1" "github.com/gardener/machine-controller-manager/pkg/controller/autoscaler" @@ -206,6 +207,7 @@ func (dc *controller) syncMachineSets(ctx context.Context, oldMachineSets []*v1a Effect: v1.TaintEffectNoSchedule, }) + klog.V(3).Infof("removing inplace labels/annotations and uncordoning node %s", node.Name) _, err = dc.targetCoreClient.CoreV1().Nodes().Update(ctx, node, metav1.UpdateOptions{}) if err != nil { return fmt.Errorf("failed to remove inplace labels/annotations and uncordon node %s: %w", node.Name, err) @@ -525,7 +527,12 @@ func (dc *controller) labelMachinesToSelectedForUpdate(ctx context.Context, mach return numOfMachinesSelectedForUpdate, err } - klog.V(3).Infof("machines selected for drain %v", machines) + machinesName := make([]string, 0, len(machines)) + for _, machine := range machines { + machinesName = append(machinesName, machine.Name) + } + + klog.V(3).Infof("machines selected for drain %v", strings.Join(machinesName, ", ")) for _, machine := range machines { // labels on the node are added cumulatively and we can find both candidate-for-update and selected-for-update labels on the node. From 43d128d7e5df9de99dc47e82ef884e0eeb7760a1 Mon Sep 17 00:00:00 2001 From: Sonu Kumar Singh Date: Tue, 22 Jul 2025 23:53:02 +0530 Subject: [PATCH 2/3] If machine is in phase `InPlaceUpdating` it should not be moved to new machine set --- pkg/controller/deployment_inplace.go | 2 +- pkg/controller/deployment_inplace_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controller/deployment_inplace.go b/pkg/controller/deployment_inplace.go index 02f0f3aad..e3a81d559 100644 --- a/pkg/controller/deployment_inplace.go +++ b/pkg/controller/deployment_inplace.go @@ -369,7 +369,7 @@ func (dc *controller) transferMachinesFromOldToNewMachineSet(ctx context.Context } cond := getMachineCondition(oldMachine, v1alpha1.NodeInPlaceUpdate) - if isUpdateNotSuccessful(cond, node.Labels) { + if isUpdateNotSuccessful(cond, node.Labels) || oldMachine.Status.CurrentStatus.Phase == v1alpha1.MachineInPlaceUpdating { continue } diff --git a/pkg/controller/deployment_inplace_test.go b/pkg/controller/deployment_inplace_test.go index b837f9fbb..6120de61d 100644 --- a/pkg/controller/deployment_inplace_test.go +++ b/pkg/controller/deployment_inplace_test.go @@ -267,7 +267,7 @@ var _ = Describe("deployment_inplace", func() { controlMachineObjects = append(controlMachineObjects, oldMachineSet, newMachineSet) machines := []*machinev1.Machine{} - machines = append(machines, newMachinesFromMachineSet(int(data.setup.oldMachineSetReplicas), oldMachineSet, &machinev1.MachineStatus{}, nil, map[string]string{"key": "value"})...) + machines = append(machines, newMachinesFromMachineSet(int(data.setup.oldMachineSetReplicas), oldMachineSet, &machinev1.MachineStatus{CurrentStatus: machinev1.CurrentStatus{Phase: machinev1.MachineInPlaceUpdateSuccessful}}, nil, map[string]string{"key": "value"})...) machines = append(machines, newMachinesFromMachineSet(int(data.setup.newMachineSetReplicas), newMachineSet, &machinev1.MachineStatus{}, nil, nil)...) machinesWithUpdateSuccessful := 0 for i := range machines { From 334f6fff3f58ea2022b1263038907486531b86be Mon Sep 17 00:00:00 2001 From: Sonu Kumar Singh Date: Wed, 23 Jul 2025 00:09:18 +0530 Subject: [PATCH 3/3] Prevent negative value calculation --- pkg/controller/deployment_inplace.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/deployment_inplace.go b/pkg/controller/deployment_inplace.go index e3a81d559..1c06091c3 100644 --- a/pkg/controller/deployment_inplace.go +++ b/pkg/controller/deployment_inplace.go @@ -303,7 +303,7 @@ func (dc *controller) reconcileOldMachineSetsInPlace(ctx context.Context, allMac maxUnavailable := MaxUnavailable(*deployment) minAvailable := deployment.Spec.Replicas - maxUnavailable - newMachineSetUnavailableMachineCount := newMachineSet.Spec.Replicas - newMachineSet.Status.AvailableReplicas + newMachineSetUnavailableMachineCount := max(0, newMachineSet.Spec.Replicas-newMachineSet.Status.AvailableReplicas) oldMachineSetsMachinesUndergoingUpdate, err := dc.getMachinesUndergoingUpdate(oldMachineSets) if err != nil { return false, err