VSAN support for VSphere Volume Plugin #29172

abrarshivani · 2016-07-19T07:02:38Z

This PR does the following,

Fixes VSphere Volume doesn't unmount #28625 (VSphere Volume doesn't unmount): modified vmdk namespace path parsing so it accurately handles VMs in folders. See file pkg/volume/vsphere_volume/vsphere_volume.go.
Updates vmware/govmomi dependency. It was quite behind. The majority of files in the change are in this category.
Adds support for VSAN datastore. Handle namespace to uuid mapping to assist unmount and detach in VSAN case as well. See file pkg/cloudprovider/providers/vsphere/vsphere.go.

Tested:
- Created a K8s cluster on VSphere with VSAN datastore. Created a vmdk in VSAN datastore and created pod which uses this vmdk. Before fix (VSphere Volume doesn't unmount) it failed. After fix the volume gets successfully unmounted and detached.
- Created a K8s cluster on VSphere with VMFS datastore. Created a vmdk in subdirectory of root in VMFS datastore and created pod which uses this vmdk. Before fix (VSphere Volume doesn't unmount) it failed. After fix the volume gets successfully unmounted and detached.

googlebot · 2016-07-19T07:02:39Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

k8s-bot · 2016-07-19T07:02:58Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-07-19T07:03:19Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-07-19T07:03:38Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-07-19T07:04:19Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-07-19T07:04:40Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

frapposelli · 2016-07-19T19:35:53Z

Ping @googlebot as I've just added @abrarshivani in the vmware contributors group.

abrarshivani · 2016-07-19T19:39:09Z

ping @googlebot

googlebot · 2016-07-19T19:39:10Z

CLAs look good, thanks!

abithap · 2016-07-20T23:23:30Z

pkg/cloudprovider/providers/vsphere/vsphere.go

+	return uuid
+}
+
+//Gets virtual disk uuid by datastore path. Here, datastore path can be both containing vsan object name or uuid.


whats a vsan object name, is it a datastore vmdk path? does this mean volPath argument can be a uuid as well?

Good catch @abhitap. I will adjust the comments to this:

// Gets virtual disk uuid by datastore (namespace) path // // volPath can be namespace path (e.g. "[vsanDatastore] volumes/test.vmdk") or // uuid path (e.g. "[vsanDatastore] 59427457-6c5a-a917-7997-0200103eedbc/test.vmdk"). // `volumes` in this case would be a symlink to // `59427457-6c5a-a917-7997-0200103eedbc`. // // We want users to use namespace path. It is good for attaching the disk, // but for detaching the API requires uuid path. Hence, to detach the right // device we have to convert the namespace path to uuid path.

abrarshivani · 2016-07-21T00:03:17Z

//cc @dagnello @imkin

pmorie · 2016-07-26T18:18:19Z

pkg/cloudprovider/providers/vsphere/vsphere.go

 	}
 	return "", ErrNoDiskUUIDFound
 }

