From 93d1d3be17b1f5dc9aeb40beebff4a2bec42311d Mon Sep 17 00:00:00 2001 From: Ian Wienand Date: Mon, 16 Sep 2019 15:28:01 +1000 Subject: [PATCH] Support nodes setting 'auto' python-path The nodepool "python-path" config variable makes it's way through from the node arguments and ends up as the "ansible_python_interpreter" variable for the inventory when running the job. Notably, Python 3 only distributions require this to be set to /usr/bin/python3 to avoid what can often be confusing red-herring errors (e.g. things like dnf packages incorrectly appearing to be missing on Fedora, for example [1]). Upstream is aware of this often confusing behaviour and has made an "ansible_python_interpreter" value of "auto" to, essentially, "do the right thing" [2] and choose the right python for the target environment. This is available in Ansible >=2.8 and will become default in 2.12. This allows, and defaults to, an interpreter value of "auto" when running with Ansible >=2.8. On the supported prior Ansible releases, "auto" will be translated into "/usr/bin/python2" to maintain backwards compatability. Of course a node explicity setting "python-path" already will override this. Nodepool is updated to set this by default with I02a1a618c8806b150049e91b644ec3c0cb826ba4. I think this is much more user friendly as it puts the work of figuring out what platform has what interpreter into Ansible. It alleviates the need for admins to know anything at all about "python-path" for node configurations unless they are actually doing something out of the ordinary like using a virtualenv. At the moment, if you put a modern Python-3 only distro into nodepool, Zuul always does the wrong thing by selecting /usr/bin/python2; you are left to debug the failures and need to know to go and manually update the python-path to Python 3. Documentation is updated. Detailed discussion is moved into the executor section; the README is simplified a bit to avoid confusion. A release note is added. A test-case is added. Note that it is also self-testing in that jobs using Ansible 2.8 use the updated value (c.f. I7cdcfc760975871f7fa9949da1015d7cec92ee67) [1] https://bugzilla.redhat.com/show_bug.cgi?id=1696404 [2] https://docs.ansible.com/ansible/2.8/reference_appendices/interpreter_discovery.html Change-Id: I2b3bc6d4f873b7d653cfaccd1598464583c561e7 --- README.rst | 9 ++-- doc/source/admin/components.rst | 36 +++++++++++++ .../ansible-interp-auto-4b4ecb4678c11470.yaml | 7 +++ tests/base.py | 2 +- .../playbooks/ansible-version.yaml | 2 + .../inventory/git/common-config/zuul.yaml | 16 ++++++ .../inventory/git/org_project/.zuul.yaml | 2 + tests/unit/test_inventory.py | 51 +++++++++++++++++++ zuul/executor/server.py | 20 ++++++-- 9 files changed, 136 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/ansible-interp-auto-4b4ecb4678c11470.yaml create mode 100644 tests/fixtures/config/inventory/git/common-config/playbooks/ansible-version.yaml diff --git a/README.rst b/README.rst index fb95f3f4dd..7b737ca175 100644 --- a/README.rst +++ b/README.rst @@ -64,9 +64,8 @@ at the tops of individual source files. Python Version Support ---------------------- -Zuul v3 requires Python 3. It does not support Python 2. +Zuul requires Python 3. It does not support Python 2. + +Since Zuul uses Ansible to drive CI jobs, Zuul can run tests anywhere +Ansible can, including Python 2 environments. -As Ansible is used for the execution of jobs, it's important to note that -while Ansible does support Python 3, not all of Ansible's modules do. Zuul -currently sets ``ansible_python_interpreter`` to python2 so that remote -content will be executed with Python 2. diff --git a/doc/source/admin/components.rst b/doc/source/admin/components.rst index 7cba2516b7..a0722c0565 100644 --- a/doc/source/admin/components.rst +++ b/doc/source/admin/components.rst @@ -781,6 +781,42 @@ To enable or disable running Ansible in verbose mode (with the ``-vvv`` argument to ansible-playbook) run ``zuul-executor verbose`` and ``zuul-executor unverbose``. +Ansible and Python 3 +~~~~~~~~~~~~~~~~~~~~ + +As noted above, the executor runs Ansible playbooks against the remote +node(s) allocated for the job. Since part of executing playbooks on +remote hosts is running Python scripts on them, Ansible needs to know +what Python interpreter to use on the remote host. With older +distributions, ``/usr/bin/python2`` was a generally sensible choice. +However, over time a heterogeneous Python ecosystem has evolved where +older distributions may only provide Python 2, most provide a mixed +2/3 environment and newer distributions may only provide Python 3 (and +then others like RHEL8 may even have separate "system" Python versions +to add to confusion!). + +Ansible's ``ansible_python_interpreter`` variable configures the path +to the remote Python interpreter to use during playbook execution. +This value is set by Zuul from the ``python-path`` specified for the +node by Nodepool; see the `nodepool configuration documentation +`__. + +This defaults to ``auto``, where Ansible will automatically discover +the interpreter available on the remote host. However, this setting +only became available in Ansible >=2.8, so Zuul will translate +``auto`` into the old default of ``/usr/bin/python2`` when configured +to use older Ansible versions. + +Thus for modern Python 3-only hosts no further configuration is needed +when using Ansible >=2.8 (e.g. Fedora, Bionic onwards). If using +earlier Ansible versions you may need to explicitly set the +``python-path`` if ``/usr/bin/python2`` is not available on the node. + +Ansible roles/modules which include Python code are generally Python 3 +safe now, but there is still a small possibility of incompatibility. +See also the Ansible `Python 3 support page +`__. + .. _web-server: Web Server diff --git a/releasenotes/notes/ansible-interp-auto-4b4ecb4678c11470.yaml b/releasenotes/notes/ansible-interp-auto-4b4ecb4678c11470.yaml new file mode 100644 index 0000000000..6a0be626d9 --- /dev/null +++ b/releasenotes/notes/ansible-interp-auto-4b4ecb4678c11470.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Nodes can set their ``python-path`` in nodepool configuration to + ``auto`` to allow for `automatic ansible_python_interpreter + discovery `__ + when using Ansible >=2.8. diff --git a/tests/base.py b/tests/base.py index 885a070d04..88f340340b 100644 --- a/tests/base.py +++ b/tests/base.py @@ -2187,7 +2187,7 @@ class FakeNodepool(object): self.remote_ansible = False self.attributes = None self.resources = None - self.python_path = '/usr/bin/python2' + self.python_path = 'auto' def stop(self): self._running = False diff --git a/tests/fixtures/config/inventory/git/common-config/playbooks/ansible-version.yaml b/tests/fixtures/config/inventory/git/common-config/playbooks/ansible-version.yaml new file mode 100644 index 0000000000..2bec3d2d19 --- /dev/null +++ b/tests/fixtures/config/inventory/git/common-config/playbooks/ansible-version.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] \ No newline at end of file diff --git a/tests/fixtures/config/inventory/git/common-config/zuul.yaml b/tests/fixtures/config/inventory/git/common-config/zuul.yaml index 7d6b7a62ad..f6ead477d2 100644 --- a/tests/fixtures/config/inventory/git/common-config/zuul.yaml +++ b/tests/fixtures/config/inventory/git/common-config/zuul.yaml @@ -89,4 +89,20 @@ label: ubuntu-xenial run: playbooks/jinja2-message.yaml +- job: + name: ansible-version27-inventory + nodeset: + nodes: + - name: ubuntu-xenial + label: ubuntu-xenial + ansible-version: '2.7' + run: playbooks/ansible-version.yaml +- job: + name: ansible-version28-inventory + nodeset: + nodes: + - name: ubuntu-xenial + label: ubuntu-xenial + ansible-version: '2.8' + run: playbooks/ansible-version.yaml diff --git a/tests/fixtures/config/inventory/git/org_project/.zuul.yaml b/tests/fixtures/config/inventory/git/org_project/.zuul.yaml index df02c3e8b7..83961d75ae 100644 --- a/tests/fixtures/config/inventory/git/org_project/.zuul.yaml +++ b/tests/fixtures/config/inventory/git/org_project/.zuul.yaml @@ -7,3 +7,5 @@ - executor-only-inventory - group-inventory - hostvars-inventory + - ansible-version27-inventory + - ansible-version28-inventory diff --git a/tests/unit/test_inventory.py b/tests/unit/test_inventory.py index 65944689d2..ef10ee4553 100644 --- a/tests/unit/test_inventory.py +++ b/tests/unit/test_inventory.py @@ -76,6 +76,57 @@ class TestInventoryPythonPath(TestInventoryBase): self.waitUntilSettled() +class TestInventoryAutoPython(TestInventoryBase): + + def test_auto_python_ansible28_inventory(self): + inventory = self._get_build_inventory('ansible-version28-inventory') + + all_nodes = ('ubuntu-xenial',) + self.assertIn('all', inventory) + self.assertIn('hosts', inventory['all']) + self.assertIn('vars', inventory['all']) + for node_name in all_nodes: + self.assertIn(node_name, inventory['all']['hosts']) + node_vars = inventory['all']['hosts'][node_name] + self.assertEqual( + 'auto', node_vars['ansible_python_interpreter']) + + self.assertIn('zuul', inventory['all']['vars']) + z_vars = inventory['all']['vars']['zuul'] + self.assertIn('executor', z_vars) + self.assertIn('src_root', z_vars['executor']) + self.assertIn('job', z_vars) + self.assertEqual(z_vars['job'], 'ansible-version28-inventory') + self.assertEqual(z_vars['message'], 'QQ==') + + self.executor_server.release() + self.waitUntilSettled() + + def test_auto_python_ansible27_inventory(self): + inventory = self._get_build_inventory('ansible-version27-inventory') + + all_nodes = ('ubuntu-xenial',) + self.assertIn('all', inventory) + self.assertIn('hosts', inventory['all']) + self.assertIn('vars', inventory['all']) + for node_name in all_nodes: + self.assertIn(node_name, inventory['all']['hosts']) + node_vars = inventory['all']['hosts'][node_name] + self.assertEqual( + '/usr/bin/python2', node_vars['ansible_python_interpreter']) + + self.assertIn('zuul', inventory['all']['vars']) + z_vars = inventory['all']['vars']['zuul'] + self.assertIn('executor', z_vars) + self.assertIn('src_root', z_vars['executor']) + self.assertIn('job', z_vars) + self.assertEqual(z_vars['job'], 'ansible-version27-inventory') + self.assertEqual(z_vars['message'], 'QQ==') + + self.executor_server.release() + self.waitUntilSettled() + + class TestInventory(TestInventoryBase): def test_single_inventory(self): diff --git a/zuul/executor/server.py b/zuul/executor/server.py index 5becb3fc56..fd58e96456 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -1370,9 +1370,23 @@ class AnsibleJob(object): private_ipv4=node.get('private_ipv4'), public_ipv6=node.get('public_ipv6')))) - host_vars.setdefault( - 'ansible_python_interpreter', - node.get('python_path', '/usr/bin/python2')) + # Ansible >=2.8 introduced "auto" as an + # ansible_python_interpreter argument that looks up + # which python to use on the remote host in an inbuilt + # table and essentially "does the right thing" + # (i.e. chooses python3 on 3-only hosts like later + # 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) username = node.get('username') if username: