From 5940d3d45b7361a92c13edd3f6379432e112a705 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 14 Apr 2016 10:31:39 -0400 Subject: [PATCH] fixes to vault/copy rm _del_ as it might leak memory renamed to tmp file cleanup added exception handling when traversing file list, even if one fails try rest added cleanup to finally to ensure removal in most cases --- lib/ansible/cli/adhoc.py | 2 ++ lib/ansible/cli/console.py | 2 ++ lib/ansible/executor/playbook_executor.py | 2 ++ lib/ansible/parsing/dataloader.py | 19 ++++++++++--------- lib/ansible/plugins/action/copy.py | 6 ++---- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/ansible/cli/adhoc.py b/lib/ansible/cli/adhoc.py index c1a23f6c25..becb43e581 100644 --- a/lib/ansible/cli/adhoc.py +++ b/lib/ansible/cli/adhoc.py @@ -194,5 +194,7 @@ class AdHocCLI(CLI): finally: if self._tqm: self._tqm.cleanup() + if loader: + loader.cleanup_all_tmp_files() return result diff --git a/lib/ansible/cli/console.py b/lib/ansible/cli/console.py index c6b27f8f66..5c844e6094 100644 --- a/lib/ansible/cli/console.py +++ b/lib/ansible/cli/console.py @@ -215,6 +215,8 @@ class ConsoleCLI(CLI, cmd.Cmd): finally: if self._tqm: self._tqm.cleanup() + if self.loader: + self.loader.cleanup_all_tmp_files() if result is None: display.error("No hosts found") diff --git a/lib/ansible/executor/playbook_executor.py b/lib/ansible/executor/playbook_executor.py index 4827ebc8d4..7179692123 100644 --- a/lib/ansible/executor/playbook_executor.py +++ b/lib/ansible/executor/playbook_executor.py @@ -194,6 +194,8 @@ class PlaybookExecutor: finally: if self._tqm is not None: self._tqm.cleanup() + if self._loader: + self._loader.cleanup_all_tmp_files() if self._options.syntax: display.display("No issues encountered") diff --git a/lib/ansible/parsing/dataloader.py b/lib/ansible/parsing/dataloader.py index bdc62d10b5..5043d0e793 100644 --- a/lib/ansible/parsing/dataloader.py +++ b/lib/ansible/parsing/dataloader.py @@ -23,11 +23,9 @@ import copy import os import json import subprocess -from yaml import YAMLError import tempfile +from yaml import YAMLError -from yaml import load, YAMLError ->>>>>>> feature/copy-vault-dataloader: Add method get_real_file(file_path) to dataloader from ansible.compat.six import text_type, string_types from ansible.errors import AnsibleFileNotFound, AnsibleParserError, AnsibleError @@ -67,10 +65,6 @@ class DataLoader(): # initialize the vault stuff with an empty password self.set_vault_password(None) - def __del__(self): - for f in self._tempfiles: - os.unlink(f) - def set_vault_password(self, vault_password): self._vault_password = vault_password self._vault = VaultLib(password=vault_password) @@ -352,9 +346,9 @@ class DataLoader(): return real_path except (IOError, OSError) as e: - raise AnsibleParserError("an error occurred while trying to read the file '%s': %s" % (file_name, str(e))) + raise AnsibleParserError("an error occurred while trying to read the file '%s': %s" % (real_path, str(e))) - def cleanup_real_file(self, file_path): + def cleanup_tmp_file(self, file_path): """ Removes any temporary files created from a previous call to get_real_file. file_path must be the path returned from a @@ -363,3 +357,10 @@ class DataLoader(): if file_path in self._tempfiles: os.unlink(file_path) self._tempfiles.remove(file_path); + + def cleanup_all_tmp_files(self): + for f in self._tempfiles: + try: + self.cleanup_tmp_file(f) + except: + pass #TODO: this should at least warn diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index 7eb805aa5d..f798f8c4b8 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -220,8 +220,7 @@ class ActionModule(ActionBase): # We have copied the file remotely and no longer require our content_tempfile self._remove_tempfile_if_content_defined(content, content_tempfile) - - self._loader.cleanup_real_file(source_full) + self._loader.cleanup_tmp_file(source_full) # fix file permissions when the copy is done as a different user self._fixup_perms(tmp, remote_user, recursive=True) @@ -250,8 +249,7 @@ class ActionModule(ActionBase): # no need to transfer the file, already correct hash, but still need to call # the file module in case we want to change attributes self._remove_tempfile_if_content_defined(content, content_tempfile) - - self._loader.cleanup_real_file(source_full) + self._loader.cleanup_tmp_file(source_full) if raw: # Continue to next iteration if raw is defined.