diff --git a/lib/ansible/plugins/action/synchronize.py b/lib/ansible/plugins/action/synchronize.py index 2ff84b4f8d..0dc98bfe39 100644 --- a/lib/ansible/plugins/action/synchronize.py +++ b/lib/ansible/plugins/action/synchronize.py @@ -76,7 +76,7 @@ class ActionModule(ActionBase): path = self._get_absolute_path(path=path) return path - def _process_remote(self, host, path, user, port_matches_localhost_port): + def _process_remote(self, task_args, host, path, user, port_matches_localhost_port): """ :arg host: hostname for the path :arg path: file path @@ -99,7 +99,7 @@ class ActionModule(ActionBase): # fix up the rsync path to use the controller's public DNS/IP # instead of "localhost" if port_matches_localhost_port and host in C.LOCALHOST: - self._task.args['_substitute_controller'] = True + task_args['_substitute_controller'] = True return self._format_rsync_rsh_target(host, path, user) if ':' not in path and not path.startswith('/'): @@ -157,6 +157,11 @@ class ActionModule(ActionBase): if task_vars is None: task_vars = dict() + # We make a copy of the args here because we may fail and be asked to + # retry. If that happens we don't want to pass the munged args through + # to our next invocation. Munged args are single use only. + _tmp_args = self._task.args.copy() + result = super(ActionModule, self).run(tmp, task_vars) # Store remote connection type @@ -189,10 +194,10 @@ class ActionModule(ActionBase): result['msg'] = "synchronize uses rsync to function. rsync needs to connect to the remote host via ssh, docker client or a direct filesystem copy. This remote host is being accessed via %s instead so it cannot work." % self._connection.transport return result - use_ssh_args = self._task.args.pop('use_ssh_args', None) + use_ssh_args = _tmp_args.pop('use_ssh_args', None) # Parameter name needed by the ansible module - self._task.args['_local_rsync_path'] = task_vars.get('ansible_rsync_path') or 'rsync' + _tmp_args['_local_rsync_path'] = task_vars.get('ansible_rsync_path') or 'rsync' # rsync thinks that one end of the connection is localhost and the # other is the host we're running the task for (Note: We use @@ -229,9 +234,9 @@ class ActionModule(ActionBase): # CHECK FOR NON-DEFAULT SSH PORT inv_port = task_vars.get('ansible_ssh_port', None) or C.DEFAULT_REMOTE_PORT - if self._task.args.get('dest_port', None) is None: + if _tmp_args.get('dest_port', None) is None: if inv_port is not None: - self._task.args['dest_port'] = inv_port + _tmp_args['dest_port'] = inv_port # Set use_delegate if we are going to run rsync on a delegated host # instead of localhost @@ -273,12 +278,12 @@ class ActionModule(ActionBase): self._override_module_replaced_vars(task_vars) # SWITCH SRC AND DEST HOST PER MODE - if self._task.args.get('mode', 'push') == 'pull': + if _tmp_args.get('mode', 'push') == 'pull': (dest_host, src_host) = (src_host, dest_host) # MUNGE SRC AND DEST PER REMOTE_HOST INFO - src = self._task.args.get('src', None) - dest = self._task.args.get('dest', None) + src = _tmp_args.get('src', None) + dest = _tmp_args.get('dest', None) if src is None or dest is None: return dict(failed=True, msg="synchronize requires both src and dest parameters are set") @@ -289,12 +294,12 @@ class ActionModule(ActionBase): if private_key is not None: private_key = os.path.expanduser(private_key) - self._task.args['private_key'] = private_key + _tmp_args['private_key'] = private_key # Src and dest rsync "path" handling # Determine if we need a user@ user = None - if boolean(self._task.args.get('set_remote_user', 'yes')): + if boolean(_tmp_args.get('set_remote_user', 'yes')): if use_delegate: user = task_vars.get('ansible_delegated_vars', dict()).get('ansible_ssh_user', None) if not user: @@ -304,14 +309,14 @@ class ActionModule(ActionBase): user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user # use the mode to define src and dest's url - if self._task.args.get('mode', 'push') == 'pull': + if _tmp_args.get('mode', 'push') == 'pull': # src is a remote path: @, dest is a local path - src = self._process_remote(src_host, src, user, inv_port in localhost_ports) + src = self._process_remote(_tmp_args, src_host, src, user, inv_port in localhost_ports) dest = self._process_origin(dest_host, dest, user) else: # src is a local path, dest is a remote path: @ src = self._process_origin(src_host, src, user) - dest = self._process_remote(dest_host, dest, user, inv_port in localhost_ports) + dest = self._process_remote(_tmp_args, dest_host, dest, user, inv_port in localhost_ports) else: # Still need to munge paths (to account for roles) even if we aren't # copying files between hosts @@ -320,11 +325,11 @@ class ActionModule(ActionBase): if not dest.startswith('/'): dest = self._get_absolute_path(path=dest) - self._task.args['src'] = src - self._task.args['dest'] = dest + _tmp_args['src'] = src + _tmp_args['dest'] = dest # Allow custom rsync path argument - rsync_path = self._task.args.get('rsync_path', None) + rsync_path = _tmp_args.get('rsync_path', None) if not dest_is_local: if self._play_context.become and not rsync_path: @@ -341,7 +346,7 @@ class ActionModule(ActionBase): # make sure rsync path is quoted. if rsync_path: - self._task.args['rsync_path'] = '"%s"' % rsync_path + _tmp_args['rsync_path'] = '"%s"' % rsync_path if use_ssh_args: ssh_args = [ @@ -349,7 +354,7 @@ class ActionModule(ActionBase): getattr(self._play_context, 'ssh_common_args', ''), getattr(self._play_context, 'ssh_extra_args', ''), ] - self._task.args['ssh_args'] = ' '.join([a for a in ssh_args if a]) + _tmp_args['ssh_args'] = ' '.join([a for a in ssh_args if a]) # If launching synchronize against docker container # use rsync_opts to support container to override rsh options @@ -364,7 +369,7 @@ class ActionModule(ActionBase): self._task.args['rsync_opts'].append("--rsh='%s exec -i'" % self._docker_cmd) # run the module and store the result - result.update(self._execute_module('synchronize', task_vars=task_vars)) + result.update(self._execute_module('synchronize', module_args=_tmp_args, task_vars=task_vars)) if 'SyntaxError' in result.get('exception', result.get('msg', '')): # Emit a warning about using python3 because synchronize is diff --git a/test/units/plugins/action/fixtures/synchronize/basic/meta.yaml b/test/units/plugins/action/fixtures/synchronize/basic/meta.yaml index d508bb373d..7608ebfaf3 100644 --- a/test/units/plugins/action/fixtures/synchronize/basic/meta.yaml +++ b/test/units/plugins/action/fixtures/synchronize/basic/meta.yaml @@ -12,6 +12,6 @@ asserts: - "SAM._connection.transport == 'local'" - "self._play_context.shell == 'sh'" - "self.execute_called" - - "self.task.args['_local_rsync_path'] == 'rsync'" - - "self.task.args['src'] == '/tmp/deleteme'" - - "self.task.args['dest'] == 'root@el6host:/tmp/deleteme'" + - "self.final_module_args['_local_rsync_path'] == 'rsync'" + - "self.final_module_args['src'] == '/tmp/deleteme'" + - "self.final_module_args['dest'] == 'root@el6host:/tmp/deleteme'" diff --git a/test/units/plugins/action/fixtures/synchronize/basic_become/meta.yaml b/test/units/plugins/action/fixtures/synchronize/basic_become/meta.yaml index 24daa108cc..c507b3271a 100644 --- a/test/units/plugins/action/fixtures/synchronize/basic_become/meta.yaml +++ b/test/units/plugins/action/fixtures/synchronize/basic_become/meta.yaml @@ -23,11 +23,11 @@ asserts: - "hasattr(SAM._connection, 'ismock')" - "SAM._connection.transport == 'local'" - "self.execute_called" - - "self.task.args['_local_rsync_path'] == 'rsync'" + - "self.final_module_args['_local_rsync_path'] == 'rsync'" # this is a crucial aspect of this scenario ... - - "self.task.args['rsync_path'] == '\"sudo rsync\"'" - - "self.task.args['src'] == '/tmp/deleteme'" - - "self.task.args['dest'] == 'root@el6host:/tmp/deleteme'" + - "self.final_module_args['rsync_path'] == '\"sudo rsync\"'" + - "self.final_module_args['src'] == '/tmp/deleteme'" + - "self.final_module_args['dest'] == 'root@el6host:/tmp/deleteme'" - "self.task.become == True" - "self.task.become_user == None" - "self._play_context.shell == 'sh'" diff --git a/test/units/plugins/action/fixtures/synchronize/basic_become_cli/meta.yaml b/test/units/plugins/action/fixtures/synchronize/basic_become_cli/meta.yaml index f4399be30c..ae8b295df2 100644 --- a/test/units/plugins/action/fixtures/synchronize/basic_become_cli/meta.yaml +++ b/test/units/plugins/action/fixtures/synchronize/basic_become_cli/meta.yaml @@ -23,11 +23,11 @@ asserts: - "hasattr(SAM._connection, 'ismock')" - "SAM._connection.transport == 'local'" - "self.execute_called" - - "self.task.args['_local_rsync_path'] == 'rsync'" + - "self.final_module_args['_local_rsync_path'] == 'rsync'" # this is a crucial aspect of this scenario ... - - "self.task.args['rsync_path'] == '\"sudo rsync\"'" - - "self.task.args['src'] == '/tmp/deleteme'" - - "self.task.args['dest'] == 'root@el6host:/tmp/deleteme'" + - "self.final_module_args['rsync_path'] == '\"sudo rsync\"'" + - "self.final_module_args['src'] == '/tmp/deleteme'" + - "self.final_module_args['dest'] == 'root@el6host:/tmp/deleteme'" - "self.task.become == None" - "self.task.become_user == None" - "self._play_context.shell == 'sh'" diff --git a/test/units/plugins/action/fixtures/synchronize/basic_vagrant/meta.yaml b/test/units/plugins/action/fixtures/synchronize/basic_vagrant/meta.yaml index 15a3a1a06f..7654cc6d77 100644 --- a/test/units/plugins/action/fixtures/synchronize/basic_vagrant/meta.yaml +++ b/test/units/plugins/action/fixtures/synchronize/basic_vagrant/meta.yaml @@ -17,10 +17,10 @@ asserts: - "hasattr(SAM._connection, 'ismock')" - "SAM._connection.transport == 'local'" - "self.execute_called" - - "self.task.args['_local_rsync_path'] == 'rsync'" - - "self.task.args['dest_port'] == 2202" - - "self.task.args['src'] == '/tmp/deleteme'" - - "self.task.args['dest'] == 'vagrant@127.0.0.1:/tmp/deleteme'" + - "self.final_module_args['_local_rsync_path'] == 'rsync'" + - "self.final_module_args['dest_port'] == 2202" + - "self.final_module_args['src'] == '/tmp/deleteme'" + - "self.final_module_args['dest'] == 'vagrant@127.0.0.1:/tmp/deleteme'" - "self._play_context.shell == 'sh'" - "self._play_context.remote_addr == '127.0.0.1'" - "self._play_context.remote_user == 'vagrant'" diff --git a/test/units/plugins/action/fixtures/synchronize/basic_vagrant_become_cli/meta.yaml b/test/units/plugins/action/fixtures/synchronize/basic_vagrant_become_cli/meta.yaml index 728147ec21..242de9208c 100644 --- a/test/units/plugins/action/fixtures/synchronize/basic_vagrant_become_cli/meta.yaml +++ b/test/units/plugins/action/fixtures/synchronize/basic_vagrant_become_cli/meta.yaml @@ -20,10 +20,10 @@ asserts: - "hasattr(SAM._connection, 'ismock')" - "SAM._connection.transport == 'local'" - "self.execute_called" - - "self.task.args['_local_rsync_path'] == 'rsync'" - - "self.task.args['dest_port'] == 2202" - - "self.task.args['src'] == '/tmp/deleteme'" - - "self.task.args['dest'] == 'vagrant@127.0.0.1:/tmp/deleteme'" + - "self.final_module_args['_local_rsync_path'] == 'rsync'" + - "self.final_module_args['dest_port'] == 2202" + - "self.final_module_args['src'] == '/tmp/deleteme'" + - "self.final_module_args['dest'] == 'vagrant@127.0.0.1:/tmp/deleteme'" - "self._play_context.shell == 'sh'" - "self._play_context.remote_addr == '127.0.0.1'" - "self._play_context.remote_user == 'vagrant'" diff --git a/test/units/plugins/action/fixtures/synchronize/basic_vagrant_sudo/meta.yaml b/test/units/plugins/action/fixtures/synchronize/basic_vagrant_sudo/meta.yaml index 15a3a1a06f..7654cc6d77 100644 --- a/test/units/plugins/action/fixtures/synchronize/basic_vagrant_sudo/meta.yaml +++ b/test/units/plugins/action/fixtures/synchronize/basic_vagrant_sudo/meta.yaml @@ -17,10 +17,10 @@ asserts: - "hasattr(SAM._connection, 'ismock')" - "SAM._connection.transport == 'local'" - "self.execute_called" - - "self.task.args['_local_rsync_path'] == 'rsync'" - - "self.task.args['dest_port'] == 2202" - - "self.task.args['src'] == '/tmp/deleteme'" - - "self.task.args['dest'] == 'vagrant@127.0.0.1:/tmp/deleteme'" + - "self.final_module_args['_local_rsync_path'] == 'rsync'" + - "self.final_module_args['dest_port'] == 2202" + - "self.final_module_args['src'] == '/tmp/deleteme'" + - "self.final_module_args['dest'] == 'vagrant@127.0.0.1:/tmp/deleteme'" - "self._play_context.shell == 'sh'" - "self._play_context.remote_addr == '127.0.0.1'" - "self._play_context.remote_user == 'vagrant'" diff --git a/test/units/plugins/action/fixtures/synchronize/delegate_remote/meta.yaml b/test/units/plugins/action/fixtures/synchronize/delegate_remote/meta.yaml index 49c96786ff..06a08e6ae5 100644 --- a/test/units/plugins/action/fixtures/synchronize/delegate_remote/meta.yaml +++ b/test/units/plugins/action/fixtures/synchronize/delegate_remote/meta.yaml @@ -21,6 +21,6 @@ asserts: - "SAM._connection.transport == 'ssh'" - "self._play_context.shell == None" - "self.execute_called" - - "self.task.args['_local_rsync_path'] == 'rsync'" - - "self.task.args['src'] == '/tmp/deleteme'" - - "self.task.args['dest'] == 'el6host:/tmp/deleteme'" + - "self.final_module_args['_local_rsync_path'] == 'rsync'" + - "self.final_module_args['src'] == '/tmp/deleteme'" + - "self.final_module_args['dest'] == 'el6host:/tmp/deleteme'" diff --git a/test/units/plugins/action/fixtures/synchronize/delegate_remote_su/meta.yaml b/test/units/plugins/action/fixtures/synchronize/delegate_remote_su/meta.yaml index 0abc8bf6c4..47ae6e1d61 100644 --- a/test/units/plugins/action/fixtures/synchronize/delegate_remote_su/meta.yaml +++ b/test/units/plugins/action/fixtures/synchronize/delegate_remote_su/meta.yaml @@ -28,6 +28,6 @@ asserts: - "not self._play_context.become" - "self._play_context.become_method == 'su'" - "self.execute_called" - - "self.task.args['_local_rsync_path'] == 'rsync'" - - "self.task.args['src'] == '/tmp/deleteme'" - - "self.task.args['dest'] == 'el6host:/tmp/deleteme'" + - "self.final_module_args['_local_rsync_path'] == 'rsync'" + - "self.final_module_args['src'] == '/tmp/deleteme'" + - "self.final_module_args['dest'] == 'el6host:/tmp/deleteme'" diff --git a/test/units/plugins/action/test_synchronize.py b/test/units/plugins/action/test_synchronize.py index a22107d0a8..8072bd0444 100644 --- a/test/units/plugins/action/test_synchronize.py +++ b/test/units/plugins/action/test_synchronize.py @@ -99,11 +99,12 @@ class SynchronizeTester(object): execute_called = False - def _execute_module(self, module_name, task_vars=None): + def _execute_module(self, module_name, module_args=None, task_vars=None): self.execute_called = True + self.final_module_args = module_args self.final_task_vars = task_vars return {} - + def runtest(self, fixturepath='fixtures/synchronize/basic'): metapath = os.path.join(fixturepath, 'meta.yaml')