[continue 912] strip rootfs prefix for run in docker

[cofyc]

Because @diafour has not responded for months, and I hope we can support it officially, so I continue his work in #912.

Summary of changes:

• Rebase his commit on latest master.
• Update docs in README to remove volume mounts: 1cbcea3

cc @SuperQ @discordianfish

[grobie]

Could we add some tests this behavior?

[SuperQ]

@cofyc Thanks for picking this up. I'm thinking about this, and trying to decide if we should default the flag to / and handle the path manipulation that way. What do you thinking?

[cofyc]

@grobie

Sure, I found there is a e2e framework end-to-end-test.sh, perhaps we can add some fixtures for containerized node_exporter and test it. Do you have some suggestions?

@SuperQ

Agree, by default rootfs is at /, for containerized node_exporter, it's only because we want to access host filesystem, so we need to mount host filesystem as subtree of container filesystem and prefix path.rootfs to access it.

However, I found there is a issue to get mount points from /proc/mounts. Because /proc/mounts is mount view of current process, for containerized node_exporter, it has different mount namespace with host, its /proc/mounts contains extra mount infos in /host, e.g.

$ docker run -d --net=host --pid=host -v "/:/host:ro,rslave" busybox sh -c 'sleep 99999'
fae0db84eee2f9a7e8d83083479afd605879d8541b32b3d41e6113ce0bbe5ba6
$ docker exec -it fae0db84eee2f9a7e8d83083479afd605879d8541b32b3d41e6113ce0bbe5ba6 sh -c 'cat /proc/mounts' | grep 92a84bc841d345cd81282dbb2e476b5f87c96d26994569d8d8a02f51a3c7f17e
none /host/data/var/lib/docker/aufs/mnt/92a84bc841d345cd81282dbb2e476b5f87c96d26994569d8d8a02f51a3c7f17e aufs rw,relatime,si=e19aa3432dcc5f0f,dio,dirperm1 0 0
none /host/data/var/lib/docker/aufs/mnt/92a84bc841d345cd81282dbb2e476b5f87c96d26994569d8d8a02f51a3c7f17e aufs rw,relatime,si=e19aa3432dcc5f0f,dio,dirperm1 0 0
proc /host/data/var/lib/docker/aufs/mnt/92a84bc841d345cd81282dbb2e476b5f87c96d26994569d8d8a02f51a3c7f17e/proc proc rw,nosuid,nodev,noexec,relatime 0 0
tmpfs /host/data/var/lib/docker/aufs/mnt/92a84bc841d345cd81282dbb2e476b5f87c96d26994569d8d8a02f51a3c7f17e/dev tmpfs rw,nosuid,size=65536k,mode=755 0 0
devpts /host/data/var/lib/docker/aufs/mnt/92a84bc841d345cd81282dbb2e476b5f87c96d26994569d8d8a02f51a3c7f17e/dev/pts devpts rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=666 0 0
mqueue /host/data/var/lib/docker/aufs/mnt/92a84bc841d345cd81282dbb2e476b5f87c96d26994569d8d8a02f51a3c7f17e/dev/mqueue mqueue rw,nosuid,nodev,noexec,relatime 0 0
...

These are container internal mount points we don't care.

Note that /data/var/lib/docker/aufs/mnt/92a84bc841d345cd81282dbb2e476b5f87c96d26994569d8d8a0 is rootfs of container fae0db84eee2f9a7e8d83083479afd605879d8541b32b3d41e6113ce0bbe5ba6.

I suggest to get mount points from /proc/1/mounts (we did this way in kubernetes too), for node_exporter running on host, it share same mount namespace with 1:

$ cat /proc/mounts | md5sum -
6afb7e3cd1acfc78666063eeb5ef8ead  -
$ cat /proc/1/mounts | md5sum -
6afb7e3cd1acfc78666063eeb5ef8ead  -

In container, with --pid=host, we can access same mount points in host, by access /proc/1/mounts, see:

$ docker exec -it fae0 sh -c 'cat /proc/1/mounts | md5sum -'
6afb7e3cd1acfc78666063eeb5ef8ead  -

We still need to mount host filesystem into container to access filesystem stats.

[cofyc]

hi, I pushed a new commit to use / as default value of path.rootfs, and get mounts from /proc/1/mounts. PTAL.

Here is my quick test to verify it works the same as on the host:

$ docker run -d --net="host" --pid="host" -v "/:/host:ro,rslave" prom/node-exporter:rootfsprefix --path.rootfs /host
$ ./node_exporter --web.listen-address=":9101" &
$ curl -s http://localhost:9100/metrics  | cut -d ' ' -f 1  | md5sum -
b27ec87736d2e3e79f9ce483d2cc9793  -
$ curl -s http://localhost:9101/metrics  | cut -d ' ' -f 1  | md5sum -
b27ec87736d2e3e79f9ce483d2cc9793  -

[cofyc]

Any other concerns?

[diafour]

Wow! Completely forgot about 912 and accidentally have found this new pr with google :)

@cofyc good work 👍

[SuperQ]

Sorry, this fell off my review radar.

Would you please update the CHANGELOG.md with an [ENHANCEMENT] line?

Otherwise, looks great!

[SuperQ]

Since we're moving to /proc/1/mounts, we should cleanup the old collector/fixtures/proc/mounts.

[discordianfish]

LGTM