system.shutdown unusable

[boltronics]

The system.shutdown execution module looks unusable in its current form on probably any GNU/Linux distribution.

root@master:~# salt '*' system.shutdown
minion01.example.com:
    shutdown: time expected
    Try `shutdown --help' for more information.

Looking at salt/modules/system.py, the 'shutdown' command is called without any provision for user-specified parameters. It doesn't have '-h now' hard-coded either.

Happy to supply a patch (which would likely be trivial), but I'm not sure which approach was intended. I see we have support for 'halt' (which is generally effectively the same as "shutdown -h now", so I guess supporting parameters is the way to go.

However, if adding parameters for shutdown, why not add them to halt, reboot, etc as well? Having them only for shutdown would seem inconsistent. Then again, if you're accepting parameters, is there actually any advantage to calling:

salt 'some-instance' system.shutdown -h now

over

salt 'some-instance' cmd.run shutdown -h now

(other than having to type one character less)? Maybe it can all disappear?

In short, it doesn't work currently, but I don't know how you would like to see it fixed. :)

[cachedout]

Thanks for the excellent write-up here, @boltronics.

I agree that the current implementation of system.shutdown is completely broken. What do you think about just modifying the function to accept a time argument and, if present, pass it along the the shutdown command itself? If not present, just shutdown immediately.

Send along a pull req with that, or if you have a different approach I'm sure we'd love to see it as well.

Cheers!

[boltronics]

That sounds like a very reasonable approach. If time permits, I'll try to add a pull request later today. Cheers.

[cachedout]

@boltronics That's wonderful to hear. Thanks so much for helping out!

[boltronics]

Whops! Sorry cachedout. I didn't get around to this immediately, and ended up completely forgetting about it. Thanks for the fix.

[cachedout]

@boltronics Don't worry about it at all. Thanks for reporting this issue and helping to make Salt better!