salt-ssh: python binary exists before version check

salt/utils/thin.py

@@ -396,19 +396,22 @@ def gen_thin(cachedir, extra_mods='', overwrite=False, so_mods='',
             return thintar
     if _six.PY3:
         # Let's check for the minimum python 2 version requirement, 2.6
-        py_shell_cmd = "{} -c 'import sys;sys.stdout.write(\"%s.%s\\n\" % sys.version_info[:2]);'".format(python2_bin)
+        if not salt.utils.path.which(python2_bin):
-        cmd = subprocess.Popen(py_shell_cmd, stdout=subprocess.PIPE, shell=True)
+            log.debug('{0} binary does not exist. Will not detect version to use for Python 2'.format(python2_bin))
-        stdout, _ = cmd.communicate()
-        if cmd.returncode == 0:
-            py2_version = tuple(int(n) for n in stdout.decode('utf-8').strip().split('.'))
-            if py2_version < (2, 6):
-                raise salt.exceptions.SaltSystemExit(
-                    'The minimum required python version to run salt-ssh is "2.6".'
-                    'The version reported by "{0}" is "{1}". Please try "salt-ssh '
-                    '--python2-bin=<path-to-python-2.6-binary-or-higher>".'.format(python2_bin, stdout.strip()))
         else:
-            log.error('Unable to detect Python-2 version')
+            py_shell_cmd = "{} -c 'import sys;sys.stdout.write(\"%s.%s\\n\" % sys.version_info[:2]);'".format(python2_bin)
-            log.debug(stdout)
+            cmd = subprocess.Popen(py_shell_cmd, stdout=subprocess.PIPE, shell=True)
+            stdout, _ = cmd.communicate()
+            if cmd.returncode == 0:
+                py2_version = tuple(int(n) for n in stdout.decode('utf-8').strip().split('.'))
+                if py2_version < (2, 6):
+                    raise salt.exceptions.SaltSystemExit(
+                        'The minimum required python version to run salt-ssh is "2.6".'
+                        'The version reported by "{0}" is "{1}". Please try "salt-ssh '
+                        '--python2-bin=<path-to-python-2.6-binary-or-higher>".'.format(python2_bin, stdout.strip()))
+            else:
+                log.debug('Unable to detect %s version', python2_bin)
+                log.debug(stdout)
 
     tops_failure_msg = 'Failed %s tops for Python binary %s.'
     tops_py_version_mapping = {}
@@ -418,38 +421,44 @@ def gen_thin(cachedir, extra_mods='', overwrite=False, so_mods='',
     # Collect tops, alternative to 2.x version
     if _six.PY2 and sys.version_info.major == 2:
         # Get python 3 tops
-        py_shell_cmd = "{0} -c 'import salt.utils.thin as t;print(t.gte())' '{1}'".format(
+        if not salt.utils.path.which(python3_bin):
-            python3_bin, salt.utils.json.dumps({'extra_mods': extra_mods, 'so_mods': so_mods}))
+            log.debug('{0} binary does not exist. Will not attempt to generate tops for Python 3'.format(python3_bin))
-        cmd = subprocess.Popen(py_shell_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
-        stdout, stderr = cmd.communicate()
-        if cmd.returncode == 0:
-            try:
-                tops = salt.utils.json.loads(stdout)
-                tops_py_version_mapping['3'] = tops
-            except ValueError as err:
-                log.error(tops_failure_msg, 'parsing', python3_bin)
-                log.exception(err)
         else:
-            log.debug(tops_failure_msg, 'collecting', python3_bin)
+            py_shell_cmd = "{0} -c 'import salt.utils.thin as t;print(t.gte())' '{1}'".format(
-            log.debug(stderr)
+                python3_bin, salt.utils.json.dumps({'extra_mods': extra_mods, 'so_mods': so_mods}))
+            cmd = subprocess.Popen(py_shell_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+            stdout, stderr = cmd.communicate()
+            if cmd.returncode == 0:
+                try:
+                    tops = salt.utils.json.loads(stdout)
+                    tops_py_version_mapping['3'] = tops
+                except ValueError as err:
+                    log.error(tops_failure_msg, 'parsing', python3_bin)
+                    log.exception(err)
+            else:
+                log.debug(tops_failure_msg, 'collecting', python3_bin)
+                log.debug(stderr)
 
     # Collect tops, alternative to 3.x version
     if _six.PY3 and sys.version_info.major == 3:
         # Get python 2 tops
-        py_shell_cmd = "{0} -c 'import salt.utils.thin as t;print(t.gte())' '{1}'".format(
+        if not salt.utils.path.which(python2_bin):
-            python2_bin, salt.utils.json.dumps({'extra_mods': extra_mods, 'so_mods': so_mods}))
+            log.debug('{0} binary does not exist. Will not attempt to generate tops for Python 2'.format(python2_bin))
-        cmd = subprocess.Popen(py_shell_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
-        stdout, stderr = cmd.communicate()
-        if cmd.returncode == 0:
-            try:
-                tops = salt.utils.json.loads(stdout.decode('utf-8'))
-                tops_py_version_mapping['2'] = tops
-            except ValueError as err:
-                log.error(tops_failure_msg, 'parsing', python2_bin)
-                log.exception(err)
         else:
-            log.debug(tops_failure_msg, 'collecting', python2_bin)
+            py_shell_cmd = "{0} -c 'import salt.utils.thin as t;print(t.gte())' '{1}'".format(
-            log.debug(stderr)
+                python2_bin, salt.utils.json.dumps({'extra_mods': extra_mods, 'so_mods': so_mods}))
+            cmd = subprocess.Popen(py_shell_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
+            stdout, stderr = cmd.communicate()
+            if cmd.returncode == 0:
+                try:
+                    tops = salt.utils.json.loads(stdout.decode('utf-8'))
+                    tops_py_version_mapping['2'] = tops
+                except ValueError as err:
+                    log.error(tops_failure_msg, 'parsing', python2_bin)
+                    log.exception(err)
+            else:
+                log.debug(tops_failure_msg, 'collecting', python2_bin)
+                log.debug(stderr)
 
     with salt.utils.files.fopen(pymap_cfg, 'wb') as fp_:
         fp_.write(_get_supported_py_config(tops=tops_py_version_mapping, extended_cfg=extended_cfg))

tests/unit/utils/test_thin.py

@@ -7,6 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 import os
 import sys
 from tests.support.unit import TestCase, skipIf
+from tests.support.helpers import TestsLoggingHandler
 from tests.support.mock import (
     NO_MOCK,
     NO_MOCK_REASON,
@@ -20,6 +21,7 @@ import salt.utils.stringutils
 import salt.utils.platform
 from salt.utils.stringutils import to_bytes as bts
 from salt.ext.six.moves import range
+import salt.ext.six
 
 try:
     import pytest
@@ -424,6 +426,53 @@ class SSHThinTestCase(TestCase):
         self.assertIn('The minimum required python version to run salt-ssh is '
                       '"2.6"', str(err.value))
 
+    @skipIf(salt.utils.platform.is_windows() and thin._six.PY2,
+            'Dies on Python2 on Windows')
+    @patch('salt.exceptions.SaltSystemExit', Exception)
+    @patch('salt.utils.thin.os.makedirs', MagicMock())
+    @patch('salt.utils.files.fopen', MagicMock())
+    @patch('salt.utils.thin._get_salt_call', MagicMock())
+    @patch('salt.utils.thin._get_ext_namespaces', MagicMock())
+    @patch('salt.utils.thin.get_tops', MagicMock(return_value=['/foo3', '/bar3']))
+    @patch('salt.utils.thin.get_ext_tops', MagicMock(return_value={}))
+    @patch('salt.utils.thin.os.path.isfile', MagicMock())
+    @patch('salt.utils.thin.os.path.isdir', MagicMock(return_value=True))
+    @patch('salt.utils.thin.os.remove', MagicMock())
+    @patch('salt.utils.thin.os.path.exists', MagicMock())
+    @patch('salt.utils.path.os_walk', MagicMock(return_value=[]))
+    @patch('salt.utils.thin.subprocess.Popen',
+           _popen(None, side_effect=[(bts('2.7'), bts('')), (bts('["/foo27", "/bar27"]'), bts(''))]))
+    @patch('salt.utils.thin.tarfile', MagicMock())
+    @patch('salt.utils.thin.zipfile', MagicMock())
+    @patch('salt.utils.thin.os.getcwd', MagicMock())
+    @patch('salt.utils.thin.os.chdir', MagicMock())
+    @patch('salt.utils.thin.tempfile.mkdtemp', MagicMock())
+    @patch('salt.utils.thin.tempfile.mkstemp', MagicMock(return_value=(3, ".temporary")))
+    @patch('salt.utils.thin.shutil', MagicMock())
+    @patch('salt.utils.path.which', MagicMock(return_value=''))
+    @patch('salt.utils.thin._get_thintar_prefix', MagicMock())
+    def test_gen_thin_python_exist_or_not(self):
+        '''
+        Test thin.gen_thin function if the opposite python
+        binary does not exist
+        '''
+        with TestsLoggingHandler() as handler:
+            thin.gen_thin('')
+            salt.utils.thin.subprocess.Popen.assert_not_called()
+
+            if salt.ext.six.PY2:
+                self.assertIn('DEBUG:python3 binary does not exist. Will not attempt to generate '
+                              'tops for Python 3',
+                              handler.messages)
+
+            if salt.ext.six.PY3:
+                self.assertIn('DEBUG:python2 binary does not exist. Will not '
+                              'detect version to use for Python 2',
+                              handler.messages)
+                self.assertIn('DEBUG:python2 binary does not exist. Will not attempt to generate '
+                              'tops for Python 2',
+                              handler.messages)
+
     @skipIf(salt.utils.platform.is_windows() and thin._six.PY2,
             'Dies on Python2 on Windows')
     @patch('salt.exceptions.SaltSystemExit', Exception)
@@ -452,6 +501,7 @@ class SSHThinTestCase(TestCase):
     @patch('salt.utils.thin._six.PY3', True)
     @patch('salt.utils.thin._six.PY2', False)
     @patch('salt.utils.thin.sys.version_info', _version_info(None, 3, 6))
+    @patch('salt.utils.path.which', MagicMock(return_value='/usr/bin/python'))
     def test_gen_thin_compression_fallback_py3(self):
         '''
         Test thin.gen_thin function if fallbacks to the gzip compression, once setup wrong.
@@ -493,6 +543,7 @@ class SSHThinTestCase(TestCase):
     @patch('salt.utils.thin._six.PY3', True)
     @patch('salt.utils.thin._six.PY2', False)
     @patch('salt.utils.thin.sys.version_info', _version_info(None, 3, 6))
+    @patch('salt.utils.path.which', MagicMock(return_value='/usr/bin/python'))
     def test_gen_thin_control_files_written_py3(self):
         '''
         Test thin.gen_thin function if control files are written (version, salt-call etc).
@@ -538,6 +589,7 @@ class SSHThinTestCase(TestCase):
     @patch('salt.utils.thin._six.PY2', False)
     @patch('salt.utils.thin.sys.version_info', _version_info(None, 3, 6))
     @patch('salt.utils.hashutils.DigestCollector', MagicMock())
+    @patch('salt.utils.path.which', MagicMock(return_value='/usr/bin/python'))
     def test_gen_thin_main_content_files_written_py3(self):
         '''
         Test thin.gen_thin function if main content files are written.
@@ -590,6 +642,7 @@ class SSHThinTestCase(TestCase):
     @patch('salt.utils.thin._six.PY2', False)
     @patch('salt.utils.thin.sys.version_info', _version_info(None, 3, 6))
     @patch('salt.utils.hashutils.DigestCollector', MagicMock())
+    @patch('salt.utils.path.which', MagicMock(return_value='/usr/bin/python'))
     def test_gen_thin_ext_alternative_content_files_written_py3(self):
         '''
         Test thin.gen_thin function if external alternative content files are written.