filesystem: take super options into account for read-only (#3387)

* filesystem: take super options into account for read-only

With the latest change implemented to use `mountinfo` instead of
`mounts` there was a regression in filesystem readonly detection
due to super options not taken into account: filesystems that would
previously be marked a "read-only" would not anymore because that
information had moved to super options instead of mount options on
certain occasions.

fixes #3157

Signed-off-by: nicbaz <[email protected]>

* filesystem: use faster integer to string implementation

Signed-off-by: nicbaz <[email protected]>

---------

Signed-off-by: nicbaz <[email protected]>

Author: nicbaz

collector/filesystem_common.go

@@ -82,7 +82,7 @@ type filesystemCollector struct {
 }
 
 type filesystemLabels struct {
-	device, mountPoint, fsType, options, deviceError, major, minor string
+	device, mountPoint, fsType, mountOptions, superOptions, deviceError, major, minor string
 }
 
 type filesystemStats struct {

collector/filesystem_linux.go

@@ -23,6 +23,8 @@ import (
 	"io"
 	"log/slog"
 	"os"
+	"slices"
+	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -110,11 +112,8 @@ func (c *filesystemCollector) GetStats() ([]filesystemStats, error) {
 
 func (c *filesystemCollector) processStat(labels filesystemLabels) filesystemStats {
 	var ro float64
-	for _, option := range strings.Split(labels.options, ",") {
-		if option == "ro" {
-			ro = 1
-			break
-		}
+	if isFilesystemReadOnly(labels) {
+		ro = 1
 	}
 
 	success := make(chan struct{})
@@ -198,7 +197,7 @@ func parseFilesystemLabels(r io.Reader) ([]filesystemLabels, error) {
 	for scanner.Scan() {
 		parts := strings.Fields(scanner.Text())
 
-		if len(parts) < 10 {
+		if len(parts) < 11 {
 			return nil, fmt.Errorf("malformed mount point information: %q", scanner.Text())
 		}
 
@@ -219,15 +218,26 @@ func parseFilesystemLabels(r io.Reader) ([]filesystemLabels, error) {
 		parts[4] = strings.ReplaceAll(parts[4], "\\011", "\t")
 
 		filesystems = append(filesystems, filesystemLabels{
-			device:      parts[m+3],
-			mountPoint:  rootfsStripPrefix(parts[4]),
-			fsType:      parts[m+2],
-			options:     parts[5],
-			major:       fmt.Sprint(major),
-			minor:       fmt.Sprint(minor),
-			deviceError: "",
+			device:       parts[m+3],
+			mountPoint:   rootfsStripPrefix(parts[4]),
+			fsType:       parts[m+2],
+			mountOptions: parts[5],
+			superOptions: parts[10],
+			major:        strconv.Itoa(major),
+			minor:        strconv.Itoa(minor),
+			deviceError:  "",
 		})
 	}
 
 	return filesystems, scanner.Err()
 }
+
+// see https://github.com/prometheus/node_exporter/issues/3157#issuecomment-2422761187
+// if either mount or super options contain "ro" the filesystem is read-only
+func isFilesystemReadOnly(labels filesystemLabels) bool {
+	if slices.Contains(strings.Split(labels.mountOptions, ","), "ro") || slices.Contains(strings.Split(labels.superOptions, ","), "ro") {
+		return true
+	}
+
+	return false
+}

collector/filesystem_linux_test.go

@@ -45,6 +45,45 @@ func Test_parseFilesystemLabelsError(t *testing.T) {
 	}
 }
 
+func Test_isFilesystemReadOnly(t *testing.T) {
+	tests := map[string]struct {
+		labels   filesystemLabels
+		expected bool
+	}{
+		"/media/volume1": {
+			labels: filesystemLabels{
+				mountOptions: "rw,nosuid,nodev,noexec,relatime",
+				superOptions: "rw,devices",
+			},
+			expected: false,
+		},
+		"/media/volume2": {
+			labels: filesystemLabels{
+				mountOptions: "ro,relatime",
+				superOptions: "rw,fd=22,pgrp=1,timeout=300,minproto=5,maxproto=5,direct",
+			}, expected: true,
+		},
+		"/media/volume3": {
+			labels: filesystemLabels{
+				mountOptions: "rw,user_id=1000,group_id=1000",
+				superOptions: "ro",
+			}, expected: true,
+		},
+		"/media/volume4": {
+			labels: filesystemLabels{
+				mountOptions: "ro,nosuid,noexec",
+				superOptions: "ro,nodev",
+			}, expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		if got := isFilesystemReadOnly(tt.labels); got != tt.expected {
+			t.Errorf("Expected %t, got %t", tt.expected, got)
+		}
+	}
+}
+
 func TestMountPointDetails(t *testing.T) {
 	if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", "./fixtures/proc"}); err != nil {
 		t.Fatal(err)