Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Commit

Permalink
Avoiding redundant mounts by using /proc/mount info (#1224)
Browse files Browse the repository at this point in the history 
* Avoiding redundant mounts by using /proc/mount info

1. Convert a short volume name to the long one. If required to a trip to vmdkops service
2. In every mount request, check if the volume is already mounted (info of /proc/mount)
3. refcount test which kills docker and verifies refcounting
  • Loading branch information
Shahzeb Patel authored May 9, 2017
1 parent 1712a34 commit 4eca684
Show file tree
Hide file tree
Showing 7 changed files with 306 additions and 67 deletions.
25 changes: 14 additions & 11 deletions misc/scripts/refcnt_test.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,11 @@ function check_files {
}

function check_recovery_record {
line=`tail -10 /var/log/docker-volume-vsphere.log | $GREP 'Volume name=' | $GREP 'mounted=true'`
expected="name=$vname count=$count mounted=true"
# log contains refcounting attempts and after success logs summary.
line=`tail -50 /var/log/docker-volume-vsphere.log | $GREP 'Volume name=' | $GREP 'mounted=true'`
expected="count=$count mounted=true"

echo $line | $GREP -q "$expected" ; if [ $? -ne 0 ] ; then
echo $line | $GREP "$vname" | $GREP -q "$expected" ; if [ $? -ne 0 ] ; then
echo Found: \"$line\"
echo Expected pattern: \"$expected\"
return 1
Expand All @@ -80,20 +81,21 @@ function check_recovery_record {

function test_crash_recovery {
timeout=$1
echo "Checking recovery for VMDK plugin kill -9"
kill -9 `pidof docker-volume-vsphere`
until pids=$(pidof docker-volume-vsphere)
echo "Checking recovery through docker kill"
# kill docker daemon forcefully
pkill -9 dockerd
until pids=$(pidof dockerd)
do
echo "Waiting for docker-volume-vsphere to restart"
echo "Waiting for docker to restart"
sleep 1
done

echo "Waiting for plugin init"
sleep 3
sleep 5
sync # give log the time to flush
wait_for check_recovery_record $timeout
if [ "$?" -ne 0 ] ; then
echo PLUGIN RESTART TEST FAILED. Did not find proper recovery record
echo DOCKER RESTART TEST FAILED. Did not find proper recovery record
exit 1
fi
}
Expand All @@ -115,8 +117,9 @@ fi
echo "$(docker volume ls)"
for i in `seq 1 $count`
do
$DOCKER run -d -v $vname:/v busybox sh -c "touch /v/file$i; sync ; \
sleep $timeout"
# run containers with restart flag so they restart after docker restart
$DOCKER run -d --restart=always -v $vname:/v busybox sh -c "touch /v/file$i; sync ; \
while true; do sleep $timeout; done"
done

echo "Checking the last refcount and mount record"
Expand Down
2 changes: 1 addition & 1 deletion vmdk_plugin/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ VMDKOPS_MODULE_SRC = $(VMDKOPS_MODULE)/*.go $(VMCI_SRC)

# All sources. We rebuild if anything changes here
SRC = main.go log_formatter.go utils/refcount/refcnt.go \
utils/fs/fs.go utils/config/config.go utils/TestInputParamsUtil.go \
utils/fs/fs.go utils/config/config.go utils/TestInputParamsUtil.go utils/plugin_utils/plugin_utils.go\
drivers/photon/photon_driver.go drivers/vmdk/vmdk_driver.go

# Canned recipe
Expand Down
1 change: 1 addition & 0 deletions vmdk_plugin/drivers/driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ type VolumeDriver interface {
MountVolume(string, string, string, bool, bool) (string, error)
UnmountVolume(string) error
GetVolume(string) (map[string]interface{}, error)
VolumesInRefMap() []string
}
65 changes: 50 additions & 15 deletions vmdk_plugin/drivers/photon/photon_driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
log "github.com/Sirupsen/logrus"
"github.com/docker/go-plugins-helpers/volume"
"github.com/vmware/docker-volume-vsphere/vmdk_plugin/utils/fs"
"github.com/vmware/docker-volume-vsphere/vmdk_plugin/utils/plugin_utils"
"github.com/vmware/docker-volume-vsphere/vmdk_plugin/utils/refcount"
"github.com/vmware/photon-controller-go-sdk/photon"
)
Expand All @@ -56,12 +57,13 @@ const (

// VolumeDriver - Photon volume driver struct
type VolumeDriver struct {
client *photon.Client
hostID string
mountRoot string
project string
refCounts *refcount.RefCountsMap
target string
client *photon.Client
hostID string
mountRoot string
project string
refCounts *refcount.RefCountsMap
target string
mountIDtoName map[string]string // map of mountID -> full volume name
}

func (d *VolumeDriver) verifyTarget() error {
Expand Down Expand Up @@ -95,6 +97,7 @@ func NewVolumeDriver(targetURL string, projectID string, hostID string, mountDir
d.mountRoot = mountDir
d.refCounts = refcount.NewRefCountsMap()
d.refCounts.Init(d, mountDir, driverName)
d.mountIDtoName = make(map[string]string)

log.WithFields(log.Fields{
"version": version,
Expand Down Expand Up @@ -360,8 +363,22 @@ func (d *VolumeDriver) MountVolume(name string, fstype string, id string, isRead
return mountpoint, fs.MountWithID(mountpoint, fstype, id, isReadOnly)
}

// VolumesInRefMap - get list of volumes names from refmap
// names are in format volume@datastore
func (d *VolumeDriver) VolumesInRefMap() []string {
return d.refCounts.GetVolumeNames()
}

// private function that does the job of mounting volume in conjunction with refcounting
func (d *VolumeDriver) processMount(r volume.MountRequest) volume.Response {
volumeInfo, err := plugin_utils.GetVolumeInfo(r.Name, "", d)
if err != nil {
log.Errorf("Unable to get volume info for volume %s. err:%v", r.Name, err)
return volume.Response{Err: err.Error()}
}
r.Name = volumeInfo.VolumeName
d.mountIDtoName[r.ID] = r.Name

// If the volume is already mounted , just increase the refcount.
// Note: for new keys, GO maps return zero value, so no need for if_exists.
refcnt := d.incrRefCount(r.Name) // save map traversal
Expand All @@ -373,22 +390,28 @@ func (d *VolumeDriver) processMount(r volume.MountRequest) volume.Response {
return volume.Response{Mountpoint: d.getMountPoint(r.Name)}
}

// There can be redundant mounts till refcounts are properly initialized
// TODO: #1220
status, err := d.GetVolume(r.Name)
if err != nil {
d.decrRefCount(r.Name)
return volume.Response{Err: err.Error()}
if plugin_utils.AlreadyMounted(r.Name, d.mountRoot) {
log.WithFields(log.Fields{"name": r.Name}).Info("Already mounted, skipping mount. ")
return volume.Response{Mountpoint: d.getMountPoint(r.Name)}
}

// get volume metadata if required
volumeMeta := volumeInfo.VolumeMeta
if volumeMeta == nil {
if volumeMeta, err = d.GetVolume(r.Name); err != nil {
d.decrRefCount(r.Name)
return volume.Response{Err: err.Error()}
}
}

fstype, exists := status[fsTypeTag]
fstype, exists := volumeMeta[fsTypeTag]
if !exists {
fstype = fs.FstypeDefault
}

skipAttach := false
// If the volume is already attached to the VM, skip the attach.
if state, stateExists := status["State"]; stateExists {
if state, stateExists := volumeMeta["State"]; stateExists {
if strings.Compare(state.(string), "DETACHED") != 0 {
skipAttach = true
}
Expand All @@ -398,7 +421,7 @@ func (d *VolumeDriver) processMount(r volume.MountRequest) volume.Response {
}

// Mount the volume and for now its always read-write.
mountpoint, err := d.MountVolume(r.Name, fstype.(string), status["ID"].(string), false, skipAttach)
mountpoint, err := d.MountVolume(r.Name, fstype.(string), volumeMeta["ID"].(string), false, skipAttach)
if err != nil {
log.WithFields(
log.Fields{"name": r.Name, "error": err.Error()},
Expand Down Expand Up @@ -618,6 +641,18 @@ func (d *VolumeDriver) Unmount(r volume.UnmountRequest) volume.Response {
return volume.Response{Err: ""}
}

if fullVolName, exist := d.mountIDtoName[r.ID]; exist {
r.Name = fullVolName
delete(d.mountIDtoName, r.ID) //cleanup the map
} else {
volumeInfo, err := plugin_utils.GetVolumeInfo(r.Name, "", d)
if err != nil {
log.Errorf("Unable to get volume info for volume %s. err:%v", r.Name, err)
return volume.Response{Err: err.Error()}
}
r.Name = volumeInfo.VolumeName
}

// if refcount has been succcessful, Normal flow.
// if the volume is still used by other containers, just return OK
refcnt, err := d.decrRefCount(r.Name)
Expand Down
55 changes: 47 additions & 8 deletions vmdk_plugin/drivers/vmdk/vmdk_driver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/docker/go-plugins-helpers/volume"
"github.com/vmware/docker-volume-vsphere/vmdk_plugin/drivers/vmdk/vmdkops"
"github.com/vmware/docker-volume-vsphere/vmdk_plugin/utils/fs"
"github.com/vmware/docker-volume-vsphere/vmdk_plugin/utils/plugin_utils"
"github.com/vmware/docker-volume-vsphere/vmdk_plugin/utils/refcount"
)

Expand All @@ -46,9 +47,10 @@ const (

// VolumeDriver - VMDK driver struct
type VolumeDriver struct {
useMockEsx bool
ops vmdkops.VmdkOps
refCounts *refcount.RefCountsMap
useMockEsx bool
ops vmdkops.VmdkOps
refCounts *refcount.RefCountsMap
mountIDtoName map[string]string // map of mountID -> full volume name
}

var mountRoot string
Expand Down Expand Up @@ -78,6 +80,7 @@ func NewVolumeDriver(port int, useMockEsx bool, mountDir string, driverName stri
}
}

d.mountIDtoName = make(map[string]string)
d.refCounts.Init(d, mountDir, driverName)

log.WithFields(log.Fields{
Expand All @@ -89,6 +92,12 @@ func NewVolumeDriver(port int, useMockEsx bool, mountDir string, driverName stri
return d
}

// VolumesInRefMap - get list of volumes names from refmap
// names are in format volume@datastore
func (d *VolumeDriver) VolumesInRefMap() []string {
return d.refCounts.GetVolumeNames()
}

// In following three operations on refcount, if refcount
// map hasn't been initialized, return 1 to prevent detach and remove.

Expand Down Expand Up @@ -212,6 +221,14 @@ func (d *VolumeDriver) UnmountVolume(name string) error {

// private function that does the job of mounting volume in conjunction with refcounting
func (d *VolumeDriver) processMount(r volume.MountRequest) volume.Response {
volumeInfo, err := plugin_utils.GetVolumeInfo(r.Name, "", d)
if err != nil {
log.Errorf("Unable to get volume info for volume %s. err:%v", r.Name, err)
return volume.Response{Err: err.Error()}
}
r.Name = volumeInfo.VolumeName
d.mountIDtoName[r.ID] = r.Name

// If the volume is already mounted , just increase the refcount.
// Note: for new keys, GO maps return zero value, so no need for if_exists.
refcnt := d.incrRefCount(r.Name) // save map traversal
Expand All @@ -223,9 +240,19 @@ func (d *VolumeDriver) processMount(r volume.MountRequest) volume.Response {
return volume.Response{Mountpoint: getMountPoint(r.Name)}
}

// There can be redundant mounts till refcounts are properly initialized
// TODO: #1220
status, err := d.ops.Get(r.Name)
if plugin_utils.AlreadyMounted(r.Name, mountRoot) {
log.WithFields(log.Fields{"name": r.Name}).Info("Already mounted, skipping mount. ")
return volume.Response{Mountpoint: getMountPoint(r.Name)}
}

// get volume metadata if required
volumeMeta := volumeInfo.VolumeMeta
if volumeMeta == nil {
if volumeMeta, err = d.ops.Get(r.Name); err != nil {
d.decrRefCount(r.Name)
return volume.Response{Err: err.Error()}
}
}

fstype := fs.FstypeDefault
isReadOnly := false
Expand All @@ -234,7 +261,7 @@ func (d *VolumeDriver) processMount(r volume.MountRequest) volume.Response {
return volume.Response{Err: err.Error()}
}
// Check access type.
value, exists := status["access"].(string)
value, exists := volumeMeta["access"].(string)
if !exists {
msg := fmt.Sprintf("Invalid access type for %s, assuming read-write access.", r.Name)
log.WithFields(log.Fields{"name": r.Name, "error": msg}).Error("")
Expand All @@ -244,7 +271,7 @@ func (d *VolumeDriver) processMount(r volume.MountRequest) volume.Response {
}

// Check file system type.
value, exists = status["fstype"].(string)
value, exists = volumeMeta["fstype"].(string)
if !exists {
msg := fmt.Sprintf("Invalid filesystem type for %s, assuming type as %s.",
r.Name, fstype)
Expand Down Expand Up @@ -458,6 +485,18 @@ func (d *VolumeDriver) Unmount(r volume.UnmountRequest) volume.Response {
return volume.Response{Err: ""}
}

if fullVolName, exist := d.mountIDtoName[r.ID]; exist {
r.Name = fullVolName
delete(d.mountIDtoName, r.ID) //cleanup the map
} else {
volumeInfo, err := plugin_utils.GetVolumeInfo(r.Name, "", d)
if err != nil {
log.Errorf("Unable to get volume info for volume %s. err:%v", r.Name, err)
return volume.Response{Err: err.Error()}
}
r.Name = volumeInfo.VolumeName
}

// if refcount has been succcessful, Normal flow
// if the volume is still used by other containers, just return OK
refcnt, err := d.decrRefCount(r.Name)
Expand Down
Loading

0 comments on commit 4eca684

Please sign in to comment.