-func getVirtualDiskID(volPath string, vmDevices object.VirtualDeviceList) (string, error) {
+func formatVirtualDiskUUID(uuid string) string {
+


spurious newline

Thanks for the comments! The newline is not needed.

pmorie · 2016-07-27T17:03:35Z

pkg/cloudprovider/providers/vsphere/vsphere.go

+}
+
+func getVirtualDiskID(volPath string, vmDevices object.VirtualDeviceList, dc *object.Datacenter, client *govmomi.Client) (string, error) {
+


spurious newline

pmorie · 2016-07-27T17:07:35Z

Just a couple nits and this LGTM. Does it need a release note? It sounds to me like it might be worthy of release note, but I'm not certain.

abrarshivani · 2016-07-29T06:22:59Z

@pmorie This PR needs release note.

k8s-bot · 2016-08-03T00:28:28Z

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

- Fix unmount for vsanDatastore - Add support for vsan datastore

pmorie · 2016-08-04T00:15:48Z

@k8s-bot ok to test

k8s-bot · 2016-08-04T00:51:26Z

GCE e2e build/test passed for commit 87e7535.

k8s-github-robot · 2016-08-04T01:55:18Z

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

k8s-bot · 2016-08-04T02:26:57Z

GCE e2e build/test passed for commit 87e7535.

k8s-github-robot · 2016-08-04T02:27:28Z

Automatic merge from submit-queue

Automatic merge from submit-queue Back porting critical vSphere bug fixes to release 1.3 vSphere cloud provider plugin was first introduced in Kubernetes 1.3.0. The two PRs listed below contain critical volume attach/detach related fixes. Without these fixes, vSphere in 1.3 lacks the ability to consistently attach/detach volumes correctly especially in a busy cluster. These changes only affect the vSphere cloud provider. - #29172 - #30535

…BugFixes Automatic merge from submit-queue Back porting critical vSphere bug fixes to release 1.3 vSphere cloud provider plugin was first introduced in Kubernetes 1.3.0. The two PRs listed below contain critical volume attach/detach related fixes. Without these fixes, vSphere in 1.3 lacks the ability to consistently attach/detach volumes correctly especially in a busy cluster. These changes only affect the vSphere cloud provider. - kubernetes#29172 - kubernetes#30535

googlebot added the cla: no label Jul 19, 2016

k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Jul 19, 2016

abrarshivani force-pushed the govmomidepupdate branch from be4ba68 to 9d248fa Compare July 19, 2016 19:22

googlebot added cla: yes and removed cla: no labels Jul 19, 2016

abrarshivani force-pushed the govmomidepupdate branch from 9d248fa to b4f9dfc Compare July 19, 2016 20:10

abrarshivani changed the title ~~Updated vmware/govmomi godep~~ VSAN support for VSphere Volume Plugin Jul 20, 2016

k8s-github-robot assigned pmorie Jul 20, 2016

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 20, 2016

abrarshivani force-pushed the govmomidepupdate branch from 45c59e3 to 9178fce Compare July 20, 2016 22:51

abithap reviewed Jul 20, 2016
View reviewed changes

abrarshivani force-pushed the govmomidepupdate branch 4 times, most recently from 9178fce to d2c7bd7 Compare July 24, 2016 02:06

pmorie reviewed Jul 26, 2016
View reviewed changes

abrarshivani force-pushed the govmomidepupdate branch from d2c7bd7 to 56739bb Compare July 26, 2016 21:26

pmorie reviewed Jul 27, 2016
View reviewed changes

k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. retest-not-required-docs-only and removed retest-not-required-docs-only labels Jul 28, 2016

abrarshivani force-pushed the govmomidepupdate branch from 56739bb to 206ad4d Compare July 29, 2016 02:53

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 29, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 3, 2016

pmorie added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Aug 3, 2016

k8s-github-robot removed the release-note-label-needed label Aug 3, 2016

- Updated vmware/govmomi godep (Needs for vsan support)

87e7535

- Fix unmount for vsanDatastore - Add support for vsan datastore

abrarshivani force-pushed the govmomidepupdate branch from 206ad4d to 87e7535 Compare August 3, 2016 23:39

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 3, 2016

pmorie added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 4, 2016

k8s-github-robot merged commit df8da19 into kubernetes:master Aug 4, 2016

dagnello mentioned this pull request Sep 2, 2016

Back porting critical vSphere bug fixes to release 1.3 #31993

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VSAN support for VSphere Volume Plugin #29172

VSAN support for VSphere Volume Plugin #29172

abrarshivani commented Jul 19, 2016 •

edited

Loading

googlebot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

frapposelli commented Jul 19, 2016

abrarshivani commented Jul 19, 2016

googlebot commented Jul 19, 2016

abithap Jul 20, 2016

abrarshivani Jul 21, 2016

abrarshivani commented Jul 21, 2016

pmorie Jul 26, 2016

abrarshivani Jul 26, 2016

pmorie Jul 27, 2016

pmorie commented Jul 27, 2016

abrarshivani commented Jul 29, 2016

k8s-bot commented Aug 3, 2016

pmorie commented Aug 4, 2016

k8s-bot commented Aug 4, 2016

k8s-github-robot commented Aug 4, 2016

k8s-bot commented Aug 4, 2016

k8s-github-robot commented Aug 4, 2016

		}

		func getVirtualDiskID(volPath string, vmDevices object.VirtualDeviceList, dc object.Datacenter, client govmomi.Client) (string, error) {

VSAN support for VSphere Volume Plugin #29172

VSAN support for VSphere Volume Plugin #29172

Conversation

abrarshivani commented Jul 19, 2016 • edited Loading

googlebot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

k8s-bot commented Jul 19, 2016

frapposelli commented Jul 19, 2016

abrarshivani commented Jul 19, 2016

googlebot commented Jul 19, 2016

abithap Jul 20, 2016

Choose a reason for hiding this comment

abrarshivani Jul 21, 2016

Choose a reason for hiding this comment

abrarshivani commented Jul 21, 2016

pmorie Jul 26, 2016

Choose a reason for hiding this comment

abrarshivani Jul 26, 2016

Choose a reason for hiding this comment

pmorie Jul 27, 2016

Choose a reason for hiding this comment

pmorie commented Jul 27, 2016

abrarshivani commented Jul 29, 2016

k8s-bot commented Aug 3, 2016

pmorie commented Aug 4, 2016

k8s-bot commented Aug 4, 2016

k8s-github-robot commented Aug 4, 2016

k8s-bot commented Aug 4, 2016

k8s-github-robot commented Aug 4, 2016

abrarshivani commented Jul 19, 2016 •

edited

Loading