Skip to content

Trigger node deletion when machine is deleted - #1102

Merged
gardener-prow[bot] merged 12 commits into
gardener:masterfrom
gagan16k:fix/machine-delete
Sep 8, 2026
Merged

Trigger node deletion when machine is deleted#1102
gardener-prow[bot] merged 12 commits into
gardener:masterfrom
gagan16k:fix/machine-delete

Conversation

@gagan16k

@gagan16k gagan16k commented May 18, 2026

Copy link
Copy Markdown
Member

What this PR does / why we need it:

  1. Machine controller now explicitly deletes the finalizer and backing node from the target cluster on machine delete events
  2. The machine status description during creation has been updated from "Creating machine on cloud provider" to "VM created on cloud provider. Waiting for node registration", so the status accurately reflects which stage the machine is in.

Which issue(s) this PR fixes:
Fixes #1044, #1064

Tests:
Passed unit tests

Manual tests performed by deleting node manually

(⎈|garden--aws-ha-external:garden shoot--i765230--demo)➜  machine-controller-manager git:(pr/gagan16k/1102) k get mc
NAME                                              STATUS    AGE     NODE
shoot--i765230--demo-worker-cpu-z1-7988d-hnw4w    Running   7h23m   ip-10-180-3-122.eu-west-1.compute.internal
shoot--i765230--demo-worker-etcd-z1-5dc7f-ttqd5   Running   7h23m   ip-10-180-137-245.eu-west-1.compute.internal

(⎈|garden-i765230--demo-external:default garden-i765230)➜  machine-controller-manager git:(pr/gagan16k/1102) k get no
NAME                                           STATUS   ROLES    AGE     VERSION
ip-10-180-137-245.eu-west-1.compute.internal   Ready    worker   7h22m   v1.34.9
ip-10-180-3-122.eu-west-1.compute.internal     Ready    worker   7h22m   v1.34.9



# Force-delete machine by removing finalizer
(⎈|garden--aws-ha-external:garden shoot--i765230--demo)➜  machine-controller-manager git:(pr/gagan16k/1102) k -n shoot--i765230--demo patch machine shoot--i765230--demo-worker-cpu-z1-7988d-hnw4w --type=merge -p '{"metadata":{"finalizers":null}}'
machine.machine.sapcloud.io/shoot--i765230--demo-worker-cpu-z1-7988d-hnw4w patched
(⎈|garden--aws-ha-external:garden shoot--i765230--demo)➜  machine-controller-manager git:(pr/gagan16k/1102) k -n shoot--i765230--demo delete machine shoot--i765230--demo-worker-cpu-z1-7988d-hnw4w
machine.machine.sapcloud.io "shoot--i765230--demo-worker-cpu-z1-7988d-hnw4w" deleted



# Machine and node deleted
(⎈|garden-i765230--demo-external:default garden-i765230)➜  machine-controller-manager git:(pr/gagan16k/1102) k get no
NAME                                           STATUS   ROLES    AGE     VERSION
ip-10-180-137-245.eu-west-1.compute.internal   Ready    worker   7h24m   v1.34.9

(⎈|garden--aws-ha-external:garden shoot--i765230--demo)➜  machine-controller-manager git:(pr/gagan16k/1102) k get mc
NAME                                              STATUS    AGE     NODE
shoot--i765230--demo-worker-cpu-z1-7988d-h299t              3s
shoot--i765230--demo-worker-etcd-z1-5dc7f-ttqd5   Running   7h25m   ip-10-180-137-245.eu-west-1.compute.internal

For #1064, machine during creation has message

Last Operation:
    Description:       VM created on cloud provider. Waiting for node registration
    Last Update Time:  2026-08-12T06:20:01Z
    State:             Processing
    Type:              Create

Release note:

MCM now deletes the backing node when its machine is force-deleted, and clears finalizers on nodes terminating without a backing machine. 
Machine creation status is also modified to distinguish cloud provider failures from node registration failures.

