-
Notifications
You must be signed in to change notification settings - Fork 95
Detach after mount failure, and a small fix for logs #1005
Conversation
Also make us a little bit more patient on slow disk attach (may happen due to slow VM reconfig on overloaded system)
refcnt, _ := d.decrRefCount(r.Name) | ||
if refcnt == 0 { | ||
log.Infof("Detaching %s - it is not used anymore", r.Name) | ||
d.ops.Detach(r.Name, nil) // try to detach before failing the request for volume |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you be calling "d.UnmountVolume(r.Name)" like in Unmount?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logically we do not want to unmount since we know the mount has failed. We just want to clean up the attach.
Technically UnmountVolume does a set of useless calls (i.e. tries to locate the name and unmount volume) which we know will fail and guarantee confusing messages about failing to unmount, and then it will do the only useful step (d.ops.Detach) which we can do directly.
I guess that was a long winded version of "no. we need to call Detach" :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😀 yes, makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
Make sure we detach after fresh volume mount failure (before returning failure to Docker)
Mount()
used to drop the refcount and immediately return failure to upstairs - without detaching.Fix some warning to print correctly
Testing
Manual:
Failed to load config file /etc/docker-volume-vsphere.conf
. (it used to beFailed to load config file %s /etc/docker-volume-vsphere.conf
Detaching vol2 - it is not used anymore
message and VMDK is detached from the VM