Salt: Clarification Use of Shell Command Pipes in Python

[damon-atkins]

Description of Issue/Question

In puppet facts collection, I have seen people do something like "command | cut | grep | sed" instead of ruby, because the person are new at ruby and do not have the skills.

I am relative new to python compare to other languages I have experience with. For example it would be far less time consuming to "command | awk script" than to write python code.

My personal preference is to aim for 100% python code.

Where does SaltStack draw the line, between effectively embedded "shell pipes" vs writing code in python?

Is their a need for python function to help with this sort of thing (see examples)?

Some existing examples.

./salt/grains/core.py:    grains['zoneid'] = __salt__['cmd.run']('zoneadm list -p | awk -F: \'{ print $1 }\'', 
./salt/utils/cloud.py:        cmd = 'echo "put {0} {1} {2}" | sftp {3} {4[username]}@{5}'.format(
./salt/states/archive.py:                        cmd = 'xz --decompress --stdout {0} | tar xvf -'
./salt/modules/mdata.py:        cmd = 'echo {2} | {0} {1}'.format(mdata, keyname, val)
./salt/modules/monit.py:        cmd = 'echo y|monit -r'
./salt/modules/pecl.py:        cmdline = 'yes ' "''" + ' | ' + cmdline
./salt/modules/salt_proxy.py:    cmd = ('ps ax | grep "salt-proxy --proxyid={0}" | grep -v grep'
./salt/modules/mac_keychain.py:    cmd = 'security find-certificate -a {0} | grep -o "alis".*\\" | ' \
./salt/modules/mac_keychain.py:    cmd = 'openssl pkcs12 -in {0} -passin pass:{1} -info -nodes -nokeys 2> /dev/null | ' \
./salt/modules/mac_system.py:        cmd = 'echo \'{0}\' | at {1}'.format(cmd, _cmd_quote(at_time))
./salt/modules/bluez.py:    cmd = 'echo {0} | bluez-simple-agent {1} {2}'.format(
./salt/modules/smartos_vmadm.py:    cmd = 'echo {image} | {vmadm} reprovision {uuid}'.format(
./salt/modules/network.py:        cmd = 'ps -a -o pid,ppid | tail -n+2'
./salt/modules/network.py:        cmd = 'ps -ax -o pid,ppid | tail -n+2'
./salt/modules/network.py:            cmd = 'netstat -f {0} -an | tail -n+3'.format(addr_family)
./salt/modules/network.py:        cmd = 'netstat -p tcp -an | tail -n+3'
./salt/modules/network.py:        cmd = 'netstat -p udp -an | tail -n+3'
./salt/modules/network.py:        cmd = 'netstat -f {0} -P tcp -an | tail -n+5'.format(addr_family)
./salt/modules/network.py:        cmd = 'netstat -f {0} -P udp -an | tail -n+5'.format(addr_family)
./salt/modules/network.py:    cmd = 'netstat -A inet -rn | tail -n+3'
./salt/modules/network.py:    cmd = 'netstat -A inet6 -rn | tail -n+3'
./salt/modules/network.py:    cmd = 'netstat -f inet -rn | tail -n+5'
./salt/modules/network.py:    cmd = 'netstat -f inet6 -rn | tail -n+5'
./salt/modules/network.py:    cmd = 'netstat -f inet -rn | tail -n+5'
./salt/modules/network.py:    cmd = 'netstat -f inet6 -rn | tail -n+5'
./salt/modules/network.py:    cmd = 'netstat -f inet -rn | tail -n+5'
./salt/modules/network.py:    cmd = 'netstat -f inet6 -rn | tail -n+5'
./salt/modules/network.py:    cmd = 'netstat -f inet -rn | tail -n+5'
./salt/modules/network.py:    cmd = 'netstat -f inet6 -rn | tail -n+5'
./salt/modules/keyboard.py:        cmd = 'localectl | grep Keymap | sed -e"s/: /=/" -e"s/^[ \t]*//"'
./salt/modules/keyboard.py:        cmd = 'grep LAYOUT /etc/sysconfig/keyboard | grep -vE "^#"'
./salt/modules/keyboard.py:        cmd = 'grep XKBLAYOUT /etc/default/keyboard | grep -vE "^#"'
./salt/modules/keyboard.py:        cmd = 'grep "^keymap" /etc/conf.d/keymaps | grep -vE "^#"'
./salt/modules/keyboard.py:    cmd = 'setxkbmap -query | grep layout'

[isbm]

I see nothing wrong with this, unless it is happening on Windows. In that case your argumentation would make sense because Windows is by definition slow on pipes. However, design of the Unix (and Unix-like) operating systems is exactly this: do less for more. Another argument why this is actually a better way then just grinding up more custom ad-hoc code, because for certain task you need certain requirements with certain tools around. Almost all of them are in the base package and comes with minimal OS installation. In some cases they aren't installed on minimal OS, but such corner cases are more like a rare exception with a bad luck, rather then a common way to do. If some tools are required, it is always better to set a package dependency (e.g. RPM packages) and install it together with Salt. Then nobody will miss anything and all the ecosystem will be equally supported. And those operating systems that do not use package managers and use custom minimal installations, are at critical minority, far less than 80/20 rule of their existence, so I would suggest not really care about them very much — bad luck (and probably a bad idea) and that's it.

That said, I would certainly vote against of re-implementing already existing tools in the operating system itself. Because adding more task-unrelated code as generally into utils package is to make Salt codebase bigger, introducing more possible bugs and duplicate what is already done and tested for decades.

However, if a certain tool requires too much resources for too little result, using common sense one need to see if it is _maybe_ a good idea to implement only a _required part_ of the functionality as an utility helper.

P.S. My last Windows I really used was Windows NT 4.0 in late 90'ies, so I am not very up to date how it looks like nowadays though. :laughing:

[damon-atkins]

Out of 1400+ cmd within the Salt Code (excluding unit tests) there are only about 100 times in which pipes are used in commands. (18 of which are in modules/network.py and 5 in modules/keyboard.py)

egrep -R  --include=\*.py "\| ?head|\| ?tail|\| ?awk|\| ?cut|\| ?grep|echo |\| ?tar|\| ?sed|\| ?perl|\| ?php\| ?cpio|'cat " . | fgrep -v "salt '*'" | wc -l

egrep -R  --include=\*.py "__salt__\['cmd.run|__salt__\['cmd.retcode|__salt__\['cmd.shell" . | wc -l

[isbm]

Sure. You don't _have_ to do that. But you don't _need_ to restrict that either. :smile:

[gtmanfred]

Personally, I only use the cmd.run or equivalent module when there is no python module that seems relatively easier than just doing it in the shell. Pipe aside, if someone else is going to maintain the shell commands or C libraries to be used in my python script, i would rather let them do it and have to maintain less of the extra stuff on my own end.

We also regularly use stuff like the GitPython module which does the shell out for you. https://github.com/gitpython-developers/GitPython/blob/master/git/cmd.py#L567

[damon-atkins]

Hi @gtmanfred
I open this up, as Python should be able to execute a command and take that input directly and process it with regex match _or something_ vs running a command and the using awk/sed/grep/tail/head to process the data. If you use a pipe the exit code is the last program in the pipe.

e.g. proc.stdout contains the the results from running a command

pattern_result = re.search(pattern, proc.stdout, flags=re.IGNORECASE)
if pattern_result:
    ret['version'] = pattern_result.group(1)

vs the not ideal use of shell pipes below

cmd | awk '/pattern/ { print $1}'

comment edited based on feedback

[isbm]

Some people, when confronted with a problem, think “I know, I'll use regular expressions.” Now they have two problems. — Jamie Zawinski (https://www.jwz.org/)

Keep it simple, stupid: http://gregblogs.com/why-you-should-try-to-avoid-regex-in-your-python/

BTW, your example is misleading, falsely giving an impression that import re is no longer a good idea and one should do everything now via awk. This is not true. What you're actually asking is:

a_classic_unix_idiom = __salt__['cmd.shell'](
    "unix-command | another-unix-command | awk-format")

versus:

a_result = __salt__['cmd.run']("unix-command")
another_result = _my_ad_hoc_reimplementation_of_what_unix_does_every_day(a_result)
final_result = _another_ad_hoc_stuff_that_is_also_outside_of_utilities(another_result)

Surely, this is something different from:

import re
my_specific_data = re.search(pattern, __salt__['some.module'](foobar)).group(1)

The bottom line is, if underlying system can return required data without an additional post-processing and with a minimal effort, then it probably should do that. If there is too much complexity to get it right away, probably Salt should do it. E.g. reimplementation of salt.utils.which makes perfect sense, since it does some other things that /usr/bin/which doesn't.

P.S. I guess salt.modules.cmdmod is designed exactly for this, right? :smile:

[stale[bot]]

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.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.