@gagan16k
gagan16k requested a review from a team as a code owner May 18, 2026 12:33
@gardener-prow gardener-prow Bot added do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. and removed cla: no Indicates the PR's author has not signed the cla-assistant.io CLA. labels May 18, 2026
@aaronfern aaronfern added the kind/bug Bug label May 22, 2026
@gardener-prow gardener-prow Bot removed the do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. label May 22, 2026

@thiyyakat thiyyakat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. Just two comments. PTAL.

Comment thread pkg/util/provider/machinecontroller/machine.go Outdated
Comment thread pkg/util/provider/machinecontroller/node.go Outdated
Comment thread pkg/util/provider/machinecontroller/machine_test.go Outdated
@gardener-ci-robot

Copy link
Copy Markdown
Contributor

The Gardener project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:

  • After 30d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 14d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as active with /lifecycle active
  • Mark this PR as fresh with /remove-lifecycle stale
  • Mark this PR as rotten with /lifecycle rotten
  • Close this PR with /close

/lifecycle stale

@gardener-prow gardener-prow Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 4, 2026
@gardener-ci-robot

Copy link
Copy Markdown
Contributor

The Gardener project currently lacks enough active contributors to adequately respond to all PRs.
This bot triages PRs according to the following rules:

  • After 30d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 14d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Mark this PR as active with /lifecycle active
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Close this PR with /close

/lifecycle rotten

@gardener-prow gardener-prow Bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Aug 3, 2026
@aaronfern aaronfern removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 3, 2026
@gardener-prow gardener-prow Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 7, 2026
@gagan16k
gagan16k force-pushed the fix/machine-delete branch from 3a45864 to 9864805 Compare August 7, 2026 08:00

@thiyyakat thiyyakat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one small nit.

Comment thread pkg/util/provider/machinecontroller/node.go Outdated
@thiyyakat

Copy link
Copy Markdown
Member

Thanks for addressing the comments.
/lgtm

@gardener-prow gardener-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2026
@gardener-prow

gardener-prow Bot commented Aug 12, 2026

Copy link
Copy Markdown

LGTM label has been added.

DetailsGit tree hash: 7c6a71242f80f6ef4851c138c0bece6fce8dbd6c

@takoverflow

Copy link
Copy Markdown
Member

/assign

@gardener-prow gardener-prow Bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2026
@gardener-prow
gardener-prow Bot requested a review from takoverflow August 12, 2026 06:44
@thiyyakat

Copy link
Copy Markdown
Member

/lgtm

@gardener-prow gardener-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 12, 2026
Comment thread pkg/util/provider/machinecontroller/machine.go Outdated

@thiyyakat thiyyakat left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Gagan. I had one concern about the logs. If error can be handled differently in the callee and caller, maybe it can be improved.

Comment thread pkg/util/provider/machinecontroller/machine.go Outdated
Comment thread pkg/util/provider/machinecontroller/machine.go Outdated
Comment thread pkg/util/provider/machinecontroller/machine.go Outdated
@gardener-prow gardener-prow Bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 27, 2026
…riptions

Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
Signed-off-by: Gagan163264 <gagan.surathkal@gmail.com>
@gagan16k
gagan16k force-pushed the fix/machine-delete branch from 0d4ef7e to cfe75f3 Compare August 27, 2026 12:40

@takoverflow takoverflow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@gardener-prow gardener-prow Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 28, 2026
@gardener-prow

gardener-prow Bot commented Aug 28, 2026

Copy link
Copy Markdown

LGTM label has been added.

DetailsGit tree hash: ce3388f6ae7953dc547a6e02da2a2e7062d0e7ce

@thiyyakat

Copy link
Copy Markdown
Member

Thanks for the multiple rounds of changes!

/lgtm

Comment thread pkg/util/provider/machinecontroller/machine.go
@r4mek

r4mek commented Sep 8, 2026

Copy link
Copy Markdown
Member

/lgtm

@gardener-prow

gardener-prow Bot commented Sep 8, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: thiyyakat

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gardener-prow gardener-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 8, 2026
@gardener-prow
gardener-prow Bot merged commit fa1ae8e into gardener:master Sep 8, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/bug Bug lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCM should always delete Node if corresponding Machine object is not present during termination flow

6 participants