Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 1689235 - Gluster mounts are displayed as NFS Mounts in cockpit-storaged UI
Summary: Gluster mounts are displayed as NFS Mounts in cockpit-storaged UI
Keywords:
Status: POST
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: cockpit
Version: 7.6
Hardware: All
OS: Linux
medium
medium
Target Milestone: rc
: 7.7
Assignee: Martin Pitt
QA Contact: Release Test Team
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2019-03-15 13:34 UTC by Renaud Métrich
Modified: 2019-04-04 10:28 UTC (History)
0 users

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Target Upstream Version:


Attachments (Terms of Use)

Description Renaud Métrich 2019-03-15 13:34:06 UTC
Description of problem:

A customer reports that his Gluster mounts are seen as NFS Mounts in cockpit.

Code analysis in "Additional info".


Version-Release number of selected component (if applicable):




How reproducible:

Always, but didn't try

Steps to Reproduce:

1. Mount gluster file systems

Actual results:

Mounts seen as NFS Mounts.


Expected results:

No mount seen there.


Additional info:

From my understanding of the cockpit-storaged code, we have a python script executed on /etc/fstab and /proc/self/mounts to find out the various mounts:

-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
$ gunzip -c /usr/share/cockpit/storaged/storage.min.js.gz | grep "# nfs-mounts"
    e.exports = '#! /usr/bin/python\n\n# nfs-mounts   --  Monitor and manage NFS mounts\n#\n# [...]
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

This "nfs-mounts.py" script is embedded in the javascript code at compilation time:

pkg/storaged/client.js:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
    var nfs_mounts_py = require("raw!./nfs-mounts.py");
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

Looking at the source code of the "nfs-mounts.py" script, I can see this:

It parses /etc/fstab and /proc/self/mounts:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
 99 def process_fstab():
100     global fstab, fstab_by_remote
101     fstab = parse_tab("/etc/fstab")
102     fstab_by_remote = index_tab(fstab)
103 
104 def process_mtab():
105     global mtab, mtab_by_remote
106     mtab = parse_tab("/proc/self/mounts")
107     mtab_by_remote = index_tab(mtab)
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

The parse_tab() method is shown below:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
 42 def parse_tab(name):
 43     entries = [ ]
 44     for line in open(name, "r").read().split("\n"):
 45         sline = line.strip()
 46         if sline == "" or sline[0] == "#":
 47             continue
 48         fields = list(map(field_unescape, re.split("[ \t]+", sline)))
 49         if len(fields) > 0 and ":" in fields[0]:
 50             entries.append(fields)
 51     return entries
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

This method does read the file, splitted by line, then does this:
- skips empty and commented out lines (line 46-47)
- splits the fields by space (line 48)
- and if there is a ":" in field 0, it's a match! (line 49-50)

So basically we can see that field 3 (FS type) is *not* checked for NFS, which *is* a bug, since Gluster filesystems appears as shown below in /proc/self/mounts:

-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
MY.SERVER.NAME:/MNTPOINT /path/to/MY.SERVER.NAME:_STORE fuse.glusterfs rw,relatime,user_id=0,group_id=0,default_permissions,allow_other,max_read=131072 0 0
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

The proper code should look like this instead (line 49 amended):

-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
 42 def parse_tab(name):
 43     entries = [ ]
 44     for line in open(name, "r").read().split("\n"):
 45         sline = line.strip()
 46         if sline == "" or sline[0] == "#":
 47             continue
 48         fields = list(map(field_unescape, re.split("[ \t]+", sline)))
 49         if len(fields) > 0 and ":" in fields[0] and fields[2] == "nfs":
 50             entries.append(fields)
 51     return entries
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------

Comment 2 Renaud Métrich 2019-03-15 13:34:53 UTC
Oups, seen in version cockpit-storaged-176-4.el7.noarch

Comment 4 Martin Pitt 2019-03-19 13:18:29 UTC
Thank you! This patch makes sense, I'll send it upstream ASAP.

Comment 5 Martin Pitt 2019-03-19 14:59:32 UTC
One note: we better check for fields[2].startswith("nfs"), to also cover "nfs4", and possible other variations (https://wiki.linux-nfs.org/wiki/index.php/Nfsv4_configuration and https://help.ubuntu.com/community/NFSv4Howto)

Comment 6 Renaud Métrich 2019-03-19 15:09:45 UTC
+1
Indeed

Comment 7 Martin Pitt 2019-03-19 15:27:30 UTC
I created some tests and posted the fix: https://github.com/cockpit-project/cockpit/pull/11428

Comment 8 Martin Pitt 2019-03-25 06:19:45 UTC
Fix is part of release 190.


Note You need to log in before you can comment on or make changes to this bug.