Don't set ansible_python_interpreter if in vars

Zuul always sets ansible_python_interpreter as a host var.  However
a user may want to set that as a regular var (to apply to the all
group) or a group var.  If that happens, disable Zuul's own setting
of the value. Note that users can still override the all-var or
group-var with a host-var of their own.

Change-Id: Id130ec1718efa25b260b39ea0587ec5794e8e2cf

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

@@ -75,11 +75,16 @@
     name: group-inventory
     nodeset: nodeset1
     run: playbooks/group-inventory.yaml
+    group-vars:
+      ceph-osd:
+        ansible_python_interpreter: python4
 
 - job:
     name: hostvars-inventory
     run: playbooks/hostvars-inventory.yaml
     nodeset: nodeset2
+    vars:
+      ansible_python_interpreter: python1.5.2
 
 - job:
     name: jinja2-message

tests/unit/test_executor.py

@@ -443,13 +443,19 @@ class TestAnsibleJob(ZuulTestCase):
                 'host_keys': ['fake-host-key'],
                 'interface_ip': 'localhost'}
         keys = self.test_job.getHostList({'nodes': [node],
-                                          'host_vars': {}})[0]['host_keys']
+                                          'host_vars': {},
+                                          'vars': {},
+                                          'groups': [],
+                                          })[0]['host_keys']
         self.assertEqual(keys[0], 'localhost fake-host-key')
 
         # Test with custom connection_port set
         node['connection_port'] = 22022
         keys = self.test_job.getHostList({'nodes': [node],
-                                          'host_vars': {}})[0]['host_keys']
+                                          'host_vars': {},
+                                          'vars': {},
+                                          'groups': [],
+                                          })[0]['host_keys']
         self.assertEqual(keys[0], '[localhost]:22022 fake-host-key')
 
 

tests/unit/test_inventory.py

@@ -212,6 +212,17 @@ class TestInventory(TestInventoryBase):
             self.assertIn(node_name,
                           inventory['all']['children']
                           ['ceph-monitor']['hosts'])
+        self.assertNotIn(
+            'ansible_python_interpreter',
+            inventory['all']['hosts']['controller'])
+        self.assertEqual(
+            'auto',
+            inventory['all']['hosts']['compute1']
+            ['ansible_python_interpreter'])
+        self.assertEqual(
+            'python4',
+            inventory['all']['children']['ceph-osd']['vars']
+            ['ansible_python_interpreter'])
         self.assertIn('zuul', inventory['all']['vars'])
         z_vars = inventory['all']['vars']['zuul']
         self.assertIn('executor', z_vars)
@@ -250,6 +261,13 @@ class TestInventory(TestInventoryBase):
                     'local',
                     inventory['all']['hosts'][node_name]['ansible_connection'])
 
+            self.assertNotIn(
+                'ansible_python_interpreter',
+                inventory['all']['hosts'][node_name])
+        self.assertEqual(
+            'python1.5.2',
+            inventory['all']['vars']['ansible_python_interpreter'])
+
         self.executor_server.release()
         self.waitUntilSettled()
 

zuul/executor/server.py

@@ -651,6 +651,15 @@ def make_setup_inventory_dict(nodes):
     return inventory
 
 
+def is_group_var_set(name, host, args):
+    for group in args['groups']:
+        if host in group['nodes']:
+            group_vars = args['group_vars'].get(group['name'], {})
+            if name in group_vars:
+                return True
+    return False
+
+
 def make_inventory_dict(nodes, args, all_vars):
     hosts = {}
     for node in nodes:
@@ -1377,15 +1386,21 @@ class AnsibleJob(object):
                 # Fedoras).  For "auto" with prior versions, fall back
                 # to the old default of /usr/bin/python2 for backwards
                 # compatability.
-                python = node.get('python_path', 'auto')
-                compat = self.arguments.get('ansible_version') in \
-                    ('2.5', '2.6', '2.7')
-                if python == "auto" and compat:
-                    self.log.debug(
-                        "ansible_version set to auto but "
-                        "overriding to python2 for Ansible <2.8")
-                    python = '/usr/bin/python2'
-                host_vars.setdefault('ansible_python_interpreter', python)
+                # If ansible_python_interpreter is set either as a group
+                # var or all-var, then don't do anything here; let the
+                # user control.
+                api = 'ansible_python_interpreter'
+                if (api not in args['vars'] and
+                    not is_group_var_set(api, name, args)):
+                    python = node.get('python_path', 'auto')
+                    compat = self.arguments.get('ansible_version') in \
+                        ('2.5', '2.6', '2.7')
+                    if python == "auto" and compat:
+                        self.log.debug(
+                            "ansible_version set to auto but "
+                            "overriding to python2 for Ansible <2.8")
+                        python = '/usr/bin/python2'
+                    host_vars.setdefault(api, python)
 
                 username = node.get('username')
                 if username: