dont skip drain for unhealthy nodes #839

elankath · 2023-08-07T18:07:25Z

What this PR does / why we need it:
See #781

Skipping drain for un-healthy nodes causes volume attachments to still be associated with old node.
attach/detach controller in KCM takes maxWaitForUnmountDuration (6m) to expire attached volumes.
After performing force drain, we also need to explicitly delete volume attachments for the old node. This prevents triggering maxWaitForUnmountDuration from taking affect and volumes are attached to replacement node (if any).

Which issue(s) this PR fixes:
Fixes #781

Special notes for your reviewer:

Release note:

Force drain and delete volume attachments for nodes un-healthy due to `ReadOnlyFileSystem` and `NotReady` for too long

elankath · 2023-08-22T02:11:34Z

Test Scenario

Apply Deployment with several replicas and volumes
Crash Kubelet for a node using systemctl stop kubelet
Wait till Old Node becomes NotReady
Wait till old machine becomes Terminating
Wait till new machine is launched by MCM
Wait till new Machine is Running and new Node is Ready
Wait till Pods are Running on new node.
Measure time taken from old node NotReady to Pods Running on new node.
Check KCM Log to see presence of maxWaitForUnmountDuration timeout

Observations

Situation Before Fix

21:22 -> crash kubelet
21:25 -> node become NotReady
21:35 -> old node becomes terminating, new node hvcf6 launches
21:39 -> new node becomes Running
21:39-> new pods scheduled ContainerCreating on new node
...unfortunately volume attachments are still associated with old node 7rrtb for several minutes. this node is no longer there. it has been terminated
I0809 21:44:02.428680       1 reconciler.go:274] "attacherDetacher.DetachVolume started: this volume is not safe to detach, but maxWaitForUnmountDuration expired, force detaching" duration="6m0s" volume={AttachedVolume:{VolumeName:kubernetes.io/csi/disk.csi.azure.com^/subscriptions/82b44c79-a5d4-4d74-8ff8-8639e79c1c39/resourceGroups/shoot--i034796--trea/providers/Microsoft.Compute/disks/pv-shoot--i034796--trea-92d86bf3-491f-4fda-bfe5-9846e5ca80ad VolumeSpec:0xc00396f098 NodeName:shoot--i034796--trea-trea-z1-66866-7rrtb PluginIsAttachable:true DevicePath: DeviceMountPath: PluginName: SELinuxMountContext:} MountedByNode:true DetachRequestedTime:2023-08-09 21:38:02.358419312 +0000 UTC m=+8110.954960133}
21:43-> new pods are in Running. new volumes associated with  7rrtb

Time Taken ~ 21 mins

Situation After Fix

06:04 -> crashed kubelet
06:07 -> hvcf6 became not ready, machine becomes Unknown
06:14 -> machine terminating
06:15-> new machine launched
06:17 -> pods in ContainerCreating
06:18 -> x78h7 becomes Running 
06:19 -> pods become Running

There is NO maxWaitForUnmountDuration in KCM log

Time Taken ~ 15 mins.

As can be seen there is an improvement of ~6 minutes.

elankath · 2023-09-21T06:39:20Z

All review comments addressed

Fix text of stage DelVolumesAttachments
Add an FAQ question: "How does MCM handle an un-healthy node ?" in the Internals section.
Undo change for constant in now := metav1.Now() while doing machineStatusUpdate.
Change godoc of function deleteNodeVolAttachments. Remove reference to wait.
Log the joined error returned from deleteVolumeAttachmentsForNode
Log after deleting volume attachments.

elankath · 2023-09-21T10:07:53Z

Re-tested after all review changes

kubelet killed at Thu Sep 21 09:07:34 UTC 2023

Poll Deployments,Pods,VolumeAttachments,Nodes at UTC Time: Thu Sep 21 09:10:37 UTC 2023, Local Time: Thu Sep 21 09:10:37 UTC 2023

NAME                         READY   UP-TO-DATE   AVAILABLE   AGE   CONTAINERS   IMAGES   SELECTOR
deployment.apps/vol-test-a   0/4     4            0           46m   ubuntu       ubuntu   app=vol-test-a

