Add support for Ansible extra-vars flag

Currently, variables using the extra-vars flags always win precedence
over any other variable in ansible. There is also a 2nd use case where
playbooks variables for serial, hosts, etc can only be set using
extra-vars CLI flag.

While this could be achieved by using secrets today, it doesn't feel
like the correct way to use them. Additionally, secrets are
dictionary values in ansible, making them hard to use the filters like
default().

Change-Id: I6d8018661f8d13b7324a012cdbf9614e983e5114
Signed-off-by: Paul Belanger <pabelanger@redhat.com>

doc/source/user/config.rst

@@ -910,6 +910,12 @@ Here is an example of two job definitions:
       same name will override a previously defined variable, but new
       variable names will be added to the set of defined variables.
 
+   .. attr:: extra-vars
+
+      A dictionary of variables to be passed to ansible command-line
+      using the --extra-vars flag. Note by using extra-vars, these
+      variables always win precedence.
+
    .. attr:: host-vars
 
       A dictionary of host variables to supply to Ansible.  The keys

doc/source/user/jobs.rst

@@ -86,6 +86,7 @@ variables defined in jobs, secrets, and site-wide variables.  The
 order of precedence is:
 
 #. :ref:`Site-wide variables <user_jobs_sitewide_variables>`
+#. :ref:`Job extra variables <user_jobs_job_extra_variables>`
 #. :ref:`Secrets <user_jobs_secrets>`
 #. :ref:`Job variables <user_jobs_job_variables>`
 #. :ref:`Parent job results <user_jobs_parent_results>`
@@ -106,6 +107,14 @@ 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.
 
+.. _user_jobs_job_extra_variables:
+
+Job Extra Variables
+~~~~~~~~~~~~~~~~~~~
+
+Any extra variables in the job definition (using the :attr:`job.extra-vars`
+attribute) are available to Ansible but not added into the inventory file.
+
 .. _user_jobs_secrets:
 
 Secrets

releasenotes/notes/job-extra-vars-9948be1ac2f99497.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    Jobs are now able to use the :attr:`job.extra-vars` which will
+    use the --extra-vars flag when a job runs.

tests/fixtures/config/ansible/git/common-config/playbooks/check-vars.yaml

@@ -41,10 +41,14 @@
     - debug:
         msg: "vartest secret {{ vartest_secret }}"
 
+    - debug:
+        msg: "vartest extra {{ vartest_extra }}"
+
     - name: Assert variable precedence.
       assert:
         that:
           - vartest_job == 'vartest_job'
+          - vartest_extra == 'vartest_extra'
           - vartest_secret.value == 'vartest_secret'
           - vartest_site == 'vartest_site'
           - base_var == 'base_var'

tests/fixtures/config/ansible/git/common-config/zuul.yaml

@@ -62,6 +62,13 @@
     data:
       value: vartest_secret
 
+# This is used by the check-vars job to evaluate variable precedence.
+# The name of this secret conflicts with an extra variable.
+- secret:
+    name: vartest_extra
+    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:
@@ -114,10 +121,15 @@
         - name: ubuntu-xenial
           label: ubuntu-xenial
     vars:
+      vartest_extra: vartest_job
       vartest_job: vartest_job
       vartest_secret: vartest_job
       vartest_site: vartest_job
+    extra-vars:
+      vartest_extra: vartest_extra
+      vartest_site: vartest_extra
     secrets:
+      - vartest_extra
       - vartest_site
       - vartest_secret
 

zuul/configloader.py

@@ -538,6 +538,7 @@ class JobParser(object):
                       'roles': to_list(role),
                       'required-projects': to_list(vs.Any(job_project, str)),
                       'vars': dict,
+                      'extra-vars': dict,
                       'host-vars': {str: dict},
                       'group-vars': {str: dict},
                       'dependencies': to_list(str),
@@ -730,6 +731,12 @@ class JobParser(object):
                 raise Exception("Variables named 'zuul' or 'nodepool' "
                                 "are not allowed.")
             job.variables = variables
+        extra_variables = conf.get('extra-vars', None)
+        if extra_variables:
+            if 'zuul' in extra_variables or 'nodepool' in extra_variables:
+                raise Exception("Variables named 'zuul' or 'nodepool' "
+                                "are not allowed.")
+            job.extra_variables = extra_variables
         host_variables = conf.get('host-vars', None)
         if host_variables:
             for host, hvars in host_variables.items():

