From 4d15925b94b747d5b2672bdc2c80e5b3fdaad9b8 Mon Sep 17 00:00:00 2001 From: Praveen M Date: Mon, 6 May 2024 11:25:00 +0530 Subject: [PATCH] cleanup: incorrect fuserecovery logging Signed-off-by: Praveen M (cherry picked from commit 0e61b826ea6ddd5c368112e4a82d139d528cdfd0) --- internal/cephfs/fuserecovery.go | 72 -------------------------- internal/cephfs/nodeserver.go | 38 ++++++++++---- internal/cephfs/store/volumeoptions.go | 4 ++ internal/util/util.go | 6 --- 4 files changed, 31 insertions(+), 89 deletions(-) diff --git a/internal/cephfs/fuserecovery.go b/internal/cephfs/fuserecovery.go index 279b0f79b63f..41f60aa2a31c 100644 --- a/internal/cephfs/fuserecovery.go +++ b/internal/cephfs/fuserecovery.go @@ -24,8 +24,6 @@ import ( fsutil "github.com/ceph/ceph-csi/internal/cephfs/util" "github.com/ceph/ceph-csi/internal/util" "github.com/ceph/ceph-csi/internal/util/log" - - mountutil "k8s.io/mount-utils" ) type ( @@ -37,9 +35,6 @@ const ( msNotMounted msMounted msCorrupted - - // ceph-fuse fsType in /proc//mountinfo. - cephFuseFsType = "fuse.ceph-fuse" ) func (ms mountState) String() string { @@ -68,30 +63,6 @@ func (ns *NodeServer) getMountState(path string) (mountState, error) { return msNotMounted, nil } -func findMountinfo(mountpoint string, minfo []mountutil.MountInfo) int { - for i := range minfo { - if minfo[i].MountPoint == mountpoint { - return i - } - } - - return -1 -} - -// Ensures that given mountpoint is of specified fstype. -// Returns true if fstype matches, or if no such mountpoint exists. -func validateFsType(mountpoint, fsType string, minfo []mountutil.MountInfo) bool { - if idx := findMountinfo(mountpoint, minfo); idx > 0 { - mi := minfo[idx] - - if mi.FsType != fsType { - return false - } - } - - return true -} - // tryRestoreFuseMountsInNodePublish tries to restore staging and publish // volume moutpoints inside the NodePublishVolume call. // @@ -158,19 +129,6 @@ func (ns *NodeServer) tryRestoreFuseMountsInNodePublish( volOptions *store.VolumeOptions ) - procMountInfo, err := util.ReadMountInfoForProc("self") - if err != nil { - return err - } - - if !validateFsType(stagingTargetPath, cephFuseFsType, procMountInfo) || - !validateFsType(targetPath, cephFuseFsType, procMountInfo) { - // We can't restore mounts not managed by ceph-fuse. - log.WarningLog(ctx, "cephfs: cannot proceed with mount recovery on non-FUSE mountpoints") - - return nil - } - volOptions, err = ns.getVolumeOptions(ctx, volID, volContext, nsMountinfo.Secrets) if err != nil { return err @@ -181,13 +139,6 @@ func (ns *NodeServer) tryRestoreFuseMountsInNodePublish( return err } - if _, ok := volMounter.(*mounter.FuseMounter); !ok { - // We can't restore mounts with non-FUSE mounter. - log.WarningLog(ctx, "cephfs: cannot proceed with mount recovery with non-FUSE mounter") - - return nil - } - // Try to restore mount in staging target path. // Unmount and mount the volume. @@ -225,7 +176,6 @@ func (ns *NodeServer) tryRestoreFuseMountsInNodePublish( // should be able to continue with mounting the volume normally afterwards. func (ns *NodeServer) tryRestoreFuseMountInNodeStage( ctx context.Context, - mnt mounter.VolumeMounter, stagingTargetPath string, ) error { // Check if there is anything to restore. @@ -245,28 +195,6 @@ func (ns *NodeServer) tryRestoreFuseMountInNodeStage( log.WarningLog(ctx, "cephfs: mountpoint problem detected when staging a volume: %s is %s; attempting recovery", stagingTargetPath, stagingTargetMs) - // Check that the existing stage mount for this volume is managed by - // ceph-fuse, and that the mounter is FuseMounter. Then try to restore them. - - procMountInfo, err := util.ReadMountInfoForProc("self") - if err != nil { - return err - } - - if !validateFsType(stagingTargetPath, cephFuseFsType, procMountInfo) { - // We can't restore mounts not managed by ceph-fuse. - log.WarningLog(ctx, "cephfs: cannot proceed with mount recovery on non-FUSE mountpoints") - - return nil - } - - if _, ok := mnt.(*mounter.FuseMounter); !ok { - // We can't restore mounts with non-FUSE mounter. - log.WarningLog(ctx, "cephfs: cannot proceed with mount recovery with non-FUSE mounter") - - return nil - } - // Restoration here means only unmounting the volume. // NodeStageVolume should take care of the rest. return mounter.UnmountAll(ctx, stagingTargetPath) diff --git a/internal/cephfs/nodeserver.go b/internal/cephfs/nodeserver.go index 2af514c3644f..ede292b31423 100644 --- a/internal/cephfs/nodeserver.go +++ b/internal/cephfs/nodeserver.go @@ -213,8 +213,10 @@ func (ns *NodeServer) NodeStageVolume( // Check if the volume is already mounted - if err = ns.tryRestoreFuseMountInNodeStage(ctx, mnt, stagingTargetPath); err != nil { - return nil, status.Errorf(codes.Internal, "failed to try to restore FUSE mounts: %v", err) + if _, ok := mnt.(*mounter.FuseMounter); ok { + if err = ns.tryRestoreFuseMountInNodeStage(ctx, stagingTargetPath); err != nil { + return nil, status.Errorf(codes.Internal, "failed to try to restore FUSE mounts: %v", err) + } } isMnt, err := util.IsMountPoint(ns.Mounter, stagingTargetPath) @@ -448,23 +450,37 @@ func (ns *NodeServer) NodePublishVolume( targetPath := req.GetTargetPath() volID := fsutil.VolumeID(req.GetVolumeId()) + volOptions := &store.VolumeOptions{} + defer volOptions.Destroy() + + if err := volOptions.DetectMounter(req.GetVolumeContext()); err != nil { + return nil, status.Errorf(codes.Internal, "failed to detect mounter for volume %s: %v", volID, err.Error()) + } + + volMounter, err := mounter.New(volOptions) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to create mounter for volume %s: %v", volID, err.Error()) + } + // Considering kubelet make sure the stage and publish operations // are serialized, we dont need any extra locking in nodePublish - if err := util.CreateMountPoint(targetPath); err != nil { + if err = util.CreateMountPoint(targetPath); err != nil { log.ErrorLog(ctx, "failed to create mount point at %s: %v", targetPath, err) return nil, status.Error(codes.Internal, err.Error()) } - if err := ns.tryRestoreFuseMountsInNodePublish( - ctx, - volID, - stagingTargetPath, - targetPath, - req.GetVolumeContext(), - ); err != nil { - return nil, status.Errorf(codes.Internal, "failed to try to restore FUSE mounts: %v", err) + if _, ok := volMounter.(*mounter.FuseMounter); ok { + if err = ns.tryRestoreFuseMountsInNodePublish( + ctx, + volID, + stagingTargetPath, + targetPath, + req.GetVolumeContext(), + ); err != nil { + return nil, status.Errorf(codes.Internal, "failed to try to restore FUSE mounts: %v", err) + } } if req.GetReadonly() { diff --git a/internal/cephfs/store/volumeoptions.go b/internal/cephfs/store/volumeoptions.go index 5ed493e86fa0..8a91b44b2878 100644 --- a/internal/cephfs/store/volumeoptions.go +++ b/internal/cephfs/store/volumeoptions.go @@ -151,6 +151,10 @@ func validateMounter(m string) error { return nil } +func (v *VolumeOptions) DetectMounter(options map[string]string) error { + return extractMounter(&v.Mounter, options) +} + func extractMounter(dest *string, options map[string]string) error { if err := extractOptionalOption(dest, "mounter", options); err != nil { return err diff --git a/internal/util/util.go b/internal/util/util.go index 1dbaf54eaece..853116f797d4 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -338,12 +338,6 @@ func IsCorruptedMountError(err error) bool { return mount.IsCorruptedMnt(err) } -// ReadMountInfoForProc reads /proc//mountpoint and marshals it into -// MountInfo structs. -func ReadMountInfoForProc(proc string) ([]mount.MountInfo, error) { - return mount.ParseMountInfo(fmt.Sprintf("/proc/%s/mountinfo", proc)) -} - // Mount mounts the source to target path. func Mount(mounter mount.Interface, source, target, fstype string, options []string) error { return mounter.MountSensitiveWithoutSystemd(source, target, fstype, options, nil)