NAME                              READY   STATUS    RESTARTS   AGE   IP           NODE                                   NOMINATED NODE   READINESS GATES
pod/vol-test-a-55bbbb77b6-4fwtx   1/1     Running   0          29m   10.180.0.5   shoot--i034796--az1-a-z1-85cf5-m9rfx   <none>           <none>
pod/vol-test-a-55bbbb77b6-6sbn9   1/1     Running   0          29m   10.180.0.5   shoot--i034796--az1-a-z1-85cf5-m9rfx   <none>           <none>
pod/vol-test-a-55bbbb77b6-7kgdw   1/1     Running   0          29m   10.180.0.5   shoot--i034796--az1-a-z1-85cf5-m9rfx   <none>           <none>
pod/vol-test-a-55bbbb77b6-sg8gs   1/1     Running   0          29m   10.180.0.5   shoot--i034796--az1-a-z1-85cf5-m9rfx   <none>           <none>

NAME                                                                                                   ATTACHER             PV                                                            NODE                                   ATTACHED   AGE
volumeattachment.storage.k8s.io/csi-08af6e9b391c406a84fe1206590764fb6076e0b657b47d9a2dc14b18c39bf2c7   disk.csi.azure.com   pv-shoot--i034796--az1-4f4b7484-3029-4603-8806-dcaad8c1da2b   shoot--i034796--az1-a-z1-85cf5-m9rfx   true       13m
volumeattachment.storage.k8s.io/csi-5615adc7d535b4b91cce4421a32a09d52b46111099c7d15188b033c8379227f5   disk.csi.azure.com   pv-shoot--i034796--az1-dba037bb-eb4b-45c0-85bf-87e19b8977c0   shoot--i034796--az1-a-z1-85cf5-m9rfx   true       13m
volumeattachment.storage.k8s.io/csi-585bf1aa7caf08a2686aa04ca4fcb7827e46a4520bb978983a638416b2957680   disk.csi.azure.com   pv-shoot--i034796--az1-040457ee-eaac-4b01-b690-92a814a91788   shoot--i034796--az1-a-z1-85cf5-m9rfx   true       13m
volumeattachment.storage.k8s.io/csi-99c05671376884fd7dc6663ba0a118195ae5b118a22c7991dd7e7785b6b16e11   disk.csi.azure.com   pv-shoot--i034796--az1-fcae9b4f-28b1-4483-aaf0-00a71f368b32   shoot--i034796--az1-a-z1-85cf5-m9rfx   true       13m
NAME                                   STATUS     ROLES    AGE    VERSION
shoot--i034796--az1-a-z1-85cf5-djz2j   Ready      <none>   19m    v1.26.7
shoot--i034796--az1-a-z1-85cf5-m9rfx   NotReady   <none>   121m   v1.26.7

Poll Deployments,Pods,VolumeAttachments,Nodes at UTC Time: Thu Sep 21 09:16:39 UTC 2023, Local Time: Thu Sep 21 09:16:39 UTC 2023

NAME                         READY   UP-TO-DATE   AVAILABLE   AGE   CONTAINERS   IMAGES   SELECTOR
deployment.apps/vol-test-a   0/4     4            0           52m   ubuntu       ubuntu   app=vol-test-a

NAME                              READY   STATUS              RESTARTS   AGE    IP           NODE                                   NOMINATED NODE   READINESS GATES
pod/vol-test-a-55bbbb77b6-dmk8n   0/1     ContainerCreating   0          118s   10.180.0.6   shoot--i034796--az1-a-z1-85cf5-djz2j   <none>           <none>
pod/vol-test-a-55bbbb77b6-fkn7k   0/1     ContainerCreating   0          118s   10.180.0.6   shoot--i034796--az1-a-z1-85cf5-djz2j   <none>           <none>
pod/vol-test-a-55bbbb77b6-mc52s   0/1     ContainerCreating   0          118s   10.180.0.6   shoot--i034796--az1-a-z1-85cf5-djz2j   <none>           <none>
pod/vol-test-a-55bbbb77b6-wnnhg   0/1     ContainerCreating   0          118s   10.180.0.6   shoot--i034796--az1-a-z1-85cf5-djz2j   <none>           <none>
NAME                                   STATUS                        ROLES    AGE    VERSION
shoot--i034796--az1-a-z1-85cf5-djz2j   Ready                         <none>   25m    v1.26.7
shoot--i034796--az1-a-z1-85cf5-m9rfx   NotReady,SchedulingDisabled   <none>   127m   v1.26.7

