Repository: prometheus/node_exporter

Issue:
Metric "node_filesystem_readonly" does not include super options when calculating
<!--
	Please note: GitHub issues should only be used for feature requests and
	bug reports. For general discussions, please refer to one of the community channels
	described in https://prometheus.io/community/.

	Before filing a bug report, note that running node_exporter in Docker is
	not recommended, for the reasons detailed in the README:

	https://github.com/prometheus/node_exporter#docker

	Finally, also note that node_exporter is focused on *NIX kernels, and the
	WMI exporter should be used instead on Windows.

	For bug reports, please fill out the below fields and provide as much detail
	as possible about your issue.  For feature requests, you may omit the
	following template.
-->
### Question description

Hello, I would like to ask a question. When using node-exporter for metric collection, I have noticed an inconsistency in the behavior of the metric "node_filesystem_readonly" across different versions. In version 1.6.1, the value is obtained through /proc/1/mounts' options, while in the latest version, it is obtained through /proc/1/mountinfo. When I execute the following commands on the host with a read-only root partition:

```
cat /proc/1/mountinfo | grep /dev/md127
44 0 9:127 / / rw,relatime shared:1 - ext4 /dev/md127 ro
```

```
cat /proc/1/mounts | grep md127
/dev/md127 / ext4 ro,relatime 0 0
```

As per the code logic, in version 1.6.1, the "ro" value is 1, whereas in the latest version, it is 0 (the options value in the new version's file does not include "ro", but the super options include it). I would like to confirm whether this metric's **meaning** does not include cases where super options are set to "ro".

The PR that caused this question: https://github.com/prometheus/node_exporter/commit/b9d0932179a0c5b3a8863f3d6cdafe8584cedc8e

### Host operating system: output of `uname -a`
Linux host 4.19.90-2307.3.0.el7.v60.x86_64 #1 SMP Mon Aug 26 14:27:04 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
### node_exporter version: output of `node_exporter --version`
<!-- If building from source, run `make` first. -->
node_exporter, version 1.6.1 (branch: HEAD, revision: 4a1b77600c1873a8233f3ffb55afcedbb63b8d84)
  build user:       root@586879db11e5
  build date:       20230717-12:10:52
  go version:       go1.20.6
  platform:         linux/amd64
  tags:             netgo osusergo static_build

### node_exporter command line flags
<!-- Please list all of the command line flags -->

### node_exporter log output

### Are you running node_exporter in Docker?
<!-- Please note the warning above. -->
no

### What did you do that produced an error?
Make the root partition read-only

### What did you expect to see?
The node_filesystem_readonly{mountpoint="/"} is set to 1 when the root partition is read-only.

### What did you see instead?
The node_filesystem_readonly{mountpoint="/"} is set to 0 when the root partition is read-only.


Hints from issue comments:
@dswarbrick: In `filesystem_linux.go`, you can see that it parses the mount options from the sixth field group, i.e. `rw,relatime` in your case. It ignores the super-options completely.

Excerpt from `mount(2)` manpage:
```
       Since Linux 2.6.16, MS_RDONLY can be set or cleared on a per-
       mount-point basis as well as on the underlying filesystem
       superblock.  The mounted filesystem will be writable only if
       neither the filesystem nor the mountpoint are flagged as read-
       only.
```
So for the `node_filesystem_readonly` metric to have any real meaning, it would need to take both the per-mount options and per-superblock options into account.

Solution patch:
diff --git a/collector/filesystem_common.go b/collector/filesystem_common.go
index 4161b3fe..1e61e8e7 100644
--- a/collector/filesystem_common.go
+++ b/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 {
diff --git a/collector/filesystem_linux.go b/collector/filesystem_linux.go
index 127a3be1..e7712856 100644
--- a/collector/filesystem_linux.go
+++ b/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
+}



Test patch:
diff --git a/collector/filesystem_linux_test.go b/collector/filesystem_linux_test.go
index d088598f..7b3e22c4 100644
--- a/collector/filesystem_linux_test.go
+++ b/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)