Fix mount collision timeout issue #29673

pmorie · 2016-07-27T14:30:17Z

Short- or medium-term workaround for #29555. The root issue being fixed here is that the recent attach/detach work in the kubelet uses a unique volume name as a key that tracks the work that has to be done for each volume in a pod to attach/mount/umount/detach. However, the non-attachable volume plugins do not report unique names for themselves, which causes collisions when a single secret or configmap is mounted multiple times in a pod.

This is still a WIP -- I need to add a couple E2E tests that ensure that tests break in the future if there is a regression -- but posting for early review.

cc @kubernetes/sig-storage

Ultimately, I would like to refine this a bit further. A couple things I would like to change:

GetUniqueVolumeName should be a property ONLY of attachable volumes
I would like to see the kubelet apparatus for attach/mount/umount/detach handle non-attachable volumes specifically to avoid things like the WaitForControllerAttach call that has to be done for those volume types now

k8s-cherrypick-bot · 2016-07-27T16:39:56Z

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

pmorie · 2016-07-27T17:10:00Z

pkg/volume/util/volumehelper/volumehelper.go

+// GetUniqueVolumeNameForNonAttachableVolume returns the unique volume name
+// for a non-attachable volume.
+func GetUniqueVolumeNameForNonAttachableVolume(volumePlugin volume.VolumePlugin, podNamespace, podName, podSpecName string) api.UniqueVolumeName {
+	return api.UniqueVolumeName(fmt.Sprintf("%s/%s-%s-%s", volumePlugin.GetPluginName(), podNamespace, podName, podSpecName))


@jingxu97 I'm changing this to use the pod UID instead of the namespace and name to aid you in #27970

Yep that would great

@pmorie, thanks for addressing this. Now I am still thinking whether this
change may affect our initial intention of avoiding operating mount/mount
for the same volume/pod. If for the same pod/volume, it is ok mount
parallel if the inner/outterspec name is different, then it should be fine.

On Wed, Jul 27, 2016 at 10:10 AM, Paul Morie notifications@github.com
wrote:

In pkg/volume/util/volumehelper/volumehelper.go
#29673 (comment)
:

@@ -52,6 +52,12 @@ func GetUniqueVolumeName(pluginName, volumeName string) api.UniqueVolumeName {
return api.UniqueVolumeName(fmt.Sprintf("%s/%s", pluginName, volumeName))
}

+// GetUniqueVolumeNameForNonAttachableVolume returns the unique volume name
+// for a non-attachable volume.
+func GetUniqueVolumeNameForNonAttachableVolume(volumePlugin volume.VolumePlugin, podNamespace, podName, podSpecName string) api.UniqueVolumeName {

return api.UniqueVolumeName(fmt.Sprintf("%s/%s-%s-%s", volumePlugin.GetPluginName(), podNamespace, podName, podSpecName))

@jingxu97 https://github.com/jingxu97 I'm changing this to use the pod
UID instead of the namespace and name to aid you in #27970
#27970

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/kubernetes/pull/29673/files/e967f0d59934d271a8eca16f6582865614008d8f#r72480743,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ASSNxfEUFfXSNMgrEIUUJwFFRtyLn6B0ks5qZ5EagaJpZM4JWQGg
.

Jing

matchstick · 2016-07-27T17:26:04Z

pkg/volume/util/nestedpendingoperations/nestedpendingoperations.go

@@ -92,6 +92,7 @@ func (grm *nestedPendingOperations) Run(
 	grm.lock.Lock()
 	defer grm.lock.Unlock()

+	// search for an existing operation


@pmorie @saad-ali This is nice to have, but should we really bundle this in this PR? Is it unrelated? Same for the following comment.

I'll take it out.

matchstick · 2016-07-27T19:00:27Z

pkg/volume/util/operationexecutor/operation_executor.go

+	//
+	// TODO: in the future, we should be able to remove the volumeName
+	// argument to this method -- since it is used only for attachable
+	// volumes.


@saad-ali @pmorie Can we open the github issue and include the number in this comment?

Also add "why we do this" namely - "to ensure that the volume name used is unique."

Yes, I'll open an issue for follow-up.

For non-attachable volumes, do not call GetVolumeName on the plugin and instead generate a unique name based on the identity of the pod and the name of the volume within the pod.

pmorie · 2016-07-27T21:54:23Z

@matchstick @saad-ali should be in a good state now, PTAL.

k8s-bot · 2016-07-27T22:32:40Z

GCE e2e build/test passed for commit f15684e.

saad-ali · 2016-07-28T03:09:19Z

LGTM Thanks @pmorie

k8s-github-robot · 2016-07-28T03:26:32Z

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

k8s-bot · 2016-07-28T04:06:23Z

GCE e2e build/test passed for commit f15684e.

k8s-github-robot · 2016-07-28T04:06:46Z

Automatic merge from submit-queue

fabioy · 2016-07-28T14:22:37Z

@pmorie, @saad-ali Cherrypick approved. Please create a cherrypick PR.

…-upstream-release-1.3 Automated cherry pick of #29673

…-of-#29673-upstream-release-1.3 Automated cherry pick of kubernetes#29673

pmorie added sig/storage Categorizes an issue or PR as relevant to SIG Storage. release-note-none Denotes a PR that doesn't merit a release note. labels Jul 27, 2016

pmorie assigned saad-ali Jul 27, 2016

googlebot added the cla: yes label Jul 27, 2016

pmorie added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Jul 27, 2016

k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 27, 2016

pmorie force-pushed the mount-collision branch from 696db7f to 8a4d020 Compare July 27, 2016 16:08

This was referenced Jul 27, 2016

Mounting (only 'default-token') volume takes a long time when creating a batch of pods #28616

Closed

1.3 upstream picks openshift/origin#9790

Closed

matchstick self-assigned this Jul 27, 2016

pmorie added the cherrypick-candidate label Jul 27, 2016

k8s-cherrypick-bot removed the cherrypick-candidate label Jul 27, 2016

pmorie force-pushed the mount-collision branch from 8a4d020 to e967f0d Compare July 27, 2016 16:45

matchstick added this to the v1.3 milestone Jul 27, 2016

pmorie reviewed Jul 27, 2016
View reviewed changes

pmorie force-pushed the mount-collision branch from e967f0d to a1d1622 Compare July 27, 2016 17:25

matchstick reviewed Jul 27, 2016
View reviewed changes

jingxu97 mentioned this pull request Jul 27, 2016

kubelet remounting default-token-xxxx repeatedly #29059

Closed

saad-ali mentioned this pull request Jul 27, 2016

Volume Manager Data Structure Should Use Pod UID in Key #29695

Closed

matchstick reviewed Jul 27, 2016
View reviewed changes

pmorie force-pushed the mount-collision branch from 526bf4a to a3f44c5 Compare July 27, 2016 20:24

k8s-github-robot mentioned this pull request Jul 27, 2016

[k8s.io] Deployment scaled rollout should not block on annotation check {Kubernetes e2e suite} #29559

Closed

pmorie force-pushed the mount-collision branch from a3f44c5 to 8f5cea2 Compare July 27, 2016 21:50

pmorie changed the title ~~WIP: fix mount collision timeout issue~~ Fix mount collision timeout issue Jul 27, 2016

pmorie added 2 commits July 27, 2016 17:53

Fix collisions issues / timeouts for mounts

c884297

For non-attachable volumes, do not call GetVolumeName on the plugin and instead generate a unique name based on the identity of the pod and the name of the volume within the pod.

Add e2e coverage for mount collisions

f15684e

pmorie force-pushed the mount-collision branch from 8f5cea2 to f15684e Compare July 27, 2016 21:54

pmorie added the cherrypick-candidate label Jul 27, 2016

saad-ali added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 28, 2016

saad-ali mentioned this pull request Jul 28, 2016

Secret and ConfigMap volume do not report unique volume name correctly #29555

Closed

k8s-github-robot merged commit 1ae9b73 into kubernetes:master Jul 28, 2016

fabioy added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jul 28, 2016

pmorie mentioned this pull request Jul 28, 2016

Automated cherry pick of #29673 #29746

Merged

fabioy added a commit that referenced this pull request Jul 28, 2016

Merge pull request #29746 from pmorie/automated-cherry-pick-of-#29673…

8b3de26

…-upstream-release-1.3 Automated cherry pick of #29673

fabioy removed the cherrypick-candidate label Jul 28, 2016

pmorie mentioned this pull request Aug 1, 2016

3.3 Upstream patches for storage openshift/origin#10111

Merged

saad-ali mentioned this pull request Aug 1, 2016

Error while tearing down pod, "device or resource busy" on service account secret #28750

Closed

fabioy mentioned this pull request Aug 4, 2016

kubernetes-e2e-gke-staging: broken test run #28440

Closed

shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016

Merge pull request kubernetes#29746 from pmorie/automated-cherry-pick…

338d57a

…-of-#29673-upstream-release-1.3 Automated cherry pick of kubernetes#29673

shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017

Merge pull request kubernetes#29746 from pmorie/automated-cherry-pick…

594b597

…-of-#29673-upstream-release-1.3 Automated cherry pick of kubernetes#29673

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix mount collision timeout issue #29673

Fix mount collision timeout issue #29673

pmorie commented Jul 27, 2016 •

edited

k8s-cherrypick-bot commented Jul 27, 2016

pmorie Jul 27, 2016

saad-ali Jul 27, 2016

jingxu97 Jul 27, 2016

matchstick Jul 27, 2016

pmorie Jul 27, 2016

matchstick Jul 27, 2016

matchstick Jul 27, 2016

pmorie Jul 27, 2016

pmorie commented Jul 27, 2016

k8s-bot commented Jul 27, 2016

saad-ali commented Jul 28, 2016

k8s-github-robot commented Jul 28, 2016

k8s-bot commented Jul 28, 2016

k8s-github-robot commented Jul 28, 2016

fabioy commented Jul 28, 2016

Fix mount collision timeout issue #29673

Fix mount collision timeout issue #29673

Conversation

pmorie commented Jul 27, 2016 • edited

k8s-cherrypick-bot commented Jul 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmorie commented Jul 27, 2016

k8s-bot commented Jul 27, 2016

saad-ali commented Jul 28, 2016

k8s-github-robot commented Jul 28, 2016

k8s-bot commented Jul 28, 2016

k8s-github-robot commented Jul 28, 2016

fabioy commented Jul 28, 2016

pmorie commented Jul 27, 2016 •

edited