Poll Deployments,Pods,VolumeAttachments,Nodes at UTC Time: Thu Sep 21 09:18:49 UTC 2023, Local Time: Thu Sep 21 09:18:49 UTC 2023

NAME                         READY   UP-TO-DATE   AVAILABLE   AGE   CONTAINERS   IMAGES   SELECTOR
deployment.apps/vol-test-a   4/4     4            4           54m   ubuntu       ubuntu   app=vol-test-a

NAME                              READY   STATUS    RESTARTS   AGE    IP           NODE                                   NOMINATED NODE   READINESS GATES
pod/vol-test-a-55bbbb77b6-dmk8n   1/1     Running   0          4m9s   10.180.0.6   shoot--i034796--az1-a-z1-85cf5-djz2j   <none>           <none>
pod/vol-test-a-55bbbb77b6-fkn7k   1/1     Running   0          4m9s   10.180.0.6   shoot--i034796--az1-a-z1-85cf5-djz2j   <none>           <none>
pod/vol-test-a-55bbbb77b6-mc52s   1/1     Running   0          4m9s   10.180.0.6   shoot--i034796--az1-a-z1-85cf5-djz2j   <none>           <none>
pod/vol-test-a-55bbbb77b6-wnnhg   1/1     Running   0          4m9s   10.180.0.6   shoot--i034796--az1-a-z1-85cf5-djz2j   <none>           <none>

NAME                                                                                                   ATTACHER             PV                                                            NODE                                   ATTACHED   AGE
volumeattachment.storage.k8s.io/csi-0779eb4f890f6377eb07997244fcbd794e9a63fc3fb0c13ea4414ac8ca67ca84   disk.csi.azure.com   pv-shoot--i034796--az1-040457ee-eaac-4b01-b690-92a814a91788   shoot--i034796--az1-a-z1-85cf5-djz2j   true       111s
volumeattachment.storage.k8s.io/csi-07f618ac82174b622379ae088200d2390615e7005fb2fbf589f2a8cdbea9f45b   disk.csi.azure.com   pv-shoot--i034796--az1-fcae9b4f-28b1-4483-aaf0-00a71f368b32   shoot--i034796--az1-a-z1-85cf5-djz2j   true       111s
volumeattachment.storage.k8s.io/csi-9e2659a49d9a40e9f5d7b1d2378e52e54739464c4c266a1b82e973d9c0983bb7   disk.csi.azure.com   pv-shoot--i034796--az1-dba037bb-eb4b-45c0-85bf-87e19b8977c0   shoot--i034796--az1-a-z1-85cf5-djz2j   true       111s
volumeattachment.storage.k8s.io/csi-c59e72b600d9a63c5a5a702a05cd3be93b0de263c782e199f255439d52def5ac   disk.csi.azure.com   pv-shoot--i034796--az1-4f4b7484-3029-4603-8806-dcaad8c1da2b   shoot--i034796--az1-a-z1-85cf5-djz2j   true       111s
NAME                                   STATUS   ROLES    AGE   VERSION
shoot--i034796--az1-a-z1-85cf5-74rpz   Ready    <none>   44s   v1.26.7
shoot--i034796--az1-a-z1-85cf5-djz2j   Ready    <none>   27m   v1.26.7

Pods on new node come up in ~12 mins. There is no maxWaitForUnmountDuration in the kube controller log.

pkg/util/provider/machinecontroller/machine_util.go

docs/FAQ.md

elankath · 2023-09-22T07:52:57Z

Additional Test Requested

Attempt to skip the drain and move directly to deletion of node volumes.

Test Steps

Introduced code for testing in controller.drainNode to skip drain and simulate error.

			if false { // SKIPPED OUT TO CHECK BEHAVIOUR IF POD DEL DIDN't OCCUR
				err = drainOptions.RunDrain(ctx) 
			}
			err = fmt.Errorf("PRETEND DRAIN RAN INTO ERR")

