feat(zfspv): handling unmounted volume

There can be cases where openebs namespace has been accidently deleted (Optoro case: https://mdap.zendesk.com/agent/tickets/963), There the driver attempted to destroy the dataset which will first umount the dataset and then try to destroy it, the destroy will fail as volume is busy. Here, as mentioned in the steps to recover, we have to manually mount the dataset
```
6. The driver might have attempted to destroy the volume before going down, which sets the mount as no(this strange behavior on gke ubuntu 18.04), we have to mount the dataset, go to the each node and check if there is any unmounted volume
zfs get mounted
if there is any unmounted dataset with this option as "no", we should do the below :-
mountpath=zfs get -Hp -o value mountpoint <dataset name>
zfs set mountpoint=none
zfs set mountpoint=<mountpath>
this will set the dataset to be mounted.
```

So in this case the volume will be  unmounted and still mountpoint will set to the mountpath, so if application pod is deleted later on, it will try to mount the zfs dataset, here just setting the `mountpoint` is not sufficient, as if we have unmounted the zfs dataset (via zfs destroy in this case), so we have to explicitely mount the dataset **otherwise application will start running without any persistence storage**. Here automating the manual steps performed to resolve the problem, we are checking in the code that if zfs dataset is not mounted after setting the mountpoint property, attempt to mount it.

This is not the case with the zvol as it does not attempt to unmount it, so zvols are fine.

Also NodeUnPublish operation MUST be idempotent. If this RPC failed, or the CO does not know if it failed or not, it can choose to call NudeUnPublishRequest again. So handled this and returned successful if volume is not mounted also added descriptive error messages at few places.

Signed-off-by: Pawan <pawan@mayadata.io>
This commit is contained in:
Pawan 2020-04-06 20:09:07 +05:30 committed by Kiran Mova
parent 6033789c17
commit 3a1a8e78e6
4 changed files with 53 additions and 37 deletions

View file

@ -133,10 +133,8 @@ func (ns *node) NodeUnpublishVolume(
) (*csi.NodeUnpublishVolumeResponse, error) { ) (*csi.NodeUnpublishVolumeResponse, error) {
var ( var (
err error err error
vol *apis.ZFSVolume vol *apis.ZFSVolume
devpath string
currentMounts []string
) )
if err = ns.validateNodeUnpublishReq(req); err != nil { if err = ns.validateNodeUnpublishReq(req); err != nil {
@ -147,37 +145,17 @@ func (ns *node) NodeUnpublishVolume(
volumeID := req.GetVolumeId() volumeID := req.GetVolumeId()
if vol, err = zfs.GetZFSVolume(volumeID); err != nil { if vol, err = zfs.GetZFSVolume(volumeID); err != nil {
return nil, err return nil, status.Errorf(codes.Internal,
"not able to get the ZFSVolume %s err : %s",
volumeID, err.Error())
} }
if devpath, err = zfs.GetVolumeDevPath(vol); err != nil { err = zfs.UmountVolume(vol, targetPath)
goto NodeUnpublishResponse
}
currentMounts, err = zfs.GetMounts(devpath)
if err != nil { if err != nil {
return nil, err return nil, status.Errorf(codes.Internal,
} else if len(currentMounts) == 0 { "unable to umount the volume %s err : %s",
return nil, status.Error(codes.Internal, "umount request for not mounted volume") volumeID, err.Error())
} else if len(currentMounts) == 1 {
if currentMounts[0] != targetPath {
return nil, status.Error(codes.Internal, "device not mounted at right path")
}
} else {
logrus.Errorf(
"can not unmount, more than one mounts for volume:%s path %s mounts: %v",
volumeID, targetPath, currentMounts,
)
return nil, status.Error(codes.Internal, "device not mounted at rightpath")
}
if err = zfs.UmountVolume(vol, req.GetTargetPath()); err != nil {
goto NodeUnpublishResponse
}
NodeUnpublishResponse:
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
} }
logrus.Infof("hostpath: volume %s path: %s has been unmounted.", logrus.Infof("hostpath: volume %s path: %s has been unmounted.",
volumeID, targetPath) volumeID, targetPath)

View file

@ -119,8 +119,8 @@ func CreateZFSVolume(req *csi.CreateVolumeRequest) (string, error) {
err = zfs.ProvisionVolume(volObj) err = zfs.ProvisionVolume(volObj)
if err != nil { if err != nil {
return "", status.Error(codes.Internal, return "", status.Errorf(codes.Internal,
"not able to provision the volume") "not able to provision the volume %s", err.Error())
} }
return selected, nil return selected, nil
@ -168,8 +168,8 @@ func CreateZFSClone(req *csi.CreateVolumeRequest, snapshot string) (string, erro
err = zfs.ProvisionVolume(volObj) err = zfs.ProvisionVolume(volObj)
if err != nil { if err != nil {
return "", status.Error(codes.Internal, return "", status.Errorf(codes.Internal,
"not able to provision the volume") "not able to provision the clone volume %s", err.Error())
} }
return selected, nil return selected, nil

View file

@ -32,7 +32,7 @@ func UmountVolume(vol *apis.ZFSVolume, targetPath string,
) error { ) error {
mounter := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: mount.NewOsExec()} mounter := &mount.SafeFormatAndMount{Interface: mount.New(""), Exec: mount.NewOsExec()}
_, _, err := mount.GetDeviceNameFromMount(mounter, targetPath) dev, ref, err := mount.GetDeviceNameFromMount(mounter, targetPath)
if err != nil { if err != nil {
logrus.Errorf( logrus.Errorf(
"zfspv umount volume: failed to get device from mnt: %s\nError: %v", "zfspv umount volume: failed to get device from mnt: %s\nError: %v",
@ -41,6 +41,15 @@ func UmountVolume(vol *apis.ZFSVolume, targetPath string,
return err return err
} }
// device has already been un-mounted, return successful
if len(dev) == 0 || ref == 0 {
logrus.Warningf(
"Warning: Unmount skipped because volume %s not mounted: %v",
vol.Name, targetPath,
)
return nil
}
if pathExists, pathErr := mount.PathExists(targetPath); pathErr != nil { if pathExists, pathErr := mount.PathExists(targetPath); pathErr != nil {
return fmt.Errorf("Error checking if path exists: %v", pathErr) return fmt.Errorf("Error checking if path exists: %v", pathErr)
} else if !pathExists { } else if !pathExists {

View file

@ -387,7 +387,36 @@ func SetDatasetMountProp(volume string, mountpath string) error {
func MountZFSDataset(vol *apis.ZFSVolume, mountpath string) error { func MountZFSDataset(vol *apis.ZFSVolume, mountpath string) error {
volume := vol.Spec.PoolName + "/" + vol.Name volume := vol.Spec.PoolName + "/" + vol.Name
return SetDatasetMountProp(volume, mountpath) // set the mountpoint to the path where this volume should be mounted
err := SetDatasetMountProp(volume, mountpath)
if err != nil {
return err
}
/*
* see if we should attempt to mount the dataset.
* Setting the mountpoint is sufficient to mount the zfs dataset,
* but if dataset has been unmounted, then setting the mountpoint
* is not sufficient, we have to mount the dataset explicitly
*/
mounted, err := GetVolumeProperty(vol, "mounted")
if err != nil {
return err
}
if mounted == "no" {
var MountVolArg []string
MountVolArg = append(MountVolArg, "mount", volume)
cmd := exec.Command(ZFSVolCmd, MountVolArg...)
out, err := cmd.CombinedOutput()
if err != nil {
logrus.Errorf("zfs: could not mount the dataset %v cmd %v error: %s",
volume, MountVolArg, string(out))
return err
}
}
return nil
} }
// UmountZFSDataset umounts the dataset // UmountZFSDataset umounts the dataset