From: Zack Cerza Date: Wed, 21 Mar 2018 23:37:32 +0000 (-0600) Subject: task.ansible: Allow passing in custom group_vars X-Git-Tag: 1.1.0~353^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=24a667b71b3424054f385e42f044e982ef4e308c;p=teuthology.git task.ansible: Allow passing in custom group_vars Up until now, if you wanted to inject vars to a playbook run, you had to use --extra-vars, which don't behave the same way that group_vars do. This commit adds that functionality. We look for a 'group_vars' dict in the task's config object. If it's there, we create group_vars files with names taken from the keys, and content taken from the values. Signed-off-by: Zack Cerza --- diff --git a/teuthology/task/ansible.py b/teuthology/task/ansible.py index 3a2c1d9bc..baf5d5a80 100644 --- a/teuthology/task/ansible.py +++ b/teuthology/task/ansible.py @@ -7,7 +7,7 @@ import yaml import shutil from cStringIO import StringIO -from tempfile import NamedTemporaryFile +from tempfile import mkdtemp, NamedTemporaryFile from teuthology.config import config as teuth_config from teuthology.exceptions import CommandFailedError, AnsibleFailedError @@ -64,6 +64,10 @@ class Ansible(Task): directly to ansible-playbook. vars: A dict of vars to be passed to ansible-playbook via the --extra-vars flag + group_vars: A dict with keys matching relevant group names in the + playbook, and values to be written in the corresponding + inventory's group_vars files. Only applies to inventories + generated by this task. cleanup: If present, the given or generated playbook will be run again during teardown with a 'cleanup' var set to True. This will allow the playbook to clean up after itself, @@ -114,7 +118,7 @@ class Ansible(Task): super(Ansible, self).setup() self.find_repo() self.get_playbook() - self.get_inventory() or self.generate_hosts_file() + self.get_inventory() or self.generate_inventory() if not hasattr(self, 'playbook_file'): self.generate_playbook() @@ -187,7 +191,7 @@ class Ansible(Task): self.inventory = etc_ansible_hosts return self.inventory - def generate_hosts_file(self): + def generate_inventory(self): """ Generate a hosts (inventory) file to use. This should not be called if we're using an existing file. @@ -200,21 +204,42 @@ class Ansible(Task): inventory.append('[{0}]'.format(self.inventory_group)) inventory.extend(hostnames + ['']) hosts_str = '\n'.join(inventory) - self.inventory = self._write_hosts_file(hosts_str) + self.inventory = self._write_inventory_files(hosts_str) self.generated_inventory = True - def _write_hosts_file(self, content, suffix=''): + def _write_inventory_files(self, inventory, inv_suffix=''): """ - Actually write the hosts file + Actually write the inventory files. Writes out group_vars files as + necessary based on configuration. + + :param inventory: The content of the inventory file itself, as a + string + :param inv_suffix: The suffix to use for the inventory filename """ - hosts_file = NamedTemporaryFile( - prefix="teuth_ansible_hosts_", - suffix=suffix, - delete=False, + # First, create the inventory directory + inventory_dir = mkdtemp( + prefix="teuth_ansible_inventory", ) - hosts_file.write(content) - hosts_file.flush() - return hosts_file.name + inv_fn = os.path.join(inventory_dir, 'inventory') + if inv_suffix: + inv_fn = '.'.join(inv_fn, inv_suffix) + # Write out the inventory file + inv_file = file(inv_fn, 'w') + inv_file.write(inventory) + # Next, write the group_vars files + all_group_vars = self.config.get('group_vars') + if all_group_vars: + group_vars_dir = os.path.join(inventory_dir, 'group_vars') + os.mkdir(group_vars_dir) + # We loop over a sorted list of keys here because we want our tests + # to be able to mock predictably + for group_name in sorted(all_group_vars): + group_vars = all_group_vars[group_name] + path = os.path.join(group_vars_dir, group_name + '.yml') + gv_file = file(path, 'w') + yaml.safe_dump(group_vars, gv_file) + + return inventory_dir def generate_playbook(self): """ @@ -333,7 +358,7 @@ class Ansible(Task): def teardown(self): self._cleanup() if self.generated_inventory: - os.remove(self.inventory) + shutil.rmtree(self.inventory) if self.generated_playbook: os.remove(self.playbook_file.name) super(Ansible, self).teardown() diff --git a/teuthology/task/cephmetrics.py b/teuthology/task/cephmetrics.py index c8ef573ae..5f68934ed 100644 --- a/teuthology/task/cephmetrics.py +++ b/teuthology/task/cephmetrics.py @@ -23,7 +23,7 @@ class CephMetrics(Ansible): def get_inventory(self): return False - def generate_hosts_file(self): + def generate_inventory(self): groups_to_roles = { 'mons': 'mon', 'mgrs': 'mgr', @@ -65,7 +65,7 @@ class CephMetrics(Ansible): hosts_lines.append(host_line) hosts_lines.append('') hosts_str = '\n'.join(hosts_lines) - self.inventory = self._write_hosts_file(hosts_str) + self.inventory = self._write_inventory_files(hosts_str) self.generated_inventory = True def begin(self): diff --git a/teuthology/test/task/test_ansible.py b/teuthology/test/task/test_ansible.py index f8b2829fa..2fa0aa703 100644 --- a/teuthology/test/task/test_ansible.py +++ b/teuthology/test/task/test_ansible.py @@ -3,7 +3,7 @@ import os import yaml from mock import patch, DEFAULT, Mock -from pytest import raises +from pytest import raises, mark from StringIO import StringIO from teuthology.config import config, FakeNamespace @@ -31,26 +31,38 @@ class TestAnsibleTask(TestTask): self.start_patchers() def start_patchers(self): + self.patchers = dict() + self.mocks = dict() + self.patchers['mkdtemp'] = patch( + 'teuthology.task.ansible.mkdtemp', + ) m_NTF = Mock() m_file = Mock() m_file.name = 'file_name' m_NTF.return_value = m_file - self.patcher_NTF = patch( + self.patchers['NTF'] = patch( 'teuthology.task.ansible.NamedTemporaryFile', m_NTF, ) - self.patcher_NTF.start() - self.patcher_os_remove = patch( + self.patchers['os_mkdir'] = patch( + 'teuthology.task.ansible.os.mkdir', + ) + self.patchers['os_remove'] = patch( 'teuthology.task.ansible.os.remove', ) - self.patcher_os_remove.start() + self.patchers['shutil_rmtree'] = patch( + 'teuthology.task.ansible.shutil.rmtree', + ) + for name, patcher in self.patchers.items(): + if name not in self.mocks: + self.mocks[name] = patcher.start() def teardown(self): self.stop_patchers() def stop_patchers(self): - self.patcher_NTF.stop() - self.patcher_os_remove.stop() + for patcher in self.mocks.values(): + patcher.stop() def test_setup(self): self.task_config.update(dict( @@ -65,7 +77,7 @@ class TestAnsibleTask(TestTask): find_repo=DEFAULT, get_playbook=fake_get_playbook, get_inventory=DEFAULT, - generate_hosts_file=DEFAULT, + generate_inventory=DEFAULT, generate_playbook=Mock(side_effect=Exception), ): task = self.klass(self.ctx, self.task_config) @@ -80,7 +92,7 @@ class TestAnsibleTask(TestTask): find_repo=DEFAULT, get_playbook=DEFAULT, get_inventory=DEFAULT, - generate_hosts_file=DEFAULT, + generate_inventory=DEFAULT, generate_playbook=DEFAULT, ): task = self.klass(self.ctx, self.task_config) @@ -216,29 +228,62 @@ class TestAnsibleTask(TestTask): assert task.inventory == '/etc/ansible/hosts' assert task.generated_inventory is False - def test_generate_hosts_file(self): + @mark.parametrize( + 'group_vars', + [ + dict(), + dict(all=dict(var0=0, var1=1)), + dict(foo=dict(var0=0), bar=dict(var0=1)), + ] + ) + def test_generate_inventory(self, group_vars): self.task_config.update(dict( playbook=[] )) + if group_vars: + self.task_config.update(dict(group_vars=group_vars)) task = self.klass(self.ctx, self.task_config) - hosts_file_path = '/my/hosts/file' + hosts_file_path = '/my/hosts/inventory' hosts_file_obj = StringIO() hosts_file_obj.name = hosts_file_path - with patch.object(ansible, 'NamedTemporaryFile') as m_NTF: - m_NTF.return_value = hosts_file_obj - task.generate_hosts_file() - m_NTF.assert_called_once_with( - prefix="teuth_ansible_hosts_", - delete=False, - suffix='', - ) + inventory_dir = os.path.dirname(hosts_file_path) + gv_dir = os.path.join(inventory_dir, 'group_vars') + self.mocks['mkdtemp'].return_value = inventory_dir + with patch('teuthology.task.ansible.file', create=True) as m_file: + fake_files = [hosts_file_obj] + # Create StringIOs for each group_vars file + if group_vars: + fake_files += [StringIO() for i in sorted(group_vars)] + m_file.side_effect = fake_files + task.generate_inventory() + file_calls = m_file.call_args_list + # Verify the inventory file was created + assert file_calls[0][0][0] == hosts_file_path + # Verify each group_vars file was created + for gv_name, call_obj in zip(sorted(group_vars), file_calls[1:]): + gv_path = call_obj[0][0] + assert gv_path == os.path.join(gv_dir, '%s.yml' % gv_name) + # Verify the group_vars dir was created + if group_vars: + mkdir_call = self.mocks['os_mkdir'].call_args_list + assert mkdir_call[0][0][0] == gv_dir assert task.generated_inventory is True - assert task.inventory == hosts_file_path + assert task.inventory == inventory_dir + # Verify the content of the inventory *file* hosts_file_obj.seek(0) assert hosts_file_obj.readlines() == [ 'remote1\n', 'remote2\n', ] + # Verify the contents of each group_vars file + gv_names = sorted(group_vars) + for i in range(len(gv_names)): + gv_name = gv_names[i] + in_val = group_vars[gv_name] + gv_stringio = fake_files[1 + i] + gv_stringio.seek(0) + out_val = yaml.safe_load(gv_stringio) + assert in_val == out_val def test_generate_playbook(self): playbook = [ @@ -375,9 +420,9 @@ class TestAnsibleTask(TestTask): task = self.klass(self.ctx, self.task_config) task.generated_inventory = True task.inventory = 'fake' - with patch.object(ansible.os, 'remove') as m_remove: + with patch.object(ansible.shutil, 'rmtree') as m_rmtree: task.teardown() - assert m_remove.called_once_with('fake') + assert m_rmtree.called_once_with('fake') def test_teardown_playbook(self): self.task_config.update(dict( @@ -439,23 +484,33 @@ class TestCephLabTask(TestTask): self.ctx.cluster.add(Remote('user@remote2'), ['role2']) self.ctx.config = dict() self.task_config = dict() - self.patcher_fetch_repo = patch('teuthology.task.ansible.fetch_repo') - self.patcher_fetch_repo.return_value = 'PATH' - self.patcher_fetch_repo.start() + self.start_patchers() + + def start_patchers(self): + self.patchers = dict() + self.mocks = dict() + self.patchers['fetch_repo'] = patch( + 'teuthology.task.ansible.fetch_repo', + ) + self.patchers['fetch_repo'].return_value = 'PATH' def fake_get_playbook(self): self.playbook_file = Mock() self.playbook_file.name = 'cephlab.yml' - self.patcher_get_playbook = patch( + self.patchers['get_playbook'] = patch( 'teuthology.task.ansible.CephLab.get_playbook', new=fake_get_playbook, ) - self.patcher_get_playbook.start() + self.patchers['mkdtemp'] = patch( + 'teuthology.task.ansible.mkdtemp', + ) + for name, patcher in self.patchers.items(): + self.mocks[name] = patcher.start() def teardown(self): - self.patcher_fetch_repo.stop() - self.patcher_get_playbook.stop() + for patcher in self.patchers.values(): + patcher.stop() @patch('teuthology.task.ansible.fetch_repo') def test_find_repo_http(self, m_fetch_repo): @@ -477,7 +532,7 @@ class TestCephLabTask(TestTask): task.get_playbook() assert task.playbook_file.name == playbook - def test_generate_hosts_file(self): + def test_generate_inventory(self): self.task_config.update(dict( playbook=[] )) @@ -485,16 +540,12 @@ class TestCephLabTask(TestTask): hosts_file_path = '/my/hosts/file' hosts_file_obj = StringIO() hosts_file_obj.name = hosts_file_path - with patch.object(ansible, 'NamedTemporaryFile') as m_NTF: - m_NTF.return_value = hosts_file_obj - task.generate_hosts_file() - m_NTF.assert_called_once_with( - prefix="teuth_ansible_hosts_", - delete=False, - suffix='', - ) + self.mocks['mkdtemp'].return_value = os.path.dirname(hosts_file_path) + with patch('teuthology.task.ansible.file', create=True) as m_file: + m_file.return_value = hosts_file_obj + task.generate_inventory() assert task.generated_inventory is True - assert task.inventory == hosts_file_path + assert task.inventory == os.path.dirname(hosts_file_path) hosts_file_obj.seek(0) assert hosts_file_obj.readlines() == [ '[testnodes]\n',