Now crash the node kubelet and check behaviour. New machine should be launched and Pod's should be moved to new node.

Test Result.

Test success.

kubelet crashed at

k attach -it vol-test-a-55bbbb77b6-4mgn8
date -u && systemctl stop kubelet
Fri Sep 22 08:56:48 UTC 2023

the kcm already handles this case. one gets logs of the form I0922 09:07:45.010348 1 gc_controller.go:247] "Found orphaned Pod assigned to the Node, deleting." pod="kube-system/csi-driver-node-file-ltrdt" node="shoot--i034796--az1-a-z1-85cf5-g69js"
Pods up and running on new node in ~12 mins time

Poll Deployments,Pods,VolumeAttachments,Nodes at UTC Time: Fri Sep 22 09:08:16 UTC 2023, Local Time: Fri Sep 22 09:08:16 UTC 2023

NAME                         READY   UP-TO-DATE   AVAILABLE   AGE   CONTAINERS   IMAGES   SELECTOR
deployment.apps/vol-test-a   4/4     4            4           24h   ubuntu       ubuntu   app=vol-test-a

NAME                              READY   STATUS    RESTARTS   AGE     IP           NODE                                   NOMINATED NODE   READINESS GATES
pod/vol-test-a-55bbbb77b6-2w222   1/1     Running   0          4m26s   10.180.0.4   shoot--i034796--az1-a-z1-85cf5-hpncg   <none>           <none>
pod/vol-test-a-55bbbb77b6-2z8jl   1/1     Running   0          4m26s   10.180.0.4   shoot--i034796--az1-a-z1-85cf5-hpncg   <none>           <none>
pod/vol-test-a-55bbbb77b6-7cb9t   1/1     Running   0          4m26s   10.180.0.4   shoot--i034796--az1-a-z1-85cf5-hpncg   <none>           <none>
pod/vol-test-a-55bbbb77b6-jmngh   1/1     Running   0          4m26s   10.180.0.4   shoot--i034796--az1-a-z1-85cf5-hpncg   <none>           <none>

NAME                                                                                                   ATTACHER             PV                                                            NODE                                   ATTACHED   AGE
volumeattachment.storage.k8s.io/csi-00cbc2e2e1dd3245a500825ccf34f412b9bca85284719edf4b8cb20b9b103f9e   disk.csi.azure.com   pv-shoot--i034796--az1-fcae9b4f-28b1-4483-aaf0-00a71f368b32   shoot--i034796--az1-a-z1-85cf5-hpncg   true       2m21s
volumeattachment.storage.k8s.io/csi-0c4157b87ede74c377a9e83909b650c371c37000fa4779ab616003d9856ad2d8   disk.csi.azure.com   pv-shoot--i034796--az1-040457ee-eaac-4b01-b690-92a814a91788   shoot--i034796--az1-a-z1-85cf5-hpncg   true       2m21s
volumeattachment.storage.k8s.io/csi-45576c441448edda597f5f3c251b35c575546f0c0a6a395a878bd917e29ceba7   disk.csi.azure.com   pv-shoot--i034796--az1-4f4b7484-3029-4603-8806-dcaad8c1da2b   shoot--i034796--az1-a-z1-85cf5-hpncg   true       2m21s
volumeattachment.storage.k8s.io/csi-59f80936ca21d7dd7521f65a26ecae266b60f602ebcb6c240b5fa37532c87347   disk.csi.azure.com   pv-shoot--i034796--az1-dba037bb-eb4b-45c0-85bf-87e19b8977c0   shoot--i034796--az1-a-z1-85cf5-hpncg   true       2m21s
NAME                                   STATUS   ROLES    AGE   VERSION
shoot--i034796--az1-a-z1-85cf5-2g4q8   Ready    <none>   54s   v1.26.7
shoot--i034796--az1-a-z1-85cf5-hpncg   Ready    <none>   24m   v1.26.7

himanshu-kun

/lgtm

After gardener/machine-controller-manager#839 MCM provider extensions need to delete VolumeAttachments. See the PR description for more details. Co-authored-by: elankath <tarun.ramakrishna.elankath@sap.com> Co-authored-by: kon-angelo <konstantinos.angelopoulos@sap.com>

