diff --git a/controlplane/kubeadm/internal/controllers/controller.go b/controlplane/kubeadm/internal/controllers/controller.go index 39e5a7bb908d..383f9dfda1b5 100644 --- a/controlplane/kubeadm/internal/controllers/controller.go +++ b/controlplane/kubeadm/internal/controllers/controller.go @@ -610,8 +610,29 @@ func (r *KubeadmControlPlaneReconciler) syncMachines(ctx context.Context, contro patchHelpers := map[string]*patch.Helper{} for machineName := range controlPlane.Machines { m := controlPlane.Machines[machineName] - // If the machine is already being deleted, we don't need to update it. + // If the Machine is already being deleted, we only need to sync + // the subset of fields that impact tearing down the Machine. if !m.DeletionTimestamp.IsZero() { + patchHelper, err := patch.NewHelper(m, r.Client) + if err != nil { + return err + } + + // Set all other in-place mutable fields that impact the ability to tear down existing machines. + m.Spec.NodeDrainTimeout = controlPlane.KCP.Spec.MachineTemplate.NodeDrainTimeout + m.Spec.NodeDeletionTimeout = controlPlane.KCP.Spec.MachineTemplate.NodeDeletionTimeout + m.Spec.NodeVolumeDetachTimeout = controlPlane.KCP.Spec.MachineTemplate.NodeVolumeDetachTimeout + + if err := patchHelper.Patch(ctx, m); err != nil { + return err + } + + controlPlane.Machines[machineName] = m + patchHelper, err = patch.NewHelper(m, r.Client) + if err != nil { + return err + } + patchHelpers[machineName] = patchHelper continue } diff --git a/controlplane/kubeadm/internal/controllers/controller_test.go b/controlplane/kubeadm/internal/controllers/controller_test.go index da71a74ff1b3..7376556a2bdf 100644 --- a/controlplane/kubeadm/internal/controllers/controller_test.go +++ b/controlplane/kubeadm/internal/controllers/controller_test.go @@ -1554,6 +1554,9 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) { Bootstrap: clusterv1.Bootstrap{ DataSecretName: ptr.To("machine-bootstrap-secret"), }, + NodeDrainTimeout: duration5s, + NodeVolumeDetachTimeout: duration5s, + NodeDeletionTimeout: duration5s, }, } g.Expect(env.Create(ctx, deletingMachine, client.FieldOwner(classicManager))).To(Succeed()) @@ -1636,16 +1639,16 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) { KCP: kcp, Cluster: testCluster, Machines: collections.Machines{ - inPlaceMutatingMachine.Name: inPlaceMutatingMachine, - deletingMachine.Name: deletingMachine, - nilInfraMachineMachine.Name: nilInfraMachineMachine, + inPlaceMutatingMachine.Name: inPlaceMutatingMachine.DeepCopy(), + deletingMachine.Name: deletingMachine.DeepCopy(), + nilInfraMachineMachine.Name: nilInfraMachineMachine.DeepCopy(), }, KubeadmConfigs: map[string]*bootstrapv1.KubeadmConfig{ - inPlaceMutatingMachine.Name: existingKubeadmConfig, + inPlaceMutatingMachine.Name: existingKubeadmConfig.DeepCopy(), deletingMachine.Name: nil, }, InfraResources: map[string]*unstructured.Unstructured{ - inPlaceMutatingMachine.Name: existingInfraMachine, + inPlaceMutatingMachine.Name: existingInfraMachine.DeepCopy(), deletingMachine.Name: nil, }, } @@ -1806,7 +1809,14 @@ func TestKubeadmControlPlaneReconciler_syncMachines(t *testing.T) { // Verify the machine labels and annotations are unchanged. g.Expect(updatedDeletingMachine.Labels).Should(Equal(deletingMachine.Labels)) g.Expect(updatedDeletingMachine.Annotations).Should(Equal(deletingMachine.Annotations)) - // Verify the machine spec is unchanged. + // Verify Node timeout values + g.Expect(updatedDeletingMachine.Spec.NodeDrainTimeout).Should(Equal(kcp.Spec.MachineTemplate.NodeDrainTimeout)) + g.Expect(updatedDeletingMachine.Spec.NodeDeletionTimeout).Should(Equal(kcp.Spec.MachineTemplate.NodeDeletionTimeout)) + g.Expect(updatedDeletingMachine.Spec.NodeVolumeDetachTimeout).Should(Equal(kcp.Spec.MachineTemplate.NodeVolumeDetachTimeout)) + // Verify the machine spec is otherwise unchanged. + deletingMachine.Spec.NodeDrainTimeout = kcp.Spec.MachineTemplate.NodeDrainTimeout + deletingMachine.Spec.NodeDeletionTimeout = kcp.Spec.MachineTemplate.NodeDeletionTimeout + deletingMachine.Spec.NodeVolumeDetachTimeout = kcp.Spec.MachineTemplate.NodeVolumeDetachTimeout g.Expect(updatedDeletingMachine.Spec).Should(BeComparableTo(deletingMachine.Spec)) } diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go index b4e2cc149c85..9fe9d950e7b0 100644 --- a/internal/controllers/machineset/machineset_controller.go +++ b/internal/controllers/machineset/machineset_controller.go @@ -375,7 +375,7 @@ func (r *Reconciler) syncMachines(ctx context.Context, machineSet *clusterv1.Mac if !m.DeletionTimestamp.IsZero() { patchHelper, err := patch.NewHelper(m, r.Client) if err != nil { - return errors.Wrapf(err, "failed to generate patch for Machine %q", klog.KObj(m)) + return err } // Set all other in-place mutable fields that impact the ability to tear down existing machines. @@ -383,10 +383,8 @@ func (r *Reconciler) syncMachines(ctx context.Context, machineSet *clusterv1.Mac m.Spec.NodeDeletionTimeout = machineSet.Spec.Template.Spec.NodeDeletionTimeout m.Spec.NodeVolumeDetachTimeout = machineSet.Spec.Template.Spec.NodeVolumeDetachTimeout - err = patchHelper.Patch(ctx, m) - if err != nil { - log.Error(err, "Failed to update Machine", "Machine", klog.KObj(m)) - return errors.Wrapf(err, "failed to update Machine %q", klog.KObj(m)) + if err := patchHelper.Patch(ctx, m); err != nil { + return err } continue }