Allow loading additional variables file for site config

It would be useful to allow deployment specific configuration that can
be fed into the project-config deployments so that we can customize
things like host ip without having to change job definitions for each
site.

Also, add a method to display the build log from a failed assertion in
the Ansible test (this was used in the development of the tests for
this change).

Change-Id: I87e8bffc540bcafab543c46244f3d5327b56fcae
Co-Authored-By: James E. Blair <jeblair@redhat.com>
diff --git a/doc/source/admin/components.rst b/doc/source/admin/components.rst
index cc9d181..99912d7 100644
--- a/doc/source/admin/components.rst
+++ b/doc/source/admin/components.rst
@@ -317,6 +317,24 @@
 
      user=zuul
 
+.. _admin_sitewide_variables:
+
+**variables**
+  Path to an Ansible variables file to supply site-wide variables.
+  This should be a YAML-formatted file consisting of a single
+  dictionary.  The contents will be made available to all jobs as
+  Ansible variables.  These variables take precedence over all other
+  forms (job variables and secrets).  Care should be taken when naming
+  these variables to avoid potential collisions with those used by
+  jobs.  Prefixing variable names with a site-specific identifier is
+  recommended.  The default is not to add any site-wide variables.
+  See the :ref:`User's Guide <user_sitewide_variables>` for more
+  information.
+
+  Example::
+
+     variables=/etc/zuul/variables.yaml
+
 merger
 """"""
 
diff --git a/doc/source/user/jobs.rst b/doc/source/user/jobs.rst
index a367aa0..1812bc5 100644
--- a/doc/source/user/jobs.rst
+++ b/doc/source/user/jobs.rst
@@ -73,6 +73,24 @@
 Variables
 ---------
 
+There are several sources of variables which are available to Ansible:
+variables defined in jobs, secrets, and site-wide variables.  The
+order of precedence is:
+
+* Site-wide variables
+
+* Secrets
+
+* Job variables
+
+Meaning that a site-wide variable with the same name as any other will
+override its value, and similarly, secrets override job variables of
+the same name.  Each of the three sources is described below.
+
+
+Job Variables
+~~~~~~~~~~~~~
+
 Any variables specified in the job definition are available as Ansible
 host variables.  They are added to the `vars` section of the inventory
 file under the `all` hosts group, so they are available to all hosts.
@@ -293,6 +311,19 @@
 **zuul.executor.log_root**
   The path to the logs directory.
 
+
+.. _user_sitewide_variables:
+
+Site-wide Variables
+~~~~~~~~~~~~~~~~~~~
+
+The Zuul administrator may define variables which will be available to
+all jobs running in the system.  These are statically defined and may
+not be altered by jobs.  See the :ref:`Administrator's Guide
+<admin_sitewide_variables>` for information on how a site
+administrator may define these variables.
+
+
 SSH Keys
 --------
 
diff --git a/tests/base.py b/tests/base.py
index 2c478ad..98f4c7f 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -16,6 +16,7 @@
 # under the License.
 
 import configparser
+from contextlib import contextmanager
 import datetime
 import gc
 import hashlib
@@ -2769,6 +2770,24 @@
     """ZuulTestCase but with an actual ansible executor running"""
     run_ansible = True
 
+    @contextmanager
+    def jobLog(self, build):
+        """Print job logs on assertion errors
+
+        This method is a context manager which, if it encounters an
+        ecxeption, adds the build log to the debug output.
+
+        :arg Build build: The build that's being asserted.
+        """
+        try:
+            yield
+        except Exception:
+            path = os.path.join(self.test_root, build.uuid,
+                                'work', 'logs', 'job-output.txt')
+            with open(path) as f:
+                self.log.debug(f.read())
+            raise
+
 
 class SSLZuulTestCase(ZuulTestCase):
     """ZuulTestCase but using SSL when possible"""
diff --git a/tests/fixtures/config/ansible/git/common-config/playbooks/check-vars.yaml b/tests/fixtures/config/ansible/git/common-config/playbooks/check-vars.yaml
index ce392a4..3f62c4c 100644
--- a/tests/fixtures/config/ansible/git/common-config/playbooks/check-vars.yaml
+++ b/tests/fixtures/config/ansible/git/common-config/playbooks/check-vars.yaml
@@ -20,3 +20,13 @@
           - zuul.project.name == 'org/project'
           - zuul.project.canonical_hostname == 'review.example.com'
           - zuul.project.canonical_name == 'review.example.com/org/project'
+
+    - debug:
+        msg: "vartest secret {{ vartest_secret }}"
+
+    - name: Assert variable precedence.
+      assert:
+        that:
+          - vartest_job == 'vartest_job'
+          - vartest_secret.value == 'vartest_secret'
+          - vartest_site == 'vartest_site'
\ No newline at end of file
diff --git a/tests/fixtures/config/ansible/git/common-config/zuul.yaml b/tests/fixtures/config/ansible/git/common-config/zuul.yaml
index 1a1b22f..f050c6f 100644
--- a/tests/fixtures/config/ansible/git/common-config/zuul.yaml
+++ b/tests/fixtures/config/ansible/git/common-config/zuul.yaml
@@ -48,6 +48,27 @@
         Z3QSO1NjbBxWnaHKZYT7nkrJm8AMCgZU0ZArFLpaufKCeiK5ECSsDxic4FIsY1OkWT42qEUfL0Wd
         +150AKGNZpPJnnP3QYY4W/MWcKH/zdO400+zWN52WevbSqZy90tqKDJrBkMl1ydqbuw1E4ZHvIs=
 
