Skip resize if fs is readonly (#114325)

* skip resize if fs is readonly

* run formatter + add log line for early return case

* fix error

staging/src/k8s.io/mount-utils/resizefs_linux.go

@@ -28,6 +28,10 @@ import (
 	utilexec "k8s.io/utils/exec"
 )
 
+const (
+	blockDev = "blockdev"
+)
+
 // ResizeFs Provides support for resizing file systems
 type ResizeFs struct {
 	exec utilexec.Interface
@@ -104,6 +108,17 @@ func (resizefs *ResizeFs) btrfsResize(deviceMountPath string) (bool, error) {
 }
 
 func (resizefs *ResizeFs) NeedResize(devicePath string, deviceMountPath string) (bool, error) {
+	// Do nothing if device is mounted as readonly
+	readonly, err := resizefs.getDeviceRO(devicePath)
+	if err != nil {
+		return false, err
+	}
+
+	if readonly {
+		klog.V(3).Infof("ResizeFs.needResize - no resize possible since filesystem %s is readonly", devicePath)
+		return false, nil
+	}
+
 	deviceSize, err := resizefs.getDeviceSize(devicePath)
 	if err != nil {
 		return false, err
@@ -147,7 +162,7 @@ func (resizefs *ResizeFs) NeedResize(devicePath string, deviceMountPath string)
 	return true, nil
 }
 func (resizefs *ResizeFs) getDeviceSize(devicePath string) (uint64, error) {
-	output, err := resizefs.exec.Command("blockdev", "--getsize64", devicePath).CombinedOutput()
+	output, err := resizefs.exec.Command(blockDev, "--getsize64", devicePath).CombinedOutput()
 	outStr := strings.TrimSpace(string(output))
 	if err != nil {
 		return 0, fmt.Errorf("failed to read size of device %s: %s: %s", devicePath, err, outStr)
@@ -159,6 +174,22 @@ func (resizefs *ResizeFs) getDeviceSize(devicePath string) (uint64, error) {
 	return size, nil
 }
 
+func (resizefs *ResizeFs) getDeviceRO(devicePath string) (bool, error) {
+	output, err := resizefs.exec.Command(blockDev, "--getro", devicePath).CombinedOutput()
+	outStr := strings.TrimSpace(string(output))
+	if err != nil {
+		return false, fmt.Errorf("failed to get readonly bit from device %s: %s: %s", devicePath, err, outStr)
+	}
+	switch outStr {
+	case "0":
+		return false, nil
+	case "1":
+		return true, nil
+	default:
+		return false, fmt.Errorf("Failed readonly device check. Expected 1 or 0, got '%s'", outStr)
+	}
+}
+
 func (resizefs *ResizeFs) getExtSize(devicePath string) (uint64, uint64, error) {
 	output, err := resizefs.exec.Command("dumpe2fs", "-h", devicePath).CombinedOutput()
 	if err != nil {

staging/src/k8s.io/mount-utils/resizefs_linux_test.go

@@ -20,9 +20,11 @@ limitations under the License.
 package mount
 
 import (
+	"fmt"
+	"testing"
+
 	"k8s.io/utils/exec"
 	fakeexec "k8s.io/utils/exec/testing"
-	"testing"
 )
 
 func TestGetFileSystemSize(t *testing.T) {
@@ -179,7 +181,7 @@ flags                   0x1
 magic                   _BHRfS_M [match]
 fsid                    3f53c8f7-3c57-4185-bf1d-b305b42cce97
 metadata_uuid           3f53c8f7-3c57-4185-bf1d-b305b42cce97
-label                   
+label
 generation              7
 root                    30441472
 sys_array_size          129
@@ -289,7 +291,7 @@ flags                   0x1
 magic                   _BHRfS_M [match]
 fsid                    3f53c8f7-3c57-4185-bf1d-b305b42cce97
 metadata_uuid           3f53c8f7-3c57-4185-bf1d-b305b42cce97
-label                   
+label
 generation              7
 root                    30441472
 sys_array_size          129
@@ -496,16 +498,53 @@ func TestNeedResize(t *testing.T) {
 		name            string
 		devicePath      string
 		deviceMountPath string
+		readonly        string
 		deviceSize      string
+		extSize         string
 		cmdOutputFsType string
 		expectError     bool
 		expectResult    bool
 	}{
+		{
+			name:            "True",
+			devicePath:      "/dev/test1",
+			deviceMountPath: "/mnt/test1",
+			readonly:        "0",
+			deviceSize:      "2048",
+			cmdOutputFsType: "TYPE=ext3",
+			extSize:         "20",
+			expectError:     false,
+			expectResult:    true,
+		},
+		{
+			name:            "False - needed by size but fs is readonly",
+			devicePath:      "/dev/test1",
+			deviceMountPath: "/mnt/test1",
+			readonly:        "1",
+			deviceSize:      "2048",
+			cmdOutputFsType: "TYPE=ext3",
+			extSize:         "20",
+			expectError:     false,
+			expectResult:    false,
+		},
+		{
+			name:            "False - Not needed by size",
+			devicePath:      "/dev/test1",
+			deviceMountPath: "/mnt/test1",
+			readonly:        "0",
+			deviceSize:      "20",
+			cmdOutputFsType: "TYPE=ext3",
+			extSize:         "2048",
+			expectError:     false,
+			expectResult:    false,
+		},
 		{
 			name:            "False - Unsupported fs type",
 			devicePath:      "/dev/test1",
 			deviceMountPath: "/mnt/test1",
+			readonly:        "0",
 			deviceSize:      "2048",
+			extSize:         "1",
 			cmdOutputFsType: "TYPE=ntfs",
 			expectError:     true,
 			expectResult:    false,
@@ -516,25 +555,31 @@ func TestNeedResize(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			fcmd := fakeexec.FakeCmd{
 				CombinedOutputScript: []fakeexec.FakeAction{
+					func() ([]byte, []byte, error) { return []byte(test.readonly), nil, nil },
 					func() ([]byte, []byte, error) { return []byte(test.deviceSize), nil, nil },
 					func() ([]byte, []byte, error) { return []byte(test.cmdOutputFsType), nil, nil },
+					func() ([]byte, []byte, error) {
+						return []byte(fmt.Sprintf("block size: %s\nblock count: 1", test.extSize)), nil, nil
+					},
 				},
 			}
 			fexec := fakeexec.FakeExec{
 				CommandScript: []fakeexec.FakeCommandAction{
 					func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
 					func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+					func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+					func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
 				},
 			}
 			resizefs := ResizeFs{exec: &fexec}
 
 			needResize, err := resizefs.NeedResize(test.devicePath, test.deviceMountPath)
-			if needResize != test.expectResult {
-				t.Fatalf("Expect result is %v but got %v", test.expectResult, needResize)
-			}
 			if !test.expectError && err != nil {
 				t.Fatalf("Expect no error but got %v", err)
 			}
+			if needResize != test.expectResult {
+				t.Fatalf("Expect result is %v but got %v", test.expectResult, needResize)
+			}
 		})
 	}
 }