Skip to content

Commit

Permalink
issue-2936: NodePublishVolume must return error if FS was remounted t…
Browse files Browse the repository at this point in the history
…o RO (#2937)
  • Loading branch information
antonmyagkov authored Jan 31, 2025
1 parent c43ddfe commit 1bf0460
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 0 deletions.
33 changes: 33 additions & 0 deletions cloud/blockstore/tests/csi_driver/e2e_tests_part2/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,3 +168,36 @@ def test_node_volume_expand_vm_mode():
raise
finally:
csi.cleanup_after_test(env, volume_name, access_type, [pod_id])


def test_publish_volume_must_fail_after_fs_error():
env, run = csi.init()
try:
volume_name = "example-disk"
volume_size = 1024 ** 3
pod_name = "example-pod"
pod_id = "deadbeef1"
access_type = "mount"
env.csi.create_volume(name=volume_name, size=volume_size)
env.csi.stage_volume(volume_name, access_type)
env.csi.publish_volume(pod_id, volume_name, pod_name, access_type)

with open('/sys/fs/ext4/nbd0/trigger_fs_error', 'w') as f:
f.write("test error")

env.csi.unpublish_volume(pod_id, volume_name, access_type)

stage_path = "/var/lib/kubelet/plugins/kubernetes.io/csi/nbs.csi.nebius.ai/a/globalmount"
assert "ro" == get_access_mode(stage_path)

try:
env.csi.publish_volume(pod_id, volume_name, pod_name, access_type)
assert False
except subprocess.CalledProcessError:
pass

except subprocess.CalledProcessError as e:
csi.log_called_process_error(e)
raise
finally:
csi.cleanup_after_test(env, volume_name, access_type, [pod_id])
7 changes: 7 additions & 0 deletions cloud/blockstore/tools/csi_driver/internal/driver/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,13 @@ func (s *nodeService) nodePublishDiskAsFilesystem(
"Staging target path is not mounted: %w", req.VolumeId)
}

readOnly, _ := s.mounter.IsFilesystemRemountedAsReadonly(req.StagingTargetPath)
if readOnly {
return s.statusErrorf(
codes.Internal,
"Filesystem was remounted as readonly")
}

mounted, _ = s.mounter.IsMountPoint(req.TargetPath)
if !mounted {
targetPerm := os.FileMode(0775)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ func TestPublishUnpublishDiskForInfrakuber(t *testing.T) {

mounter.On("IsMountPoint", stagingTargetPath).Return(true, nil)
mounter.On("IsMountPoint", targetPath).Return(false, nil)
mounter.On("IsFilesystemRemountedAsReadonly", stagingTargetPath).Return(false, nil)

mounter.On("Mount", stagingTargetPath, targetPath, "", []string{"bind"}).Return(nil)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type Interface interface {
HasBlockDevice(device string) (bool, error)

IsFilesystemExisted(device string) (bool, error)
IsFilesystemRemountedAsReadonly(mountPoint string) (bool, error)
MakeFilesystem(device string, fsType string) ([]byte, error)

NeedResize(devicePath string, deviceMountPath string) (bool, error)
Expand Down
5 changes: 5 additions & 0 deletions cloud/blockstore/tools/csi_driver/internal/mounter/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ func (c *Mock) IsFilesystemExisted(device string) (bool, error) {
return args.Get(0).(bool), args.Error(1)
}

func (c *Mock) IsFilesystemRemountedAsReadonly(mountPoint string) (bool, error) {
args := c.Called(mountPoint)
return args.Get(0).(bool), args.Error(1)
}

func (c *Mock) MakeFilesystem(device string, fsType string) ([]byte, error) {
args := c.Called(device, fsType)
return args.Get(0).([]byte), args.Error(1)
Expand Down
33 changes: 33 additions & 0 deletions cloud/blockstore/tools/csi_driver/internal/mounter/mounter.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,39 @@ func (m *mounter) IsFilesystemExisted(device string) (bool, error) {
return err == nil && string(out) != "", nil
}

func (m *mounter) IsFilesystemRemountedAsReadonly(mountPoint string) (bool, error) {
mountInfoList, err := mount.ParseMountInfo("/proc/self/mountinfo")
if err != nil {
return false, err
}

for _, mountInfo := range mountInfoList {
if mountInfo.MountPoint == mountPoint {
// The filesystem was remounted as read-only
// if the mount options included a read-write option, while
// the superblock options specified a read-only option.
var readWriteFs = false
for _, mountOption := range mountInfo.MountOptions {
if mountOption == "rw" {
readWriteFs = true
break
}
}

if !readWriteFs {
return false, nil
}

for _, superOption := range mountInfo.SuperOptions {
if superOption == "ro" {
return true, nil
}
}
}
}
return false, nil
}

func (m *mounter) MakeFilesystem(device string, fsType string) ([]byte, error) {
options := []string{"-t", fsType}
if fsType == "ext4" {
Expand Down

0 comments on commit 1bf0460

Please sign in to comment.