-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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.
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
Unable to detach the vSphere volume from Powered off node #52575
Unable to detach the vSphere volume from Powered off node #52575
Conversation
Hi @BaluDontu. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
301756b
to
c7fe832
Compare
/ok-to-test |
/unassign |
/retest |
1 similar comment
/retest |
Do you have tests for this change? What are the test scenarios? |
@jingxu97 : I plan to add the E2E test once this PR is checked in. The following test scenarios are done for this PR:
|
Please find the following logs when node goes into "NotReady" state after node is powered off in vCenter. Overall I don't see pod getting evicted from powered off node nor the volume getting detached from powered off node. Initially when pod is deployed, it is deployed on node3.
Now after node3 is powered off in vCenter, I see the following
From the below logs, I see the pod is never evicted from powered off node - node3. But a new pod is created on node1 after 5 mins when node3 is powered off but the old pod is never deleted. Also I don't see the detach call on node3 at all.
From the below you can see, a new pod is being created on node2 but not evicted from powered off node - node3.
For the old pod - "balu-master-ccd4cd947-9f84z" which went into "Unknown" state, I see the following.
For the new pod - balu-master-ccd4cd947-d4bfd which is now in "Ready" state, I see the following events.
|
Few more logs where it creates a new pod on available node but old pod went into unknown state.
|
Debug with @BaluDontu offline.
Attach_detach controller should try to detach the volume for terminated pods (after release 1.7). See PR #45286. However, there is a check to see whether a pod is terminated or not in the controller. This function also checks containers' status (not the same as pod status). If container status is still running, the pod will not be considered as terminated. In this scenario, since node is powered off, kubelet will not update container status so it is still showing as running. So attach_detach controller will not treat this pod as terminated and will not detach volume for this pod.
I think we should remove the check for container status when determining the pod is terminated or not. |
Thanks @jingxu97 for helping me out on this. I have one more query. Also wanted to know how does GCE/AWS handle this scenario? |
@jingxu97 shouldn't pod enter state So what you are saying would mean detach the volume if pod is in |
@gnufied The problem I see is this code https://github.com/kubernetes/kubernetes/blob/master/pkg/volume/util/volumehelper/volumehelper.go#L102 It checks the container status which is not updated by kubelet since node is down. Even though the pod is in terminating state, this function return false due to incorrect container status (running state) |
This issue is also related to #46442 @BaluDontu Please also try to have e2e test for this change. You can put it in a different PR /approve |
/test all Tests are more than 96 hours old. Re-running tests. |
@jingxu: I am already working on E2E test and will have a PR ready soon. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BaluDontu, jingxu97, luomiao Associated issue: 46442 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
/test all [submit-queue is verifying that this PR is safe to merge] |
/retest |
@BaluDontu: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Automatic merge from submit-queue (batch tested with PRs 51311, 52575, 53169). If you want to cherry-pick this change to another branch, please follow the instructions here. |
@jingxu97 : Though this fix was able to resolve the issue when node was powered off. However it still fails when kubelet was stopped on the node where pod got provisioned. It is because, the node goes into NotReadyState when kubelet is stopped on the node. |
…52575-upstream-release-1.8 Automatic merge from submit-queue. Automated cherry pick of #52131 #52575 upstream release 1.8 Cherry pick of #52131, #52575 on release-1.8 #52131: Implement bulk polling of volumes for vSphere #52575: Unable to detach the vSphere volume from Powered off node @BaluDontu **Release note**: ```release-note BulkVerifyVolumes() implementation for vSphere Cloud Provider Fixed volume detachment from powered off node ```
With the existing implementation when a vSphere node is powered off, the node is not deleted by the node controller and is in "NotReady" state. Following the approach similar to GCE as mentioned here - #46442.
I observe the following issues:
So inorder to resolve this problem, we have decided to back with the approach where the powered off node will be removed by the Node controller. So the above 2 problems will be resolved as follows:
For now, we would want to go ahead with deleting the node from node controller when a node is powered off in vCenter until we have a better approach. I think the best possible solution would be to introduce power handler in volume controller to see if the node is powered off before we can take any appropriate for attach/detach operations.
@jingxu97 @saad-ali @divyenpatel @luomiao @rohitjogvmw