Skip to content

Commit

Permalink
Set FS type from LINSTOR properties during mounts
Browse files Browse the repository at this point in the history
If a user restores a volume from a snapshot, the file system type of the
restored volume may not match the requested FS type of the storage class. This
may happen if for example the snapshot was taken from an xfs volume, but the
new StorageClass is configured to provision ext4 volumes. We then would try
to call "mount -t ext4" on a xfs-formatted block device, leading to a failure
mode where the volume is created fine, but you cannot make use of it.

To fix this, instead of taking the FS type directly from Kubernetes, we use
the existing LINSTOR FsType property to determine what fs type is actually
on the volume. Only if this information is not provided (possible with very
old snapshots or some other funky setups), we fall back to the information
provided by Kubernetes.

One alternative would be to disallow such mixing of filesystem types
completely, ensuring when a volume is restored from a snapshot that old and new
volume have matching FS type in Kubernetes. This would permanently lock in some
users to their initial choice of FS, so we instead chose the option that allows
for gradual migration to a new StorageClass/FS.

Signed-off-by: Moritz Wanzenböck <[email protected]>
  • Loading branch information
WanzenBug committed Feb 18, 2025
1 parent dbe913d commit 6468e6e
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 17 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Set the FS type during mount operations based on the FS type stored in LINSTOR.

## [1.7.0] - 2025-02-13

### Added
Expand Down
37 changes: 20 additions & 17 deletions pkg/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,13 +342,31 @@ func (d Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolum

volCtx.MountOptions = append(volCtx.MountOptions, "_netdev")

publishCtx := PublishContextFromMap(req.GetPublishContext())
if publishCtx == nil {
assignment, err := d.Assignments.FindAssignmentOnNode(ctx, req.GetVolumeId(), d.nodeID)
if err != nil {
return nil, status.Errorf(codes.Internal, "NodePublishVolume failed for %s: %v", req.GetVolumeId(), err)
}

if assignment == nil {
return nil, status.Errorf(codes.NotFound, "NodePublishVolume failed for %s: assignment not found", req.GetVolumeId())
}

publishCtx = &PublishContext{
DevicePath: assignment.Path,
}
}

var fsType string

if block := req.GetVolumeCapability().GetBlock(); block != nil {
volCtx.MountOptions = []string{"bind"}
}

if mnt := req.GetVolumeCapability().GetMount(); mnt != nil {
if publishCtx.FsType != "" {
fsType = publishCtx.FsType
} else if mnt := req.GetVolumeCapability().GetMount(); mnt != nil {
volCtx.MountOptions = append(volCtx.MountOptions, mnt.GetMountFlags()...)
fsType = "ext4"
if mnt.FsType != "" {
Expand All @@ -362,22 +380,6 @@ func (d Driver) NodePublishVolume(ctx context.Context, req *csi.NodePublishVolum
volCtx.MountOptions = append(volCtx.MountOptions, "nouuid")
}

publishCtx := PublishContextFromMap(req.GetPublishContext())
if publishCtx == nil {
assignment, err := d.Assignments.FindAssignmentOnNode(ctx, req.GetVolumeId(), d.nodeID)
if err != nil {
return nil, status.Errorf(codes.Internal, "NodePublishVolume failed for %s: %v", req.GetVolumeId(), err)
}

if assignment == nil {
return nil, status.Errorf(codes.NotFound, "NodePublishVolume failed for %s: assignment not found", req.GetVolumeId())
}

publishCtx = &PublishContext{
DevicePath: assignment.Path,
}
}

ro := req.GetReadonly() || req.GetVolumeCapability().GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY

err = d.Mounter.Mount(ctx, publishCtx.DevicePath, req.GetTargetPath(), fsType, ro, volCtx.MountOptions)
Expand Down Expand Up @@ -683,6 +685,7 @@ func (d Driver) ControllerPublishVolume(ctx context.Context, req *csi.Controller
return &csi.ControllerPublishVolumeResponse{
PublishContext: (&PublishContext{
DevicePath: devPath,
FsType: existingVolume.FsType,
}).ToMap(),
}, nil
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/driver/publish_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import (
const (
PublishContextMarker = linstor.ParameterNamespace + "/uses-publish-context"
DevicePath = linstor.ParameterNamespace + "/device-path"
FsType = linstor.ParameterNamespace + "/fs-type"
)

type PublishContext struct {
FsType string
DevicePath string
}

Expand All @@ -21,12 +23,14 @@ func PublishContextFromMap(ctx map[string]string) *PublishContext {

return &PublishContext{
DevicePath: ctx[DevicePath],
FsType: ctx[FsType],
}
}

func (p *PublishContext) ToMap() map[string]string {
return map[string]string{
PublishContextMarker: "true",
DevicePath: p.DevicePath,
FsType: p.FsType,
}
}

0 comments on commit 6468e6e

Please sign in to comment.