zuul/executor/client.py

@@ -231,6 +231,7 @@ class ExecutorClient(object):
         params['nodes'] = nodes
         params['groups'] = [group.toDict() for group in nodeset.getGroups()]
         params['vars'] = job.variables
+        params['extra_vars'] = job.extra_variables
         params['host_vars'] = job.host_variables
         params['group_vars'] = job.group_variables
         params['zuul'] = zuul_params

zuul/executor/server.py

@@ -296,6 +296,7 @@ class JobDir(object):
         #   ansible (mounted in bwrap read-only)
         #     logging.json
         #     inventory.yaml
+        #     extra_vars.yaml
         #   .ansible (mounted in bwrap read-write)
         #     fact-cache/localhost
         #     cp
@@ -367,6 +368,7 @@ class JobDir(object):
             pass
         self.known_hosts = os.path.join(ssh_dir, 'known_hosts')
         self.inventory = os.path.join(self.ansible_root, 'inventory.yaml')
+        self.extra_vars = os.path.join(self.ansible_root, 'extra_vars.yaml')
         self.setup_inventory = os.path.join(self.ansible_root,
                                             'setup-inventory.yaml')
         self.logging_json = os.path.join(self.ansible_root, 'logging.json')
@@ -1386,6 +1388,10 @@ class AnsibleJob(object):
                 for key in node['host_keys']:
                     known_hosts.write('%s\n' % key)
 
+        with open(self.jobdir.extra_vars, 'w') as extra_vars:
+            extra_vars.write(
+                yaml.safe_dump(args['extra_vars'], default_flow_style=False))
+
     def writeLoggingConfig(self):
         self.log.debug("Writing logging config for job %s %s",
                        self.jobdir.job_output_file,
@@ -1742,6 +1748,8 @@ class AnsibleJob(object):
         if playbook.secrets_content:
             cmd.extend(['-e', '@' + playbook.secrets])
 
+        cmd.extend(['-e', '@' + self.jobdir.extra_vars])
+
         if success is not None:
             cmd.extend(['-e', 'zuul_success=%s' % str(bool(success))])
 

zuul/model.py

@@ -1012,6 +1012,7 @@ class Job(ConfigObject):
             timeout=None,
             post_timeout=None,
             variables={},
+            extra_variables={},
             host_variables={},
             group_variables={},
             nodeset=NodeSet(),
@@ -1194,9 +1195,13 @@ class Job(ConfigObject):
             matchers.append(self.branch_matcher)
         self.branch_matcher = change_matcher.MatchAll(matchers)
 
-    def updateVariables(self, other_vars, other_host_vars, other_group_vars):
+    def updateVariables(self, other_vars, other_extra_vars, other_host_vars,
+                        other_group_vars):
         if other_vars is not None:
             self.variables = Job._deepUpdate(self.variables, other_vars)
+        if other_extra_vars is not None:
+            self.extra_variables = Job._deepUpdate(
+                self.extra_variables, other_extra_vars)
         if other_host_vars is not None:
             self.host_variables = Job._deepUpdate(
                 self.host_variables, other_host_vars)
@@ -1288,9 +1293,9 @@ class Job(ConfigObject):
                                         "from other projects."
                                         % (repr(self), this_origin))
                 if k not in set(['pre_run', 'run', 'post_run', 'roles',
-                                 'variables', 'host_variables',
-                                 'group_variables', 'required_projects',
-                                 'allowed_projects']):
+                                 'variables', 'extra_variables',
+                                 'host_variables', 'group_variables',
+                                 'required_projects', 'allowed_projects']):
                     setattr(self, k, other._get(k))
 
         # Don't set final above so that we don't trip an error halfway
@@ -1332,8 +1337,8 @@ class Job(ConfigObject):
         if other._get('post_run') is not None:
             other_post_run = self.freezePlaybooks(other.post_run, layout)
             self.post_run = other_post_run + self.post_run
-        self.updateVariables(other.variables, other.host_variables,
-                             other.group_variables)
+        self.updateVariables(other.variables, other.extra_variables,
+                             other.host_variables, other.group_variables)
         if other._get('required_projects') is not None:
             self.updateProjects(other.required_projects)
         if (other._get('allowed_projects') is not None and