wbrawner/ansible
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Optimize template (#28044)

Browse source 
* Optimize template

* In fixing template to handle diff correctly #24477, I introduced more
  round trips to the remote end which slowed things down  The new code now
  uses one fewer round trips than the old code.
* Reimplement a large part of template by calling the copy action plugin
  instead of doing it in template's code.  This reduces the code in
  template and gives us one place to fix bugs and optimize.
* Add a follow parameter to template that mirrors the follow parameters
  for file and copy.
* Fix copy's diff handling (probably broken in my rewrite for in 2.4
  development)
* Adjusted when copy creates tmp dirs to rduce round trips in copy and
  template.

Fixes #27956
This commit is contained in:
Toshio Kuratomi 2017-08-11 17:50:49 -07:00 committed by GitHub
parent caf8bbf3bd
commit a3132e5dd6
4 changed files with 51 additions and 86 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -149,6 +149,9 @@ Ansible Changes By Release
absolute path.
- The archive module has a new parameter exclude_path which lists paths to exclude from the archive
- The yum module has a new parameter security which limits state=latest to security updates
- The template module gained a follow parameter to match with copy and file.
Like those modules, template defaults this parameter to False. Previously,
template hardcoded this to true.
### New Modules
- aci

7
lib/ansible/modules/files/template.py
View file

@ -81,6 +81,13 @@ options:
if the destination does not exist.
type: bool
default: 'yes'
follow:
description:
- This flag indicates that filesystem links in the destination, if they exist, should be followed.
- Previous to Ansible 2.4, this was hardcoded as C(yes).
type: bool
default: 'no'
version_added: "2.4"
notes:
- For Windows you can use M(win_template) which uses '\r\n' as C(newline_sequence).
- Including a string that uses a date in the template will result in the template being marked 'changed' each time

21
lib/ansible/plugins/action/copy.py
View file

@ -219,6 +219,12 @@ class ActionModule(ActionBase):
else:
dest_file = self._connection._shell.join_path(dest)
# Create a tmp path if missing only if this is not recursive.
# If this is recursive we already have a tmp path.
if delete_remote_tmp:
if tmp is None or "-tmp-" not in tmp:
tmp = self._make_tmp_path()
# Attempt to get remote file info
dest_status = self._execute_remote_stat(dest_file, all_vars=task_vars, follow=follow, tmp=tmp, checksum=force)
@ -246,19 +252,13 @@ class ActionModule(ActionBase):
if local_checksum != dest_status['checksum']:
# The checksums don't match and we will change or error out.
# Create a tmp path if missing only if this is not recursive.
# If this is recursive we already have a tmp path.
if delete_remote_tmp:
if tmp is None or "-tmp-" not in tmp:
tmp = self._make_tmp_path()
if self._play_context.diff and not raw:
result['diff'].append(self._get_diff_data(dest_file, source_full, task_vars))
if self._play_context.check_mode:
self._remove_tempfile_if_content_defined(content, content_tempfile)
module_return = dict(changed=True)
return module_return
result['changed'] = True
return result
# Define a remote directory that we will copy the file to.
tmp_src = self._connection._shell.join_path(tmp, 'source')
@ -321,7 +321,7 @@ class ActionModule(ActionBase):
# If checksums match, and follow = True, find out if 'dest' is a link. If so,
# change it to point to the source of the link.
if follow:
dest_status_nofollow = self._execute_remote_stat(dest_file, all_vars=task_vars, follow=False)
dest_status_nofollow = self._execute_remote_stat(dest_file, all_vars=task_vars, tmp=tmp, follow=False)
if dest_status_nofollow['islnk'] and 'lnk_source' in dest_status_nofollow.keys():
dest = dest_status_nofollow['lnk_source']
@ -345,7 +345,8 @@ class ActionModule(ActionBase):
if not module_return.get('checksum'):
module_return['checksum'] = local_checksum
return module_return
result.update(module_return)
return result
def _get_file_args(self):
new_module_args = {'recurse': False}

106
lib/ansible/plugins/action/template.py
View file

