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

Kubelet Volume Attach/Detach/Mount/Unmount Redesign #26801

Merged
merged 3 commits into from
Jun 15, 2016

Conversation

saad-ali
Copy link
Member

@saad-ali saad-ali commented Jun 3, 2016

This PR redesigns the Volume Attach/Detach/Mount/Unmount in Kubelet as proposed in #21931

A new volume manager was introduced in kubelet that synchronizes volume mount/unmount (and attach/detach, if attach/detach controller is not enabled).

This eliminates the race conditions between the pod creation loop and the orphaned volumes loops. It also removes the unmount/detach from the `syncPod()` path so volume clean up never blocks the `syncPod` loop.

@saad-ali saad-ali added priority/backlog Higher priority than priority/awaiting-more-evidence. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jun 3, 2016
@saad-ali saad-ali added this to the v1.3 milestone Jun 3, 2016
@saad-ali saad-ali self-assigned this Jun 3, 2016
@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 3, 2016
@saad-ali saad-ali force-pushed the mountUnmountRedesign branch from 493760a to b6baed7 Compare June 3, 2016 19:45
@pmorie pmorie assigned pmorie and thockin and unassigned saad-ali Jun 3, 2016
@pmorie pmorie added sig/storage Categorizes an issue or PR as relevant to SIG Storage. area/kubelet labels Jun 3, 2016
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 3, 2016
@saad-ali saad-ali force-pushed the mountUnmountRedesign branch from b6baed7 to 840f0be Compare June 4, 2016 02:59
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 4, 2016
@saad-ali saad-ali force-pushed the mountUnmountRedesign branch from 840f0be to 11f1393 Compare June 4, 2016 03:21
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 4, 2016
@saad-ali saad-ali force-pushed the mountUnmountRedesign branch from 98c504a to 6cdeb6d Compare June 4, 2016 07:59
@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 4, 2016
@saad-ali saad-ali force-pushed the mountUnmountRedesign branch from 6cdeb6d to 557556e Compare June 6, 2016 20:51
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 6, 2016
@saad-ali saad-ali force-pushed the mountUnmountRedesign branch from 557556e to 6b92518 Compare June 6, 2016 21:15
@@ -2386,11 +2386,11 @@ type NodeStatus struct {
NodeInfo NodeSystemInfo `json:"nodeInfo,omitempty" protobuf:"bytes,7,opt,name=nodeInfo"`
// List of container images on this node
Images []ContainerImage `json:"images,omitempty" protobuf:"bytes,8,rep,name=images"`
// List of volumes in use (mounted) by the node.
VolumesInUse []UniqueDeviceName `json:"volumesInUse,omitempty" protobuf:"bytes,9,rep,name=volumesInUse"`
// List of attachable volume devices in use (mounted) by the node.
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't match the internal API description.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@pmorie pmorie added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 15, 2016
@saad-ali saad-ali added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed priority/backlog Higher priority than priority/awaiting-more-evidence. labels Jun 15, 2016
@saad-ali
Copy link
Member Author

saad-ali commented Jun 15, 2016

@k8s-bot node e2e test this issue #27296

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 15, 2016
@saad-ali saad-ali force-pushed the mountUnmountRedesign branch from b2031c3 to 3c09a88 Compare June 15, 2016 07:17
@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 15, 2016
@saad-ali saad-ali added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 15, 2016
@saad-ali saad-ali force-pushed the mountUnmountRedesign branch from 3c09a88 to 2f97e51 Compare June 15, 2016 11:50
@k8s-github-robot k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jun 15, 2016
saad-ali added 3 commits June 15, 2016 09:32
Rename UniqueDeviceName to UniqueVolumeName and move helper functions
from attacherdetacher to volumehelper package.
Introduce UniquePodName alias
This commit adds a new volume manager in kubelet that synchronizes
volume mount/unmount (and attach/detach, if attach/detach controller
is not enabled).

This eliminates the race conditions between the pod creation loop
and the orphaned volumes loops. It also removes the unmount/detach
from the `syncPod()` path so volume clean up never blocks the
`syncPod` loop.
@saad-ali saad-ali force-pushed the mountUnmountRedesign branch from 2f97e51 to cfab536 Compare June 15, 2016 16:35
@saad-ali saad-ali added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 15, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 15, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jun 15, 2016

GCE e2e build/test passed for commit cfab536.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 12b1e4a into kubernetes:master Jun 15, 2016
@saad-ali saad-ali deleted the mountUnmountRedesign branch August 15, 2016 22:17
dims pushed a commit to dims/kubernetes that referenced this pull request Feb 8, 2018
Automatic merge from submit-queue

Kubelet Volume Attach/Detach/Mount/Unmount Redesign

This PR redesigns the Volume Attach/Detach/Mount/Unmount in Kubelet as proposed in kubernetes#21931

```release-note
A new volume manager was introduced in kubelet that synchronizes volume mount/unmount (and attach/detach, if attach/detach controller is not enabled).

This eliminates the race conditions between the pod creation loop and the orphaned volumes loops. It also removes the unmount/detach from the `syncPod()` path so volume clean up never blocks the `syncPod` loop.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/kubelet kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.