goss icon indicating copy to clipboard operation
goss copied to clipboard

Check for mount options doesn't work

Open XenGi opened this issue 6 years ago • 4 comments

I'm trying to check my mount points with goss. It reports an error about missing mount options but the options are there.

$ cat mounts.yml
mount:
  /zpool/media/download:
    exists: true
    opts:
      - rw
      - relatime
      - posixacl
    source: zpool/media/download
    filesystem: zfs
  /home:
    exists: true
    opts:
      - rw
      - noatime
      - compress=zstd
      - ssd
      - discard
      - subvol=@home
    source: /dev/mapper/cryptroot
    filesystem: btrfs
$ goss -g mounts.yml validate -f tap
1..8
ok 1 - Mount: /home: exists: matches expectation: [true]
not ok 2 - Mount: /home: opts: doesn't match, expect: [["rw","noatime","compress=zstd","ssd","discard","subvol=@home"]] found: [["rw","noatime"]]
ok 3 - Mount: /home: source: matches expectation: ["/dev/mapper/cryptroot"]
ok 4 - Mount: /home: filesystem: matches expectation: ["btrfs"]
ok 5 - Mount: /zpool/media/download: exists: matches expectation: [true]
not ok 6 - Mount: /zpool/media/download: opts: doesn't match, expect: [["rw","relatime","posixacl"]] found: [["rw","relatime"]]
ok 7 - Mount: /zpool/media/download: source: matches expectation: ["zpool/media/download"]
ok 8 - Mount: /zpool/media/download: filesystem: matches expectation: ["zfs"]
$ goss -v
goss version v0.3.7

Any idea what I'm doing wrong?

XenGi avatar May 07 '19 19:05 XenGi

I have the same issue. It seems that that check only gets the mount options(6) of /proc/self/mountpoints. You want to check the super block mount points(11) that do not seem to be managed by the goss check.

https://github.com/aelsabbahy/goss/blob/master/system/mount.go https://github.com/moby/moby/blob/master/pkg/mount/mountinfo_linux.go

lechuk47 avatar Sep 30 '19 16:09 lechuk47

These are all the ways I now to get mounts and they are all consistent:

$ mount | grep download
zpool/media/download on /zpool/media/download type zfs (rw,relatime,xattr,posixacl)
$ cat /proc/self/mounts | grep download
zpool/media/download /zpool/media/download zfs rw,relatime,xattr,posixacl 0 0
$ cat /proc/self/mountinfo | grep download
156 135 0:64 / /zpool/media/download rw,relatime shared:89 - zfs zpool/media/download rw,xattr,posixacl
$ cat /proc/self/mountstats  | grep download
device zpool/media/download mounted on /zpool/media/download with fstype zfs

The entry is not in the /etc/fstab because these are zfs mountpoints handled by the zfs mount target.

There is no /proc/self/mountpoints in arch linux. Is this a default path or distribution specific?

Here's an example with /home:

$ cat /etc/fstab | grep home
UUID=f26bce25-38a7-4330-9888-4e89d19d20fd	/home     	btrfs     	rw,noatime,compress=zstd,ssd,discard,space_cache,subvolid=258,subvol=/@home,subvol=@home	0 0
$ mount | grep home
/dev/mapper/cryptroot on /home type btrfs (rw,noatime,compress=zstd:3,ssd,discard,space_cache,subvolid=258,subvol=/@home)
$ cat /proc/self/mounts | grep home
/dev/mapper/cryptroot /home btrfs rw,noatime,compress=zstd:3,ssd,discard,space_cache,subvolid=258,subvol=/@home 0 0
$ cat /proc/self/mountinfo | grep home
115 31 0:28 /@home /home rw,noatime shared:61 - btrfs /dev/mapper/cryptroot rw,compress=zstd:3,ssd,discard,space_cache,subvolid=258,subvol=/@home
$ cat /proc/self/mountstats  | grep home
device /dev/mapper/cryptroot mounted on /home with fstype btrfs

This is also consistent.

I just followed the code to https://github.com/moby/moby/blob/092cba3727bb9b4a2f0e922cd6c0f93ea270e363/pkg/mount/mountinfo_linux.go#L33. This is where goss gets the mount infos and it seems that this library also just reads the contents of /proc/self/mountinfo. So it "should" just work.

