Hide underlying implementation and fix bz1328588

[runcom]

• hide the underlying implementation detail that we use lv* tools or mkfs by returning generic error messages (we will log details in syslog in a follow up PR)

• Fix https://bugzilla.redhat.com/show_bug.cgi?id=1328588#c15

• Cleanup the volume if there's an error on create time

• Cleanup the mountpoint if there's an error on create time

• Fix .gitignore (add the binary output)

• Error out directly if no --size or empty --size is provided (otherwise we'll show lvcreate specific error:

  Error response from daemon: create test6: VolumeDriver.Create: Please specify either size or extents. Run 'lvcreate --help' for more information.)

ping @rhvgoyal @shishir-a412ed @rhatdan @mrunalp

Signed-off-by: Antonio Murdaca runcom@redhat.com

[rhatdan]

This looks good to me, but we definitely needs to syslog the output of the lvm commands, otherwise diagnosing why something is breaking will be difficult.

[runcom]

@rhatdan totally agree with that but didn't have time right now to go that way - me or @shishir-a412ed can take a peek at logging to syslog in a follow up PR I guess. But right now, exposing lvm and mkfs specific error is awful.

[mrunalp]

Lgtm

[shishir-a412ed]

@runcom I just tried to create an invalid logical volume using the current implementation.

docker volume create -d lvm --name invalid_foobar --opt size=0.01G.

I do get the ugly error and see the logical volume listed under lvs command (Note: No invalid_foobar listed when I do docker volume ls). so lvremove is needed here for cleanup.

LGTM. Just one small ques.
What do you mean by cleanup the mountpoint if there is an error on create time ?

I didn't see anything mounted on the FS. I checked at /var/lib/docker-lvm-plugin and after the lvremove of invalid_foobar, I checked at /dev/volume_group_one (where volume_group_one is the volume group name). Am I missing something ?

Shishir

[runcom]

See the os.RemoveAll in the defer for mount point cleanup. After this patch also, no logical volumes is listed with lvs

[shishir-a412ed]

LGTM. Merging ! thanks @runcom