diff --git a/tripleo_common/actions/ansible.py b/tripleo_common/actions/ansible.py index 847fa796d..ca675f5ef 100644 --- a/tripleo_common/actions/ansible.py +++ b/tripleo_common/actions/ansible.py @@ -185,12 +185,14 @@ class AnsibleAction(actions.Action): self.gather_facts = self._kwargs_for_run.pop('gather_facts', False) self._work_dir = None + self._remove_work_dir = False @property def work_dir(self): if self._work_dir: return self._work_dir self._work_dir = tempfile.mkdtemp(prefix='ansible-mistral-action') + self._remove_work_dir = True return self._work_dir @property @@ -322,7 +324,7 @@ class AnsibleAction(actions.Action): "log_path": os.path.join(self.work_dir, 'ansible.log')} finally: # NOTE(flaper87): clean the mess if debug is disabled. - if not self.verbosity: + if not self.verbosity and self._remove_work_dir: shutil.rmtree(self.work_dir) @@ -383,11 +385,14 @@ class AnsiblePlaybookAction(base.TripleOAction): self.command_timeout = self._kwargs_for_run.pop( 'command_timeout', None) + self._remove_work_dir = False + @property def work_dir(self): if self._work_dir: return self._work_dir self._work_dir = tempfile.mkdtemp(prefix='ansible-mistral-action') + self._remove_work_dir = True return self._work_dir @property @@ -658,7 +663,7 @@ class AnsiblePlaybookAction(base.TripleOAction): "log_path": os.path.join(self.work_dir, 'ansible.log')} finally: # NOTE(flaper87): clean the mess if debug is disabled. - if not self.verbosity: + if not self.verbosity and self._remove_work_dir: shutil.rmtree(self.work_dir) diff --git a/tripleo_common/tests/actions/test_ansible.py b/tripleo_common/tests/actions/test_ansible.py index 76d1105e9..0f4606bc4 100644 --- a/tripleo_common/tests/actions/test_ansible.py +++ b/tripleo_common/tests/actions/test_ansible.py @@ -18,6 +18,7 @@ import mock import os import random from six.moves import configparser +import shutil import string import sys import tempfile @@ -184,6 +185,50 @@ class AnsiblePlaybookActionTest(base.TestCase): mock.call(action.format_message( message[message_size * 3:2048]))) + @mock.patch("shutil.rmtree") + @mock.patch("tripleo_common.actions.ansible.write_default_ansible_cfg") + @mock.patch("oslo_concurrency.processutils.execute") + def test_work_dir_cleanup(self, mock_execute, mock_write_cfg, mock_rmtree): + + mock_execute.return_value = ('', '') + + action = ansible.AnsiblePlaybookAction( + playbook=self.playbook, limit_hosts=self.limit_hosts, + remote_user=self.remote_user, become=self.become, + become_user=self.become_user, extra_vars=self.extra_vars, + verbosity=0) + + try: + action.run(self.ctx) + mock_rmtree.assert_called_once_with(action.work_dir) + finally: + # Since we mocked the delete we need to manually cleanup. + shutil.rmtree(action.work_dir) + + @mock.patch("shutil.rmtree") + @mock.patch("tripleo_common.actions.ansible.write_default_ansible_cfg") + @mock.patch("oslo_concurrency.processutils.execute") + def test_work_dir_no_cleanup(self, mock_execute, mock_write_cfg, + mock_rmtree): + + mock_execute.return_value = ('', '') + + # Specity a work_dir, this should not be deleted automatically. + work_dir = tempfile.mkdtemp() + try: + action = ansible.AnsiblePlaybookAction( + playbook=self.playbook, limit_hosts=self.limit_hosts, + remote_user=self.remote_user, become=self.become, + become_user=self.become_user, extra_vars=self.extra_vars, + verbosity=self.verbosity, work_dir=work_dir) + + action.run(self.ctx) + + # verify the rmtree is not called + mock_rmtree.assert_not_called() + finally: + shutil.rmtree(work_dir) + class CopyConfigFileTest(base.TestCase):