Skip to content
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

Implement ListVolumes #464

Closed
msau42 opened this issue Mar 12, 2020 · 12 comments
Closed

Implement ListVolumes #464

msau42 opened this issue Mar 12, 2020 · 12 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@msau42
Copy link
Contributor

msau42 commented Mar 12, 2020

Is your feature request related to a problem?/Why is this needed
When a Node gets deleted, and Pod is rescheduled, Kubernetes thinks that the the volume is still attached to the original node and fails to attach to the new node with Multi-Attach error.

/feature

Describe the solution you'd like in detail
The latest csi-attacher v2.1 adds support for the ListVolumes capability: https://github.com/kubernetes-csi/external-attacher/blob/v2.1.1/CHANGELOG-2.1.md

It addresses problems when volumes may get detached out of band, and need to be reattached to a node. It could potentially help the other way around too, but that hasn't been tested.

Describe alternatives you've considered
n/a

Additional context
n/a

@msau42
Copy link
Contributor Author

msau42 commented Mar 12, 2020

cc @jsafrane

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 10, 2020
@qianzhangxa
Copy link

I am thinking about this case: an EBS volume has been attached to an AWS instance via ControllerPublishVolume call, and then this instance is down by accident. The container orchestrator may choose to issue another ControllerPublishVolume call to attach the volume to another healthy instance, but I guess this call will fail (since the volume is still being attached in the failed instance), right?

@msau42 Do you think the latest csi-attacher can help resolve the above problem?

@msau42
Copy link
Contributor Author

msau42 commented Jun 17, 2020

The latest csi-attacher does not help with that case. It only helps for the case where the disk actually got detached out of band and needs to be reattached to the same node.

For your case, assuming the instance is down and then the Kubernetes Node gets deleted as a result, then the volume will be force detached from the node after 5 minutes, and then the new Pod will be able to start up. Some discussion if we can recover better: kubernetes-csi/external-attacher#215

@qianzhangxa
Copy link

Thanks @msau42 for your reply!

For your case, assuming the instance is down and then the Kubernetes Node gets deleted as a result, then the volume will be force detached from the node after 5 minutes

Could you please let me know who will be responsible for forcily detaching the volume after 5 mins? Is it kube-controller-manager to create a VolumeAttachment object in this case to trigger csi-attacher to forcily detach the volume?

@msau42
Copy link
Contributor Author

msau42 commented Jun 18, 2020

It's the attach detach controller in kube-controller-manager. This is where it waits for 5 minutes if it thinks the volume may still be mounted on the node: https://github.com/kubernetes/kubernetes/blob/1faf097f3f7294322a574d2c813d21657ab61a81/pkg/controller/volume/attachdetach/reconciler/reconciler.go#L173

After the timeout, then we proceed to detach. However, there's an interesting case here where we skip calling Detach if we failed to update the Node status (maybe because the Node is gone):
https://github.com/kubernetes/kubernetes/blob/1faf097f3f7294322a574d2c813d21657ab61a81/pkg/controller/volume/attachdetach/reconciler/reconciler.go#L191

I need to verify if Detach is properly called in this case or if we end up leaking the VolumeAttachment.

@qianzhangxa
Copy link

@msau42 I think the timeout you are talking about is maxWaitForUnmountDuration , right? In code comments I see:

// maxWaitForUnmountDuration is the max amount of time the reconciler will wait
// for the volume to be safely unmounted, after this it will detach the volume
// anyway (to handle crashed/unavailable nodes). If during this time the volume
// becomes used by a new pod, the detach request will be aborted and the timer
// cleared.

So the reconciler will wait for maxWaitForUnmountDuration before detaching volume anyway. My question is, who will detach the volume when a node is down in the first place before reconciler tries to detach the volume anyway. Or which component notifies the reconciler that a volume needs to be detached because the node is down?

@leakingtapan
Copy link
Contributor

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 25, 2020
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 23, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot 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 Oct 23, 2020
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

5 participants