From 0b6fadaad7d6ba173e844af0d3a7d3147eb5d5fd Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Sun, 26 Jul 2015 12:21:38 -0400 Subject: [PATCH] started implementing diff diff now works with template also fixed check mode for template and copy --- lib/ansible/constants.py | 1 + lib/ansible/playbook/play_context.py | 3 ++ lib/ansible/plugins/action/__init__.py | 3 +- lib/ansible/plugins/action/copy.py | 25 ++++----- lib/ansible/plugins/action/template.py | 63 ++++++++++++---------- lib/ansible/plugins/callback/__init__.py | 13 +++-- lib/ansible/plugins/strategies/__init__.py | 9 ++-- lib/ansible/utils/display.py | 6 ++- 8 files changed, 71 insertions(+), 52 deletions(-) diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py index a6efc5f5e7..6751afe54a 100644 --- a/lib/ansible/constants.py +++ b/lib/ansible/constants.py @@ -237,3 +237,4 @@ DEFAULT_SUBSET = None DEFAULT_SU_PASS = None VAULT_VERSION_MIN = 1.0 VAULT_VERSION_MAX = 1.0 +MAX_FILE_SIZE_FOR_DIFF = 1*1024*1024 diff --git a/lib/ansible/playbook/play_context.py b/lib/ansible/playbook/play_context.py index 8b856310dc..f6ae473927 100644 --- a/lib/ansible/playbook/play_context.py +++ b/lib/ansible/playbook/play_context.py @@ -175,6 +175,7 @@ class PlayContext(Base): _force_handlers = FieldAttribute(isa='bool', default=False) _start_at_task = FieldAttribute(isa='string') _step = FieldAttribute(isa='bool', default=False) + _diff = FieldAttribute(isa='bool', default=False) def __init__(self, play=None, options=None, passwords=None): @@ -253,6 +254,8 @@ class PlayContext(Base): self.step = boolean(options.step) if hasattr(options, 'start_at_task') and options.start_at_task: self.start_at_task = to_unicode(options.start_at_task) + if hasattr(options, 'diff') and options.diff: + self.diff = boolean(options.diff) # get the tag info from options, converting a comma-separated list # of values into a proper list if need be. We check to see if the diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 51b5ceb338..0ba8734d3c 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -233,8 +233,7 @@ class ActionBase: Takes a remote checksum and returns 1 if no file ''' - # FIXME: figure out how this will work, probably pulled from the - # variable manager data + # FIXME: figure out how this will work, probably pulled from the variable manager data #python_interp = inject['hostvars'][inject['inventory_hostname']].get('ansible_python_interpreter', 'python') python_interp = 'python' cmd = self._connection._shell.checksum(path, python_interp) diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index 34fd7b1dbb..fc4727d3aa 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -174,17 +174,11 @@ class ActionModule(ActionBase): if tmp is None or "-tmp-" not in tmp: tmp = self._make_tmp_path() - # FIXME: runner shouldn't have the diff option there - #if self.runner.diff and not raw: - # diff = self._get_diff_data(tmp, dest_file, source_full, task_vars) - #else: - # diff = {} - diff = {} + if self._play_context.diff and not raw: + diffs.append(self._get_diff_data(tmp, dest_file, source_full, task_vars)) if self._play_context.check_mode: self._remove_tempfile_if_content_defined(content, content_tempfile) - # FIXME: diff stuff - #diffs.append(diff) changed = True module_return = dict(changed=True) continue @@ -231,7 +225,7 @@ class ActionModule(ActionBase): if raw: # Continue to next iteration if raw is defined. - # self._remove_tmp_path(tmp) + self._remove_tmp_path(tmp) continue # Build temporary module_args. @@ -244,6 +238,9 @@ class ActionModule(ActionBase): ) ) + if self._play_context.check_mode: + new_module_args['CHECKMODE'] = True + # Execute the file module. module_return = self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars, delete_remote_tmp=delete_remote_tmp) module_executed = True @@ -299,9 +296,8 @@ class ActionModule(ActionBase): diff['before'] = '' elif peek_result['appears_binary']: diff['dst_binary'] = 1 - # FIXME: this should not be in utils.. - #elif peek_result['size'] > utils.MAX_FILE_SIZE_FOR_DIFF: - # diff['dst_larger'] = utils.MAX_FILE_SIZE_FOR_DIFF + elif peek_result['size'] > C.MAX_FILE_SIZE_FOR_DIFF: + diff['dst_larger'] = C.MAX_FILE_SIZE_FOR_DIFF else: dest_result = self._execute_module(module_name='slurp', module_args=dict(path=destination), task_vars=task_vars, tmp=tmp, persist_files=True) if 'content' in dest_result: @@ -318,9 +314,8 @@ class ActionModule(ActionBase): st = os.stat(source) if "\x00" in src_contents: diff['src_binary'] = 1 - # FIXME: this should not be in utils - #elif st[stat.ST_SIZE] > utils.MAX_FILE_SIZE_FOR_DIFF: - # diff['src_larger'] = utils.MAX_FILE_SIZE_FOR_DIFF + elif st[stat.ST_SIZE] > C.MAX_FILE_SIZE_FOR_DIFF: + diff['src_larger'] = C.MAX_FILE_SIZE_FOR_DIFF else: src.seek(0) diff['after_header'] = source diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index 67a8469d11..e0ea4f3ea1 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -125,40 +125,44 @@ class ActionModule(ActionBase): return remote_checksum if local_checksum != remote_checksum: - # if showing diffs, we need to get the remote value dest_contents = '' - # FIXME: still need to implement diff mechanism - #if self.runner.diff: - # # using persist_files to keep the temp directory around to avoid needing to grab another - # dest_result = self.runner._execute_module(conn, tmp, 'slurp', "path=%s" % dest, task_vars=task_vars, persist_files=True) - # if 'content' in dest_result.result: - # dest_contents = dest_result.result['content'] - # if dest_result.result['encoding'] == 'base64': - # dest_contents = base64.b64decode(dest_contents) - # else: - # raise Exception("unknown encoding, failed: %s" % dest_result.result) + # if showing diffs, we need to get the remote value + if self._play_context.diff: + # using persist_files to keep the temp directory around to avoid needing to grab another + my_args = dict(path=dest) + dest_result = self._execute_module(module_name='slurp', module_args=my_args, task_vars=task_vars, persist_files=True) + if 'content' in dest_result: + dest_contents = dest_result['content'] + if dest_result['encoding'] == 'base64': + dest_contents = base64.b64decode(dest_contents) + else: + raise Exception("unknown encoding, failed: %s" % dest_result) - xfered = self._transfer_data(self._connection._shell.join_path(tmp, 'source'), resultant) + if not self._play_context.check_mode: # do actual work thorugh copy + xfered = self._transfer_data(self._connection._shell.join_path(tmp, 'source'), resultant) - # fix file permissions when the copy is done as a different user - if self._play_context.become and self._play_context.become_user != 'root': - self._remote_chmod('a+r', xfered, tmp) + # fix file permissions when the copy is done as a different user + if self._play_context.become and self._play_context.become_user != 'root': + self._remote_chmod('a+r', xfered, tmp) - # run the copy module - new_module_args = self._task.args.copy() - new_module_args.update( - dict( - src=xfered, - dest=dest, - original_basename=os.path.basename(source), - follow=True, - ), - ) + # run the copy module + new_module_args = self._task.args.copy() + new_module_args.update( + dict( + src=xfered, + dest=dest, + original_basename=os.path.basename(source), + follow=True, + ), + ) + result = self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars) + else: + result=dict(changed=True) + + if result.['changed'] and self._play_context.diff: + result['diff'] = dict(before=dest_contents, after=resultant, before_header=dest, after_header=source) - result = self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars) - if result.get('changed', False): - result['diff'] = dict(before=dest_contents, after=resultant) return result else: @@ -177,5 +181,8 @@ class ActionModule(ActionBase): ), ) + if self._play_context.check_mode: + new_module_args['CHECKMODE'] = True + return self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars) diff --git a/lib/ansible/plugins/callback/__init__.py b/lib/ansible/plugins/callback/__init__.py index 273f74e9b7..8f484caad2 100644 --- a/lib/ansible/plugins/callback/__init__.py +++ b/lib/ansible/plugins/callback/__init__.py @@ -40,9 +40,9 @@ class CallbackBase: def __init__(self, display): self._display = display if self._display.verbosity >= 4: - name = getattr(self, 'CALLBACK_NAME', 'with no defined name') - ctype = getattr(self, 'CALLBACK_TYPE', 'unknwon') - version = getattr(self, 'CALLBACK_VERSION', 'unknwon') + name = getattr(self, 'CALLBACK_NAME', 'unnamed') + ctype = getattr(self, 'CALLBACK_TYPE', 'old') + version = getattr(self, 'CALLBACK_VERSION', '1.0') self._display.vvvv('Loaded callback %s of type %s, v%s' % (name, ctype, version)) def _dump_results(self, result, indent=4, sort_keys=True): @@ -117,6 +117,9 @@ class CallbackBase: def playbook_on_stats(self, stats): pass + def on_file_diff(self, host, diff): + self._display.display(self._dump_results(diff)) + ####### V2 METHODS, by default they call v1 counterparts if possible ###### def v2_on_any(self, *args, **kwargs): self.on_any(args, kwargs) @@ -204,3 +207,7 @@ class CallbackBase: def v2_playbook_on_stats(self, stats): self.playbook_on_stats(stats) + def v2_on_file_diff(self, result): + host = result._host.get_name() + if 'diff' in result._result: + self.on_file_diff(host, result._result['diff']) diff --git a/lib/ansible/plugins/strategies/__init__.py b/lib/ansible/plugins/strategies/__init__.py index cac7a60870..83ddd1d2c3 100644 --- a/lib/ansible/plugins/strategies/__init__.py +++ b/lib/ansible/plugins/strategies/__init__.py @@ -71,6 +71,7 @@ class StrategyBase: self._loader = tqm.get_loader() self._final_q = tqm._final_q self._step = getattr(tqm._options, 'step', False) + self._diff = getattr(tqm._options, 'diff', False) self._display = display # internal counters @@ -191,6 +192,9 @@ class StrategyBase: self._tqm._stats.increment('changed', host.name) self._tqm.send_callback('v2_runner_on_ok', task_result) + if self._diff and 'diff' in task_result._result: + self._tqm.send_callback('v2_on_file_diff', task_result) + self._pending_results -= 1 if host.name in self._blocked_hosts: del self._blocked_hosts[host.name] @@ -215,7 +219,7 @@ class StrategyBase: elif result[0] == 'add_host': task_result = result[1] new_host_info = task_result.get('add_host', dict()) - + self._add_host(new_host_info) elif result[0] == 'add_group': @@ -479,8 +483,7 @@ class StrategyBase: for host in included_file._hosts: iterator.mark_host_failed(host) self._tqm._failed_hosts[host.name] = True - # FIXME: callback here? - print(e) + self._display.warning(str(e)) continue self._display.debug("done running handlers, result is: %s" % result) return result diff --git a/lib/ansible/utils/display.py b/lib/ansible/utils/display.py index 42ebd1a90f..fa16b7af05 100644 --- a/lib/ansible/utils/display.py +++ b/lib/ansible/utils/display.py @@ -34,6 +34,11 @@ from ansible.errors import AnsibleError from ansible.utils.color import stringc from ansible.utils.unicode import to_bytes + + +# These are module level as we currently fork and serialize the whole process and locks in the objects don't play well with that +debug_lock = Lock() + #TODO: make this a logging callback instead if C.DEFAULT_LOG_PATH: path = C.DEFAULT_LOG_PATH @@ -47,7 +52,6 @@ if C.DEFAULT_LOG_PATH: else: logger = None -debug_lock = Lock() class Display: