From d70914e47349ba04ec0e5baa59642618cd616cd7 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Tue, 23 Jun 2015 16:36:26 -0700 Subject: [PATCH 1/4] Don't refetch the template 100% of the time-- Performance optimization for templated files This module was actually re-fetching the template *every* run, even if we had the template cached locally (unless it was served from the master itself). With this change we check our local cache before fetching the template. This cuts ~1 RTT from the minion to the source of the file, which ranges from moderate to large savings. --- salt/fileclient.py | 34 ++++++++++-------- salt/modules/file.py | 83 +++++++++++++++++++++++++------------------- 2 files changed, 68 insertions(+), 49 deletions(-) diff --git a/salt/fileclient.py b/salt/fileclient.py index 4bbd75d053..c1e0ec2283 100644 --- a/salt/fileclient.py +++ b/salt/fileclient.py @@ -356,11 +356,14 @@ class Client(object): self.opts['cachedir'], 'localfiles', path.lstrip('/')) filesdest = os.path.join( self.opts['cachedir'], 'files', saltenv, path.lstrip('salt://')) + extrndest = self._extrn_path(path, saltenv) if os.path.exists(filesdest): return filesdest elif os.path.exists(localsfilesdest): return localsfilesdest + elif os.path.exists(extrndest): + return extrndest return '' @@ -537,13 +540,7 @@ class Client(object): netloc = salt.utils.sanitize_win_path_string(url_data.netloc) else: netloc = url_data.netloc - dest = salt.utils.path_join( - self.opts['cachedir'], - 'extrn_files', - saltenv, - netloc, - url_data.path - ) + dest = self._extrn_path(url, saltenv) destdir = os.path.dirname(dest) if not os.path.isdir(destdir): os.makedirs(destdir) @@ -670,13 +667,7 @@ class Client(object): return '' if not dest: # No destination passed, set the dest as an extrn_files cache - dest = salt.utils.path_join( - self.opts['cachedir'], - 'extrn_files', - saltenv, - url_data.netloc, - url_data.path - ) + dest = self._extrn_path(url, saltenv) # If Salt generated the dest name, create any required dirs makedirs = True @@ -690,6 +681,21 @@ class Client(object): shutil.move(data['data'], dest) return dest + def _extrn_path(self, url, saltenv): + ''' + Return the extn_filepath for a given url + ''' + url_data = urlparse(url) + + return salt.utils.path_join( + self.opts['cachedir'], + 'extrn_files', + saltenv, + url_data.netloc, + url_data.path + ) + + class LocalClient(Client): ''' diff --git a/salt/modules/file.py b/salt/modules/file.py index bec2edbeab..bcc669d7c3 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -2758,8 +2758,54 @@ def get_managed( # Copy the file to the minion and templatize it sfn = '' source_sum = {} - if template and source: - sfn = __salt__['cp.cache_file'](source, saltenv) + # if we have a source defined, lets figure out what the hash is + if source: + if _urlparse(source).scheme == 'salt': + source_sum = __salt__['cp.hash_file'](source, saltenv) + if not source_sum: + return '', {}, 'Source file {0} not found'.format(source) + elif source_hash: + protos = ['salt', 'http', 'https', 'ftp', 'swift'] + if _urlparse(source_hash).scheme in protos: + # The source_hash is a file on a server + hash_fn = __salt__['cp.cache_file'](source_hash, saltenv) + if not hash_fn: + return '', {}, 'Source hash file {0} not found'.format( + source_hash) + source_sum = extract_hash(hash_fn, '', name) + if source_sum is None: + return '', {}, ('Source hash file {0} contains an invalid ' + 'hash format, it must be in the format =.' + ).format(source_hash) + + else: + # The source_hash is a hash string + comps = source_hash.split('=') + if len(comps) < 2: + return '', {}, ('Source hash file {0} contains an ' + 'invalid hash format, it must be in ' + 'the format =' + ).format(source_hash) + source_sum['hsum'] = comps[1].strip() + source_sum['hash_type'] = comps[0].strip() + else: + return '', {}, ('Unable to determine upstream hash of' + ' source file {0}').format(source) + + # if the file is a template we need to actually template the file to get + # a checksum, but we can cache the template itselt + if template: + # check if we have the template cached + template_dest = __salt__['cp.is_cached'](source, saltenv) + if template_dest: + comps = source_hash.split('=') + cached_template_sum = get_hash(template_dest, form=source_sum['hash_type']) + if cached_template_sum == source_sum['hsum']: + sfn = template_dest + # if we didn't have the template file, lets get it + if not sfn: + sfn = __salt__['cp.cache_file'](source, saltenv) + # exists doesn't play nice with sfn as bool # but if cache failed, sfn == False if not sfn or not os.path.exists(sfn): @@ -2798,40 +2844,7 @@ def get_managed( else: __clean_tmp(sfn) return sfn, {}, data['data'] - else: - # Copy the file down if there is a source - if source: - if _urlparse(source).scheme == 'salt': - source_sum = __salt__['cp.hash_file'](source, saltenv) - if not source_sum: - return '', {}, 'Source file {0} not found'.format(source) - elif source_hash: - protos = ['salt', 'http', 'https', 'ftp', 'swift'] - if _urlparse(source_hash).scheme in protos: - # The source_hash is a file on a server - hash_fn = __salt__['cp.cache_file'](source_hash, saltenv) - if not hash_fn: - return '', {}, 'Source hash file {0} not found'.format( - source_hash) - source_sum = extract_hash(hash_fn, '', name) - if source_sum is None: - return '', {}, ('Source hash file {0} contains an invalid ' - 'hash format, it must be in the format =.' - ).format(source_hash) - else: - # The source_hash is a hash string - comps = source_hash.split('=') - if len(comps) < 2: - return '', {}, ('Source hash file {0} contains an ' - 'invalid hash format, it must be in ' - 'the format =' - ).format(source_hash) - source_sum['hsum'] = comps[1].strip() - source_sum['hash_type'] = comps[0].strip() - else: - return '', {}, ('Unable to determine upstream hash of' - ' source file {0}').format(source) return sfn, source_sum, '' From 4cf78a0a95259fa71a0e5906f1b333ff25e39425 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Wed, 24 Jun 2015 11:20:33 -0700 Subject: [PATCH 2/4] pylint --- salt/fileclient.py | 1 - 1 file changed, 1 deletion(-) diff --git a/salt/fileclient.py b/salt/fileclient.py index c1e0ec2283..45f5399828 100644 --- a/salt/fileclient.py +++ b/salt/fileclient.py @@ -696,7 +696,6 @@ class Client(object): ) - class LocalClient(Client): ''' Use the local_roots option to parse a local file root From c03a6fa9d18c145419451cf899525d462a33fb4b Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Wed, 24 Jun 2015 11:20:38 -0700 Subject: [PATCH 3/4] Add support for sources of managed files to be local --- salt/modules/file.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/salt/modules/file.py b/salt/modules/file.py index bcc669d7c3..5e9fc58cd4 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -2758,14 +2758,20 @@ def get_managed( # Copy the file to the minion and templatize it sfn = '' source_sum = {} + urlparsed_source = _urlparse(source) # if we have a source defined, lets figure out what the hash is if source: - if _urlparse(source).scheme == 'salt': + if urlparsed_source.scheme == 'salt': source_sum = __salt__['cp.hash_file'](source, saltenv) if not source_sum: return '', {}, 'Source file {0} not found'.format(source) + # if its a local file + elif urlparsed_source.scheme == 'file': + source_sum = get_hash(urlparsed_source.path) + elif source.startswith('/'): + source_sum = get_hash(source) elif source_hash: - protos = ['salt', 'http', 'https', 'ftp', 'swift'] + protos = ('salt', 'http', 'https', 'ftp', 'swift') if _urlparse(source_hash).scheme in protos: # The source_hash is a file on a server hash_fn = __salt__['cp.cache_file'](source_hash, saltenv) From 5fb75346efeedded2738a2c25a4914f94b364bff Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Wed, 24 Jun 2015 12:31:51 -0700 Subject: [PATCH 4/4] Only parse the source if we have one --- salt/modules/file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/file.py b/salt/modules/file.py index 5e9fc58cd4..01af8b8754 100644 --- a/salt/modules/file.py +++ b/salt/modules/file.py @@ -2758,9 +2758,9 @@ def get_managed( # Copy the file to the minion and templatize it sfn = '' source_sum = {} - urlparsed_source = _urlparse(source) # if we have a source defined, lets figure out what the hash is if source: + urlparsed_source = _urlparse(source) if urlparsed_source.scheme == 'salt': source_sum = __salt__['cp.hash_file'](source, saltenv) if not source_sum: