Validate ansible extra packages

Currently when validating the ansible installation, zuul only checks
if ansible is installed and not any packages that would have been
installed with ANSIBLE_EXTRA_PACKAGES. Since the executor image has
ansible pre installed the ANSIBLE_EXTRA_PACKAGES environment variable
has no effect unless ansible is removed.

This adds a check to make sure packages specified with
ANSIBLE_EXTRA_PACKAGES are installed as well.

Change-Id: I7ee4125d6716db718bb355b837e90dbcfce9b857

tests/unit/test_executor.py

@@ -842,3 +842,27 @@ class TestExecutorStart(ZuulTestCase):
 
     def test_executor_start(self):
         self.assertFalse(os.path.exists(self.junk_dir))
+
+
+class TestExecutorExtraPackages(AnsibleZuulTestCase):
+    tenant_config_file = 'config/single-tenant/main.yaml'
+
+    test_package = 'pywinrm'
+
+    def setUp(self):
+        super(TestExecutorExtraPackages, self).setUp()
+        import subprocess
+        ansible_manager = self.executor_server.ansible_manager
+        for version in ansible_manager._supported_versions:
+            command = [ansible_manager.getAnsibleCommand(version, 'pip'),
+                       'uninstall', '-y', self.test_package]
+            subprocess.run(command)
+
+    @mock.patch('zuul.lib.ansible.ManagedAnsible.extra_packages',
+                new_callable=mock.PropertyMock)
+    def test_extra_packages(self, mock_extra_packages):
+        mock_extra_packages.return_value = [self.test_package]
+        ansible_manager = self.executor_server.ansible_manager
+        self.assertFalse(ansible_manager.validate())
+        ansible_manager.install()
+        self.assertTrue(ansible_manager.validate())

tests/unit/test_lib_ansible.py

@@ -13,7 +13,6 @@
 # under the License.
 
 import collections
-import subprocess
 from unittest import mock
 
 from tests.base import BaseTestCase
@@ -23,8 +22,11 @@ from zuul.lib.ansible import AnsibleManager
 class TestLibAnsibleManager(BaseTestCase):
 
     @mock.patch('zuul.lib.ansible.AnsibleManager.load_ansible_config')
-    @mock.patch('zuul.lib.ansible.AnsibleManager.getAnsibleCommand')
+    @mock.patch('zuul.lib.ansible.AnsibleManager._validate_packages')
-    def test_validate_remembers_failures(self, getAnsibleCommand, _):
+    @mock.patch('zuul.lib.ansible.AnsibleManager._validate_ansible')
+    def test_validate_remembers_failures(self,
+                                         mock_validate_ansible,
+                                         mock_validate_packages, _):
 
         okish = mock.Mock(
             'subprocess.CompletedProcess',
@@ -33,17 +35,22 @@ class TestLibAnsibleManager(BaseTestCase):
 
         am = AnsibleManager()
         am._supported_versions = collections.OrderedDict([
-            ('1.0', subprocess.CalledProcessError(1, 'fake failure')),
+            ('1.0', False),
-            ('2.8', okish),
+            ('2.8', True),
         ])
 
-        with mock.patch('subprocess.run') as ansible:
+        mock_validate_packages.side_effect = am._supported_versions.values()
-            ansible.side_effect = am._supported_versions.values()
+        mock_validate_ansible.side_effect = am._supported_versions.values()
-            self.assertFalse(
+        self.assertFalse(
-                am.validate(),
+            am.validate(),
-                'A valid ansible should not mask a previous failure')
+            'A valid ansible should not mask a previous failure')
+
         self.assertEquals(
-            [mock.call('1.0', 'ansible'),
+            [mock.call('1.0'),
-             mock.call('2.8', 'ansible'),
+             mock.call('2.8')
             ],
-            getAnsibleCommand.mock_calls)
+            mock_validate_ansible.mock_calls)
+
+        self.assertEquals(
+            [mock.call('2.8')],
+            mock_validate_packages.mock_calls)

zuul/lib/ansible.py

@@ -202,29 +202,54 @@ class AnsibleManager:
             for future in concurrent.futures.as_completed(futures):
                 future.result()
 
+    def _validate_ansible(self, version):
+        result = True
+        try:
+            command = [
+                self.getAnsibleCommand(version, 'ansible'),
+                '--version',
+            ]
+
+            ret = subprocess.run(command,
+                                 stdout=subprocess.PIPE,
+                                 stderr=subprocess.PIPE,
+                                 check=True)
+            self.log.info('Ansible version %s information: \n%s',
+                          version, ret.stdout.decode())
+        except subprocess.CalledProcessError:
+            result = False
+            self.log.exception("Ansible version %s not working" % version)
+        except Exception:
+            result = False
+            self.log.exception(
+                'Ansible version %s not installed' % version)
+        return result
+
+    def _validate_packages(self, version):
+        result = True
+        try:
+            extra_packages = self._getAnsible(version).extra_packages
+            python_package_check = \
+                "import pkg_resources; pkg_resources.require({})".format(
+                    repr(extra_packages))
+
+            command = [self.getAnsibleCommand(version, 'python'),
+                       '-c', python_package_check]
+            subprocess.run(command, check=True)
+        except Exception:
+            result = False
+            self.log.exception(
+                'Ansible version %s installation is missing packages' %
+                version)
+        return result
+
     def validate(self):
         result = True
         for version in self._supported_versions:
-            try:
+            if not self._validate_ansible(version):
-                command = [
-                    self.getAnsibleCommand(version, 'ansible'),
-                    '--version',
-                ]
-
-                ret = subprocess.run(command,
-                                     stdout=subprocess.PIPE,
-                                     stderr=subprocess.PIPE,
-                                     check=True)
-                self.log.info('Ansible version %s information: \n%s',
-                              version, ret.stdout.decode())
-            except subprocess.CalledProcessError:
                 result = False
-                self.log.exception("Ansible version %s not working" % version)
+            elif not self._validate_packages(version):
-            except Exception:
                 result = False
-                self.log.exception(
-                    'Ansible version %s not installed' % version)
-
         return result
 
     def _getAnsible(self, version):