VMDK server plugin doesn't return FS type and causes container run error

[govint]

This is seen on the current TOT code. The new changes to return fstype on the GET call is causing an issue when there is no fstype in the volume opts. In vmdk_ops.py vol_info() there is a new check added to see if fstype is there in the volume opts and the returned status (on GET) has the fs type only if its there, Otherwise no.

Client accesses status["fstype"] and its not there and seems to hang up. Docker daemon fails as below:

---

$ docker run --rm -it -w /data -v dvol_2:/data busybox
docker: Error response from daemon: Post http://%2Frun%2Fdocker%2Fplugins%2Fvmdk.sock/VolumeDriver.Mount: http: ContentLength=90 with Body length 0.

---

And the client plugin hangs here,

---

2016-10-20 03:30:14.523188855 -0700 PDT [INFO] No config file found. Using defaults.
2016-10-20 03:30:14.523621362 -0700 PDT [INFO] Docker VMDK plugin started version="VMDK Volume Driver v0.3" port=1019 mock_esx=false log_level=debug config="/etc/docker-volume-vsphere.conf"
2016-10-20 03:30:14.523838499 -0700 PDT [INFO] Getting volume data from unix:///var/run/docker.sock
2016-10-20 03:30:14.629758124 -0700 PDT [DEBUG] Docker info: version=1.12.2, root=/var/lib/docker, OS=Ubuntu 14.04.1 LTS
2016-10-20 03:30:14.630803269 -0700 PDT [DEBUG] Found 0 running or paused containers
2016-10-20 03:30:14.631533162 -0700 PDT [INFO] Discovered 0 volumes in use.
2016-10-20 03:30:14.631717317 -0700 PDT [INFO] Going into ServeUnix - Listening on Unix socket address="/run/docker/plugins/vmdk.sock"
2016-10-20 03:30:14.632257413 -0700 PDT [DEBUG] root group found. gid: 0
2016-10-20 03:32:08.561885931 -0700 PDT [DEBUG] vmdkOps.Get name=dvol_2
2016-10-20 03:32:09.73342527 -0700 PDT [INFO] Mounting volume name="dvol_2"

---

[govint]

Any changes to vol_info() must always follow the approach: that if the option isn't in vol opts then fill in a default.

[brunotm]

How the volume was successfully created without a fstype?

[govint]

Its an old volume that i'd created much earlier.

[brunotm]

For old volumes (ext2) it's ok, but this can wrongly hint the plugin about the fstype to mount

[govint]

The server needs to send back an fstype for disks that don;t have the
option set on it. Yes, the default should be ext2.

On Thu, Oct 20, 2016 at 6:15 PM, Bruno Moura notifications@github.com
wrote:

For old volumes (ext2) it's ok, but this can wrongly hint the plugin about
the fstype to mount

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#638 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/APHseMB4XhBEQIj0y6C0xN1I3CY7lJuaks5q12KGgaJpZM4KcC_r
.

[brunotm]

I didn't explained myself correctly, sorry.

As of now, it's not supposed to have a vmdk created without the fstype attribute. If this happen for some reason (other than this specific case of old volumes) we should return an error.

[pdhamdhere]

Ability to specify FS type is not backward compatible with existing volumes. Do we want to fix it?

[govint]

Updated changes, the client at least catches a missing FS type and handles it.

---

administrator@hte-1s-eng-dhcp98:~/vpl/docker-volume-vsphere$ docker run --rm -it -w /data -v dvol_3:/data busybox
docker: Error response from daemon: VolumeDriver.Mount: Failed mount - invalid filesystem type..

---

[brunotm]

@govint
I think this would be the expected behaviour, but from a backward compatibility perspective (if needed at this point) maybe you previous change can make more sense if we log the absence of the fstype attr for the volume. Because we will always get the error in client from syscall.Mount() if the fstype is invalid.

The only scenario i can think of an invalid or empty fstype going into the volume metadata after creation is tampering the metadata.

[pdhamdhere]

I am fine with proposed change. Report an appropriate error. No backward compatibility. I believe this change will also help if Host is attempting to mount volume created and formatted with filesystem which is not available on current Host.

[brunotm]

@govint
After thinking more throughly, i really think your previous proposed change makes more sense:

• We would catch an invalid or unsupported fstype is error anyway in syscall.Mount().
• My previous case for fstype being empty in KV is not possible without previous errors in the current code.
• It assures backward compatibility. (if needed)
• Its cleaner
• Bot ext4,ext3 and ext2 can be mounted using fstype=ext2

[msterin]

fixed by #639

[govint]

Agree, this isn't backward compatible and took some time figuring out why
the TOT code just doesn't complete a mount(). The client is using an fstype
from the server that may not be set, which typically shouldn't happen for
new volumes. At best the client should be checking that it didn't get back
a null string for the fstype.

ANy reason why the default fsttype is even defined in volume_kv.go if the
fstype is always set?

On Fri, Oct 21, 2016 at 6:28 AM, Prashant Dhamdhere <
notifications@github.com> wrote:

Ability to specify FS type is not backward compatible with existing
volumes. Do we want to fix it?

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#638 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/APHseHZO3WXJ2SPGCI-xFKVo_i-ZxgFZks5q2A4ygaJpZM4KcC_r
.