Merge pull request #2067 from s0undt3ch/issues/2028

Fix #2028

salt/states/pip.py

@@ -70,38 +70,47 @@ def installed(name,
                 name)
         return ret
 
-    if __salt__['pip.install'](pkgs=name,
-                               requirements=requirements,
-                               bin_env=bin_env,
-                               log=log,
-                               proxy=proxy,
-                               timeout=timeout,
-                               editable=editable,
-                               find_links=find_links,
-                               index_url=index_url,
-                               extra_index_url=extra_index_url,
-                               no_index=no_index,
-                               mirrors=mirrors,
-                               build=build,
-                               target=target,
-                               download=download,
-                               download_cache=download_cache,
-                               source=source,
-                               upgrade=upgrade,
-                               force_reinstall=force_reinstall,
-                               ignore_installed=ignore_installed,
-                               no_deps=no_deps,
-                               no_install=no_install,
-                               no_download=no_download,
-                               install_options=install_options,
-                               runas=user,
-                               cwd=cwd):
+    pip_install_call = __salt__['pip.install'](
+        pkgs=name,
+        requirements=requirements,
+        bin_env=bin_env,
+        log=log,
+        proxy=proxy,
+        timeout=timeout,
+        editable=editable,
+        find_links=find_links,
+        index_url=index_url,
+        extra_index_url=extra_index_url,
+        no_index=no_index,
+        mirrors=mirrors,
+        build=build,
+        target=target,
+        download=download,
+        download_cache=download_cache,
+        source=source,
+        upgrade=upgrade,
+        force_reinstall=force_reinstall,
+        ignore_installed=ignore_installed,
+        no_deps=no_deps,
+        no_install=no_install,
+        no_download=no_download,
+        install_options=install_options,
+        runas=user,
+        cwd=cwd
+    )
+
+    if pip_install_call and pip_install_call['retcode']==0:
         pkg_list = __salt__['pip.list'](name, bin_env, runas=user, cwd=cwd)
         version = list(pkg_list.values())[0]
         pkg_name = next(iter(pkg_list))
         ret['result'] = True
         ret['changes']["{0}=={1}".format(pkg_name, version)] = 'Installed'
         ret['comment'] = 'Package was successfully installed'
+    elif pip_install_call:
+        ret['result'] = False
+        ret['comment'] = 'Failed to install package {0}. Error: {1}'.format(
+            name, pip_install_call['stderr']
+        )
     else:
         ret['result'] = False
         ret['comment'] = 'Could not install package'

tests/integration/files/file/base/issue-2028-pip-installed.sls

@@ -0,0 +1,11 @@
+/tmp/issue-2028-pip-installed:
+  virtualenv.managed:
+    - no_site_packages: True
+    - distribute: True
+
+supervisord-pip:
+    pip.installed:
+      - name: supervisor
+      - bin_env: /tmp/issue-2028-pip-installed
+      - require:
+        - virtualenv: /tmp/issue-2028-pip-installed

tests/integration/modules/virtualenv.py

@@ -1,5 +1,6 @@
 # Import python libs
 import os
+import shutil
 import tempfile
 import integration
 

tests/integration/states/file.py

@@ -328,8 +328,6 @@ class FileTest(integration.ModuleCase):
                 ignore_errors=True
             )
 
-
-
     def test_touch(self):
         '''
         file.touch

tests/integration/states/pip.py

@@ -0,0 +1,73 @@
+# -*- coding: utf-8 -*-
+'''
+    :copyright: © 2012 UfSoft.org - :email:`Pedro Algarvio (pedro@algarvio.me)`
+    :license: Apache 2.0, see LICENSE for more details
+'''
+
+import os
+import shutil
+import tempfile
+
+# Import salt libs
+from saltunittest import skipIf
+import integration
+
+
+class PipStateTest(integration.ModuleCase):
+
+    def setUp(self):
+        super(PipStateTest, self).setUp()
+        ret = self.run_function('cmd.has_exec', ['virtualenv'])
+        if not ret:
+            self.skipTest('virtualenv not installed')
+
+    def test_pip_installed_errors(self):
+        venv_dir = '/tmp/pip-installed-errors'
+        try:
+            # Since we don't have the virtualenv created, pip.installed will
+            # thrown and error.
+            ret = self.run_function('state.sls', mods='pip-installed-errors')
+            self.assertTrue(isinstance(ret, dict))
+            self.assertNotEqual(ret, {})
+
+            for key in ret.keys():
+                self.assertFalse(ret[key]['result'])
+                self.assertEqual(
+                    ret[key]['comment'],
+                    'Failed to install package supervisor. Error: /bin/bash: '
+                    '/tmp/pip-installed-errors: No such file or directory'
+                )
+
+            # We now create the missing virtualenv
+            ret = self.run_function('virtualenv.create', [venv_dir])
+            self.assertTrue(ret['retcode']==0)
+
+            # The state should not have any issues running now
+            ret = self.run_function('state.sls', mods='pip-installed-errors')
+            self.assertTrue(isinstance(ret, dict))
+            self.assertNotEqual(ret, {})
+
+            for key in ret.keys():
+                self.assertTrue(ret[key]['result'])
+        finally:
+            if os.path.isdir(venv_dir):
+                shutil.rmtree(venv_dir)
+
+    def test_issue_2028_pip_installed_state(self):
+        ret = self.run_function('state.sls', mods='issue-2028-pip-installed')
+
+        venv_dir = '/tmp/issue-2028-pip-installed'
+
+        try:
+            self.assertTrue(isinstance(ret, dict)), ret
+            self.assertNotEqual(ret, {})
+
+            for key in ret.iterkeys():
+                self.assertTrue(ret[key]['result'])
+
+            self.assertTrue(
+                os.path.isfile(os.path.join(venv_dir, 'bin', 'supervisord'))
+            )
+        finally:
+            if os.path.isdir(venv_dir):
+                shutil.rmtree(venv_dir)