+# This is used by the check-vars job to evaluate variable precedence.
+# The name of this secret conflicts with a site variable.
+- secret:
+    name: vartest_site
+    data:
+      value: vartest_secret
+
+# This is used by the check-vars job to evaluate variable precedence.
+# The name of this secret conflicts with a job variable.
+- secret:
+    name: vartest_job
+    data:
+      value: vartest_secret
+
+# This is used by the check-vars job to evaluate variable precedence.
+# The name of this secret should not conflict.
+- secret:
+    name: vartest_secret
+    data:
+      value: vartest_secret
+
 - job:
     name: base-urls
     success-url: https://success.example.com/zuul-logs/{build.uuid}/
@@ -77,6 +98,14 @@
     nodes:
       - name: ubuntu-xenial
         label: ubuntu-xenial
+    vars:
+      vartest_job: vartest_job
+      vartest_secret: vartest_job
+      vartest_site: vartest_job
+    auth:
+      secrets:
+        - vartest_site
+        - vartest_secret
 
 - job:
     parent: base-urls
diff --git a/tests/fixtures/config/ansible/variables.yaml b/tests/fixtures/config/ansible/variables.yaml
new file mode 100644
index 0000000..692eb7f
--- /dev/null
+++ b/tests/fixtures/config/ansible/variables.yaml
@@ -0,0 +1 @@
+vartest_site: vartest_site
diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py
index fb80660..ad0b2cc 100644
--- a/tests/unit/test_v3.py
+++ b/tests/unit/test_v3.py
@@ -595,54 +595,70 @@
     tenant_config_file = 'config/ansible/main.yaml'
 
     def test_playbook(self):
+        # Keep the jobdir around so we can inspect contents if an
+        # assert fails.
+        self.executor_server.keep_jobdir = True
+        # Output extra ansible info so we might see errors.
+        self.executor_server.verbose = True
+        # Add a site variables file, used by check-vars
+        path = os.path.join(FIXTURE_DIR, 'config', 'ansible',
+                            'variables.yaml')
+        self.config.set('executor', 'variables', path)
         A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
         self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
         self.waitUntilSettled()
         build_timeout = self.getJobFromHistory('timeout')
-        self.assertEqual(build_timeout.result, 'TIMED_OUT')
+        with self.jobLog(build_timeout):
+            self.assertEqual(build_timeout.result, 'TIMED_OUT')
         build_faillocal = self.getJobFromHistory('faillocal')
-        self.assertEqual(build_faillocal.result, 'FAILURE')
+        with self.jobLog(build_faillocal):
+            self.assertEqual(build_faillocal.result, 'FAILURE')
         build_failpost = self.getJobFromHistory('failpost')
-        self.assertEqual(build_failpost.result, 'POST_FAILURE')
+        with self.jobLog(build_failpost):
+            self.assertEqual(build_failpost.result, 'POST_FAILURE')
         build_check_vars = self.getJobFromHistory('check-vars')
-        self.assertEqual(build_check_vars.result, 'SUCCESS')
+        with self.jobLog(build_check_vars):
+            self.assertEqual(build_check_vars.result, 'SUCCESS')
         build_hello = self.getJobFromHistory('hello-world')
-        self.assertEqual(build_hello.result, 'SUCCESS')
+        with self.jobLog(build_hello):
+            self.assertEqual(build_hello.result, 'SUCCESS')
         build_python27 = self.getJobFromHistory('python27')
