From 144760abb4c9140e35a5f982c6238df6b362bf86 Mon Sep 17 00:00:00 2001 From: Ronald van Zantvoort Date: Tue, 22 Dec 2015 20:11:35 +0100 Subject: [PATCH 1/8] #6691 part 1; Add option to not wait/timeout in timed_subprocess.TimedProc --- salt/utils/timed_subprocess.py | 51 ++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 17 deletions(-) diff --git a/salt/utils/timed_subprocess.py b/salt/utils/timed_subprocess.py index 34ec10061c..dd6bc10051 100644 --- a/salt/utils/timed_subprocess.py +++ b/salt/utils/timed_subprocess.py @@ -14,13 +14,31 @@ class TimedProc(object): ''' def __init__(self, args, **kwargs): - self.stdin = kwargs.pop('stdin', None) - if self.stdin is not None: - # Translate a newline submitted as '\n' on the CLI to an actual - # newline character. - self.stdin = self.stdin.replace('\\n', '\n') - kwargs['stdin'] = subprocess.PIPE - self.with_communicate = kwargs.pop('with_communicate', True) + self.wait = kwargs.pop('wait', True) + + # If you're not willing to wait for the process + # you can't define any stdin, stdout or stderr + if not self.wait: + self.stdin = kwargs['stdin'] = None + else: + self.stdin = kwargs.pop('stdin', None) + if self.stdin is not None: + # Translate a newline submitted as '\n' on the CLI to an actual + # newline character. + self.stdin = self.stdin.replace('\\n', '\n') + kwargs['stdin'] = subprocess.PIPE + + self.with_communicate = kwargs.pop('with_communicate', self.wait) + if not self.with_communicate: + self.stdout = kwargs['stdout'] = None + self.stderr = kwargs['stderr'] = None + + if not self.wait or 'timeout' not in kwargs: + self.timeout = None + else: + self.timeout = kwargs.pop('timeout') + if not isinstance(self.timeout, (int, float)): + raise salt.exceptions.TimedProcTimeoutError('Error: timeout must be a number') try: self.process = subprocess.Popen(args, **kwargs) @@ -35,24 +53,25 @@ class TimedProc(object): self.process = subprocess.Popen(args, **kwargs) self.command = args - def wait(self, timeout=None): + def run(self): ''' wait for subprocess to terminate and return subprocess' return code. If timeout is reached, throw TimedProcTimeoutError ''' def receive(): if self.with_communicate: - (self.stdout, self.stderr) = self.process.communicate(input=self.stdin) + self.stdout, self.stderr = self.process.communicate(input=self.stdin) + elif not self.wait: + self.process.communicate() else: self.process.wait() - (self.stdout, self.stderr) = (None, None) - if timeout: - if not isinstance(timeout, (int, float)): - raise salt.exceptions.TimedProcTimeoutError('Error: timeout must be a number') + if not self.wait or not self.timeout: + receive() + else: rt = threading.Thread(target=receive) rt.start() - rt.join(timeout) + rt.join(self.timeout) if rt.isAlive(): # Subprocess cleanup (best effort) self.process.kill() @@ -64,9 +83,7 @@ class TimedProc(object): raise salt.exceptions.TimedProcTimeoutError( '{0} : Timed out after {1} seconds'.format( self.command, - str(timeout), + str(self.timeout), ) ) - else: - receive() return self.process.returncode From 54359c7094edc522ded0e664c9ad84cee4fdca61 Mon Sep 17 00:00:00 2001 From: Ronald van Zantvoort Date: Tue, 22 Dec 2015 20:49:38 +0100 Subject: [PATCH 2/8] #6691 part 1; adjust cmdmod to new syntax --- salt/modules/cmdmod.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index a6481b0f1f..4b1dcdec00 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -418,7 +418,9 @@ def _run(cmd, 'stdin': str(stdin) if stdin is not None else stdin, 'stdout': stdout, 'stderr': stderr, - 'with_communicate': with_communicate} + 'with_communicate': with_communicate, + 'timeout': timeout, + } if umask is not None: _umask = str(umask).lstrip('0') @@ -470,7 +472,7 @@ def _run(cmd, ) try: - proc.wait(timeout) + proc.run() except TimedProcTimeoutError as exc: ret['stdout'] = str(exc) ret['stderr'] = '' From 5de4b9df70dab9ae0fb5cdd7454a732f9a18259b Mon Sep 17 00:00:00 2001 From: Ronald van Zantvoort Date: Tue, 22 Dec 2015 22:21:53 +0100 Subject: [PATCH 3/8] #6691 wire in 'wait' in cmdmod._run, fix TimedProc & return incorrect timeout if given in exception msg --- salt/modules/cmdmod.py | 2 ++ salt/utils/timed_subprocess.py | 8 +++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 4b1dcdec00..a9545d5d9f 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -237,6 +237,7 @@ def _run(cmd, pillar_override=None, use_vt=False, password=None, + wait=True, **kwargs): ''' Do the DRY thing and only call subprocess.Popen() once @@ -420,6 +421,7 @@ def _run(cmd, 'stderr': stderr, 'with_communicate': with_communicate, 'timeout': timeout, + 'wait': wait, } if umask is not None: diff --git a/salt/utils/timed_subprocess.py b/salt/utils/timed_subprocess.py index dd6bc10051..d26f87858d 100644 --- a/salt/utils/timed_subprocess.py +++ b/salt/utils/timed_subprocess.py @@ -37,8 +37,8 @@ class TimedProc(object): self.timeout = None else: self.timeout = kwargs.pop('timeout') - if not isinstance(self.timeout, (int, float)): - raise salt.exceptions.TimedProcTimeoutError('Error: timeout must be a number') + if self.timeout is not None and not isinstance(self.timeout, (int, float)): + raise salt.exceptions.TimedProcTimeoutError('Error: timeout {0} must be a number'.format(timeout)) try: self.process = subprocess.Popen(args, **kwargs) @@ -61,9 +61,7 @@ class TimedProc(object): def receive(): if self.with_communicate: self.stdout, self.stderr = self.process.communicate(input=self.stdin) - elif not self.wait: - self.process.communicate() - else: + elif self.wait: self.process.wait() if not self.wait or not self.timeout: From 53c44593f9c674d568a361fc3cae56db17fb021c Mon Sep 17 00:00:00 2001 From: Ronald van Zantvoort Date: Tue, 22 Dec 2015 23:11:50 +0100 Subject: [PATCH 4/8] #6691 final refinements to timed_subprocess, add wait to run, shell, run_chroot, script and initial add of run_bg() --- salt/modules/cmdmod.py | 77 ++++++++++++++++++++++++++++++++-- salt/utils/timed_subprocess.py | 27 ++++++------ 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index a9545d5d9f..955630fa61 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -666,6 +666,7 @@ def run(cmd, ignore_retcode=False, saltenv='base', use_vt=False, + wait=True, **kwargs): r''' Execute the passed command and return the output as a string @@ -833,7 +834,8 @@ def run(cmd, pillarenv=kwargs.get('pillarenv'), pillar_override=kwargs.get('pillar'), use_vt=use_vt, - password=kwargs.get('password', None)) + password=kwargs.get('password', None), + wait=wait) log_callback = _check_cb(log_callback) @@ -892,6 +894,7 @@ def shell(cmd, ignore_retcode=False, saltenv='base', use_vt=False, + wait=False, **kwargs): ''' Execute the passed command and return the output as a string. @@ -1052,6 +1055,7 @@ def shell(cmd, saltenv=saltenv, use_vt=use_vt, python_shell=python_shell, + wait=wait, **kwargs) @@ -1845,6 +1849,7 @@ def script(source, __env__=None, saltenv='base', use_vt=False, + wait=True, **kwargs): ''' Download a script from a remote location and execute the script locally. @@ -2030,7 +2035,8 @@ def script(source, pillarenv=kwargs.get('pillarenv'), pillar_override=kwargs.get('pillar'), use_vt=use_vt, - password=kwargs.get('password', None)) + password=kwargs.get('password', None), + wait=wait) _cleanup_tempfile(path) return ret @@ -2320,6 +2326,7 @@ def run_chroot(root, ignore_retcode=False, saltenv='base', use_vt=False, + wait=True, **kwargs): ''' .. versionadded:: 2014.7.0 @@ -2468,7 +2475,8 @@ def run_chroot(root, saltenv=saltenv, pillarenv=kwargs.get('pillarenv'), pillar=kwargs.get('pillar'), - use_vt=use_vt) + use_vt=use_vt, + wait=wait) # Kill processes running in the chroot for i in range(6): @@ -2712,3 +2720,66 @@ def powershell(cmd, except Exception: log.error("Error converting PowerShell JSON return", exc_info=True) return {} + + +def run_bg(cmd, + cwd=None, + runas=None, + shell=DEFAULT_SHELL, + python_shell=None, + env=None, + clean_env=False, + template=None, + rstrip=True, + umask=None, + output_loglevel='debug', + log_callback=None, + timeout=None, + reset_system_locale=True, + saltenv='base', + wait=True, + **kwargs): + ''' + Dodadoc + :param cmd: + :param cwd: + :param runas: + :param shell: + :param env: + :param clean_env: + :param template: + :param rstrip: + :param umask: + :param log_callback: + :param quiet: + :param timeout: + :param reset_system_locale: + :param ignore_retcode: + :param saltenv: + :param kwargs: + :return: + ''' + + return run(cmd, + cwd=cwd, + stdin=None, + stdout=None, + output_loglevel=None, + use_vt=None, + wait=True, + + runas=runas, + shell=shell, + env=env, + clean_env=clean_env, + template=template, + rstrip=rstrip, + umask=umask, + log_callback=log_callback, + quiet=True, + timeout=timeout, + reset_system_locale=reset_system_locale, + ignore_retcode=True, + saltenv=saltenv, + python_shell=python_shell, + **kwargs) diff --git a/salt/utils/timed_subprocess.py b/salt/utils/timed_subprocess.py index d26f87858d..c58a656de2 100644 --- a/salt/utils/timed_subprocess.py +++ b/salt/utils/timed_subprocess.py @@ -15,30 +15,27 @@ class TimedProc(object): def __init__(self, args, **kwargs): self.wait = kwargs.pop('wait', True) + self.stdin = kwargs.pop('stdin', None) + self.with_communicate = kwargs.pop('with_communicate', self.wait) + self.timeout = kwargs.pop('timeout', None) # If you're not willing to wait for the process # you can't define any stdin, stdout or stderr if not self.wait: self.stdin = kwargs['stdin'] = None - else: - self.stdin = kwargs.pop('stdin', None) - if self.stdin is not None: - # Translate a newline submitted as '\n' on the CLI to an actual - # newline character. - self.stdin = self.stdin.replace('\\n', '\n') - kwargs['stdin'] = subprocess.PIPE + self.with_communicate = False + elif self.stdin is not None: + # Translate a newline submitted as '\n' on the CLI to an actual + # newline character. + self.stdin = self.stdin.replace('\\n', '\n') + kwargs['stdin'] = subprocess.PIPE - self.with_communicate = kwargs.pop('with_communicate', self.wait) if not self.with_communicate: self.stdout = kwargs['stdout'] = None self.stderr = kwargs['stderr'] = None - if not self.wait or 'timeout' not in kwargs: - self.timeout = None - else: - self.timeout = kwargs.pop('timeout') - if self.timeout is not None and not isinstance(self.timeout, (int, float)): - raise salt.exceptions.TimedProcTimeoutError('Error: timeout {0} must be a number'.format(timeout)) + if self.timeout and not isinstance(self.timeout, (int, float)): + raise salt.exceptions.TimedProcTimeoutError('Error: timeout {0} must be a number'.format(self.timeout)) try: self.process = subprocess.Popen(args, **kwargs) @@ -64,7 +61,7 @@ class TimedProc(object): elif self.wait: self.process.wait() - if not self.wait or not self.timeout: + if not self.timeout: receive() else: rt = threading.Thread(target=receive) From a4b432f59c818f06bf3bae85d68364ee850ad08a Mon Sep 17 00:00:00 2001 From: Ronald van Zantvoort Date: Tue, 22 Dec 2015 23:26:09 +0100 Subject: [PATCH 5/8] #6691 run_bg() improvement --- salt/modules/cmdmod.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 955630fa61..07edcf7baf 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -2760,26 +2760,37 @@ def run_bg(cmd, :return: ''' - return run(cmd, - cwd=cwd, + python_shell = _python_shell_default(python_shell, + kwargs.get('__pub_jid', '')) + res = _run(cmd, stdin=None, + stderr=None, stdout=None, output_loglevel=None, use_vt=None, - wait=True, + wait=False, + with_communicate=False, runas=runas, shell=shell, + python_shell=python_shell, + cwd=cwd, env=env, clean_env=clean_env, template=template, rstrip=rstrip, umask=umask, + # output_loglevel=output_loglevel, log_callback=log_callback, - quiet=True, timeout=timeout, reset_system_locale=reset_system_locale, - ignore_retcode=True, + # ignore_retcode=ignore_retcode, saltenv=saltenv, - python_shell=python_shell, - **kwargs) + pillarenv=kwargs.get('pillarenv'), + pillar_override=kwargs.get('pillar'), + password=kwargs.get('password', None), + ) + + return { + 'pid': res['pid'] + } From d94a7f40a4ac1669f9a0146a02d32b3c9b65c80d Mon Sep 17 00:00:00 2001 From: Ronald van Zantvoort Date: Tue, 5 Jan 2016 11:15:59 +0100 Subject: [PATCH 6/8] Change arg to 'bg' for better consistency & user expectancy; add documentation --- salt/modules/cmdmod.py | 169 ++++++++++++++++++++++++++------- salt/utils/timed_subprocess.py | 2 +- 2 files changed, 136 insertions(+), 35 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 07edcf7baf..a6accc54e8 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -237,7 +237,7 @@ def _run(cmd, pillar_override=None, use_vt=False, password=None, - wait=True, + bg=False, **kwargs): ''' Do the DRY thing and only call subprocess.Popen() once @@ -421,7 +421,7 @@ def _run(cmd, 'stderr': stderr, 'with_communicate': with_communicate, 'timeout': timeout, - 'wait': wait, + 'bg': bg, } if umask is not None: @@ -666,7 +666,7 @@ def run(cmd, ignore_retcode=False, saltenv='base', use_vt=False, - wait=True, + bg=False, **kwargs): r''' Execute the passed command and return the output as a string @@ -697,6 +697,8 @@ def run(cmd, :param bool python_shell: If False, let python handle the positional arguments. Set to True to use shell features, such as pipes or redirection + :param bool bg: If True, run command in background and do not await or deliver it's results + :param list env: A list of environment variables to be set prior to execution. @@ -835,7 +837,7 @@ def run(cmd, pillar_override=kwargs.get('pillar'), use_vt=use_vt, password=kwargs.get('password', None), - wait=wait) + bg=bg) log_callback = _check_cb(log_callback) @@ -894,7 +896,7 @@ def shell(cmd, ignore_retcode=False, saltenv='base', use_vt=False, - wait=False, + bg=False, **kwargs): ''' Execute the passed command and return the output as a string. @@ -921,6 +923,8 @@ def shell(cmd, :param int shell: Shell to execute under. Defaults to the system default shell. + :param bool bg: If True, run command in background and do not await or deliver it's results + :param list env: A list of environment variables to be set prior to execution. @@ -1055,7 +1059,7 @@ def shell(cmd, saltenv=saltenv, use_vt=use_vt, python_shell=python_shell, - wait=wait, + bg=bg, **kwargs) @@ -1849,7 +1853,7 @@ def script(source, __env__=None, saltenv='base', use_vt=False, - wait=True, + bg=False, **kwargs): ''' Download a script from a remote location and execute the script locally. @@ -1889,6 +1893,8 @@ def script(source, :param bool python_shell: If False, let python handle the positional arguments. Set to True to use shell features, such as pipes or redirection + :param bool bg: If True, run script in background and do not await or deliver it's results + :param list env: A list of environment variables to be set prior to execution. @@ -2036,7 +2042,7 @@ def script(source, pillar_override=kwargs.get('pillar'), use_vt=use_vt, password=kwargs.get('password', None), - wait=wait) + bg=bg) _cleanup_tempfile(path) return ret @@ -2326,7 +2332,7 @@ def run_chroot(root, ignore_retcode=False, saltenv='base', use_vt=False, - wait=True, + bg=False, **kwargs): ''' .. versionadded:: 2014.7.0 @@ -2476,7 +2482,7 @@ def run_chroot(root, pillarenv=kwargs.get('pillarenv'), pillar=kwargs.get('pillar'), use_vt=use_vt, - wait=wait) + bg=bg) # Kill processes running in the chroot for i in range(6): @@ -2730,34 +2736,130 @@ def run_bg(cmd, env=None, clean_env=False, template=None, - rstrip=True, umask=None, output_loglevel='debug', log_callback=None, timeout=None, reset_system_locale=True, saltenv='base', - wait=True, **kwargs): ''' - Dodadoc - :param cmd: - :param cwd: - :param runas: - :param shell: - :param env: - :param clean_env: - :param template: - :param rstrip: - :param umask: - :param log_callback: - :param quiet: - :param timeout: - :param reset_system_locale: - :param ignore_retcode: - :param saltenv: - :param kwargs: - :return: + .. versionadded: Boron + + Execute the passed command in the background and return it's PID + + Note that ``env`` represents the environment variables for the command, and + should be formatted as a dict, or a YAML string which resolves to a dict. + + :param str cmd: The command to run. ex: 'ls -lart /home' + + :param str cwd: The current working directory to execute the command in, + defaults to `/root` (`C:\` in windows) + + :param str runas: User to run script as. If running on a Windows minion you + must also pass a password + + :param str shell: Shell to execute under. Defaults to the system default + shell. + + :param bool python_shell: If False, let python handle the positional + arguments. Set to True to use shell features, such as pipes or redirection + + :param list env: A list of environment variables to be set prior to + execution. + + Example: + + .. code-block:: yaml + + salt://scripts/foo.sh: + cmd.script: + - env: + - BATCH: 'yes' + + .. warning:: + + The above illustrates a common PyYAML pitfall, that **yes**, + **no**, **on**, **off**, **true**, and **false** are all loaded as + boolean ``True`` and ``False`` values, and must be enclosed in + quotes to be used as strings. More info on this (and other) PyYAML + idiosyncrasies can be found :doc:`here + `. + + Variables as values are not evaluated. So $PATH in the following + example is a literal '$PATH': + + .. code-block:: yaml + + salt://scripts/bar.sh: + cmd.script: + - env: "PATH=/some/path:$PATH" + + One can still use the existing $PATH by using a bit of Jinja: + + .. code-block:: yaml + + {% set current_path = salt['environ.get']('PATH', '/bin:/usr/bin') %} + + mycommand: + cmd.run: + - name: ls -l / + - env: + - PATH: {{ [current_path, '/my/special/bin']|join(':') }} + + :param bool clean_env: Attempt to clean out all other shell environment + variables and set only those provided in the 'env' argument to this + function. + + :param str template: If this setting is applied then the named templating + engine will be used to render the downloaded file. Currently jinja, mako, + and wempy are supported + + :param str umask: The umask (in octal) to use when running the command. + + :param int timeout: A timeout in seconds for the executed process to return. + + .. warning:: + + This function does not process commands through a shell + unless the python_shell flag is set to True. This means that any + shell-specific functionality such as 'echo' or the use of pipes, + redirection or &&, should either be migrated to cmd.shell or + have the python_shell=True flag set here. + + The use of python_shell=True means that the shell will accept _any_ input + including potentially malicious commands such as 'good_command;rm -rf /'. + Be absolutely certain that you have sanitized your input prior to using + python_shell=True + + CLI Example: + + .. code-block:: bash + + salt '*' cmd.run_bg "ls -l | awk '/foo/{print \\$2}'" + + The template arg can be set to 'jinja' or another supported template + engine to render the command arguments before execution. + For example: + + .. code-block:: bash + + salt '*' cmd.run_bg template=jinja "ls -l /tmp/{{grains.id}} | awk '/foo/{print \\$2}'" + + Specify an alternate shell with the shell parameter: + + .. code-block:: bash + + salt '*' cmd.run "Get-ChildItem C:\\ " shell='powershell' + + If an equal sign (``=``) appears in an argument to a Salt command it is + interpreted as a keyword argument in the format ``key=val``. That + processing can be bypassed in order to pass an equal sign through to the + remote shell command by manually specifying the kwarg: + + .. code-block:: bash + + salt '*' cmd.run cmd='sed -e s/=/:/g' ''' python_shell = _python_shell_default(python_shell, @@ -2768,8 +2870,9 @@ def run_bg(cmd, stdout=None, output_loglevel=None, use_vt=None, - wait=False, + bg=True, with_communicate=False, + rstrip=False, runas=runas, shell=shell, @@ -2778,9 +2881,7 @@ def run_bg(cmd, env=env, clean_env=clean_env, template=template, - rstrip=rstrip, umask=umask, - # output_loglevel=output_loglevel, log_callback=log_callback, timeout=timeout, reset_system_locale=reset_system_locale, @@ -2788,7 +2889,7 @@ def run_bg(cmd, saltenv=saltenv, pillarenv=kwargs.get('pillarenv'), pillar_override=kwargs.get('pillar'), - password=kwargs.get('password', None), + # password=kwargs.get('password', None), ) return { diff --git a/salt/utils/timed_subprocess.py b/salt/utils/timed_subprocess.py index c58a656de2..bdb654936d 100644 --- a/salt/utils/timed_subprocess.py +++ b/salt/utils/timed_subprocess.py @@ -14,7 +14,7 @@ class TimedProc(object): ''' def __init__(self, args, **kwargs): - self.wait = kwargs.pop('wait', True) + self.wait = not kwargs.pop('bg', False) self.stdin = kwargs.pop('stdin', None) self.with_communicate = kwargs.pop('with_communicate', self.wait) self.timeout = kwargs.pop('timeout', None) From 69661c61ce0820ed9e875324768591a57fa6da0a Mon Sep 17 00:00:00 2001 From: Ronald van Zantvoort Date: Tue, 5 Jan 2016 11:49:23 +0100 Subject: [PATCH 7/8] pylint fix --- salt/modules/cmdmod.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index a6accc54e8..26c79e30c9 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -2743,7 +2743,7 @@ def run_bg(cmd, reset_system_locale=True, saltenv='base', **kwargs): - ''' + r''' .. versionadded: Boron Execute the passed command in the background and return it's PID From 7668325242c82c21daf57390b4cd95df42b049c1 Mon Sep 17 00:00:00 2001 From: Ronald van Zantvoort Date: Tue, 5 Jan 2016 11:57:46 +0100 Subject: [PATCH 8/8] remove output_loglevel from run_bg named kwargs --- salt/modules/cmdmod.py | 1 - 1 file changed, 1 deletion(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 26c79e30c9..4dd6c3ec06 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -2737,7 +2737,6 @@ def run_bg(cmd, clean_env=False, template=None, umask=None, - output_loglevel='debug', log_callback=None, timeout=None, reset_system_locale=True,