From b3e8d7f2819bbedb45b51bed5bbcbb7a405551d3 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 8 Apr 2019 11:54:53 +0200 Subject: [PATCH 1/5] st2-self-check should ignore any pack actions which name doesn't start with test_. --- st2common/bin/st2-self-check | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/bin/st2-self-check b/st2common/bin/st2-self-check index 822730e2eb..db4514a86b 100755 --- a/st2common/bin/st2-self-check +++ b/st2common/bin/st2-self-check @@ -119,7 +119,7 @@ rm -R st2tests/ popd # Retrieve test action list -TEST_ACTION_LIST=`st2 action list --pack=tests -w 90 | awk '{ print $2 }' | grep -v "|" | grep -v "ref"` +TEST_ACTION_LIST=`st2 action list --pack=tests -w 90 | awk '{ print $2 }' | grep -v "|" | grep -v "ref" | grep tests.test_` # Run all the tests for TEST in $TEST_ACTION_LIST From bb8be3224015f6a1e820199333ae8fd42371555b Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 8 Apr 2019 13:41:01 +0200 Subject: [PATCH 2/5] Don't run WinRM tests when run_windows_tests variable is false (default). --- st2common/bin/st2-self-check | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/st2common/bin/st2-self-check b/st2common/bin/st2-self-check index db4514a86b..1800dd7ecb 100755 --- a/st2common/bin/st2-self-check +++ b/st2common/bin/st2-self-check @@ -125,12 +125,21 @@ TEST_ACTION_LIST=`st2 action list --pack=tests -w 90 | awk '{ print $2 }' | grep for TEST in $TEST_ACTION_LIST do # If -w option is not set, skip Windows related tests - # because smbclient is not installed by default. if [ ${RUN_WINDOWS_TESTS} = "false" ] && [ ${TEST} = "tests.test_windows_runners" ]; then echo "Skipping ${TEST}..." continue fi + if [ ${RUN_WINDOWS_TESTS} = "false" ] && [ ${TEST} = "tests.test_winrm_runners" ]; then + echo "Skipping ${TEST}..." + continue + fi + + if [ ${RUN_WINDOWS_TESTS} = "false" ] && [ ${TEST} = "test_winrm_large_script" ]; then + echo "Skipping ${TEST}..." + continue + fi + # Skip Mistral Inquiry test if we're not running Mistral tests if [ ${RUN_MISTRAL_TESTS} = "false" ] && [ ${TEST} = "tests.test_inquiry_mistral" ]; then echo "Skipping ${TEST}..." From a26af65a14f09929f03ef1b6e4a51c72976d18a2 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 8 Apr 2019 15:01:11 +0200 Subject: [PATCH 3/5] Fix action reference. --- st2common/bin/st2-self-check | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/bin/st2-self-check b/st2common/bin/st2-self-check index 1800dd7ecb..ca847bdd97 100755 --- a/st2common/bin/st2-self-check +++ b/st2common/bin/st2-self-check @@ -135,7 +135,7 @@ do continue fi - if [ ${RUN_WINDOWS_TESTS} = "false" ] && [ ${TEST} = "test_winrm_large_script" ]; then + if [ ${RUN_WINDOWS_TESTS} = "false" ] && [ ${TEST} = "tests.test_winrm_large_script" ]; then echo "Skipping ${TEST}..." continue fi From 21fac2cfb6ecdf89f2cfd5bd544087f66a3140a9 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 8 Apr 2019 18:22:28 +0200 Subject: [PATCH 4/5] Update the paramiko runner code to replace all occurances of \r\n in the stdout and stderr data with \n when pty is used. When pty is used, default behavior is to replace all new line characters \n with \r\n which is almost never desired. --- st2common/st2common/runners/paramiko_ssh.py | 28 ++++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/st2common/st2common/runners/paramiko_ssh.py b/st2common/st2common/runners/paramiko_ssh.py index fdca3dc19a..24826a76d5 100644 --- a/st2common/st2common/runners/paramiko_ssh.py +++ b/st2common/st2common/runners/paramiko_ssh.py @@ -363,7 +363,13 @@ def run(self, cmd, timeout=None, quote=False, call_line_handler_func=False): if cmd.startswith('sudo'): # Note that fabric does this as well. If you set pty, stdout and stderr # streams will be combined into one. + # NOTE: If pty is used, every new line character \n will be converted to \r\n which + # isn't desired. Because of that we sanitize the output and replace \r\n with \n at the + # bottom of this method + uses_pty = True chan.get_pty() + else: + uses_pty = False chan.exec_command(cmd) stdout = StringIO() @@ -404,8 +410,8 @@ def run(self, cmd, timeout=None, quote=False, call_line_handler_func=False): # TODO: Is this the right way to clean up? chan.close() - stdout = strip_shell_chars(stdout.getvalue()) - stderr = strip_shell_chars(stderr.getvalue()) + stdout = self._sanitize_output(stdout.getvalue(), uses_pty=uses_pty) + stderr = self._sanitize_output(stderr.getvalue(), uses_pty=uses_pty) raise SSHCommandTimeoutError(cmd=cmd, timeout=timeout, stdout=stdout, stderr=stderr) @@ -434,8 +440,8 @@ def run(self, cmd, timeout=None, quote=False, call_line_handler_func=False): # Receive the exit status code of the command we ran. status = chan.recv_exit_status() - stdout = strip_shell_chars(stdout.getvalue()) - stderr = strip_shell_chars(stderr.getvalue()) + stdout = self._sanitize_output(stdout.getvalue(), uses_pty=uses_pty) + stderr = self._sanitize_output(stderr.getvalue(), uses_pty=uses_pty) extra = {'_status': status, '_stdout': stdout, '_stderr': stderr} self.logger.debug('Command finished', extra=extra) @@ -736,6 +742,20 @@ def _get_ssh_config_for_host(self, host): return ssh_config_info + def _sanitize_output(self, output, uses_pty=False): + """ + Function which sanitizes the output (stdout / stderr). + + It strips trailing carriage return and new line characters and if pty is used, it also + replaces all occurances of \r\n with \n. + """ + output = strip_shell_chars(output) + + if uses_pty: + output = output.replace('\r\n', '\n') + + return output + @staticmethod def _is_key_file_needs_passphrase(file): for cls in [paramiko.RSAKey, paramiko.DSSKey, paramiko.ECDSAKey]: From 2c66d537fc1d92542223e39396fe0ab3e3ec9176 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 9 Apr 2019 11:53:00 +0200 Subject: [PATCH 5/5] Revert "Update the paramiko runner code to replace all occurances of \r\n in the" This reverts commit 21fac2cfb6ecdf89f2cfd5bd544087f66a3140a9. --- st2common/st2common/runners/paramiko_ssh.py | 28 +++------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/st2common/st2common/runners/paramiko_ssh.py b/st2common/st2common/runners/paramiko_ssh.py index 24826a76d5..fdca3dc19a 100644 --- a/st2common/st2common/runners/paramiko_ssh.py +++ b/st2common/st2common/runners/paramiko_ssh.py @@ -363,13 +363,7 @@ def run(self, cmd, timeout=None, quote=False, call_line_handler_func=False): if cmd.startswith('sudo'): # Note that fabric does this as well. If you set pty, stdout and stderr # streams will be combined into one. - # NOTE: If pty is used, every new line character \n will be converted to \r\n which - # isn't desired. Because of that we sanitize the output and replace \r\n with \n at the - # bottom of this method - uses_pty = True chan.get_pty() - else: - uses_pty = False chan.exec_command(cmd) stdout = StringIO() @@ -410,8 +404,8 @@ def run(self, cmd, timeout=None, quote=False, call_line_handler_func=False): # TODO: Is this the right way to clean up? chan.close() - stdout = self._sanitize_output(stdout.getvalue(), uses_pty=uses_pty) - stderr = self._sanitize_output(stderr.getvalue(), uses_pty=uses_pty) + stdout = strip_shell_chars(stdout.getvalue()) + stderr = strip_shell_chars(stderr.getvalue()) raise SSHCommandTimeoutError(cmd=cmd, timeout=timeout, stdout=stdout, stderr=stderr) @@ -440,8 +434,8 @@ def run(self, cmd, timeout=None, quote=False, call_line_handler_func=False): # Receive the exit status code of the command we ran. status = chan.recv_exit_status() - stdout = self._sanitize_output(stdout.getvalue(), uses_pty=uses_pty) - stderr = self._sanitize_output(stderr.getvalue(), uses_pty=uses_pty) + stdout = strip_shell_chars(stdout.getvalue()) + stderr = strip_shell_chars(stderr.getvalue()) extra = {'_status': status, '_stdout': stdout, '_stderr': stderr} self.logger.debug('Command finished', extra=extra) @@ -742,20 +736,6 @@ def _get_ssh_config_for_host(self, host): return ssh_config_info - def _sanitize_output(self, output, uses_pty=False): - """ - Function which sanitizes the output (stdout / stderr). - - It strips trailing carriage return and new line characters and if pty is used, it also - replaces all occurances of \r\n with \n. - """ - output = strip_shell_chars(output) - - if uses_pty: - output = output.replace('\r\n', '\n') - - return output - @staticmethod def _is_key_file_needs_passphrase(file): for cls in [paramiko.RSAKey, paramiko.DSSKey, paramiko.ECDSAKey]: