Skip to content

Merge node finalizer removal and node deletion into one step #1097

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
gardener-prow merged 11 commits into from
Apr 24, 2026
Merged

Merge node finalizer removal and node deletion into one step #1097

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
84353c6
Atomically perform finalizer removal
gagan16k Apr 10, 2026
0e10a8e
update tests
gagan16k Apr 10, 2026
2d52801
Check machine deletion status before adding finalizers
gagan16k Apr 10, 2026
acfa4a1
Refactor node update logic
gagan16k Apr 10, 2026
439698b
cleanup
gagan16k Apr 10, 2026
dcaf05a
Address review comments
gagan16k Apr 17, 2026
6bb7293
test fix
gagan16k Apr 17, 2026
136039e
consistency changes
gagan16k Apr 17, 2026
582a6ec
Address review comments
gagan16k Apr 20, 2026
9899422
fix test
gagan16k Apr 20, 2026
c1f5e06
revert parameter name
gagan16k Apr 20, 2026
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
3 changes: 0 additions & 3 deletions pkg/util/provider/machinecontroller/machine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,9 +665,6 @@ func (c *controller) triggerDeletionFlow(ctx context.Context, deleteMachineReque
case strings.Contains(machine.Status.LastOperation.Description, machineutils.InitiateVMDeletion):
return c.deleteVM(ctx, deleteMachineRequest)

case strings.Contains(machine.Status.LastOperation.Description, machineutils.RemoveNodeFinalizers):
return c.deleteNodeFinalizers(ctx, machine)

case strings.Contains(machine.Status.LastOperation.Description, machineutils.InitiateNodeDeletion):
return c.deleteNodeObject(ctx, machine)

Expand Down
116 changes: 7 additions & 109 deletions pkg/util/provider/machinecontroller/machine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3015,7 +3015,7 @@ var _ = Describe("machine", func() {
},
},
expect: expect{
err: fmt.Errorf("Machine deletion in process. VM deletion was successful. " + machineutils.RemoveNodeFinalizers),
err: fmt.Errorf("Machine deletion in process. VM deletion was successful. " + machineutils.InitiateNodeDeletion),
retry: machineutils.ShortRetry,
machine: newMachine(
&v1alpha1.MachineTemplateSpec{
Expand All @@ -3034,7 +3034,7 @@ var _ = Describe("machine", func() {
LastUpdateTime: metav1.Now(),
},
LastOperation: v1alpha1.LastOperation{
Description: fmt.Sprintf("VM deletion was successful. %s", machineutils.RemoveNodeFinalizers),
Description: fmt.Sprintf("VM deletion was successful. %s", machineutils.InitiateNodeDeletion),
State: v1alpha1.MachineStateProcessing,
Type: v1alpha1.MachineOperationDelete,
LastUpdateTime: metav1.Now(),
Expand All @@ -3052,7 +3052,7 @@ var _ = Describe("machine", func() {
),
},
}),
Entry("Delete node finalizers successfully", &data{
Entry("Delete node object successfully", &data{
setup: setup{
secrets: []*corev1.Secret{
{
Expand Down Expand Up @@ -3083,7 +3083,7 @@ var _ = Describe("machine", func() {
LastUpdateTime: metav1.Now(),
},
LastOperation: v1alpha1.LastOperation{
Description: fmt.Sprintf("VM deletion was successful. %s", machineutils.RemoveNodeFinalizers),
Description: fmt.Sprintf("VM deletion was successful. %s", machineutils.InitiateNodeDeletion),
State: v1alpha1.MachineStateProcessing,
Type: v1alpha1.MachineOperationDelete,
LastUpdateTime: metav1.Now(),
Expand Down Expand Up @@ -3120,109 +3120,7 @@ var _ = Describe("machine", func() {
},
},
expect: expect{
err: fmt.Errorf("machine deletion in process. Removal of finalizers from Node Object \"fakeID-0\" is successful. " + machineutils.InitiateNodeDeletion),
retry: machineutils.ShortRetry,
machine: newMachine(
&v1alpha1.MachineTemplateSpec{
ObjectMeta: *newObjectMeta(objMeta, 0),
Spec: v1alpha1.MachineSpec{
Class: v1alpha1.ClassSpec{
Kind: "MachineClass",
Name: "machine-0",
},
ProviderID: "fakeID",
},
},
&v1alpha1.MachineStatus{
CurrentStatus: v1alpha1.CurrentStatus{
Phase: v1alpha1.MachineTerminating,
LastUpdateTime: metav1.Now(),
},
LastOperation: v1alpha1.LastOperation{
Description: fmt.Sprintf("Removal of finalizers from Node Object %q is successful. %s", "fakeID-0", machineutils.InitiateNodeDeletion),
State: v1alpha1.MachineStateProcessing,
Type: v1alpha1.MachineOperationDelete,
LastUpdateTime: metav1.Now(),
},
},
nil,
map[string]string{
machineutils.MachinePriority: "3",
},
map[string]string{
v1alpha1.NodeLabelKey: "fakeID-0",
},
true,
metav1.Now(),
),
},
}),
Entry("Delete node object successfully", &data{
setup: setup{
secrets: []*corev1.Secret{
{
ObjectMeta: *newObjectMeta(objMeta, 0),
},
},
machineClasses: []*v1alpha1.MachineClass{
{
ObjectMeta: *newObjectMeta(objMeta, 0),
SecretRef: newSecretReference(objMeta, 0),
},
},
machines: newMachines(
1,
&v1alpha1.MachineTemplateSpec{
ObjectMeta: *newObjectMeta(objMeta, 0),
Spec: v1alpha1.MachineSpec{
Class: v1alpha1.ClassSpec{
Kind: "MachineClass",
Name: "machine-0",
},
ProviderID: "fakeID",
},
},
&v1alpha1.MachineStatus{
CurrentStatus: v1alpha1.CurrentStatus{
Phase: v1alpha1.MachineTerminating,
LastUpdateTime: metav1.Now(),
},
LastOperation: v1alpha1.LastOperation{
Description: fmt.Sprintf("Removal of finalizers from Node Object %q is successful. %s", "fakeID-0", machineutils.InitiateNodeDeletion),
State: v1alpha1.MachineStateProcessing,
Type: v1alpha1.MachineOperationDelete,
LastUpdateTime: metav1.Now(),
},
},
nil,
map[string]string{
machineutils.MachinePriority: "3",
},
map[string]string{
v1alpha1.NodeLabelKey: "fakeID-0",
},
true,
metav1.Now(),
),
nodes: []*corev1.Node{
{
ObjectMeta: metav1.ObjectMeta{
Name: "fakeID-0",
},
},
},
},
action: action{
machine: "machine-0",
fakeDriver: &driver.FakeDriver{
VMExists: true,
ProviderID: "fakeID-0",
NodeName: "fakeNode-0",
Err: nil,
},
},
expect: expect{
err: fmt.Errorf("Machine deletion in process. Deletion of node object was successful"),
err: fmt.Errorf("machine deletion in process: Deletion of Node Object %q is successful. %s", "fakeID-0", machineutils.InitiateFinalizerRemoval),
retry: machineutils.ShortRetry,
nodeDeleted: true,
machine: newMachine(
Expand Down Expand Up @@ -3604,7 +3502,7 @@ var _ = Describe("machine", func() {
},
},
expect: expect{
err: fmt.Errorf("Machine deletion in process. No node object found"),
err: fmt.Errorf("machine deletion in process: no node object found"),
retry: machineutils.ShortRetry,
nodeDeleted: false,
machine: newMachine(
Expand All @@ -3624,7 +3522,7 @@ var _ = Describe("machine", func() {
LastUpdateTime: metav1.Now(),
},
LastOperation: v1alpha1.LastOperation{
Description: fmt.Sprintf("Label %q not present on machine %q or no associated node object found, continuing deletion flow. %s", v1alpha1.NodeLabelKey, "machine-0", machineutils.InitiateFinalizerRemoval),
Description: fmt.Sprintf("Label %q not present on machine %q, continuing deletion flow. %s", v1alpha1.NodeLabelKey, "machine-0", machineutils.InitiateFinalizerRemoval),
State: v1alpha1.MachineStateProcessing,
Type: v1alpha1.MachineOperationDelete,
LastUpdateTime: metav1.Now(),
Expand Down
139 changes: 56 additions & 83 deletions pkg/util/provider/machinecontroller/machine_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1832,7 +1832,7 @@ func (c *controller) deleteVM(ctx context.Context, deleteMachineRequest *driver.
state = v1alpha1.MachineStateFailed
case codes.NotFound:
retryRequired = machineutils.ShortRetry
description = fmt.Sprintf("VM not found. Continuing deletion flow. %s", machineutils.RemoveNodeFinalizers)
description = fmt.Sprintf("VM not found. Continuing deletion flow. %s", machineutils.InitiateNodeDeletion)
state = v1alpha1.MachineStateProcessing
default:
retryRequired = machineutils.LongRetry
Expand All @@ -1847,7 +1847,7 @@ func (c *controller) deleteVM(ctx context.Context, deleteMachineRequest *driver.

} else {
retryRequired = machineutils.ShortRetry
description = fmt.Sprintf("VM deletion was successful. %s", machineutils.RemoveNodeFinalizers)
description = fmt.Sprintf("VM deletion was successful. %s", machineutils.InitiateNodeDeletion)
state = v1alpha1.MachineStateProcessing

err = fmt.Errorf("Machine deletion in process. %s", description)
Expand Down Expand Up @@ -1880,105 +1880,78 @@ func (c *controller) deleteVM(ctx context.Context, deleteMachineRequest *driver.
return retryRequired, err
}

// deleteNodeFinalizers attempts to remove finalizers from the node object backed by the machine object
func (c *controller) deleteNodeFinalizers(ctx context.Context, machine *v1alpha1.Machine) (machineutils.RetryPeriod, error) {
// deleteNodeObject attempts to remove finalizers from and delete the node object backed by the machine object
func (c *controller) deleteNodeObject(ctx context.Context, machine *v1alpha1.Machine) (machineutils.RetryPeriod, error) {
var (
err error
retryReason error
Comment thread
takoverflow marked this conversation as resolved.
Outdated
description string
state v1alpha1.MachineState
node *v1.Node
)

// nodeName label is expected to be present on machine object, but in case it is not present
// we should not block deletion of machine and should move forward with flow.
nodeName := machine.Labels[v1alpha1.NodeLabelKey]

if nodeName != "" {
var node *v1.Node
node, err = c.nodeLister.Get(nodeName)
if err != nil {
switch {
case apierrors.IsNotFound(err):
description = fmt.Sprintf("No node object found for %q, skipping node finalizer removal. %s", nodeName, machineutils.InitiateNodeDeletion)
klog.Warning(description)
state = v1alpha1.MachineStateProcessing
default:
description = fmt.Sprintf("Retrieval of Node Object %q failed due to error: %s, Retrying node finalizer removal. %s", nodeName, err, machineutils.RemoveNodeFinalizers)
klog.Errorf(description)
state = v1alpha1.MachineStateFailed
}
} else {
err = c.removeNodeFinalizers(ctx, node)
if err != nil {
description = fmt.Sprintf("Removal of finalizers from Node Object %q failed due to error: %s, Retrying node finalizer removal. %s", nodeName, err, machineutils.RemoveNodeFinalizers)
klog.Errorf(description)
state = v1alpha1.MachineStateFailed
} else {
description = fmt.Sprintf("Removal of finalizers from Node Object %q is successful. %s", nodeName, machineutils.InitiateNodeDeletion)
state = v1alpha1.MachineStateProcessing
err = fmt.Errorf("machine deletion in process. %s", description)
}
}
} else {
description = fmt.Sprintf("Label %q not present on machine %q, skipping node finalizer removal. %s", v1alpha1.NodeLabelKey, machine.Name, machineutils.InitiateNodeDeletion)
if nodeName == "" {
description = fmt.Sprintf("Label %q not present on machine %q, continuing deletion flow. %s", v1alpha1.NodeLabelKey, machine.Name, machineutils.InitiateFinalizerRemoval)
klog.Warning(description)
state = v1alpha1.MachineStateProcessing
}
retryReason = errors.New("machine deletion in process: no node object found")
Comment thread
takoverflow marked this conversation as resolved.
Outdated

updateRetryPeriod, updateErr := c.machineStatusUpdate(
ctx,
machine,
v1alpha1.LastOperation{
Description: description,
State: state,
Type: v1alpha1.MachineOperationDelete,
LastUpdateTime: metav1.Now(),
},
// Let the clone.Status.CurrentStatus (LastUpdateTime) be as it was before.
// This helps while computing when the drain timeout to determine if force deletion is to be triggered.
machine.Status.CurrentStatus,
machine.Status.LastKnownState,
)

if updateErr != nil {
return updateRetryPeriod, updateErr
return c.updateStatusForNodeDeletion(ctx, machine, description, state, retryReason)
}
return machineutils.ShortRetry, err
}

// deleteNodeObject attempts to delete the node object backed by the machine object
func (c *controller) deleteNodeObject(ctx context.Context, machine *v1alpha1.Machine) (machineutils.RetryPeriod, error) {
var (
err error
description string
state v1alpha1.MachineState
)

nodeName := machine.Labels[v1alpha1.NodeLabelKey]

if nodeName != "" {
// Delete node object
err = c.targetCoreClient.CoreV1().Nodes().Delete(ctx, nodeName, metav1.DeleteOptions{})
klog.V(3).Infof("Deleting node %q associated with machine %q", nodeName, machine.Name)
if err != nil && !apierrors.IsNotFound(err) {
// If its an error, and any other error than object not found
description = fmt.Sprintf("Deletion of Node Object %q failed due to error: %s. %s", nodeName, err, machineutils.InitiateNodeDeletion)
klog.Error(description)
state = v1alpha1.MachineStateFailed
} else if err == nil {
description = fmt.Sprintf("Deletion of Node Object %q is successful. %s", nodeName, machineutils.InitiateFinalizerRemoval)
klog.V(3).Info(description)
state = v1alpha1.MachineStateProcessing
err = fmt.Errorf("Machine deletion in process. Deletion of node object was successful")
} else {
// If node object is not found, then it could have already been deleted,
Comment thread
takoverflow marked this conversation as resolved.
Outdated
// therefore we should not block deletion of machine. But if error is other than NotFound,
// then we should retry as it could be a transient error.
node, retryReason = c.nodeLister.Get(nodeName)
if retryReason != nil {
switch {
case apierrors.IsNotFound(retryReason):
description = fmt.Sprintf("No node object found for %q, continuing deletion flow. %s", nodeName, machineutils.InitiateFinalizerRemoval)
klog.Warning(description)
state = v1alpha1.MachineStateProcessing
default:
description = fmt.Sprintf("Retrieval of Node Object %q failed due to error: %s, Retrying. %s", nodeName, retryReason, machineutils.InitiateNodeDeletion)
klog.Error(description)
state = v1alpha1.MachineStateFailed
}
} else {
description = fmt.Sprintf("Label %q not present on machine %q or no associated node object found, continuing deletion flow. %s", v1alpha1.NodeLabelKey, machine.Name, machineutils.InitiateFinalizerRemoval)

return c.updateStatusForNodeDeletion(ctx, machine, description, state, retryReason)
}

// Remove finalizers from node object before deletion.
klog.V(3).Infof("Removing finalizers from node %q associated with machine %q", nodeName, machine.Name)
retryReason = c.removeNodeFinalizers(ctx, node)
if retryReason != nil {
description = fmt.Sprintf("Removal of finalizers from Node Object %q failed due to error: %s, Retrying. %s", nodeName, retryReason, machineutils.InitiateNodeDeletion)
klog.Error(description)
state = v1alpha1.MachineStateFailed
return c.updateStatusForNodeDeletion(ctx, machine, description, state, retryReason)
}

// Delete node object.
klog.V(3).Infof("Deleting node %q associated with machine %q", nodeName, machine.Name)
retryReason = c.targetCoreClient.CoreV1().Nodes().Delete(ctx, nodeName, metav1.DeleteOptions{})
switch {
case retryReason == nil:
description = fmt.Sprintf("Deletion of Node Object %q is successful. %s", nodeName, machineutils.InitiateFinalizerRemoval)
klog.V(3).Info(description)
state = v1alpha1.MachineStateProcessing
retryReason = fmt.Errorf("machine deletion in process: %s", description)
case !apierrors.IsNotFound(retryReason):
Comment thread
takoverflow marked this conversation as resolved.
Outdated
description = fmt.Sprintf("Deletion of Node Object %q failed due to error: %s. %s", nodeName, retryReason, machineutils.InitiateNodeDeletion)
klog.Error(description)
state = v1alpha1.MachineStateFailed
default:
description = fmt.Sprintf("No node object found for %q, continuing deletion flow. %s", nodeName, machineutils.InitiateFinalizerRemoval)
klog.Warning(description)
state = v1alpha1.MachineStateProcessing
err = fmt.Errorf("Machine deletion in process. No node object found")
}
return c.updateStatusForNodeDeletion(ctx, machine, description, state, retryReason)
}

func (c *controller) updateStatusForNodeDeletion(ctx context.Context, machine *v1alpha1.Machine, description string, state v1alpha1.MachineState, retryReason error) (machineutils.RetryPeriod, error) {
updateRetryPeriod, updateErr := c.machineStatusUpdate(
ctx,
machine,
Expand All @@ -1999,7 +1972,7 @@ func (c *controller) deleteNodeObject(ctx context.Context, machine *v1alpha1.Mac
return updateRetryPeriod, updateErr
}

return machineutils.ShortRetry, err
return machineutils.ShortRetry, retryReason
}

// getEffectiveDrainTimeout returns the drainTimeout set on the machine-object, otherwise returns the timeout set using the global-flag.
Expand Down
Loading
Loading