XenGi avatar Nov 19 '19 10:11 XenGi

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 09 '20 19:07 stale[bot]

I'm interested in proposed solutions for this one.

The mount file looks like this:

		   See http://man7.org/linux/man-pages/man5/proc.5.html
		   36 35 98:0 /mnt1 /mnt2 rw,noatime master:1 - ext3 /dev/root rw,errors=continue
		   (1)(2)(3)   (4)   (5)      (6)      (7)   (8) (9)   (10)         (11)
		   (1) mount ID:  unique identifier of the mount (may be reused after umount)
		   (2) parent ID:  ID of parent (or of self for the top of the mount tree)
		   (3) major:minor:  value of st_dev for files on filesystem
		   (4) root:  root of the mount within the filesystem
		   (5) mount point:  mount point relative to the process's root
		   (6) mount options:  per mount options
		   (7) optional fields:  zero or more fields of the form "tag[:value]"
		   (8) separator:  marks the end of the optional fields
		   (9) filesystem type:  name of filesystem of the form "type[.subtype]"
		   (10) mount source:  filesystem specific information or "none"
		   (11) super options:  per super block options

Should goss combine mount options with super options or should it be a separate test attribute?

aelsabbahy avatar Jul 09 '20 20:07 aelsabbahy

Hi there, I stumbled across this issue, and I think there might still be an issue. I tried to use the new goss beta, to validate whether some loop filesystems that were remounted as read only, would cause goss to fail with the below mountinfo details

138 132 7:3 / /var/log rw,relatime shared:77 - ext4 /dev/loop3 ro,discard 141 138 7:4 / /var/log/audit rw,relatime shared:79 - ext4 /dev/loop4 ro,discard

But it seems that as goss combines the mount options and super options, checking for "rw" passes, as they are in the original "mount options", even though the filesystem is actually ro.

Perhaps having separate matchers for super options vs mount options would be better? Or perhaps also having negative matchers (string does not contain ro for example)?

rorylshanks avatar Jul 23 '23 10:07 rorylshanks

Awesome feedback, super appreciate this as it allows me to catch the issue before the full version is released.

I was on the fence on this, but your comment confirms it. These two should be separate tests:

https://github.com/goss-org/goss/blob/54a77a9eb9452d1547234f3097f4e4103eb3385d/system/mount.go#L81

m.mountInfo.Options vs m.mountInfo.VFSOptions.

There are ways to do negative checks, if you send me a sample goss.yaml, I can help with that.

Any thoughts on naming: super-opts or vfs-opts? https://pkg.go.dev/github.com/moby/sys/mountinfo#Info

Again really really appreciate the fast real world results and use-case.

aelsabbahy avatar Jul 23 '23 17:07 aelsabbahy

Hey no problem! Thanks for developing this awesome tool :)

I think vfs-opts is a bit clearer, as the word "super" can be a bit ambiguous. It might also be a good idea to add a short note to the docs too to explain the difference.

rorylshanks avatar Jul 23 '23 17:07 rorylshanks

Quick update on the exclusion, something like this may work:

mount:
  /:
    exists: true
    opts:
      not:
        contain-element: rw

Example output:

$ goss v
.F

Failures/Skipped:

Mount: /: opts:
Expected
    ["rw","relatime","seclabel","attr2","inode64","noquota"]
not to contain element matching
    "rw"

Total Duration: 0.000s
Count: 2, Failed: 1, Skipped: 0

Also, I've created a PR that fixes this in #826. I'll make this part of rc2

aelsabbahy avatar Jul 31 '23 22:07 aelsabbahy

re-opening to wait for verification

aelsabbahy avatar Jul 31 '23 22:07 aelsabbahy

Just released v0.4.0-rc.2

Would love your feedback on this issue (or any others related to that release).

aelsabbahy avatar Aug 07 '23 16:08 aelsabbahy

Hi there, I just checked the new vfs-ops option in my environment and it works as expected! Now filesystems that are remounted as ro for whatever reason are correctly managed as "ro"!

Thanks again, I think this issue can now be closed.

rorylshanks avatar Aug 08 '23 09:08 rorylshanks

Thank you for reporting the issue and validating the fix. Much appreciated!

aelsabbahy avatar Aug 08 '23 13:08 aelsabbahy