Persisting VM name in KV while attaching volume #1064

pshahzeb · 2017-03-22T19:20:13Z

Persisting vm name along with uuid in KV when a volume is attached
to a VM
If vm name cannot be retrieved using uuid (VM might reside on another host)
then use the vm name persisted in KV. Doing this in both for admincli and plugin
requests

After Fix:
Setup : 2 ESXs ESX1 & ESX2 with VM1 and VM2 respectively and shared DS
Created 2 volumes, vol1 from VM1 and vol2 from VM2
admin ls from ESX1

[root@sc-rdops-vm05-dhcp-133-3:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py ls
Volume  Datastore     VM-Group  Capacity  Used  Filesystem  Policy  Disk Format  Attached-to  Access      Attach-as               Created By    Created Date
------  ------------  --------  --------  ----  ----------  ------  -----------  -----------  ----------  ----------------------  ------------  ------------------------
vol1    sharedVmfs-0  _DEFAULT  100MB     13MB  ext4        N/A     thin         detached     read-write  independent_persistent  photon-VM0.1  Wed Mar 22 00:08:11 2017
vol2    sharedVmfs-0  _DEFAULT  100MB     15MB  ext4        N/A     thin         detached     read-write  independent_persistent  photon-VM0.0  Wed Mar 22 00:08:31 2017

admin ls from ESX2

[root@sc-rdops-vm05-dhcp-138-185:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py ls
Volume  Datastore     VM-Group  Capacity  Used  Filesystem  Policy  Disk Format  Attached-to  Access      Attach-as               Created By    Created Date
------  ------------  --------  --------  ----  ----------  ------  -----------  -----------  ----------  ----------------------  ------------  ------------------------
vol1    sharedVmfs-0  _DEFAULT  100MB     13MB  ext4        N/A     thin         detached     read-write  independent_persistent  photon-VM0.1  Wed Mar 22 00:08:11 2017
vol2    sharedVmfs-0  _DEFAULT  100MB     15MB  ext4        N/A     thin         detached     read-write  independent_persistent  photon-VM0.0  Wed Mar 22 00:08:31 2017

Ran the container from VM1 and mounted the volume - vol2

root@photon-xVmYMbyTn [ ~ ]# docker run -it -d --volume-driver=vsphere -v vol2@sharedVmfs-0:/vol1 --name ub ubuntu
2804888386b32862fa0c6a82bbae55618cba5a1fa085a947b7b1b219ad380a6f

docker volume inspect vol2 from VM1

root@photon-xVmYMbyTn [ ~ ]# docker volume inspect vol2
[
    {
        "Name": "vol2",
        "Driver": "vsphere",
        "Mountpoint": "/mnt/vmdk/vol2",
        "Status": {
            "access": "read-write",
            "attach-as": "independent_persistent",
            "attached to VM": "photon-VM0.1",
            "capacity": {
                "allocated": "15MB",
                "size": "100MB"
            },
            "clone-from": "None",
            "created": "Wed Mar 22 00:08:31 2017",
            "created by VM": "photon-VM0.0",
            "datastore": "sharedVmfs-0",
            "diskformat": "thin",
            "fstype": "ext4",
            "status": "attached"
        },
        "Labels": {},
        "Scope": "global"
    }
]

docker volume inspect vol2 from VM2

root@photon-xVmYMbyTn [ ~ ]# docker volume inspect vol2
[
    {
        "Name": "vol2",
        "Driver": "vsphere",
        "Mountpoint": "/mnt/vmdk/vol2",
        "Status": {
            "access": "read-write",
            "attach-as": "independent_persistent",
            "attached to VM": "photon-VM0.1",
            "capacity": {
                "allocated": "15MB",
                "size": "100MB"
            },
            "clone-from": "None",
            "created": "Wed Mar 22 00:08:31 2017",
            "created by VM": "photon-VM0.0",
            "datastore": "sharedVmfs-0",
            "diskformat": "thin",
            "fstype": "ext4",
            "status": "attached"
        },
        "Labels": {},
        "Scope": "global"
    }
]

admin ls from ESX1

[root@sc-rdops-vm05-dhcp-133-3:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py ls
Volume  Datastore     VM-Group  Capacity  Used  Filesystem  Policy  Disk Format  Attached-to   Access      Attach-as               Created By    Created Date
------  ------------  --------  --------  ----  ----------  ------  -----------  ------------  ----------  ----------------------  ------------  ------------------------
vol1    sharedVmfs-0  _DEFAULT  100MB     13MB  ext4        N/A     thin         detached      read-write  independent_persistent  photon-VM0.1  Wed Mar 22 00:08:11 2017
vol2    sharedVmfs-0  _DEFAULT  100MB     15MB  ext4        N/A     thin         photon-VM0.1  read-write  independent_persistent  photon-VM0.0  Wed Mar 22 00:08:31 2017

admin ls from ESX2

[root@sc-rdops-vm05-dhcp-138-185:~] /usr/lib/vmware/vmdkops/bin/vmdkops_admin.py ls
Volume  Datastore     VM-Group  Capacity  Used  Filesystem  Policy  Disk Format  Attached-to   Access      Attach-as               Created By    Created Date
------  ------------  --------  --------  ----  ----------  ------  -----------  ------------  ----------  ----------------------  ------------  ------------------------
vol1    sharedVmfs-0  _DEFAULT  100MB     13MB  ext4        N/A     thin         detached      read-write  independent_persistent  photon-VM0.1  Wed Mar 22 00:08:11 2017
vol2    sharedVmfs-0  _DEFAULT  100MB     15MB  ext4        N/A     thin         photon-VM0.1  read-write  independent_persistent  photon-VM0.0  Wed Mar 22 00:08:31 2017

Fixes #1053

1. Persisting vm name along with uuid in KV when a volume is attached to a VM 2. If vm name cannot be retrieved using uuid (VM might reside on another host) then use the vm name persisted in KV. Using this in both for admincli and plugin requests Fixes #1053

pdhamdhere · 2017-03-22T19:24:50Z

esx_service/cli/vmdkops_admin.py

            if metadata[kv.ATTACHED_VM_UUID]:
-                return metadata[kv.ATTACHED_VM_UUID]
+                return metadata[kv.ATTACHED_VM_NAME]


For existing volumes which do not have VM_NAME in KV, it will return NULL. Did you check if code handles it correctly?

I see. For such cases, fixing it to return vm uuid (like before). Thanks

In ideal behavior, there will be name persisted in KV (while attach) even for volume that were created previously (with version of DVS without this change).

yes, but people can upgrade VIB while the volumes are attached ...

Got it!.thanks.
We will print uuid then!

shaominchen · 2017-03-22T21:42:06Z

Change looks good except that case Prashant pointed out. Moreover, KV Store is just one of our persistence stores. We still need to fix the same in the auth_db. Do we plan to address that in a separate issue&PR (maybe as part of the cross-esx multi tenancy support)?

pshahzeb · 2017-03-22T22:23:05Z

@shaominchen
I think we don't store explicit information about which VMs have created/mounted a volume in authdb(volumes and vms tables). This information is always managed in and taken from KV store. So I don't see a place to fix anything for authdb as of now.
If need be about having such knowledge for cross-esx tenancy; I think that can be added with its related PR.
What do you think?

shaominchen · 2017-03-22T22:48:45Z

@pshahzeb
I'm talking about the "vms" table in the auth_db. For now we only persist vm_id in the DB. We will need to add vm_name as well to support cross-esx multi tenancy and other scenarios. Sure, this can be addressed as part of that feature.

…persisting VM name in metadata

pshahzeb · 2017-03-23T03:01:23Z

OK cool. Thanks

govint · 2017-03-23T04:19:06Z

Fix is good but can we just print the UUID (like for the admin ls) when the UUID doesn't resolve to the a VM object on the local host.

govint · 2017-03-23T04:21:51Z

esx_service/cli/vmdkops_admin.py

+            if vm_name:
+                return vm_name
+            # If vm name couldn't be retrieved through uuid, use name from KV
+            elif metadata[kv.ATTACHED_VM_NAME]:


Better to check kv.ATTACHED_VM_NAME is in metadata

govint · 2017-03-23T04:24:38Z

esx_service/vmdk_ops.py

-       if vm:
-          vinfo[ATTACHED_TO_VM] = vm.config.name
+        vm = findVmByUuid(vol_meta[kv.ATTACHED_VM_UUID])
+        if vm:


Can we use vmdk_ops.vm_uuid2name{} here as well.

govint · 2017-03-23T04:27:54Z

esx_service/vmdk_ops.py

+        vm = findVmByUuid(vol_meta[kv.ATTACHED_VM_UUID])
+        if vm:
+            vinfo[ATTACHED_TO_VM] = vm.config.name
+        elif vol_meta[kv.ATTACHED_VM_NAME]:


Again check if kv.ATTACHED_VM_NAME is in vol_meta before indexing vol_meta with it,

govint

Couple of changes.

msterin

Overall looks good and looks like all proper comments have already been made. So I second a few of them
(1) need to make sure if behaves fine with current KV (they one that does not have attached-vm info
(2) I agree with @govint that a fallback (if no name anywhere) should bu just to print UUID
(3) I think any work related to DB is out of scope... Please note that DB is really access/quota config DB, not a generic tracking mechanism

…on host and is not persisted in KV store

pshahzeb · 2017-03-23T19:18:14Z

Addressed all the comments and also fallback to uuid if no name anywhere. But this shouldn't happen unless admin did an upgrade between mount and unmount unmount which looks impossible.

shaominchen

LGTM.

vmwclabot added the cla-not-required label Mar 22, 2017

pdhamdhere reviewed Mar 22, 2017

View reviewed changes

Handling VM names for in volume metadata for volumes created without …

fb5795c

…persisting VM name in metadata

govint reviewed Mar 23, 2017

View reviewed changes

msterin reviewed Mar 23, 2017

View reviewed changes

Populating attached VM with uuid in case vm name cannot be retrieved …

8978bcc

…on host and is not persisted in KV store

shaominchen approved these changes Mar 23, 2017

View reviewed changes

pdhamdhere approved these changes Mar 24, 2017

View reviewed changes

pdhamdhere merged commit 5c7614b into master Mar 24, 2017

pdhamdhere deleted the issue_1053.pshahzeb branch March 24, 2017 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persisting VM name in KV while attaching volume #1064

Persisting VM name in KV while attaching volume #1064

pshahzeb commented Mar 22, 2017

pdhamdhere Mar 22, 2017

pshahzeb Mar 22, 2017

pshahzeb Mar 23, 2017

msterin Mar 23, 2017

pshahzeb Mar 23, 2017

shaominchen commented Mar 22, 2017

pshahzeb commented Mar 22, 2017

shaominchen commented Mar 22, 2017

pshahzeb commented Mar 23, 2017

govint commented Mar 23, 2017

govint Mar 23, 2017

govint Mar 23, 2017

govint Mar 23, 2017

govint left a comment

msterin left a comment

pshahzeb commented Mar 23, 2017 •

edited

shaominchen left a comment

Persisting VM name in KV while attaching volume #1064

Persisting VM name in KV while attaching volume #1064

Conversation

pshahzeb commented Mar 22, 2017

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

shaominchen commented Mar 22, 2017

pshahzeb commented Mar 22, 2017

shaominchen commented Mar 22, 2017

pshahzeb commented Mar 23, 2017

govint commented Mar 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

govint left a comment

Choose a reason for hiding this comment

msterin left a comment

Choose a reason for hiding this comment

pshahzeb commented Mar 23, 2017 • edited

shaominchen left a comment

Choose a reason for hiding this comment

pshahzeb commented Mar 23, 2017 •

edited