Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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 <932244+nicbaz@users.noreply.github.com>
  • Loading branch information
nicbaz committed Aug 27, 2025
commit 04595cd4e4701deaab81045e9e7458440f433460
2 changes: 1 addition & 1 deletion collector/filesystem_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
35 changes: 22 additions & 13 deletions collector/filesystem_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"io"
"log/slog"
"os"
"slices"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -110,11 +111,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{})
Expand Down Expand Up @@ -198,7 +196,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())
}

Expand All @@ -219,15 +217,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: fmt.Sprint(major),
minor: fmt.Sprint(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
}
39 changes: 39 additions & 0 deletions collector/filesystem_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down