From 9d55bff91491777368a3d2a8aa5cef4dc30d773e Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 22 Sep 2016 17:38:31 -0600 Subject: [PATCH 01/33] Use cachedir for Windows --- salt/modules/cmdmod.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 08eb4f941a6..d818ca48043 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -2004,6 +2004,13 @@ def script(source, # Backwards compatibility saltenv = __env__ + if salt.utils.is_windows() and runas and cwd is None: + cwd = os.path.join(__opts__['cachedir'], 'wintmp') + if not os.path.isdir(cwd): + ret = __salt__['win_dacl.add_ace']( + cwd, 'File', runas, 'READ&EXECUTE', 'ALLOW', + 'FOLDER&SUBFOLDERS&FILES') + path = salt.utils.mkstemp(dir=cwd, suffix=os.path.splitext(source)[1]) if template: From 18d41f771193520e46e05d9213d094bc7435933d Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 22 Sep 2016 17:41:16 -0600 Subject: [PATCH 02/33] Add mkdir --- salt/modules/cmdmod.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index d818ca48043..8a4bfd14ec5 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -2007,9 +2007,10 @@ def script(source, if salt.utils.is_windows() and runas and cwd is None: cwd = os.path.join(__opts__['cachedir'], 'wintmp') if not os.path.isdir(cwd): - ret = __salt__['win_dacl.add_ace']( - cwd, 'File', runas, 'READ&EXECUTE', 'ALLOW', - 'FOLDER&SUBFOLDERS&FILES') + __salt__['file.mkdir'](root) + ret = __salt__['win_dacl.add_ace']( + cwd, 'File', runas, 'READ&EXECUTE', 'ALLOW', + 'FOLDER&SUBFOLDERS&FILES') path = salt.utils.mkstemp(dir=cwd, suffix=os.path.splitext(source)[1]) From 25d52efeac3c4e8d792e812c3bc0668dc2cc9b4f Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 22 Sep 2016 17:44:52 -0600 Subject: [PATCH 03/33] Fix mkdir --- salt/modules/cmdmod.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 8a4bfd14ec5..35897e6885f 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -2007,7 +2007,7 @@ def script(source, if salt.utils.is_windows() and runas and cwd is None: cwd = os.path.join(__opts__['cachedir'], 'wintmp') if not os.path.isdir(cwd): - __salt__['file.mkdir'](root) + __salt__['file.mkdir'](cwd) ret = __salt__['win_dacl.add_ace']( cwd, 'File', runas, 'READ&EXECUTE', 'ALLOW', 'FOLDER&SUBFOLDERS&FILES') From 20c2c91bba17da0f3ccd4f922ce97f6aa14b9dd8 Mon Sep 17 00:00:00 2001 From: Clint Armstrong Date: Fri, 23 Sep 2016 14:45:16 -0400 Subject: [PATCH 04/33] daemon-reload on call to service.avaliable --- salt/modules/systemd.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/salt/modules/systemd.py b/salt/modules/systemd.py index 15e96fb884a..795f8e0ec74 100644 --- a/salt/modules/systemd.py +++ b/salt/modules/systemd.py @@ -298,7 +298,7 @@ def _untracked_custom_unit_found(name): ''' unit_path = os.path.join('/etc/systemd/system', _canonical_unit_name(name)) - return os.access(unit_path, os.R_OK) and not available(name) + return os.access(unit_path, os.R_OK) and not available(name, False) def _unit_file_changed(name): @@ -468,7 +468,7 @@ def get_all(): return sorted(ret) -def available(name): +def available(name, check_units=True): ''' .. versionadded:: 0.10.4 @@ -481,6 +481,8 @@ def available(name): salt '*' service.available sshd ''' + if check_units: + _check_for_unit_changes(name) out = _systemctl_status(name).lower() for line in salt.utils.itertools.split(out, '\n'): match = re.match(r'\s+loaded:\s+(\S+)', line) From 47c3d03035ab38785ab1c4162996ee8b7cc2f0e2 Mon Sep 17 00:00:00 2001 From: Mathieu Le Marec - Pasquet Date: Sun, 25 Sep 2016 09:49:30 +0000 Subject: [PATCH 05/33] Fix pkg.latest_version using localized output This fixes #36561. --- salt/modules/aptpkg.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/salt/modules/aptpkg.py b/salt/modules/aptpkg.py index 509433aab89..f4d970ed309 100644 --- a/salt/modules/aptpkg.py +++ b/salt/modules/aptpkg.py @@ -261,7 +261,8 @@ def latest_version(*names, **kwargs): if isinstance(repo, list): cmd = cmd + repo out = __salt__['cmd.run_all'](cmd, python_shell=False, - output_loglevel='trace') + output_loglevel='trace', + env={'LC_ALL': 'C', 'LANG': 'C'}) candidate = '' for line in out['stdout'].splitlines(): if 'Candidate' in line: From 377ced5c24c80e8ea76d309db742735289338e9d Mon Sep 17 00:00:00 2001 From: twangboy Date: Mon, 26 Sep 2016 18:03:21 -0600 Subject: [PATCH 06/33] Remove directory in Windows with runas --- salt/modules/cmdmod.py | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/salt/modules/cmdmod.py b/salt/modules/cmdmod.py index 35897e6885f..83116c4477f 100644 --- a/salt/modules/cmdmod.py +++ b/salt/modules/cmdmod.py @@ -18,6 +18,7 @@ import subprocess import sys import time import traceback +import tempfile from salt.utils import vt # Import salt libs @@ -25,7 +26,8 @@ import salt.utils import salt.utils.timed_subprocess import salt.grains.extra import salt.ext.six as six -from salt.exceptions import CommandExecutionError, TimedProcTimeoutError +from salt.exceptions import CommandExecutionError, TimedProcTimeoutError, \ + SaltInvocationError from salt.log import LOG_LEVELS from salt.ext.six.moves import range from salt.ext.six.moves import shlex_quote as _cmd_quote @@ -1986,8 +1988,8 @@ def script(source, def _cleanup_tempfile(path): try: - os.remove(path) - except (IOError, OSError) as exc: + __salt__['file.remove'](path) + except (SaltInvocationError, CommandExecutionError) as exc: log.error( 'cmd.script: Unable to clean tempfile \'{0}\': {1}'.format( path, @@ -2005,10 +2007,8 @@ def script(source, saltenv = __env__ if salt.utils.is_windows() and runas and cwd is None: - cwd = os.path.join(__opts__['cachedir'], 'wintmp') - if not os.path.isdir(cwd): - __salt__['file.mkdir'](cwd) - ret = __salt__['win_dacl.add_ace']( + cwd = tempfile.mkdtemp(dir=__opts__['cachedir']) + __salt__['win_dacl.add_ace']( cwd, 'File', runas, 'READ&EXECUTE', 'ALLOW', 'FOLDER&SUBFOLDERS&FILES') @@ -2024,7 +2024,10 @@ def script(source, saltenv, **kwargs) if not fn_: - _cleanup_tempfile(path) + if salt.utils.is_windows() and runas: + _cleanup_tempfile(cwd) + else: + _cleanup_tempfile(path) return {'pid': 0, 'retcode': 1, 'stdout': '', @@ -2033,7 +2036,10 @@ def script(source, else: fn_ = __salt__['cp.cache_file'](source, saltenv) if not fn_: - _cleanup_tempfile(path) + if salt.utils.is_windows() and runas: + _cleanup_tempfile(cwd) + else: + _cleanup_tempfile(path) return {'pid': 0, 'retcode': 1, 'stdout': '', @@ -2061,7 +2067,10 @@ def script(source, use_vt=use_vt, bg=bg, password=password) - _cleanup_tempfile(path) + if salt.utils.is_windows() and runas: + _cleanup_tempfile(cwd) + else: + _cleanup_tempfile(path) return ret From 58577d342e2154f613131a34427fcebf49b93434 Mon Sep 17 00:00:00 2001 From: Bo Maryniuk Date: Tue, 27 Sep 2016 10:59:07 +0200 Subject: [PATCH 07/33] Generate thin with configured extrta modules --- salt/runners/thin.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/salt/runners/thin.py b/salt/runners/thin.py index 0a8b398a028..5a3386f42ad 100644 --- a/salt/runners/thin.py +++ b/salt/runners/thin.py @@ -30,4 +30,8 @@ def generate(extra_mods='', overwrite=False, so_mods=''): salt-run thin.generate mako,wempy 1 salt-run thin.generate overwrite=1 ''' + conf_mods = __opts__.get('thin_extra_mods') + if conf_mods: + extra_mods = ','.join([conf_mods, extra_mods]) + return salt.utils.thin.gen_thin(__opts__['cachedir'], extra_mods, overwrite, so_mods) From 2be9330be6709e110e4748aac2d08c43550e9b8a Mon Sep 17 00:00:00 2001 From: Bo Maryniuk Date: Tue, 27 Sep 2016 11:03:38 +0200 Subject: [PATCH 08/33] Add thin options to the master config. --- salt/config.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/salt/config.py b/salt/config.py index 3a590a29ce0..bb40c887c12 100644 --- a/salt/config.py +++ b/salt/config.py @@ -784,6 +784,9 @@ VALID_OPTS = { # Delay in seconds before executing bootstrap (salt cloud) 'bootstrap_delay': int, + + # Extra modules for Salt Thin + 'thin_extra_mods': str, } # default configurations @@ -1240,6 +1243,7 @@ DEFAULT_MASTER_OPTS = { 'dummy_pub': False, 'http_request_timeout': 1 * 60 * 60.0, # 1 hour 'http_max_body': 100 * 1024 * 1024 * 1024, # 100GB + 'thin_extra_mods': '', } From 3d878f9da5d0cb3c4108002993fe9920f776103b Mon Sep 17 00:00:00 2001 From: Bo Maryniuk Date: Tue, 27 Sep 2016 11:05:28 +0200 Subject: [PATCH 09/33] Get the thin Salt with configured extra modules on SSH --- salt/client/ssh/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/client/ssh/__init__.py b/salt/client/ssh/__init__.py index 203bf07177d..da9fa81f4c5 100644 --- a/salt/client/ssh/__init__.py +++ b/salt/client/ssh/__init__.py @@ -252,7 +252,7 @@ class SSH(object): self.serial = salt.payload.Serial(opts) self.returners = salt.loader.returners(self.opts, {}) self.fsclient = salt.fileclient.FSClient(self.opts) - self.thin = salt.utils.thin.gen_thin(self.opts['cachedir']) + self.thin = salt.utils.thin.gen_thin(self.opts['cachedir'], extra_mods=self.opts.get('thin_extra_mods')) self.mods = mod_data(self.fsclient) def get_pubkey(self): From 3bfb17ee62dffeb6b219f8bd636e05f27aa4a8a7 Mon Sep 17 00:00:00 2001 From: Bo Maryniuk Date: Tue, 27 Sep 2016 11:06:43 +0200 Subject: [PATCH 10/33] Add a description of the thin/min parameters to the master config --- conf/master | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/conf/master b/conf/master index e001390ac43..2224890a817 100644 --- a/conf/master +++ b/conf/master @@ -976,3 +976,10 @@ ############################################ # Default match type for filtering events tags: startswith, endswith, find, regex, fnmatch #event_match_type: startswith + +# Permanently include any available Python 3rd party modules into Salt Thin +# when they are generated for Salt-SSH or other purposes. +# The modules should be named by the names they are actually imported inside the Python. +# The value of the parameters can be either one module or a comma separated list of them. +#thin_extra_mods: foo,bar + From a441b35588dbae12b2f343bedfa99513be8bff05 Mon Sep 17 00:00:00 2001 From: Bo Maryniuk Date: Tue, 27 Sep 2016 11:08:32 +0200 Subject: [PATCH 11/33] Add documentation about Salt Thin configuration --- doc/ref/configuration/master.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index f181a661f06..e1d65082a4a 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -696,6 +696,15 @@ overridden on a per-minion basis in the roster (``minion_opts``) minion_opts: gpg_keydir: /root/gpg +``thin_extra_mods`` +------------------- + +Default: None + +List of additional modules, needed to be included into the Salt Thin. +Pass a list of importable Python modules that are typically located in +the `site-packages` Python directory so they will be also always included +into the Salt Thin, once generated. Master Security Settings ======================== From 975f8bb27dc4509e2e236bef187a739a59f46994 Mon Sep 17 00:00:00 2001 From: Bo Maryniuk Date: Tue, 27 Sep 2016 11:11:11 +0200 Subject: [PATCH 12/33] Add extra-mods options to the Salt-Thin via SSH CLI --- salt/utils/parsers.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/salt/utils/parsers.py b/salt/utils/parsers.py index 6019883dd3c..5a741fbb76e 100644 --- a/salt/utils/parsers.py +++ b/salt/utils/parsers.py @@ -2475,6 +2475,12 @@ class SaltSSHOptionParser(six.with_metaclass(OptionParserMeta, default=None, help='Pass in extra files to include in the state tarball' ) + self.add_option( + '--thin-extra-modules', + dest='thin_extra_mods', + default=None, + help='One or comma-separated list of extra Python modules' + 'to be included into Thin Salt.') self.add_option( '-v', '--verbose', default=False, From c4f899b3b321822f9404e5c0b5d4c411cfd9a03f Mon Sep 17 00:00:00 2001 From: Denys Havrysh Date: Tue, 27 Sep 2016 17:41:53 +0300 Subject: [PATCH 13/33] `salt.states.service`: detect that service failed to start/stop --- salt/states/service.py | 152 +++++++++++++++++++++--------- tests/unit/states/service_test.py | 28 +++++- 2 files changed, 133 insertions(+), 47 deletions(-) diff --git a/salt/states/service.py b/salt/states/service.py index 56f84be0a79..668d123e5f1 100644 --- a/salt/states/service.py +++ b/salt/states/service.py @@ -62,6 +62,9 @@ service, then set the reload value to True: from __future__ import absolute_import import time +# Import Salt libs +from salt.exceptions import CommandExecutionError + __virtualname__ = 'service' @@ -98,9 +101,17 @@ def _enable(name, started, result=True, **kwargs): ret = {} # is service available? - if not _available(name, ret): + try: + if not _available(name, ret): + return ret + except CommandExecutionError as exc: + ret['result'] = False + ret['comment'] = exc.strerror return ret + # Set default expected result + ret['result'] = result + # Check to see if this minion supports enable if 'service.enable' not in __salt__ or 'service.enabled' not in __salt__: if started is True: @@ -139,10 +150,9 @@ def _enable(name, started, result=True, **kwargs): return ret if __salt__['service.enable'](name, **kwargs): - after_toggle_enable_status = __salt__['service.enabled'](name, - **kwargs) # Service has been enabled ret['changes'] = {} + after_toggle_enable_status = __salt__['service.enabled'](name, **kwargs) # on upstart, certain services like apparmor will always return # False, even if correctly activated # do not trigger a change @@ -160,16 +170,14 @@ def _enable(name, started, result=True, **kwargs): return ret # Service failed to be enabled + ret['result'] = False if started is True: - ret['result'] = False ret['comment'] = ('Failed when setting service {0} to start at boot,' ' but the service is running').format(name) elif started is None: - ret['result'] = False ret['comment'] = ('Failed when setting service {0} to start at boot,' ' but the service was already running').format(name) else: - ret['result'] = False ret['comment'] = ('Failed when setting service {0} to start at boot,' ' and the service is dead').format(name) return ret @@ -182,10 +190,18 @@ def _disable(name, started, result=True, **kwargs): ret = {} # is service available? - if not _available(name, ret): - ret['result'] = True + try: + if not _available(name, ret): + ret['result'] = True + return ret + except CommandExecutionError as exc: + ret['result'] = False + ret['comment'] = exc.strerror return ret + # Set default expected result + ret['result'] = result + # is enable/disable available? if 'service.disable' not in __salt__ or 'service.disabled' not in __salt__: if started is True: @@ -200,8 +216,8 @@ def _disable(name, started, result=True, **kwargs): ' service {0} is dead').format(name) return ret - before_toggle_disable_status = __salt__['service.disabled'](name) # Service can be disabled + before_toggle_disable_status = __salt__['service.disabled'](name) if before_toggle_disable_status: # Service is disabled if started is True: @@ -227,8 +243,6 @@ def _disable(name, started, result=True, **kwargs): # Service has been disabled ret['changes'] = {} after_toggle_disable_status = __salt__['service.disabled'](name) - # Service has been disabled - ret['changes'] = {} # on upstart, certain services like apparmor will always return # False, even if correctly activated # do not trigger a change @@ -254,7 +268,6 @@ def _disable(name, started, result=True, **kwargs): ret['comment'] = ('Failed when setting service {0} to not start' ' at boot, but the service was already running' ).format(name) - return ret else: ret['comment'] = ('Failed when setting service {0} to not start' ' at boot, and the service is dead').format(name) @@ -315,13 +328,21 @@ def running(name, enable=None, sig=None, init_delay=None, **kwargs): return _enabled_used_error(ret) # Check if the service is available - if not _available(name, ret): + try: + if not _available(name, ret): + return ret + except CommandExecutionError as exc: + ret['result'] = False + ret['comment'] = exc.strerror return ret # lot of custom init script won't or mis implement the status # command, so it is just an indicator but can not be fully trusted before_toggle_status = __salt__['service.status'](name, sig) - before_toggle_enable_status = __salt__['service.enabled'](name) + if 'service.enabled' in __salt__: + before_toggle_enable_status = __salt__['service.enabled'](name) + else: + before_toggle_enable_status = True # See if the service is already running if before_toggle_status: @@ -347,28 +368,39 @@ def running(name, enable=None, sig=None, init_delay=None, **kwargs): ret.update(_enable(name, False, result=False, **kwargs)) elif enable is False: ret.update(_disable(name, False, result=False, **kwargs)) - else: - ret['comment'] = 'Started Service {0}'.format(name) - if enable is True: - ret.update(_enable(name, True, **kwargs)) - elif enable is False: - ret.update(_disable(name, True, **kwargs)) - - # only force a change state if we have explicitly detected them - after_toggle_status = __salt__['service.status'](name) - after_toggle_enable_status = __salt__['service.enabled'](name) - if ( - (before_toggle_enable_status != after_toggle_enable_status) or - (before_toggle_status != after_toggle_status) - ) and not ret.get('changes', {}): - ret['changes'][name] = func_ret + return ret if init_delay: time.sleep(init_delay) + + # only force a change state if we have explicitly detected them + after_toggle_status = __salt__['service.status'](name) + if 'service.enabled' in __salt__: + after_toggle_enable_status = __salt__['service.enabled'](name) + else: + after_toggle_enable_status = True + if ( + (before_toggle_enable_status != after_toggle_enable_status) or + (before_toggle_status != after_toggle_status) + ) and not ret.get('changes', {}): + ret['changes'][name] = after_toggle_status + + if after_toggle_status: + ret['comment'] = 'Started Service {0}'.format(name) + else: + ret['comment'] = 'Service {0} failed to start'.format(name) + + if enable is True: + ret.update(_enable(name, after_toggle_status, result=after_toggle_status, **kwargs)) + elif enable is False: + ret.update(_disable(name, after_toggle_status, result=after_toggle_status, **kwargs)) + + if init_delay: ret['comment'] = ( '{0}\nDelayed return for {1} seconds' .format(ret['comment'], init_delay) ) + return ret @@ -397,14 +429,23 @@ def dead(name, enable=None, sig=None, **kwargs): return _enabled_used_error(ret) # Check if the service is available - if not _available(name, ret): - ret['result'] = True + try: + if not _available(name, ret): + return ret + except CommandExecutionError as exc: + ret['result'] = False + ret['comment'] = exc.strerror return ret # lot of custom init script won't or mis implement the status # command, so it is just an indicator but can not be fully trusted before_toggle_status = __salt__['service.status'](name, sig) - before_toggle_enable_status = __salt__['service.enabled'](name) + if 'service.enabled' in __salt__: + before_toggle_enable_status = __salt__['service.enabled'](name) + else: + before_toggle_enable_status = True + + # See if the service is already dead if not before_toggle_status: ret['comment'] = 'The service {0} is already dead'.format(name) if enable is True and not before_toggle_enable_status: @@ -413,12 +454,12 @@ def dead(name, enable=None, sig=None, **kwargs): ret.update(_disable(name, None, **kwargs)) return ret + # Run the tests if __opts__['test']: ret['result'] = None ret['comment'] = 'Service {0} is set to be killed'.format(name) return ret - # be sure to stop, in case we mis detected in the check func_ret = __salt__['service.stop'](name) if not func_ret: ret['result'] = False @@ -427,20 +468,32 @@ def dead(name, enable=None, sig=None, **kwargs): ret.update(_enable(name, True, result=False, **kwargs)) elif enable is False: ret.update(_disable(name, True, result=False, **kwargs)) - else: - ret['comment'] = 'Service {0} was killed'.format(name) - if enable is True: - ret.update(_enable(name, False, **kwargs)) - elif enable is False: - ret.update(_disable(name, False, **kwargs)) + return ret + # only force a change state if we have explicitly detected them after_toggle_status = __salt__['service.status'](name) - after_toggle_enable_status = __salt__['service.enabled'](name) + if 'service.enabled' in __salt__: + after_toggle_enable_status = __salt__['service.enabled'](name) + else: + after_toggle_enable_status = True if ( - (before_toggle_enable_status != after_toggle_enable_status) or - (before_toggle_status != after_toggle_status) + (before_toggle_enable_status != after_toggle_enable_status) or + (before_toggle_status != after_toggle_status) ) and not ret.get('changes', {}): - ret['changes'][name] = func_ret + ret['changes'][name] = after_toggle_status + + # be sure to stop, in case we mis detected in the check + if after_toggle_status: + ret['result'] = False + ret['comment'] = 'Service {0} failed to die'.format(name) + else: + ret['comment'] = 'Service {0} was killed'.format(name) + + if enable is True: + ret.update(_enable(name, after_toggle_status, result=not after_toggle_status, **kwargs)) + elif enable is False: + ret.update(_disable(name, after_toggle_status, result=not after_toggle_status, **kwargs)) + return ret @@ -502,6 +555,19 @@ def mod_watch(name, sig The string to search for when looking for the service process with ps + + reload + Use reload instead of the default restart (exclusive option with full_restart, + defaults to reload if both are used) + + full_restart + Use service.full_restart instead of restart (exclusive option with reload) + + force + Use service.force_reload instead of reload (needs reload to be set to True) + + init_delay + Add a sleep command (in seconds) before the service is restarted/reloaded ''' ret = {'name': name, 'changes': {}, diff --git a/tests/unit/states/service_test.py b/tests/unit/states/service_test.py index 8bb036b5875..5073c1dada2 100644 --- a/tests/unit/states/service_test.py +++ b/tests/unit/states/service_test.py @@ -58,7 +58,10 @@ class ServiceTestCase(TestCase): 'result': True}, {'changes': {}, 'comment': 'The service salt is already running', - 'name': 'salt', 'result': True}] + 'name': 'salt', 'result': True}, + {'changes': 'saltstack', + 'comment': 'Service salt failed to start', 'name': 'salt', + 'result': True}] tmock = MagicMock(return_value=True) fmock = MagicMock(return_value=False) @@ -134,6 +137,22 @@ class ServiceTestCase(TestCase): ): self.assertDictEqual(service.running("salt", True), ret[4]) + with contextlib.nested( + patch.dict(service.__opts__, {'test': False}), + patch.dict( + service.__salt__, { + 'service.status': + MagicMock(side_effect=[False, False]), + 'service.enabled': + MagicMock(side_effect=[True, True]), + 'service.start': + MagicMock(return_value="stack")}), + patch.object( + service, '_enable', + MagicMock(return_value={'changes': 'saltstack'})) + ): + self.assertDictEqual(service.running("salt", True), ret[6]) + def test_dead(self): ''' Test to ensure that the named service is dead @@ -149,8 +168,8 @@ class ServiceTestCase(TestCase): 'comment': 'Service salt was killed', 'name': 'salt', 'result': True}, {'changes': {}, - 'comment': 'Service salt was killed', 'name': 'salt', - 'result': True}, + 'comment': 'Service salt failed to die', 'name': 'salt', + 'result': False}, {'changes': 'saltstack', 'comment': 'The service salt is already dead', 'name': 'salt', 'result': True}] @@ -176,6 +195,7 @@ class ServiceTestCase(TestCase): patch.object(service, '_enable', mock) ): self.assertDictEqual(service.dead("salt", True), ret[5]) + with contextlib.nested( patch.dict(service.__opts__, {'test': False}), patch.dict( @@ -203,7 +223,7 @@ class ServiceTestCase(TestCase): {'service.enabled': MagicMock(side_effect=[True, True, False]), 'service.status': - MagicMock(side_effect=[True, True, False]), + MagicMock(side_effect=[True, False, False]), 'service.stop': MagicMock(return_value="stack")}), patch.object( service, '_enable', From 6e36191fc4c6f424a2f9d60b8d925788bf6971f2 Mon Sep 17 00:00:00 2001 From: elainearbaugh Date: Tue, 27 Sep 2016 09:28:28 -0700 Subject: [PATCH 14/33] Fix trust key 2015.8 (#36540) * Fix gpg.trust_key user bug * Separated user and no user case --- salt/modules/gpg.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/salt/modules/gpg.py b/salt/modules/gpg.py index d2012aa6b0e..3bf5058603b 100644 --- a/salt/modules/gpg.py +++ b/salt/modules/gpg.py @@ -811,7 +811,10 @@ def trust_key(keyid=None, if not fingerprint: if keyid: - key = get_key(keyid) + if user: + key = get_key(keyid, user=user) + else: + key = get_key(keyid) if key: if 'fingerprint' not in key: ret['res'] = False From 79fdc12395f6158ac74020ce7001e30d18ece134 Mon Sep 17 00:00:00 2001 From: Jonathan Ballet Date: Tue, 27 Sep 2016 18:52:49 +0200 Subject: [PATCH 15/33] jinja: fix YAML terminator removal in Jinja's "yaml" filter --- salt/utils/jinja.py | 4 ++-- tests/unit/templates/jinja_test.py | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/salt/utils/jinja.py b/salt/utils/jinja.py index 6492c6d9d45..563a837a5cc 100644 --- a/salt/utils/jinja.py +++ b/salt/utils/jinja.py @@ -384,8 +384,8 @@ class SerializerExtension(Extension, object): def format_yaml(self, value, flow_style=True): yaml_txt = yaml.dump(value, default_flow_style=flow_style, Dumper=OrderedDictDumper).strip() - if yaml_txt.endswith('\n...\n'): - yaml_txt = yaml_txt[:len(yaml_txt-5)] + if yaml_txt.endswith('\n...'): + yaml_txt = yaml_txt[:len(yaml_txt)-4] return Markup(yaml_txt) def format_python(self, value): diff --git a/tests/unit/templates/jinja_test.py b/tests/unit/templates/jinja_test.py index c09bdcec3ca..8974f71f131 100644 --- a/tests/unit/templates/jinja_test.py +++ b/tests/unit/templates/jinja_test.py @@ -477,6 +477,12 @@ class TestCustomExtensions(TestCase): rendered = env.from_string('{{ dataset|yaml }}').render(dataset=dataset) self.assertEqual(dataset, yaml.load(rendered)) + def test_serialize_yaml_str(self): + dataset = "str value" + env = Environment(extensions=[SerializerExtension]) + rendered = env.from_string('{{ dataset|yaml }}').render(dataset=dataset) + self.assertEqual(dataset, rendered) + def test_serialize_python(self): dataset = { "foo": True, From 867638ff48d189a8f6a5b1282efb8d9656393259 Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Fri, 23 Sep 2016 17:03:18 -0600 Subject: [PATCH 16/33] Test for pkg.upgrade. Most robust on Suse but better than nothing elsewhere --- tests/integration/modules/pkg.py | 63 ++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/tests/integration/modules/pkg.py b/tests/integration/modules/pkg.py index 1d0dc6660f8..b96ae1c654c 100644 --- a/tests/integration/modules/pkg.py +++ b/tests/integration/modules/pkg.py @@ -38,6 +38,14 @@ class PkgModuleTest(integration.ModuleCase, eq = ['0.2.4-0ubuntu1', '0.2.4-0ubuntu1'] gt = ['0.2.4.1-0ubuntu1', '0.2.4-0ubuntu1'] + self.assertEqual(self.run_function(func, lt), -1) + self.assertEqual(self.run_function(func, eq), 0) + self.assertEqual(self.run_function(func, gt), 1) + elif os_family == 'Suse': + lt = ['2.3.0-1', '2.3.1-15.1'] + eq = ['2.3.1-15.1', '2.3.1-15.1'] + gt = ['2.3.2-15.1', '2.3.1-15.1'] + self.assertEqual(self.run_function(func, lt), -1) self.assertEqual(self.run_function(func, eq), 0) self.assertEqual(self.run_function(func, gt), 1) @@ -55,11 +63,11 @@ class PkgModuleTest(integration.ModuleCase, try: repo = None if os_grain == 'Ubuntu': - repo = 'ppa:otto-kesselgulasch/gimp-edge' - uri = 'http://ppa.launchpad.net/otto-kesselgulasch/gimp-edge/ubuntu' + repo = 'ppa:silvenga/3proxy' + uri = 'http://ppa.launchpad.net/silvenga/3proxy/ubuntu' ret = self.run_function('pkg.mod_repo', [repo, 'comps=main']) self.assertNotEqual(ret, {}) - self.assertIn(repo, ret) + self.assertIn('deb '+uri, ret.keys()[0]) ret = self.run_function('pkg.get_repo', [repo]) self.assertEqual(ret['uri'], uri) elif os_grain == 'CentOS': @@ -191,6 +199,11 @@ class PkgModuleTest(integration.ModuleCase, if os_family == 'RedHat': ret = self.run_function(func) self.assertIn(ret, (True, None)) + elif os_family == 'Suse': + ret = self.run_function(func) + if not isinstance(ret, dict): + self.skipTest('Upstream repo did not return coherent results. Skipping test.') + self.assertNotEqual(ret, {}) elif os_family == 'Debian': ret = self.run_function(func) if not isinstance(ret, dict): @@ -228,6 +241,50 @@ class PkgModuleTest(integration.ModuleCase, self.assertIn('less', keys) self.assertIn('zypper', keys) + @requires_network() + @destructiveTest + def test_pkg_upgrade_has_pending_upgrades(self): + ''' + Test running a system upgrade when there are packages that need upgrading + ''' + func = 'pkg.upgrade' + os_family = self.run_function('grains.item', ['os_family'])['os_family'] + + # First make sure that an up-to-date copy of the package db is available + self.run_function('pkg.refresh_db') + + if os_family == 'Suse': + # pkg.latest version returns empty if the latest version is already installed + vim_version_dict = self.run_function('pkg.latest_version', ['vim']) + print('vim_version_dict: {0}'.format(vim_version_dict)) + if vim_version_dict == {}: + # Latest version is installed, get its version and construct a version selector so the immediately previous version is selected + vim_version_dict = self.run_function('pkg.info_available', ['vim']) + vim_version = 'version=<'+vim_version_dict['vim']['version'] + else: + # Vim was not installed, so pkg.latest_version returns the latest one. + # Construct a version selector so immediately previous version is selected + vim_version = 'version=<'+vim_version_dict + + # Install a version of vim that should need upgrading + ret = self.run_function('pkg.install', ['vim', vim_version]) + print('pkg.install result: {0}'.format(ret)) + + # Run a system upgrade, which should catch the fact that Vim needs upgrading, and upgrade it. + ret = self.run_function(func) + + # The changes dictionary should not be empty. + self.assertIn('vim', ret) + else: + ret = self.run_function('pkg.list_updates') + if ret == '': + self.skipTest('No updates available for this machine. Skipping pkg.upgrade test.') + else: + ret = self.run_function(func) + + # The changes dictionary should not be empty. + self.assertNotEqual(ret, {}) + if __name__ == '__main__': from integration import run_tests From 38705894623b03a866212e81063498353da95385 Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Tue, 27 Sep 2016 14:32:20 -0600 Subject: [PATCH 17/33] Test for pkg.upgrade. Most robust on Suse but better than nothing elsewhere --- tests/integration/modules/pkg.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/modules/pkg.py b/tests/integration/modules/pkg.py index b96ae1c654c..28adb3e9ab4 100644 --- a/tests/integration/modules/pkg.py +++ b/tests/integration/modules/pkg.py @@ -274,7 +274,8 @@ class PkgModuleTest(integration.ModuleCase, ret = self.run_function(func) # The changes dictionary should not be empty. - self.assertIn('vim', ret) + self.assertIn('changes', ret) + self.assertIn('vim', ret['changes']) else: ret = self.run_function('pkg.list_updates') if ret == '': From 3904dfc5a809b286d08a777f009380e82868630b Mon Sep 17 00:00:00 2001 From: rallytime Date: Tue, 27 Sep 2016 17:29:47 -0600 Subject: [PATCH 18/33] Don't allow mercurial states to return True with errors Fixes #36553 The hg.present state was not checking for errors for any of the calls to the hg.py execution module functions. All of the hg.py execution module functions were using cmd.run, instead of cmd.run_all, which allowed for hg.py execution module functions to log an error at the CLI but the hg.present state would return True, even though there were problems executing the module functions. This change adds try/except blocks around the calls to the mercurial execution module functions and retuns False when a CommandExecutionError is raised by the module. The module has been changes to use cmd.run_all instead of cmd.run in order to check for the retcode of the underlying mercurial calls and raises a CommandExecutionError is the retcode != 0. --- salt/modules/hg.py | 36 ++++++++++++++++++++++++++++++------ salt/states/hg.py | 38 ++++++++++++++++++++++++++++++++------ 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/salt/modules/hg.py b/salt/modules/hg.py index 1ca1cf5dc63..3ff524cf32a 100644 --- a/salt/modules/hg.py +++ b/salt/modules/hg.py @@ -2,11 +2,14 @@ ''' Support for the Mercurial SCM ''' + +# Import Python libs from __future__ import absolute_import import logging # Import salt libs -from salt import utils +from salt.exceptions import CommandExecutionError +import salt.utils log = logging.getLogger(__name__) @@ -15,7 +18,7 @@ def __virtual__(): ''' Only load if hg is installed ''' - if utils.which('hg') is None: + if salt.utils.which('hg') is None: return (False, 'The hg execution module cannot be loaded: hg unavailable.') else: @@ -187,7 +190,14 @@ def pull(cwd, opts=None, user=None, identity=None, repository=None): cmd.append(opt) if repository is not None: cmd.append(repository) - return __salt__['cmd.run'](cmd, cwd=cwd, runas=user, python_shell=False) + + ret = __salt__['cmd.run_all'](cmd, cwd=cwd, runas=user, python_shell=False) + if ret['retcode'] != 0: + raise CommandExecutionError( + 'Hg command failed: {0}'.format(ret.get('stderr', ret['stdout'])) + ) + + return ret['stdout'] def update(cwd, rev, force=False, user=None): @@ -215,7 +225,14 @@ def update(cwd, rev, force=False, user=None): cmd = ['hg', 'update', '{0}'.format(rev)] if force: cmd.append('-C') - return __salt__['cmd.run'](cmd, cwd=cwd, runas=user, python_shell=False) + + ret = __salt__['cmd.run_all'](cmd, cwd=cwd, runas=user, python_shell=False) + if ret['retcode'] != 0: + raise CommandExecutionError( + 'Hg command failed: {0}'.format(ret.get('stderr', ret['stdout'])) + ) + + return ret['stdout'] def clone(cwd, repository, opts=None, user=None, identity=None): @@ -251,7 +268,14 @@ def clone(cwd, repository, opts=None, user=None, identity=None): cmd.append('{0}'.format(opt)) if identity: cmd.extend(_ssh_flag(identity)) - return __salt__['cmd.run'](cmd, runas=user, python_shell=False) + + ret = __salt__['cmd.run_all'](cmd, runas=user, python_shell=False) + if ret['retcode'] != 0: + raise CommandExecutionError( + 'Hg command failed: {0}'.format(ret.get('stderr', ret['stdout'])) + ) + + return ret['stdout'] def status(cwd, opts=None, user=None): @@ -298,7 +322,7 @@ def status(cwd, opts=None, user=None): ret[t].append(f) return ret - if utils.is_iter(cwd): + if salt.utils.is_iter(cwd): return dict((cwd, _status(cwd)) for cwd in cwd) else: return _status(cwd) diff --git a/salt/states/hg.py b/salt/states/hg.py index fefb0e4dc23..79e8ea88796 100644 --- a/salt/states/hg.py +++ b/salt/states/hg.py @@ -13,15 +13,16 @@ in ~/.ssh/known_hosts, and the remote host has this host's public key. - rev: tip - target: /tmp/example_repo ''' -from __future__ import absolute_import # Import python libs +from __future__ import absolute_import import logging import os import shutil # Import salt libs import salt.utils +from salt.exceptions import CommandExecutionError from salt.states.git import _fail, _neutral_test log = logging.getLogger(__name__) @@ -130,12 +131,27 @@ def _update_repo(ret, name, target, clean, user, identity, rev, opts): ret, test_result) - pull_out = __salt__['hg.pull'](target, user=user, identity=identity, opts=opts, repository=name) + try: + pull_out = __salt__['hg.pull'](target, user=user, identity=identity, opts=opts, repository=name) + except CommandExecutionError as err: + ret['result'] = False + ret['comment'] = err + return ret if rev: - __salt__['hg.update'](target, rev, force=clean, user=user) + try: + __salt__['hg.update'](target, rev, force=clean, user=user) + except CommandExecutionError as err: + ret['result'] = False + ret['comment'] = err + return ret else: - __salt__['hg.update'](target, 'tip', force=clean, user=user) + try: + __salt__['hg.update'](target, 'tip', force=clean, user=user) + except CommandExecutionError as err: + ret['result'] = False + ret['comment'] = err + return ret new_rev = __salt__['hg.revision'](cwd=target, user=user, rev='.') @@ -172,13 +188,23 @@ def _handle_existing(ret, target, force): def _clone_repo(ret, target, name, user, identity, rev, opts): - result = __salt__['hg.clone'](target, name, user=user, identity=identity, opts=opts) + try: + result = __salt__['hg.clone'](target, name, user=user, identity=identity, opts=opts) + except CommandExecutionError as err: + ret['result'] = False + ret['comment'] = err + return ret if not os.path.isdir(target): return _fail(ret, result) if rev: - __salt__['hg.update'](target, rev, user=user) + try: + __salt__['hg.update'](target, rev, user=user) + except CommandExecutionError as err: + ret['result'] = False + ret['comment'] = err + return ret new_rev = __salt__['hg.revision'](cwd=target, user=user) message = 'Repository {0} cloned to {1}'.format(name, target) From 0f158b5eddad765890939463531b5a0c84df982d Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Tue, 27 Sep 2016 21:53:32 -0500 Subject: [PATCH 19/33] Fix shadowed builtins id, hex, and list were all shadowed, this commit fixes that. --- salt/modules/mac_xattr.py | 52 ++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/salt/modules/mac_xattr.py b/salt/modules/mac_xattr.py index bc8f46b8625..c6d3e88ade5 100644 --- a/salt/modules/mac_xattr.py +++ b/salt/modules/mac_xattr.py @@ -16,7 +16,11 @@ import salt.utils.mac_utils from salt.exceptions import CommandExecutionError log = logging.getLogger(__name__) + __virtualname__ = "xattr" +__func_alias__ = { + 'list_': 'list', +} def __virtual__(): @@ -28,7 +32,7 @@ def __virtual__(): return False -def list(path, hex=False): +def list_(path, **kwargs): ''' List all of the extended attributes on the given file/directory @@ -49,7 +53,12 @@ def list(path, hex=False): salt '*' xattr.list /path/to/file salt '*' xattr.list /path/to/file hex=True ''' - cmd = 'xattr "{0}"'.format(path) + kwargs = salt.utils.clean_kwargs(**kwargs) + hex_ = kwargs.pop('hex', False) + if kwargs: + salt.utils.invalid_kwargs(kwargs) + + cmd = ['xattr', path] try: ret = salt.utils.mac_utils.execute_return_result(cmd) except CommandExecutionError as exc: @@ -63,13 +72,13 @@ def list(path, hex=False): attrs_ids = ret.split("\n") attrs = {} - for id in attrs_ids: - attrs[id] = read(path, id, hex) + for id_ in attrs_ids: + attrs[id_] = read(path, id_, **{'hex': hex_}) return attrs -def read(path, attribute, hex=False): +def read(path, attribute, **kwargs): ''' Read the given attributes on the given file/directory @@ -92,11 +101,15 @@ def read(path, attribute, hex=False): salt '*' xattr.read /path/to/file com.test.attr salt '*' xattr.read /path/to/file com.test.attr hex=True ''' - hex_flag = "" - if hex: - hex_flag = "-x" + kwargs = salt.utils.clean_kwargs(**kwargs) + hex_ = kwargs.pop('hex', False) + if kwargs: + salt.utils.invalid_kwargs(kwargs) - cmd = 'xattr -p {0} "{1}" "{2}"'.format(hex_flag, attribute, path) + cmd = ['xattr', '-p'] + if hex_: + cmd.append('-x') + cmd.extend([attribute, path]) try: ret = salt.utils.mac_utils.execute_return_result(cmd) @@ -110,7 +123,7 @@ def read(path, attribute, hex=False): return ret -def write(path, attribute, value, hex=False): +def write(path, attribute, value, **kwargs): ''' Causes the given attribute name to be assigned the given value @@ -134,11 +147,16 @@ def write(path, attribute, value, hex=False): salt '*' xattr.write /path/to/file "com.test.attr" "value" ''' - hex_flag = "" - if hex: - hex_flag = "-x" + kwargs = salt.utils.clean_kwargs(**kwargs) + hex_ = kwargs.pop('hex', False) + if kwargs: + salt.utils.invalid_kwargs(kwargs) + + cmd = ['xattr', '-w'] + if hex_: + cmd.append('-x') + cmd.extend([attribute, value, path]) - cmd = 'xattr -w {0} "{1}" "{2}" "{3}"'.format(hex_flag, attribute, value, path) try: salt.utils.mac_utils.execute_return_success(cmd) except CommandExecutionError as exc: @@ -146,7 +164,7 @@ def write(path, attribute, value, hex=False): raise CommandExecutionError('File not found: {0}'.format(path)) raise CommandExecutionError('Unknown Error: {0}'.format(exc.strerror)) - return read(path, attribute, hex) == value + return read(path, attribute, **{'hex': hex_}) == value def delete(path, attribute): @@ -180,7 +198,7 @@ def delete(path, attribute): raise CommandExecutionError('Attribute not found: {0}'.format(attribute)) raise CommandExecutionError('Unknown Error: {0}'.format(exc.strerror)) - return attribute not in list(path) + return attribute not in list_(path) def clear(path): @@ -207,4 +225,4 @@ def clear(path): raise CommandExecutionError('File not found: {0}'.format(path)) raise CommandExecutionError('Unknown Error: {0}'.format(exc.strerror)) - return list(path) == {} + return list_(path) == {} From a828bdd0b80c8316b1d254461563271ca6961c1c Mon Sep 17 00:00:00 2001 From: rallytime Date: Tue, 27 Sep 2016 20:55:39 -0600 Subject: [PATCH 20/33] Update test mocks for cmd.run_all dicts --- tests/unit/modules/hg_test.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/unit/modules/hg_test.py b/tests/unit/modules/hg_test.py index ad8be31bd2a..9e3c8c878a2 100644 --- a/tests/unit/modules/hg_test.py +++ b/tests/unit/modules/hg_test.py @@ -59,24 +59,27 @@ class HgTestCase(TestCase): ''' Test for Perform a pull on the given repository ''' - with patch.dict(hg.__salt__, {'cmd.run': - MagicMock(return_value='A')}): + with patch.dict(hg.__salt__, {'cmd.run_all': + MagicMock(return_value={'retcode': 0, + 'stdout': 'A'})}): self.assertEqual(hg.pull('cwd'), 'A') def test_update(self): ''' Test for Update to a given revision ''' - with patch.dict(hg.__salt__, {'cmd.run': - MagicMock(return_value='A')}): + with patch.dict(hg.__salt__, {'cmd.run_all': + MagicMock(return_value={'retcode': 0, + 'stdout': 'A'})}): self.assertEqual(hg.update('cwd', 'rev'), 'A') def test_clone(self): ''' Test for Clone a new repository ''' - with patch.dict(hg.__salt__, {'cmd.run': - MagicMock(return_value='A')}): + with patch.dict(hg.__salt__, {'cmd.run_all': + MagicMock(return_value={'retcode': 0, + 'stdout': 'A'})}): self.assertEqual(hg.clone('cwd', 'repository'), 'A') def test_status_single(self): From 283aca8f2a601b7d16b73ca787baf21ebbc24dc7 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Tue, 27 Sep 2016 23:13:19 -0500 Subject: [PATCH 21/33] Update test to reflect new function signature --- tests/unit/modules/mac_xattr_test.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/unit/modules/mac_xattr_test.py b/tests/unit/modules/mac_xattr_test.py index d11b26799f5..c20f4129533 100644 --- a/tests/unit/modules/mac_xattr_test.py +++ b/tests/unit/modules/mac_xattr_test.py @@ -55,8 +55,10 @@ class XAttrTestCase(TestCase): ''' with patch.object(salt.utils.mac_utils, 'execute_return_result', MagicMock(return_value='expected results')) as mock: - self.assertEqual(xattr.read('/path/to/file', 'com.attr', True), - 'expected results') + self.assertEqual( + xattr.read('/path/to/file', 'com.attr', **{'hex': True}), + 'expected results' + ) mock.assert_called_once_with('xattr -p -x "com.attr" "/path/to/file"') @patch('salt.utils.mac_utils.execute_return_result', From 275845c3d24899dee71bbeacffc1a04acb3837cc Mon Sep 17 00:00:00 2001 From: Yaroslav Molochko Date: Tue, 27 Sep 2016 13:49:05 -0700 Subject: [PATCH 22/33] Fix memory leak for 0mq transport In case anyone is trying to "DDOS" zeromq transport with TCP messages, we've got ZeroMQPubServerChannel process utilized more than 8GB of RAM. With this patch, we have got ZeroMQPubServerChannel stabilized at 300MB for > 500 nodes. It doesn't affect performance for 0mq minions, as it perform gc.collect only in case of exception. --- salt/payload.py | 1 + 1 file changed, 1 insertion(+) diff --git a/salt/payload.py b/salt/payload.py index 7f8e8acc483..93b5ad700a6 100644 --- a/salt/payload.py +++ b/salt/payload.py @@ -120,6 +120,7 @@ class Serial(object): 'This often happens when trying to read a file not in binary mode' 'To see message payload, enable debug logging and retry. Exception: {0}'.format(exc)) log.debug('Msgpack deserialization failure on message: {0}'.format(msg)) + gc.collect() raise finally: gc.enable() From 62729eff8d0e03500490c5a7c807fb064ad10ef1 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Wed, 28 Sep 2016 11:00:33 -0500 Subject: [PATCH 23/33] Update tests to include fix for renamed function --- tests/unit/modules/mac_xattr_test.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/unit/modules/mac_xattr_test.py b/tests/unit/modules/mac_xattr_test.py index c20f4129533..85cd23fe37e 100644 --- a/tests/unit/modules/mac_xattr_test.py +++ b/tests/unit/modules/mac_xattr_test.py @@ -30,7 +30,7 @@ class XAttrTestCase(TestCase): 'squidward': 'patrick'} with patch.object(xattr, 'read', MagicMock(side_effect=['squarepants', 'patrick'])): - self.assertEqual(xattr.list('path/to/file'), expected) + self.assertEqual(xattr.list_('path/to/file'), expected) @patch('salt.utils.mac_utils.execute_return_result', MagicMock(side_effect=CommandExecutionError('No such file'))) @@ -38,7 +38,7 @@ class XAttrTestCase(TestCase): ''' Test listing attributes of a missing file ''' - self.assertRaises(CommandExecutionError, xattr.list, '/path/to/file') + self.assertRaises(CommandExecutionError, xattr.list_, '/path/to/file') @patch('salt.utils.mac_utils.execute_return_result', MagicMock(return_value='expected results')) @@ -59,7 +59,8 @@ class XAttrTestCase(TestCase): xattr.read('/path/to/file', 'com.attr', **{'hex': True}), 'expected results' ) - mock.assert_called_once_with('xattr -p -x "com.attr" "/path/to/file"') + mock.assert_called_once_with( + ['xattr', '-p', '-x', 'com.attr', '/path/to/file']) @patch('salt.utils.mac_utils.execute_return_result', MagicMock(side_effect=CommandExecutionError('No such file'))) @@ -103,7 +104,7 @@ class XAttrTestCase(TestCase): Test deleting a specific attribute from a file ''' mock_cmd = MagicMock(return_value={'spongebob': 'squarepants'}) - with patch.object(xattr, 'list', mock_cmd): + with patch.object(xattr, 'list_', mock_cmd): self.assertTrue(xattr.delete('/path/to/file', 'attribute')) @patch('salt.utils.mac_utils.execute_return_success', @@ -124,7 +125,7 @@ class XAttrTestCase(TestCase): Test clearing all attributes on a file ''' mock_cmd = MagicMock(return_value={}) - with patch.object(xattr, 'list', mock_cmd): + with patch.object(xattr, 'list_', mock_cmd): self.assertTrue(xattr.clear('/path/to/file')) @patch('salt.utils.mac_utils.execute_return_success', From 787c1f557e3285ba807773c8b47e625f797001b9 Mon Sep 17 00:00:00 2001 From: rallytime Date: Wed, 28 Sep 2016 10:20:18 -0600 Subject: [PATCH 24/33] Pylint fix --- salt/client/ssh/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/client/ssh/__init__.py b/salt/client/ssh/__init__.py index 8d1c326e695..46a1c1d065c 100644 --- a/salt/client/ssh/__init__.py +++ b/salt/client/ssh/__init__.py @@ -295,7 +295,7 @@ class SSH(object): self.returners = salt.loader.returners(self.opts, {}) self.fsclient = salt.fileclient.FSClient(self.opts) self.thin = salt.utils.thin.gen_thin(self.opts['cachedir'], - extra_mods=self.opts.get('thin_extra_mods'), + extra_mods=self.opts.get('thin_extra_mods'), python2_bin=self.opts['python2_bin'], python3_bin=self.opts['python3_bin']) self.mods = mod_data(self.fsclient) From ae021d6dec0b9a28748c422ff4a000f79976b484 Mon Sep 17 00:00:00 2001 From: rallytime Date: Wed, 28 Sep 2016 13:58:06 -0600 Subject: [PATCH 25/33] Provide an error message when invalid transport is set Fixes #36304 --- salt/cli/daemons.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/salt/cli/daemons.py b/salt/cli/daemons.py index 735fb4ddec1..d4954eb60ba 100644 --- a/salt/cli/daemons.py +++ b/salt/cli/daemons.py @@ -308,8 +308,10 @@ class Minion(parsers.MinionOptionParser, DaemonsMixin): # pylint: disable=no-in log.exception('Salt minion is already running. Exiting.') self.shutdown(1) + transport = self.config.get('transport').lower() + # TODO: AIO core is separate from transport - if self.config['transport'].lower() in ('zeromq', 'tcp'): + if transport in ('zeromq', 'tcp'): # Late import so logging works correctly import salt.minion # If the minion key has not been accepted, then Salt enters a loop @@ -325,11 +327,19 @@ class Minion(parsers.MinionOptionParser, DaemonsMixin): # pylint: disable=no-in self.minion = salt.minion.MultiMinion(self.config) else: self.minion = salt.minion.Minion(self.config) - else: + elif transport == 'raet': import salt.daemons.flo self.daemonize_if_required() self.set_pidfile() self.minion = salt.daemons.flo.IofloMinion(self.config) + else: + log.error( + 'The transport \'{0}\' is not supported. Please use one of the following: ' + 'tcp, ' + 'raet, ' + 'or zeromq.'.format(transport) + ) + self.shutdown(1) def start(self): ''' From c4060ba2c1b802257ac84156b2351f64b5c1a0df Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Wed, 28 Sep 2016 15:29:25 -0500 Subject: [PATCH 26/33] Move check for service availability to a helper function This allows for us to check for unit file changes at the beginning of the available() function without invoking the available() function a second time, potentially resulting in a recursive loop if not handled delicately. --- salt/modules/systemd.py | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/salt/modules/systemd.py b/salt/modules/systemd.py index 795f8e0ec74..f01bb155283 100644 --- a/salt/modules/systemd.py +++ b/salt/modules/systemd.py @@ -71,6 +71,23 @@ def _canonical_unit_name(name): return '%s.service' % name +def _check_available(name): + ''' + Returns boolean telling whether or not the named service is available + ''' + out = _systemctl_status(name).lower() + for line in salt.utils.itertools.split(out, '\n'): + match = re.match(r'\s+loaded:\s+(\S+)', line) + if match: + ret = match.group(1) != 'not-found' + break + else: + raise CommandExecutionError( + 'Failed to get information on unit \'%s\'' % name + ) + return ret + + def _check_for_unit_changes(name): ''' Check for modified/updated unit files, and run a daemon-reload if any are @@ -298,7 +315,7 @@ def _untracked_custom_unit_found(name): ''' unit_path = os.path.join('/etc/systemd/system', _canonical_unit_name(name)) - return os.access(unit_path, os.R_OK) and not available(name, False) + return os.access(unit_path, os.R_OK) and not _check_available(name) def _unit_file_changed(name): @@ -468,7 +485,7 @@ def get_all(): return sorted(ret) -def available(name, check_units=True): +def available(name): ''' .. versionadded:: 0.10.4 @@ -481,19 +498,8 @@ def available(name, check_units=True): salt '*' service.available sshd ''' - if check_units: - _check_for_unit_changes(name) - out = _systemctl_status(name).lower() - for line in salt.utils.itertools.split(out, '\n'): - match = re.match(r'\s+loaded:\s+(\S+)', line) - if match: - ret = match.group(1) != 'not-found' - break - else: - raise CommandExecutionError( - 'Failed to get information on unit \'%s\'' % name - ) - return ret + _check_for_unit_changes(name) + return _check_available(name) def missing(name): From 315b031de9c16251a225538a52bdf9e4b4dd75f7 Mon Sep 17 00:00:00 2001 From: Justin Findlay Date: Tue, 27 Sep 2016 16:26:44 -0600 Subject: [PATCH 27/33] modules.archive: use less redundant message --- salt/modules/archive.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/archive.py b/salt/modules/archive.py index d442055d61a..59859b3f32b 100644 --- a/salt/modules/archive.py +++ b/salt/modules/archive.py @@ -38,7 +38,7 @@ def __virtual__(): commands = ('tar', 'gzip', 'gunzip', 'zip', 'unzip', 'rar', 'unrar') # If none of the above commands are in $PATH this module is a no-go if not any(salt.utils.which(cmd) for cmd in commands): - return (False, 'The archive module could not be loaded: unable to find commands tar,gzip,gunzip,zip,unzip,rar,unrar') + return (False, 'Unable to find commands tar,gzip,gunzip,zip,unzip,rar,unrar') return True From c1219e68c58e2789a915dd2dc2088c1626a5c431 Mon Sep 17 00:00:00 2001 From: Justin Findlay Date: Tue, 27 Sep 2016 16:27:19 -0600 Subject: [PATCH 28/33] modules.archive.unzip: depend on zipfile module --- salt/modules/archive.py | 1 + 1 file changed, 1 insertion(+) diff --git a/salt/modules/archive.py b/salt/modules/archive.py index 59859b3f32b..c3c77863aff 100644 --- a/salt/modules/archive.py +++ b/salt/modules/archive.py @@ -466,6 +466,7 @@ def cmd_unzip(zip_file, dest, excludes=None, return _trim_files(files, trim_output) +@salt.utils.decorators.depends('zipfile', fallback_function=cmd_unzip) def unzip(zip_file, dest, excludes=None, template=None, runas=None, trim_output=False, password=None): ''' From 99bf89447b0ca6090185f124a23070dbdecfedd9 Mon Sep 17 00:00:00 2001 From: Justin Findlay Date: Wed, 28 Sep 2016 12:50:44 -0600 Subject: [PATCH 29/33] modules.archive: add opts arg to g(un)zip --- salt/modules/archive.py | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/salt/modules/archive.py b/salt/modules/archive.py index c3c77863aff..7c412271dd0 100644 --- a/salt/modules/archive.py +++ b/salt/modules/archive.py @@ -132,7 +132,7 @@ def tar(options, tarfile, sources=None, dest=None, @salt.utils.decorators.which('gzip') -def gzip(sourcefile, template=None, runas=None): +def gzip(sourcefile, template=None, runas=None, options=None): ''' Uses the gzip command to create gzip files @@ -144,14 +144,27 @@ def gzip(sourcefile, template=None, runas=None): salt '*' archive.gzip template=jinja /tmp/{{grains.id}}.txt + runas : None + The user with which to run the gzip command line + + options : None + Pass any additional arguments to gzip + + .. versionadded:: 2016.3.4 + CLI Example: .. code-block:: bash # Create /tmp/sourcefile.txt.gz salt '*' archive.gzip /tmp/sourcefile.txt + salt '*' archive.gzip /tmp/sourcefile.txt options='-9 --verbose' ''' - cmd = ['gzip', '{0}'.format(sourcefile)] + cmd = ['gzip'] + if options: + cmd.append(options) + cmd.append('{0}'.format(sourcefile)) + return __salt__['cmd.run'](cmd, template=template, runas=runas, @@ -159,7 +172,7 @@ def gzip(sourcefile, template=None, runas=None): @salt.utils.decorators.which('gunzip') -def gunzip(gzipfile, template=None, runas=None): +def gunzip(gzipfile, template=None, runas=None, options=None): ''' Uses the gunzip command to unpack gzip files @@ -171,14 +184,27 @@ def gunzip(gzipfile, template=None, runas=None): salt '*' archive.gunzip template=jinja /tmp/{{grains.id}}.txt.gz + runas : None + The user with which to run the gzip command line + + options : None + Pass any additional arguments to gzip + + .. versionadded:: 2016.3.4 + CLI Example: .. code-block:: bash # Create /tmp/sourcefile.txt salt '*' archive.gunzip /tmp/sourcefile.txt.gz + salt '*' archive.gunzip /tmp/sourcefile.txt options='--verbose' ''' - cmd = ['gunzip', '{0}'.format(gzipfile)] + cmd = ['gunzip'] + if options: + cmd.append(options) + cmd.append('{0}'.format(gzipfile)) + return __salt__['cmd.run'](cmd, template=template, runas=runas, From 33ef5bffe62137725688ac56faf1874f67aad66d Mon Sep 17 00:00:00 2001 From: Nicole Thomas Date: Wed, 28 Sep 2016 15:51:48 -0600 Subject: [PATCH 30/33] Revert "Pr 36386" --- salt/config/__init__.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/salt/config/__init__.py b/salt/config/__init__.py index 4e1d5096b22..2d6dd29c7e0 100644 --- a/salt/config/__init__.py +++ b/salt/config/__init__.py @@ -1388,8 +1388,8 @@ CLOUD_CONFIG_DEFAULTS = { DEFAULT_API_OPTS = { # ----- Salt master settings overridden by Salt-API ---------------------> - 'api_pidfile': os.path.join(salt.syspaths.PIDFILE_DIR, 'salt-api.pid'), - 'api_logfile': os.path.join(salt.syspaths.LOGS_DIR, 'api'), + 'pidfile': '/var/run/salt-api.pid', + 'logfile': '/var/log/salt/api', 'rest_timeout': 300, # <---- Salt master settings overridden by Salt-API ---------------------- } @@ -3284,15 +3284,12 @@ def api_config(path): Read in the salt master config file and add additional configs that need to be stubbed out for salt-api ''' - # Let's grab a copy of salt's master opts - opts = client_config(path, defaults=DEFAULT_MASTER_OPTS) + # Let's grab a copy of salt's master default opts + defaults = DEFAULT_MASTER_OPTS # Let's override them with salt-api's required defaults - api_opts = { - 'log_file': opts.get('api_logfile', DEFAULT_API_OPTS['api_logfile']), - 'pidfile': opts.get('api_pidfile', DEFAULT_API_OPTS['api_pidfile']) - } - opts.update(api_opts) - return opts + defaults.update(DEFAULT_API_OPTS) + + return client_config(path, defaults=defaults) def spm_config(path): From b618a5c07d003f7bc76d9288b179aa51db532659 Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Wed, 28 Sep 2016 16:26:56 -0600 Subject: [PATCH 31/33] Remove debugging --- tests/integration/modules/pkg.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/integration/modules/pkg.py b/tests/integration/modules/pkg.py index 28adb3e9ab4..87d49c650b3 100644 --- a/tests/integration/modules/pkg.py +++ b/tests/integration/modules/pkg.py @@ -256,9 +256,9 @@ class PkgModuleTest(integration.ModuleCase, if os_family == 'Suse': # pkg.latest version returns empty if the latest version is already installed vim_version_dict = self.run_function('pkg.latest_version', ['vim']) - print('vim_version_dict: {0}'.format(vim_version_dict)) if vim_version_dict == {}: - # Latest version is installed, get its version and construct a version selector so the immediately previous version is selected + # Latest version is installed, get its version and construct + # a version selector so the immediately previous version is selected vim_version_dict = self.run_function('pkg.info_available', ['vim']) vim_version = 'version=<'+vim_version_dict['vim']['version'] else: @@ -268,7 +268,6 @@ class PkgModuleTest(integration.ModuleCase, # Install a version of vim that should need upgrading ret = self.run_function('pkg.install', ['vim', vim_version]) - print('pkg.install result: {0}'.format(ret)) # Run a system upgrade, which should catch the fact that Vim needs upgrading, and upgrade it. ret = self.run_function(func) From cc4d9585579f2ffbf3eeb08f1f30a5ca71dc01f2 Mon Sep 17 00:00:00 2001 From: Justin Findlay Date: Tue, 27 Sep 2016 16:27:39 -0600 Subject: [PATCH 32/33] modules.archive: add integration tests --- tests/integration/modules/archive.py | 268 +++++++++++++++++++++++++++ 1 file changed, 268 insertions(+) create mode 100644 tests/integration/modules/archive.py diff --git a/tests/integration/modules/archive.py b/tests/integration/modules/archive.py new file mode 100644 index 00000000000..f463d774672 --- /dev/null +++ b/tests/integration/modules/archive.py @@ -0,0 +1,268 @@ +# -*- coding: utf-8 -*- +''' +Tests for the archive state +''' +# Import python libs +from __future__ import absolute_import +import os +import shutil +import textwrap + +# Import Salt Testing libs +from salttesting import skipIf +from salttesting.helpers import ( + destructiveTest, + ensure_in_syspath +) +ensure_in_syspath('../../') + +# Import salt libs +import integration +import salt.utils + +# Import 3rd party libs +try: + import zipfile # pylint: disable=W0611 + HAS_ZIPFILE = True +except ImportError: + HAS_ZIPFILE = False + + +@destructiveTest +class ArchiveTest(integration.ModuleCase): + ''' + Validate the archive module + ''' + # Base path used for test artifacts + base_path = os.path.join(integration.TMP, 'modules', 'archive') + + def _set_artifact_paths(self, arch_fmt): + ''' + Define the paths for the source, archive, and destination files + + :param str arch_fmt: The archive format used in the test + ''' + self.src = os.path.join(self.base_path, '{0}_src_dir'.format(arch_fmt)) + self.src_file = os.path.join(self.src, 'file') + self.arch = os.path.join(self.base_path, 'archive.{0}'.format(arch_fmt)) + self.dst = os.path.join(self.base_path, '{0}_dst_dir'.format(arch_fmt)) + + def _set_up(self, arch_fmt): + ''' + Create source file tree and destination directory + + :param str arch_fmt: The archive format used in the test + ''' + # Setup artifact paths + self._set_artifact_paths(arch_fmt) + + # Remove the artifacts if any present + if any([os.path.exists(f) for f in (self.src, self.arch, self.dst)]): + self._tear_down() + + # Create source + os.makedirs(self.src) + with salt.utils.fopen(os.path.join(self.src, 'file'), 'w') as theorem: + theorem.write(textwrap.dedent(r'''\ + Compression theorem of computational complexity theory: + + Given a Gödel numbering $φ$ of the computable functions and a + Blum complexity measure $Φ$ where a complexity class for a + boundary function $f$ is defined as + + $\mathrm C(f) := \{φ_i ∈ \mathbb R^{(1)} | (∀^∞ x) Φ_i(x) ≤ f(x)\}$. + + Then there exists a total computable function $f$ so that for + all $i$ + + $\mathrm{Dom}(φ_i) = \mathrm{Dom}(φ_{f(i)})$ + + and + + $\mathrm C(φ_i) ⊊ \mathrm{C}(φ_{f(i)})$. + ''')) + + # Create destination + os.makedirs(self.dst) + + def _tear_down(self): + ''' + Remove source file tree, archive, and destination file tree + ''' + for f in (self.src, self.arch, self.dst): + if os.path.exists(f): + if os.path.isdir(f): + shutil.rmtree(f, ignore_errors=True) + else: + os.remove(f) + + def _assert_artifacts_in_ret(self, ret, file_only=False): + ''' + Assert that the artifact source files are printed in the source command + output + ''' + # Try to find source directory and file in output lines + dir_in_ret = None + file_in_ret = None + for line in ret: + if self.src.lstrip('/') in line \ + and not self.src_file.lstrip('/') in line: + dir_in_ret = True + if self.src_file.lstrip('/') in line: + file_in_ret = True + + # Assert number of lines, reporting of source directory and file + self.assertTrue(len(ret) >= 1 if file_only else 2) + if not file_only: + self.assertTrue(dir_in_ret) + self.assertTrue(file_in_ret) + + @skipIf(not salt.utils.which('tar'), 'Cannot find tar executable') + def test_tar_pack(self): + ''' + Validate using the tar function to create archives + ''' + self._set_up(arch_fmt='tar') + + # Test create archive + ret = self.run_function('archive.tar', ['-cvf', self.arch], sources=self.src) + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret) + + self._tear_down() + + @skipIf(not salt.utils.which('tar'), 'Cannot find tar executable') + def test_tar_unpack(self): + ''' + Validate using the tar function to extract archives + ''' + self._set_up(arch_fmt='tar') + self.run_function('archive.tar', ['-cvf', self.arch], sources=self.src) + + # Test extract archive + ret = self.run_function('archive.tar', ['-xvf', self.arch], dest=self.dst) + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret) + + self._tear_down() + + @skipIf(not salt.utils.which('gzip'), 'Cannot find gzip executable') + def test_gzip(self): + ''' + Validate using the gzip function + ''' + self._set_up(arch_fmt='gz') + + # Test create archive + ret = self.run_function('archive.gzip', [self.src_file], options='-v') + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret, file_only=True) + + self._tear_down() + + @skipIf(not salt.utils.which('gunzip'), 'Cannot find gunzip executable') + def test_gunzip(self): + ''' + Validate using the gunzip function + ''' + self._set_up(arch_fmt='gz') + self.run_function('archive.gzip', [self.src_file], options='-v') + + # Test extract archive + ret = self.run_function('archive.gunzip', [self.src_file + '.gz'], options='-v') + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret, file_only=True) + + self._tear_down() + + @skipIf(not salt.utils.which('zip'), 'Cannot find zip executable') + def test_cmd_zip(self): + ''' + Validate using the cmd_zip function + ''' + self._set_up(arch_fmt='zip') + + # Test create archive + ret = self.run_function('archive.cmd_zip', [self.arch, self.src]) + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret) + + self._tear_down() + + @skipIf(not salt.utils.which('unzip'), 'Cannot find unzip executable') + def test_cmd_unzip(self): + ''' + Validate using the cmd_unzip function + ''' + self._set_up(arch_fmt='zip') + self.run_function('archive.cmd_zip', [self.arch, self.src]) + + # Test create archive + ret = self.run_function('archive.cmd_unzip', [self.arch, self.dst]) + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret) + + self._tear_down() + + @skipIf(not HAS_ZIPFILE, 'Cannot find zip python module') + def test_zip(self): + ''' + Validate using the zip function + ''' + self._set_up(arch_fmt='zip') + + # Test create archive + ret = self.run_function('archive.zip', [self.arch, self.src]) + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret) + + self._tear_down() + + @skipIf(not HAS_ZIPFILE, 'Cannot find zip python module') + def test_unzip(self): + ''' + Validate using the unzip function + ''' + self._set_up(arch_fmt='zip') + self.run_function('archive.zip', [self.arch, self.src]) + + # Test create archive + ret = self.run_function('archive.unzip', [self.arch, self.dst]) + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret) + + self._tear_down() + + @skipIf(not salt.utils.which('rar'), 'Cannot find rar executable') + def test_rar(self): + ''' + Validate using the rar function + ''' + self._set_up(arch_fmt='rar') + + # Test create archive + ret = self.run_function('archive.rar', [self.arch, self.src]) + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret) + + self._tear_down() + + @skipIf(not salt.utils.which_bin(('rar', 'unrar')), 'Cannot find rar or unrar executable') + def test_unrar(self): + ''' + Validate using the unrar function + ''' + self._set_up(arch_fmt='rar') + self.run_function('archive.rar', [self.arch, self.src]) + + # Test create archive + ret = self.run_function('archive.unrar', [self.arch, self.dst]) + self.assertTrue(isinstance(ret, list)) + self._assert_artifacts_in_ret(ret) + + self._tear_down() + + +if __name__ == '__main__': + from integration import run_tests + run_tests(ArchiveTest) From 45352b36bdd43bbdfc06005063672223f33c33d9 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Wed, 28 Sep 2016 21:13:21 -0500 Subject: [PATCH 33/33] Support dynamic env in new-style git_pillar This was accidentally omitted when I implemented the gitfs-backed git_pillar for 2015.8.0. This commit adds that functionality back to git_pillar. --- salt/utils/gitfs.py | 33 +++++++++++++++++++++++---------- 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 6db58352e35..e91a087f867 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -569,6 +569,17 @@ class GitProvider(object): ''' raise NotImplementedError() + def get_checkout_target(self): + ''' + Resolve dynamically-set branch + ''' + if self.branch == '__env__': + target = self.opts.get('environment') or 'base' + return self.opts['{0}_base'.format(self.role)] \ + if target == 'base' \ + else target + return self.branch + def get_tree(self, tgt_env): ''' This function must be overridden in a sub-class @@ -622,6 +633,7 @@ class GitPython(GitProvider): GitPython when running these functions vary in different versions of GitPython. ''' + tgt_ref = self.get_checkout_target() try: head_sha = self.repo.rev_parse('HEAD').hexsha except Exception: @@ -629,11 +641,11 @@ class GitPython(GitProvider): # we fetch first before ever checking anything out. head_sha = None - # 'origin/' + self.branch ==> matches a branch head - # 'tags/' + self.branch + '@{commit}' ==> matches tag's commit + # 'origin/' + tgt_ref ==> matches a branch head + # 'tags/' + tgt_ref + '@{commit}' ==> matches tag's commit for rev_parse_target, checkout_ref in ( - ('origin/' + self.branch, 'origin/' + self.branch), - ('tags/' + self.branch + '@{commit}', 'tags/' + self.branch)): + ('origin/' + tgt_ref, 'origin/' + tgt_ref), + ('tags/' + tgt_ref, 'tags/' + tgt_ref)): try: target_sha = self.repo.rev_parse(rev_parse_target).hexsha except Exception: @@ -677,7 +689,7 @@ class GitPython(GitProvider): return self.check_root() log.error( 'Failed to checkout %s from %s remote \'%s\': remote ref does ' - 'not exist', self.branch, self.role, self.id + 'not exist', tgt_ref, self.role, self.id ) return None @@ -934,9 +946,10 @@ class Pygit2(GitProvider): ''' Checkout the configured branch/tag ''' - local_ref = 'refs/heads/' + self.branch - remote_ref = 'refs/remotes/origin/' + self.branch - tag_ref = 'refs/tags/' + self.branch + tgt_ref = self.get_checkout_target() + local_ref = 'refs/heads/' + tgt_ref + remote_ref = 'refs/remotes/origin/' + tgt_ref + tag_ref = 'refs/tags/' + tgt_ref try: local_head = self.repo.lookup_reference('HEAD') @@ -1103,7 +1116,7 @@ class Pygit2(GitProvider): except Exception as exc: log.error( 'Failed to checkout {0} from {1} remote \'{2}\': {3}'.format( - self.branch, + tgt_ref, self.role, self.id, exc @@ -1113,7 +1126,7 @@ class Pygit2(GitProvider): return None log.error( 'Failed to checkout {0} from {1} remote \'{2}\': remote ref ' - 'does not exist'.format(self.branch, self.role, self.id) + 'does not exist'.format(tgt_ref, self.role, self.id) ) return None