@ -18,10 +18,12 @@ from __future__ import (absolute_import, division, print_function)
__metaclass__ = type
import os
import shutil
import tempfile
from ansible import constants as C
from ansible.errors import AnsibleError, AnsibleFileNotFound
from ansible.module_utils._text import to_text
from ansible.module_utils._text import to_bytes, to_text
from ansible.module_utils.parsing.convert_bool import boolean
from ansible.plugins.action import ActionBase
from ansible.template import generate_ansible_template_vars
@ -58,6 +60,7 @@ class ActionModule(ActionBase):
source = self._task.args.get('src', None)
dest = self._task.args.get('dest', None)
force = boolean(self._task.args.get('force', True), strict=False)
follow = boolean(self._task.args.get('follow', False), strict=False)
state = self._task.args.get('state', None)
newline_sequence = self._task.args.get('newline_sequence', self.DEFAULT_NEWLINE_SEQUENCE)
variable_start_string = self._task.args.get('variable_start_string', None)
@ -92,20 +95,6 @@ class ActionModule(ActionBase):
if 'failed' in result:
return result
# Expand any user home dir specification
dest = self._remote_expand_user(dest)
directory_prepended = False
if dest.endswith(os.sep):
# Optimization. trailing slash means we know it's a directory
directory_prepended = True
dest = self._connection._shell.join_path(dest, os.path.basename(source))
else:
# Find out if it's a directory
dest_stat = self._execute_remote_stat(dest, task_vars, True, tmp=tmp)
if dest_stat['exists'] and dest_stat['isdir']:
dest = self._connection._shell.join_path(dest, os.path.basename(source))
# Get vault decrypted tmp file
try:
tmp_source = self._loader.get_real_file(source)
@ -160,70 +149,35 @@ class ActionModule(ActionBase):
finally:
self._loader.cleanup_tmp_file(tmp_source)
if not tmp:
tmp = self._make_tmp_path()
new_task = self._task.copy()
new_task.args.pop('newline_sequence', None)
new_task.args.pop('block_start_string', None)
new_task.args.pop('block_end_string', None)
new_task.args.pop('variable_start_string', None)
new_task.args.pop('variable_end_string', None)
new_task.args.pop('trim_blocks', None)
try:
tempdir = tempfile.mkdtemp()
result_file = os.path.join(tempdir, os.path.basename(source))
with open(result_file, 'wb') as f:
f.write(to_bytes(resultant, errors='surrogate_or_strict'))
local_checksum = checksum_s(resultant)
remote_checksum = self.get_checksum(dest, task_vars, not directory_prepended, source=source, tmp=tmp)
if isinstance(remote_checksum, dict):
# Error from remote_checksum is a dict. Valid return is a str
result.update(remote_checksum)
return result
diff = {}
new_module_args = self._task.args.copy()
# remove newline_sequence from standard arguments
new_module_args.pop('newline_sequence', None)
new_module_args.pop('block_start_string', None)
new_module_args.pop('block_end_string', None)
new_module_args.pop('variable_start_string', None)
new_module_args.pop('variable_end_string', None)
new_module_args.pop('trim_blocks', None)
if (remote_checksum == '1') or (force and local_checksum != remote_checksum):
result['changed'] = True
# if showing diffs, we need to get the remote value
if self._play_context.diff:
diff = self._get_diff_data(dest, resultant, task_vars, source_file=False)
if not self._play_context.check_mode: # do actual work through 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
self._fixup_perms2((tmp, xfered))
# run the copy module
new_module_args.update(
dict(
src=xfered,
dest=dest,
original_basename=os.path.basename(source),
follow=True,
),
)
result.update(self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars, tmp=tmp, delete_remote_tmp=False))
if result.get('changed', False) and self._play_context.diff:
result['diff'] = diff
else:
# when running the file module based on the template data, we do
# not want the source filename (the name of the template) to be used,
# since this would mess up links, so we clear the src param and tell
# the module to follow links. When doing that, we have to set
# original_basename to the template just in case the dest is
# a directory.
new_module_args.update(
new_task.args.update(
dict(
src=None,
original_basename=os.path.basename(source),
follow=True,
src=result_file,
dest=dest,
follow=follow,
),
)
result.update(self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars, tmp=tmp, delete_remote_tmp=False))
self._remove_tmp_path(tmp)
copy_action = self._shared_loader_obj.action_loader.get('copy',
task=new_task,
connection=self._connection,
play_context=self._play_context,
loader=self._loader,
templar=self._templar,
shared_loader_obj=self._shared_loader_obj)
result.update(copy_action.run(task_vars=task_vars))
finally:
shutil.rmtree(tempdir)
return result