After gardener/machine-controller-manager#839 MCM provider extensions need to delete VolumeAttachments. See the PR description for more details. Co-authored-by: ialidzhikov <i.alidjikov@gmail.com> Co-authored-by: elankath <tarun.ramakrishna.elankath@sap.com> Co-authored-by: kon-angelo <konstantinos.angelopoulos@sap.com>

dont skip drain for unhealthy nodes

6fdbc9b

elankath requested a review from a team as a code owner August 7, 2023 18:07

gardener-robot added kind/api-change API change with impact on API users needs/second-opinion Needs second review by someone else needs/review Needs review size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) labels Aug 7, 2023

elankath assigned himanshu-kun Aug 7, 2023

elankath self-assigned this Aug 8, 2023

Removed lastStateTransitionTime - undeeded for fix

9454fad

gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 22, 2023

removed debug message

746e3ac

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 12, 2023

corrected stage name

581780f

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 12, 2023

reverted trivial changes to drain

98e21eb

gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 12, 2023

himanshu-kun removed their assignment Sep 14, 2023

Addressed review comments and documentation

bed9967

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 21, 2023

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 21, 2023

fixed anchor link for new q in faq

51cd01c

gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 21, 2023

himanshu-kun assigned himanshu-kun and unassigned elankath Sep 21, 2023

himanshu-kun suggested changes Sep 22, 2023

View reviewed changes

gardener-robot added the needs/changes Needs (more) changes label Sep 22, 2023

del of vol attachments log machine name, minor faq update

b07a41f

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 22, 2023

gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 22, 2023

fixed unit tests after lastOperation.description adjust

632e299

gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 22, 2023

gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 22, 2023

himanshu-kun approved these changes Sep 22, 2023

View reviewed changes

gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes needs/review Needs review needs/second-opinion Needs second review by someone else labels Sep 22, 2023

gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 22, 2023

himanshu-kun merged commit e38253d into gardener:master Sep 22, 2023
8 checks passed

gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Sep 22, 2023

ialidzhikov mentioned this pull request Nov 8, 2023

Allow MCM to delete VolumeAttachments gardener/gardener#8774

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dont skip drain for unhealthy nodes #839

dont skip drain for unhealthy nodes #839

elankath commented Aug 7, 2023 •

edited by himanshu-kun

Loading

elankath commented Aug 22, 2023

elankath commented Sep 21, 2023 •

edited

Loading

elankath commented Sep 21, 2023

elankath commented Sep 22, 2023 •

edited

Loading

himanshu-kun left a comment

dont skip drain for unhealthy nodes #839

dont skip drain for unhealthy nodes #839

Conversation

elankath commented Aug 7, 2023 • edited by himanshu-kun Loading

elankath commented Aug 22, 2023

Test Scenario

Observations

Situation Before Fix

Situation After Fix

elankath commented Sep 21, 2023 • edited Loading

elankath commented Sep 21, 2023

Re-tested after all review changes

kubelet killed at Thu Sep 21 09:07:34 UTC 2023

Poll Deployments,Pods,VolumeAttachments,Nodes at UTC Time: Thu Sep 21 09:10:37 UTC 2023, Local Time: Thu Sep 21 09:10:37 UTC 2023

Poll Deployments,Pods,VolumeAttachments,Nodes at UTC Time: Thu Sep 21 09:16:39 UTC 2023, Local Time: Thu Sep 21 09:16:39 UTC 2023

Poll Deployments,Pods,VolumeAttachments,Nodes at UTC Time: Thu Sep 21 09:18:49 UTC 2023, Local Time: Thu Sep 21 09:18:49 UTC 2023

elankath commented Sep 22, 2023 • edited Loading

Additional Test Requested

Test Steps

Test Result.

Poll Deployments,Pods,VolumeAttachments,Nodes at UTC Time: Fri Sep 22 09:08:16 UTC 2023, Local Time: Fri Sep 22 09:08:16 UTC 2023

himanshu-kun left a comment

Choose a reason for hiding this comment

elankath commented Aug 7, 2023 •

edited by himanshu-kun

Loading

elankath commented Sep 21, 2023 •

edited

Loading

elankath commented Sep 22, 2023 •

edited

Loading