-        self.assertEqual(build_python27.result, 'SUCCESS')
-        flag_path = os.path.join(self.test_root, build_python27.uuid + '.flag')
-        self.assertTrue(os.path.exists(flag_path))
-        copied_path = os.path.join(self.test_root, build_python27.uuid +
-                                   '.copied')
-        self.assertTrue(os.path.exists(copied_path))
-        failed_path = os.path.join(self.test_root, build_python27.uuid +
-                                   '.failed')
-        self.assertFalse(os.path.exists(failed_path))
-        pre_flag_path = os.path.join(self.test_root, build_python27.uuid +
-                                     '.pre.flag')
-        self.assertTrue(os.path.exists(pre_flag_path))
-        post_flag_path = os.path.join(self.test_root, build_python27.uuid +
-                                      '.post.flag')
-        self.assertTrue(os.path.exists(post_flag_path))
-        bare_role_flag_path = os.path.join(self.test_root,
-                                           build_python27.uuid +
-                                           '.bare-role.flag')
-        self.assertTrue(os.path.exists(bare_role_flag_path))
+        with self.jobLog(build_python27):
+            self.assertEqual(build_python27.result, 'SUCCESS')
+            flag_path = os.path.join(self.test_root,
+                                     build_python27.uuid + '.flag')
+            self.assertTrue(os.path.exists(flag_path))
+            copied_path = os.path.join(self.test_root, build_python27.uuid +
+                                       '.copied')
+            self.assertTrue(os.path.exists(copied_path))
+            failed_path = os.path.join(self.test_root, build_python27.uuid +
+                                       '.failed')
+            self.assertFalse(os.path.exists(failed_path))
+            pre_flag_path = os.path.join(self.test_root, build_python27.uuid +
+                                         '.pre.flag')
+            self.assertTrue(os.path.exists(pre_flag_path))
+            post_flag_path = os.path.join(self.test_root, build_python27.uuid +
+                                          '.post.flag')
+            self.assertTrue(os.path.exists(post_flag_path))
+            bare_role_flag_path = os.path.join(self.test_root,
+                                               build_python27.uuid +
+                                               '.bare-role.flag')
+            self.assertTrue(os.path.exists(bare_role_flag_path))
+            secrets_path = os.path.join(self.test_root,
+                                        build_python27.uuid + '.secrets')
+            with open(secrets_path) as f:
+                self.assertEqual(f.read(), "test-username test-password")
 
-        secrets_path = os.path.join(self.test_root,
-                                    build_python27.uuid + '.secrets')
-        with open(secrets_path) as f:
-            self.assertEqual(f.read(), "test-username test-password")
-
-        msg = A.messages[0]
-        success = "{} https://success.example.com/zuul-logs/{}"
-        fail = "{} https://failure.example.com/zuul-logs/{}"
-        self.assertIn(success.format("python27", build_python27.uuid), msg)
-        self.assertIn(fail.format("faillocal", build_faillocal.uuid), msg)
-        self.assertIn(success.format("check-vars", build_check_vars.uuid), msg)
-        self.assertIn(success.format("hello-world", build_hello.uuid), msg)
-        self.assertIn(fail.format("timeout", build_timeout.uuid), msg)
-        self.assertIn(fail.format("failpost", build_failpost.uuid), msg)
+            msg = A.messages[0]
+            success = "{} https://success.example.com/zuul-logs/{}"
+            fail = "{} https://failure.example.com/zuul-logs/{}"
+            self.assertIn(success.format("python27", build_python27.uuid), msg)
+            self.assertIn(fail.format("faillocal", build_faillocal.uuid), msg)
+            self.assertIn(success.format("check-vars",
+                                         build_check_vars.uuid), msg)
+            self.assertIn(success.format("hello-world", build_hello.uuid), msg)
+            self.assertIn(fail.format("timeout", build_timeout.uuid), msg)
+            self.assertIn(fail.format("failpost", build_failpost.uuid), msg)
 
 
 class TestPrePlaybooks(AnsibleZuulTestCase):
diff --git a/zuul/driver/bubblewrap/__init__.py b/zuul/driver/bubblewrap/__init__.py
index ee81bb1..ec94d4b 100644
--- a/zuul/driver/bubblewrap/__init__.py
+++ b/zuul/driver/bubblewrap/__init__.py
@@ -82,7 +82,7 @@
         pass
 
     def setMountsMap(self, ro_dirs=[], rw_dirs=[]):
-        self.mounts_map = {'ro': ro_dirs, 'rw': [] + rw_dirs}
+        self.mounts_map = {'ro': ro_dirs, 'rw': rw_dirs}
 
     def getPopen(self, **kwargs):
         # Set zuul_dir if it was not passed in
diff --git a/zuul/executor/server.py b/zuul/executor/server.py
index 9a07b5a..ea5a37c 100644
--- a/zuul/executor/server.py
+++ b/zuul/executor/server.py
@@ -757,6 +757,12 @@
                                             '~/.ssh/id_rsa')
         self.ssh_agent = SshAgent()
 
+        self.executor_variables_file = None
+
+        if self.executor_server.config.has_option('executor', 'variables'):
+            self.executor_variables_file = self.executor_server.config.get(
+                'executor', 'variables')
+
     def run(self):
         self.running = True
         self.thread = threading.Thread(target=self.execute)
@@ -1375,6 +1381,9 @@
 
         ro_dirs.append(self.executor_server.ansible_dir)
 
+        if self.executor_variables_file:
+            ro_dirs.append(self.executor_variables_file)
+
         self.executor_server.execution_wrapper.setMountsMap(ro_dirs,
                                                             rw_dirs)
 
@@ -1477,6 +1486,9 @@
             % playbook.canonical_name_and_path])
         cmd.extend(['-e', 'zuul_execution_branch=%s' % str(playbook.branch)])
 
+        if self.executor_variables_file is not None:
+            cmd.extend(['-e@%s' % self.executor_variables_file])
+
         result, code = self.runAnsible(
             cmd=cmd, timeout=timeout,
             config_file=playbook.ansible_config,