-
Notifications
You must be signed in to change notification settings - Fork 2.6k
path.rootfs stripping in func mountPointDetails() was not merged properly #1418
Description
Host operating system
Linux ip-10-0-93-21.ec2.internal 4.14.11-coreos #1 SMP Fri Jan 5 11:00:14 UTC 2018 x86_64 GNU/Linux
node_exporter version: output of node_exporter --version
node_exporter, version 0.18.0 (branch: , revision: )
build user:
build date:
go version: go1.12.5
node_exporter command line flags
working:
ExecStart=/bin/node_exporter \
--web.listen-address :9000 \
--web.telemetry-path /metrics \
--log.level debug
pathological:
ExecStart=/bin/node_exporter \
--web.listen-address :9000 \
--web.telemetry-path /metrics \
--path.rootfs /host \
--log.level debug
Are you running node_exporter in Docker?
yes
What did you do that produced an error?
Attempt to export the node_filesystem_avail_bytes metric for a volume bind-mounted into the container via docker -v "/:/host:ro,rslave" (on the host machine, it lives at /media/MONITORDisk, so inside the container, it's at /host/media/MONITORDisk)
What did you expect to see?
The filesystem info for /host/media/MONITORDisk should be found and exported at {mountpoint="/media/MONITORDisk"} (/host being stripped, because it's the path.rootfs argument)
What did you see instead?
If path.rootfs is defined, we get the following errors in debug logs:
level=debug msg="Error on statfs() system call for \"/host/host/media/MONITORDisk\": no such file or directory" source="filesystem_linux.go:90"
If path.rootfs is undefined, we export the metric, but it doesn't have /host stripped from the mountpoint label value.
Analysis of why this is happening:
The ability to monitor bind-mounted volumes from host properly relies on removing a prefix directory they're mounted at.
In this case, we pass the parameter, --path.rootfs /host to the node_exporter binary so that /host/media/MONITORDisk can be checked and reported at /media/MONITORDisk in the labels of the resulting metric.
...but this feature was added in a PR that was closed, and then merged in another PR trying to replicate it after some discussion.
The second PR did not properly integrate the changes - so that instead of stripping /host from the metric on output, it appends it before trying to run statfs, and the binary ends up trying to run statfs on /host/host/media/MONITORDisk which of course doesn't exist.
The first (closed) PR had a function called rootfsStripPrefix which was used to strip the prefix for the mountpoint property of the filesystemLabels object which func mountPointDetails() outputs, so that when we run syscall.Statfs(rootfsFilePath(labels.mountPoint), buf), the fact that the rootfsFilePath function re-appends the stripped prefix is not a problem, as it allows the statfs() call to find the proper path on disk.
In the PR which was eventually merged instead, which was made as an imitation of this closed PR, the rootfsStripPrefix function was not included. Compare first PR and second PR (does not modify that line, which it should).
Suggested fix
Reinstate the rootfsStripPrefix function so that the code behaves as intended.
I'll be opening a PR for this shortly linking to this issue.