zfs module broken in 2018.3.0

[DeadlyMercury]

My salt minion failed after upgrade to 2018:

# salt-call saltutil.sync_all
[ERROR   ] Command '/sbin/zpool list -H -p -o name,size' failed with return code: 2
[ERROR   ] output: invalid option 'p'
usage:
        list [-gHLPv] [-o property[,...]] [-T d|u] [pool] ... [interval [count]]
...

[CRITICAL] Failed to load grains defined in grain file zfs.zfs in function <function zfs at 0x7fa3241bf2a8>, error:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 756, in grains
    ret = funcs[key]()
  File "/usr/lib/python2.7/dist-packages/salt/grains/zfs.py", line 116, in zfs
    grains = salt.utils.dictupdate.update(grains, _zfs_pool_data(), merge_lists=True)
  File "/usr/lib/python2.7/dist-packages/salt/grains/zfs.py", line 102, in _zfs_pool_data
    grains['zpool'][zpool[0]] = _conform_value(zpool[1], True)
IndexError: list index out of range
...

Problem is in: /usr/lib/python2.7/dist-packages/salt/grains/zfs.py

    98      for zpool in __salt__['cmd.run']('{zpool} list -H -p -o name,size'.format(zpool=zpool_cmd)).splitlines():

Must be -P, not -p

Also problems with lxd-formula (but I didn't dig it):

[ERROR   ] Failed to import module lxd, this is due most likely to a syntax error:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1459, in _load_module
    mod = imp.load_module(mod_namespace, fn_, fpath, desc)
  File "/var/cache/salt/minion/extmods/modules/lxd.py", line 114, in <module>
    @salt.utils.decorators.which('lxd')
AttributeError: 'module' object has no attribute 'which'

[garethgreenaway]

@DeadlyMercury Thanks for the report.

The zpool issue appears to be related to options potentially changing in the zpool binary. @sjorge Thoughts on this one?

The lxd formula issue seems to be using a custom module that might need to be updated for some changes around utils that went into 2018.3.

[sjorge]

The entire zpool/zfs stuff got a big update that just missed the boot for 2018.3.

What version of zfs/platform is this running on?

This is working fine on recent illumos/freebsd/macosx for me.

[root@carbon ~]# /sbin/zpool list -H -p -o name,size
archive 47828755808256
data    1992864825344
zones   238370684928

A few issues were fixed between 2017.x and 2018.x were all to due with not using parsable values so -p got added everywhere. So if version does not support this, this will be a problem.

     zpool list [-Hpv] [-o property[,property]...] [-T u|d] [pool]...
     <snip>
             -p      Display numbers in parsable (exact) values.

What is odd that his output seems to indicate there is a -P capital one. Hopefully they are are the same and we can detect when to use which one.

Edit: So OS and exact 'zfs' would be needed. (zfsonlinux + version, zfsonosx + version, freebsd? solaris? netbsd?)

[sjorge]

Already in bed so cannot edit the previous one... the output of:

zpool list -H -p -o name,size
zpool list -H -P -o name,size
zpool list -H -o name,size

Would also be handy to have.

[tonthon]

In case anybody needs a wuick fix to get thing work again.

Add a condition in the salt/grains/zfs.py code (line 102), in my case in /usr/lib/python2.7/dist-packages/salt/grains/zfs.py

Changing

       zpool = zpool.split()  
       grains['zpool'][zpool[0]] = _conform_value(zpool[1], True)

to

       zpool = zpool.split()  
        if len(zpool) > 1:                                                      
            grains['zpool'][zpool[0]] = _conform_value(zpool[1], True)

Be carrefull, use spaces not tabs for indentation.

[sjorge]

@tonthon the modules modules themselves will still have the problem though, most things are now also called with -p but would fix the grains. Although it might be easier to just comment the line instead. (less change of getting the indent wrong)

[DeadlyMercury]

Ok, versions: OS - Ubuntu 16.04 (latest LTS) zfsutils - 0.6.5.6-0ubuntu19

root@bl-lxc02:~# zpool list -H -p -o name,size
invalid option 'p'
root@bl-lxc02:~# zpool list -H -P -o name,size
lxc     796G
root@bl-lxc02:~# zpool list -H -o name,size
lxc     796G

[sjorge]

Interesting, so we cannot use -P as this does something else...

This is not going to be easy to fix, as we really want 'parsable' values for sizes not 1G, 59M, ... let me thing about this for a bit.

[DeadlyMercury]

Yeah, that is not what you want (I thought first that there is just "missspell", but problem is really deeper :) )

      -P    Display full paths for vdevs instead of only the last component of the path.  This can be used in conjunction with the -L flag.

And there is no any keys for size in bytes in zpool man page.

[sjorge]

I just checked on a 16.04 ubuntu and it is indeed not present. I have some vague ideas on how to fix it in the new code that is in develop branch, once I find a good way to work around that limitation. Hopefully I can find something to backport to 2018.3 also.

This seems to be contained to 16.04 because newer zfsonlinux does seem to have it: https://github.com/zfsonlinux/zfs/blob/master/cmd/zpool/zpool_main.c#L5096

[myii]

Sorry for reposting this here but the workaround may be useful for other above. Originally commented on the linked PR.

---

@sjorge Some more good/bad news for you.

I've tested a change on my 2018.3 master and for my use case, the following is working.

+++ /usr/lib/python2.7/dist-packages/salt/grains/zfs.py
@@ -95,7 +95,7 @@

     # collect zpool data
     zpool_cmd = salt.utils.path.which('zpool')
-    for zpool in __salt__['cmd.run']('{zpool} list -H -p -o name,size'.format(zpool=zpool_cmd)).splitlines():
+    for zpool in __salt__['cmd.run']('{zpool} list -H -o name,size'.format(zpool=zpool_cmd)).splitlines():
         if 'zpool' not in grains:
             grains['zpool'] = {}
         zpool = zpool.split()

• That is: https://github.com/saltstack/salt/blob/2018.3/salt/grains/zfs.py#L98

[rallytime]

This should be fixed with #47186

[rallytime]

Closed via #47186