From dbc95c6ee8b136f97fa6086b250b60bde0472ca3 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Thu, 25 Apr 2024 12:03:55 +0200 Subject: [PATCH 1/2] build: use Fedora 39 as base image for test container It seems GitHub has an issue with the Fedora 40 container image, extracting the Golang tarball fails. The Fedora 39 image does not have this problem, so use that for the time being. Signed-off-by: Niels de Vos (cherry picked from commit 60e2527917262349676d09d4678003f7beec3cf7) --- scripts/Dockerfile.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Dockerfile.test b/scripts/Dockerfile.test index 5b03911c730..2e9eb13ef39 100644 --- a/scripts/Dockerfile.test +++ b/scripts/Dockerfile.test @@ -8,7 +8,7 @@ # little different. # -FROM registry.fedoraproject.org/fedora:latest +FROM registry.fedoraproject.org/fedora:39 ARG GOPATH=/go ARG GOROOT=/usr/local/go From c4cab67d16f670c742bb8a9a5f3b455335604075 Mon Sep 17 00:00:00 2001 From: Praveen M Date: Mon, 6 May 2024 11:25:00 +0530 Subject: [PATCH 2/2] 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 279b0f79b63..41f60aa2a31 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 2af514c3644..ede292b3142 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 5ed493e86fa..8a91b44b287 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 1dbaf54eaec..853116f797d 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)