From ee0269d1600ff12231eddedd5161a53401fe8152 Mon Sep 17 00:00:00 2001 From: Payes Anand Date: Wed, 13 May 2020 14:10:04 +0530 Subject: [PATCH] fix(CStorVolumeAttachment): Remove finalizer on unsuccesful mounts and on driver restarts (#82) (#85) Signed-off-by: Payes --- pkg/driver/node.go | 21 +++++++++++++++++---- pkg/driver/service.go | 1 + pkg/utils/kubernetes.go | 5 ++--- pkg/utils/utils.go | 41 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/pkg/driver/node.go b/pkg/driver/node.go index 5b1d6ba57..e2e9980eb 100644 --- a/pkg/driver/node.go +++ b/pkg/driver/node.go @@ -126,12 +126,13 @@ func (ns *node) NodeStageVolume( } if isMountRequired { vol.Spec.Volume.StagingTargetPath = stagingTargetPath + // This is placed to clean up stale iSCSI Sessions vol.Finalizers = []string{utils.NodeIDENV} vol.Spec.Volume.DevicePath = strings.Join([]string{ "/dev/disk/by-path/ip", vol.Spec.ISCSI.TargetPortal, "iscsi", vol.Spec.ISCSI.Iqn, "lun", fmt.Sprint(0)}, "-", ) - err = utils.UpdateCStorVolumeAttachmentCR(vol) + vol, err = utils.UpdateCStorVolumeAttachmentCR(vol) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -147,6 +148,12 @@ func (ns *node) NodeStageVolume( // Login to the volume and attempt mount operation on the requested path devicePath, err := ns.attachDisk(vol) if err != nil { + vol.Finalizers = nil + // There might still be a case that the attach was successful, + // therefore not cleaning up the staging path from CR + if _, err1 := utils.UpdateCStorVolumeAttachmentCR(vol); err1 != nil { + return nil, status.Error(codes.Internal, err1.Error()) + } logrus.Errorf("NodeStageVolume: failed to attachDisk for volume %v, err: %v", volumeID, err) return nil, status.Error(codes.Internal, err.Error()) } @@ -163,6 +170,12 @@ func (ns *node) NodeStageVolume( logrus.Info("NodeStageVolume: start format and mount operation") if err := ns.formatAndMount(req, devicePath); err != nil { + vol.Finalizers = nil + // There might still be a case that the attach was successful, + // therefore not cleaning up the staging path from CR + if _, uerr := utils.UpdateCStorVolumeAttachmentCR(vol); uerr != nil { + logrus.Errorf("Failed to update CStorVolumeAttachment:%v", uerr.Error()) + } return nil, status.Error(codes.Internal, err.Error()) } @@ -236,7 +249,7 @@ func (ns *node) NodeUnstageVolume( vol.Finalizers = nil vol.Spec.Volume.StagingTargetPath = "" - if err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil { + if _, err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil { return nil, status.Error(codes.Internal, err.Error()) } logrus.Infof("hostpath: volume %s path: %s has been unmounted.", @@ -270,7 +283,7 @@ func (ns *node) NodePublishVolume( return nil, status.Error(codes.Internal, err.Error()) } vol.Spec.Volume.TargetPath = req.GetTargetPath() - if err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil { + if _, err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil { return nil, status.Error(codes.Internal, err.Error()) } @@ -316,7 +329,7 @@ func (ns *node) NodeUnpublishVolume( return nil, status.Error(codes.Internal, err.Error()) } vol.Spec.Volume.TargetPath = "" - if err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil { + if _, err = utils.UpdateCStorVolumeAttachmentCR(vol); err != nil { return nil, status.Error(codes.Internal, err.Error()) } diff --git a/pkg/driver/service.go b/pkg/driver/service.go index 678f7208d..533e1db8c 100644 --- a/pkg/driver/service.go +++ b/pkg/driver/service.go @@ -87,6 +87,7 @@ func New(config *config.Config) *CSIDriver { driver.cs = NewController(driver) case "node": + utils.CleanupOnRestart() // Start monitor goroutine to monitor the // mounted paths. If a path goes down or // becomes read only (in case of RW mount diff --git a/pkg/utils/kubernetes.go b/pkg/utils/kubernetes.go index 2e4323e51..7fe1ed58b 100644 --- a/pkg/utils/kubernetes.go +++ b/pkg/utils/kubernetes.go @@ -134,11 +134,10 @@ func CreateCStorVolumeAttachmentCR(csivol *apis.CStorVolumeAttachment, nodeID st } // UpdateCStorVolumeAttachmentCR updates CStorVolumeAttachment CR related to current nodeID -func UpdateCStorVolumeAttachmentCR(csivol *apis.CStorVolumeAttachment) error { +func UpdateCStorVolumeAttachmentCR(csivol *apis.CStorVolumeAttachment) (*apis.CStorVolumeAttachment, error) { - _, err := csivolume.NewKubeclient(). + return csivolume.NewKubeclient(). WithNamespace(OpenEBSNamespace).Update(csivol) - return err } // TODO Explain when a create of csi volume happens & when it diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 3d0c9888a..475236c6f 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -13,6 +13,7 @@ import ( "github.com/kubernetes-csi/csi-lib-utils/protosanitizer" apis "github.com/openebs/cstor-csi/pkg/apis/cstor/v1" "github.com/openebs/cstor-csi/pkg/cstor/snapshot" + iscsiutils "github.com/openebs/cstor-csi/pkg/iscsi" "github.com/sirupsen/logrus" "google.golang.org/grpc" "k8s.io/utils/mount" @@ -298,6 +299,46 @@ func MonitorMounts() { } } +// CleanupOnRestart unmounts and detaches the volumes having +// DeletionTimestamp set and removes finalizers from the +// corresponding CStorVolumeAttachment CRs +func CleanupOnRestart() { + var ( + err error + csivolList *apis.CStorVolumeAttachmentList + ) + // Get list of mounted paths present with the node + TransitionVolListLock.Lock() + defer TransitionVolListLock.Unlock() + if csivolList, err = GetVolListForNode(); err != nil { + return + } + for _, Vol := range csivolList.Items { + if Vol.DeletionTimestamp == nil { + continue + } + vol := Vol + TransitionVolList[vol.Spec.Volume.Name] = apis.CStorVolumeAttachmentStatusUnmountUnderProgress + // This is being run in a go routine so that if unmount and detach + // commands take time, the startup is not delayed + go func(vol *apis.CStorVolumeAttachment) { + if err := iscsiutils.UnmountAndDetachDisk(vol, vol.Spec.Volume.StagingTargetPath); err == nil { + vol.Finalizers = nil + if vol, err = UpdateCStorVolumeAttachmentCR(vol); err != nil { + logrus.Errorf(err.Error()) + } + } else { + logrus.Errorf(err.Error()) + } + + TransitionVolListLock.Lock() + TransitionVolList[vol.Spec.Volume.Name] = apis.CStorVolumeAttachmentStatusUnmounted + delete(TransitionVolList, vol.Spec.Volume.Name) + TransitionVolListLock.Unlock() + }(&vol) + } +} + // WaitForVolumeReadyAndReachable waits until the volume is ready to accept IOs // and is reachable, this function will not come out until both the conditions // are met. This function stops the driver from overloading the OS with iSCSI