saltstack / salt

Software to automate the management and configuration of any infrastructure or application at scale. Get access to the Salt software package repository here:
https://repo.saltproject.io/
Apache License 2.0
14.16k stars 5.48k forks source link

salt.module.zpool and salt.module.zfs could use some love. #28779

Closed sjorge closed 8 years ago

sjorge commented 8 years ago

Looks like the output is just a simple dump of the text output of the commands, e.g.:

/opt/local/salt/bin/salt-call --local zpool.list

local:
    ----------
    pools:
        - NAME      SIZE  ALLOC   FREE  EXPANDSZ   FRAG    CAP  DEDUP  HEALTH  ALTROOT
        - archive  21.8T  10.5T  11.2T         -      -    48%  1.00x  ONLINE  -
        - data     1.81T   546G  1.28T         -    13%    29%  1.00x  ONLINE  -
        - zones     111G  40.8G  70.2G         -    53%    36%  1.00x  ONLINE  -

/opt/local/salt/bin/salt-call --local zpool.status zones

local:
    -   pool: zones
    -  state: ONLINE
    -   scan: scrub repaired 0 in 0h4m with 0 errors on Mon Nov  9 02:04:09 2015
    - config:
    -     NAME        STATE     READ WRITE CKSUM
    -     zones       ONLINE       0     0     0
    -       mirror-0  ONLINE       0     0     0
    -         c2t0d0  ONLINE       0     0     0
    -         c2t1d0  ONLINE       0     0     0
    - errors: No known data errors

/opt/local/salt/bin/salt-call --local zpool.status does not work:

local:
    - cannot open '': name must begin with a letter

While it is perfectly fine to request info for all zpools!

[root@core ~]# zpool status
  pool: zones
 state: ONLINE
status: Some supported features are not enabled on the pool. The pool can
        still be used, but some features are unavailable.
action: Enable all features using 'zpool upgrade'. Once this is done,
        the pool may no longer be accessible by software that does not support
        the features. See zpool-features(5) for details.
  scan: scrub repaired 0 in 0h4m with 0 errors on Mon Nov  9 02:04:09 2015
config:

        NAME        STATE     READ WRITE CKSUM
        zones       ONLINE       0     0     0
          mirror-0  ONLINE       0     0     0
            c2t0d0  ONLINE       0     0     0
            c2t1d0  ONLINE       0     0     0

errors: No known data errors

Looking at the code the seems looks to be true for salt.modules.zfs but it is not working on SmartOS at the moment, although the zfs command is available.

[root@core ~]# /opt/local/salt/bin/salt-call --local zfs.list
[INFO    ] Executing command 'pkg_info -Q LOCALBASE pkgin' in directory '/root'
'zfs' __virtual__ returned False
sjorge commented 8 years ago

I'm happy to go over them and fix these up, although I am not sure how to do this and retain backwards compatibility with the current straight dumped output?

Any suggestions? Seems you can set a 'provider' in the minion config.

Maybe rename zfs to zfs_legacy and implement an new clean zfs/zpool module. People who really depend on the old one can enable it again.

There are also some simple additions that can be added (which I was looking for when I found this) e.g.

Stuff like above is often done from cron to send mails on failty pools, this can then be used with a scheduler and possibly reactor to automate parts of this work flow

/usr/sbin/zpool status -x | grep -v 'healthy' # asmd-cron-job [detect_zpool_health]

jfindlay commented 8 years ago

@sjorge, thanks for looking into this. I am unsure what you mean by creating a provider for zfs. If the zfs/zpool code needs an update, you are very welcome to deprecate functions and arguments or even whole modules and rewrite things in a better layout.

sjorge commented 8 years ago

zpool done, up next zfs