From c9010fb8e3df2cf60c4e8a17f8db63f09332ffd9 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Thu, 4 Dec 2014 19:15:07 -0800 Subject: [PATCH 01/33] Revert "Revert "Historically the recator has just called the "async" method of the runner and wheel clients, but this actually creates daemonized processes. In addition to creating a new daemonized process each event, the number of process it creates is unbounded, meaning that the reactor can easily use all available PIDs on a fairly busy master. In addition, there is no bound on the CPU that these are allowed to use (since they can create ALL the pids). This changes the reactor to create a threadpool for executing its master-side clients (runner/wheel). This threadpool has a configurable number of workers (max parallelism) and hwm (max queue size before dropping events)."" This reverts commit 8c7d66d6dc643bfbe4bd7c6e3b64b4630855df8a. --- salt/config.py | 4 +++ salt/utils/event.py | 16 +++++++----- salt/utils/process.py | 59 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/salt/config.py b/salt/config.py index 0d74eef68e..e81be85440 100644 --- a/salt/config.py +++ b/salt/config.py @@ -213,6 +213,8 @@ VALID_OPTS = { 'publish_session': int, 'reactor': list, 'reactor_refresh_interval': int, + 'reactor_worker_threads': int, + 'reactor_worker_hwm': int, 'serial': str, 'search': str, 'search_index_interval': int, @@ -524,6 +526,8 @@ DEFAULT_MASTER_OPTS = { 'range_server': 'range:80', 'reactor': [], 'reactor_refresh_interval': 60, + 'reactor_worker_threads': 10, + 'reactor_worker_hwm': 10000, 'serial': 'msgpack', 'state_verbose': True, 'state_output': 'full', diff --git a/salt/utils/event.py b/salt/utils/event.py index d1e431a541..9aef015956 100644 --- a/salt/utils/event.py +++ b/salt/utils/event.py @@ -73,6 +73,7 @@ import salt.loader import salt.state import salt.utils import salt.utils.cache +import salt.utils.process from salt._compat import string_types log = logging.getLogger(__name__) @@ -699,6 +700,11 @@ class ReactWrap(object): if ReactWrap.client_cache is None: ReactWrap.client_cache = salt.utils.cache.CacheDict(opts['reactor_refresh_interval']) + self.pool = salt.utils.process.ThreadPool( + self.opts['reactor_worker_threads'], # number of workers for runner/wheel + queue_size=self.opts['reactor_worker_hwm'] # queue size for those workers + ) + def run(self, low): ''' Execute the specified function in the specified state by passing the @@ -707,14 +713,12 @@ class ReactWrap(object): l_fun = getattr(self, low['state']) try: f_call = salt.utils.format_call(l_fun, low) - ret = l_fun(*f_call.get('args', ()), **f_call.get('kwargs', {})) + l_fun(*f_call.get('args', ()), **f_call.get('kwargs', {})) except Exception: log.error( 'Failed to execute {0}: {1}\n'.format(low['state'], l_fun), exc_info=True ) - return False - return ret def local(self, *args, **kwargs): ''' @@ -722,7 +726,7 @@ class ReactWrap(object): ''' if 'local' not in self.client_cache: self.client_cache['local'] = salt.client.LocalClient(self.opts['conf_file']) - return self.client_cache['local'].cmd_async(*args, **kwargs) + self.client_cache['local'].cmd_async(*args, **kwargs) cmd = local @@ -732,7 +736,7 @@ class ReactWrap(object): ''' if 'runner' not in self.client_cache: self.client_cache['runner'] = salt.runner.RunnerClient(self.opts) - return self.client_cache['runner'].async(fun, kwargs, fire_event=False) + self.pool.fire_async(self.client_cache['runner'].low, kwargs) def wheel(self, fun, **kwargs): ''' @@ -740,7 +744,7 @@ class ReactWrap(object): ''' if 'wheel' not in self.client_cache: self.client_cache['wheel'] = salt.wheel.Wheel(self.opts) - return self.client_cache['wheel'].async(fun, kwargs, fire_event=False) + self.pool.fire_async(self.client_cache['wheel'].low, kwargs) class StateFire(object): diff --git a/salt/utils/process.py b/salt/utils/process.py index 388e9aa663..734d6a03f7 100644 --- a/salt/utils/process.py +++ b/salt/utils/process.py @@ -8,6 +8,9 @@ import sys import multiprocessing import signal +import threading +import Queue + # Import salt libs import salt.utils @@ -119,6 +122,62 @@ def os_is_running(pid): return False +class ThreadPool(object): + ''' + This is a very VERY basic threadpool implementation + This was made instead of using multiprocessing ThreadPool because + we want to set max queue size and we want to daemonize threads (neither + is exposed in the stdlib version). + + Since there isn't much use for this class as of right now this implementation + Only supports daemonized threads and will *not* return results + + TODO: if this is found to be more generally useful it would be nice to pull + in the majority of code from upstream or from http://bit.ly/1wTeJtM + ''' + def __init__(self, + num_threads=None, + queue_size=0): + # if no count passed, default to number of CPUs + if num_threads is None: + num_threads = multiprocessing.cpu_count() + self.num_threads = num_threads + + # create a task queue of queue_size + self._job_queue = Queue.Queue(queue_size) + + self._workers = [] + + # create worker threads + for idx in xrange(num_threads): + thread = threading.Thread(target=self._thread_target) + thread.daemon = True + thread.start() + self._workers.append(thread) + + # intentionally not called "apply_async" since we aren't keeping track of + # the return at all, if we want to make this API compatible with multiprocessing + # threadpool we can in the future, and we won't have to worry about name collision + def fire_async(self, func, args=[], kwargs={}): + try: + self._job_queue.put((func, args, kwargs), False) + return True + except Queue.Full: + return False + + def _thread_target(self): + while True: + # 1s timeout so that if the parent dies this thread will die after 1s + try: + func, args, kwargs = self._job_queue.get(timeout=1) + except Queue.Empty: + continue + try: + func(*args, **kwargs) + except Exception: + pass + + class ProcessManager(object): ''' A class which will manage processes that should be running From d4b7642c3a553b6d0b7da06fe07a5ad754bcbe9b Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Thu, 4 Dec 2014 19:51:02 -0800 Subject: [PATCH 02/33] Fix for malformed SLS files crashing reactor Before if you had a malformed SLS file you'd get something like: ``` Process Reactor-4: Traceback (most recent call last): File "/usr/lib64/python2.6/multiprocessing/process.py", line 232, in _bootstrap self.run() File "/home/thjackso/src/salt-github/salt/utils/event.py", line 688, in run self.call_reactions(chunks) File "/home/thjackso/src/salt-github/salt/utils/event.py", line 674, in call_reactions self.wrap.run(chunk) File "/home/thjackso/src/salt-github/salt/utils/event.py", line 713, in run l_fun = getattr(self, low['state']) TypeError: string indices must be integers, not str [INFO ] Process (7656) died with exit status None, restarting... ``` Now you get something like: ``` [ERROR ] Unable to render reactions for event salt/job/20141204195047408340/new due to errors (['The type broken in runner is not formatted as a dictionary']) in one or more of the sls files (['/tmp/test.sls', '/tmp/test2.sls']) ``` --- salt/utils/event.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/salt/utils/event.py b/salt/utils/event.py index 9aef015956..cbcb39607d 100644 --- a/salt/utils/event.py +++ b/salt/utils/event.py @@ -662,7 +662,9 @@ class Reactor(multiprocessing.Process, salt.state.Compiler): if high: errors = self.verify_high(high) if errors: - return errors + log.error(('Unable to render reactions for event {0} due to ' + 'errors ({1}) in one or more of the sls files ({2})').format(tag, errors, reactors)) + return [] # We'll return nothing since there was an error chunks = self.order_chunks(self.compile_high_data(high)) return chunks From 5774c1f14cb6a62eb057a958874702f20ec9fc89 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 5 Dec 2014 02:16:05 -0800 Subject: [PATCH 03/33] Revert "Revert "Remove "fire_event" from AsyncClientMixin, since this was only added to remove infinite recusion in the reactor-- which is now not calling this API"" This reverts commit ba7f08d047afffc2b87950910cac0cdd48b76302. --- salt/client/mixins.py | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/salt/client/mixins.py b/salt/client/mixins.py index f225362bdc..a16240f6c8 100644 --- a/salt/client/mixins.py +++ b/salt/client/mixins.py @@ -62,7 +62,7 @@ class AsyncClientMixin(object): client = None tag_prefix = None - def _proc_function(self, fun, low, user, tag, jid, fire_event=True): + def _proc_function(self, fun, low, user, tag, jid): ''' Run this method in a multiprocess target to execute the function in a multiprocess and fire the return data on the event bus @@ -72,14 +72,13 @@ class AsyncClientMixin(object): 'jid': jid, 'user': user, } - if fire_event: - event = salt.utils.event.get_event( - 'master', - self.opts['sock_dir'], - self.opts['transport'], - opts=self.opts, - listen=False) - event.fire_event(data, tagify('new', base=tag)) + event = salt.utils.event.get_event( + 'master', + self.opts['sock_dir'], + self.opts['transport'], + opts=self.opts, + listen=False) + event.fire_event(data, tagify('new', base=tag)) try: data['return'] = self.low(fun, low) @@ -94,13 +93,12 @@ class AsyncClientMixin(object): data['success'] = False data['user'] = user - if fire_event: - event.fire_event(data, tagify('ret', base=tag)) - # if we fired an event, make sure to delete the event object. - # This will ensure that we call destroy, which will do the 0MQ linger - del event + event.fire_event(data, tagify('ret', base=tag)) + # if we fired an event, make sure to delete the event object. + # This will ensure that we call destroy, which will do the 0MQ linger + del event - def async(self, fun, low, user='UNKNOWN', fire_event=True): + def async(self, fun, low, user='UNKNOWN'): ''' Execute the function in a multiprocess and return the event tag to use to watch for the return @@ -110,7 +108,6 @@ class AsyncClientMixin(object): proc = multiprocessing.Process( target=self._proc_function, - args=(fun, low, user, tag, jid), - kwargs={'fire_event': fire_event}) + args=(fun, low, user, tag, jid)) proc.start() return {'tag': tag, 'jid': jid} From 616d4a360696eca8e550aabc838cdf304a36b204 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 5 Dec 2014 02:16:16 -0800 Subject: [PATCH 04/33] Revert "Revert "Remove some un-used variables"" This reverts commit 82b5567c9248f345e2b388e3d238d51b7a0d9640. --- salt/utils/event.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/salt/utils/event.py b/salt/utils/event.py index cbcb39607d..7576088e95 100644 --- a/salt/utils/event.py +++ b/salt/utils/event.py @@ -253,7 +253,7 @@ class SaltEvent(object): mtag = raw[0:20].rstrip('|') mdata = raw[20:] else: # new style - mtag, sep, mdata = raw.partition(TAGEND) # split tag from data + mtag, _, mdata = raw.partition(TAGEND) # split tag from data data = serial.loads(mdata) return mtag, data @@ -732,7 +732,7 @@ class ReactWrap(object): cmd = local - def runner(self, fun, **kwargs): + def runner(self, _, **kwargs): ''' Wrap RunnerClient for executing :ref:`runner modules ` ''' @@ -740,7 +740,7 @@ class ReactWrap(object): self.client_cache['runner'] = salt.runner.RunnerClient(self.opts) self.pool.fire_async(self.client_cache['runner'].low, kwargs) - def wheel(self, fun, **kwargs): + def wheel(self, _, **kwargs): ''' Wrap Wheel to enable executing :ref:`wheel modules ` ''' From eee14db8396a26025dbe084e220859513faa8e77 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 5 Dec 2014 02:16:39 -0800 Subject: [PATCH 05/33] Revert "Revert "Pylint cleanup for threadpool"" This reverts commit 32d01ee6c6bd4a7070be158c7188ede14cb34b60. --- salt/utils/process.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/salt/utils/process.py b/salt/utils/process.py index 734d6a03f7..b89fe6c96e 100644 --- a/salt/utils/process.py +++ b/salt/utils/process.py @@ -158,7 +158,11 @@ class ThreadPool(object): # intentionally not called "apply_async" since we aren't keeping track of # the return at all, if we want to make this API compatible with multiprocessing # threadpool we can in the future, and we won't have to worry about name collision - def fire_async(self, func, args=[], kwargs={}): + def fire_async(self, func, args=None, kwargs=None): + if args is None: + args = [] + if kwargs is None: + kwargs = {} try: self._job_queue.put((func, args, kwargs), False) return True From 83ecb5eedba506961cbace427f166bb35e11bf1c Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 12 Dec 2014 07:38:09 -0800 Subject: [PATCH 06/33] Add debug logging to threadpool targets --- salt/utils/process.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/utils/process.py b/salt/utils/process.py index b89fe6c96e..13e9a25ad9 100644 --- a/salt/utils/process.py +++ b/salt/utils/process.py @@ -178,8 +178,8 @@ class ThreadPool(object): continue try: func(*args, **kwargs) - except Exception: - pass + except Exception as err: + log.debug(err, exc_info=True) class ProcessManager(object): From e19b360e1ab3c9e7b2169fdb102cdce85e23057c Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 12 Dec 2014 07:38:28 -0800 Subject: [PATCH 07/33] Clarify comment --- salt/utils/process.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/utils/process.py b/salt/utils/process.py index 13e9a25ad9..e63bcac12d 100644 --- a/salt/utils/process.py +++ b/salt/utils/process.py @@ -171,7 +171,7 @@ class ThreadPool(object): def _thread_target(self): while True: - # 1s timeout so that if the parent dies this thread will die after 1s + # 1s timeout so that if the parent dies this thread will die within 1s try: func, args, kwargs = self._job_queue.get(timeout=1) except Queue.Empty: From 06e9b02ac94274e2fc631e52094235c9ac8f8d4c Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 12 Dec 2014 08:00:46 -0800 Subject: [PATCH 08/33] Instantiate the threadpool *after* forking. --- salt/utils/event.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/salt/utils/event.py b/salt/utils/event.py index 7576088e95..1b67c92beb 100644 --- a/salt/utils/event.py +++ b/salt/utils/event.py @@ -585,7 +585,6 @@ class Reactor(multiprocessing.Process, salt.state.Compiler): def __init__(self, opts): multiprocessing.Process.__init__(self) salt.state.Compiler.__init__(self, opts) - self.wrap = ReactWrap(self.opts) local_minion_opts = self.opts.copy() local_minion_opts['file_client'] = 'local' @@ -680,7 +679,11 @@ class Reactor(multiprocessing.Process, salt.state.Compiler): Enter into the server loop ''' salt.utils.appendproctitle(self.__class__.__name__) + + # instantiate some classes inside our new process self.event = SaltEvent('master', self.opts['sock_dir']) + self.wrap = ReactWrap(self.opts) + for data in self.event.iter_events(full=True): reactors = self.list_reactors(data['tag']) if not reactors: @@ -732,7 +735,7 @@ class ReactWrap(object): cmd = local - def runner(self, _, **kwargs): + def runner(self, **kwargs): ''' Wrap RunnerClient for executing :ref:`runner modules ` ''' @@ -740,7 +743,7 @@ class ReactWrap(object): self.client_cache['runner'] = salt.runner.RunnerClient(self.opts) self.pool.fire_async(self.client_cache['runner'].low, kwargs) - def wheel(self, _, **kwargs): + def wheel(self, **kwargs): ''' Wrap Wheel to enable executing :ref:`wheel modules ` ''' From 0026b544b360f0e0354d828320732fa0a8b58ae0 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 12 Dec 2014 08:01:06 -0800 Subject: [PATCH 09/33] Mark the task as done as soon as you pull it. Ff there is an exception while running the func, that doesn't mean we should keep trying --- salt/utils/process.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/salt/utils/process.py b/salt/utils/process.py index e63bcac12d..0268fc94dd 100644 --- a/salt/utils/process.py +++ b/salt/utils/process.py @@ -164,7 +164,7 @@ class ThreadPool(object): if kwargs is None: kwargs = {} try: - self._job_queue.put((func, args, kwargs), False) + self._job_queue.put_nowait((func, args, kwargs)) return True except Queue.Full: return False @@ -174,6 +174,7 @@ class ThreadPool(object): # 1s timeout so that if the parent dies this thread will die within 1s try: func, args, kwargs = self._job_queue.get(timeout=1) + self._job_queue.task_done() # Mark the task as done once we get it except Queue.Empty: continue try: From 77a7d9ab139f6762344762cf5d046025a92ca17e Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 12 Dec 2014 08:04:02 -0800 Subject: [PATCH 10/33] backport tests for process manager --- tests/unit/utils/process_test.py | 87 ++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 tests/unit/utils/process_test.py diff --git a/tests/unit/utils/process_test.py b/tests/unit/utils/process_test.py new file mode 100644 index 0000000000..a9f31bb32e --- /dev/null +++ b/tests/unit/utils/process_test.py @@ -0,0 +1,87 @@ +# -*- coding: utf-8 -*- + +# Import python libs +import os +import time +import signal +import multiprocessing + +# Import Salt Testing libs +from salttesting import TestCase +from salttesting.helpers import ensure_in_syspath +ensure_in_syspath('../../') + +# Import salt libs +import salt.utils.process + + +class TestProcessManager(TestCase): + + def test_basic(self): + ''' + Make sure that the process is alive 2s later + ''' + def spin(): + while True: + time.sleep(1) + + process_manager = salt.utils.process.ProcessManager() + process_manager.add_process(spin) + initial_pid = process_manager._process_map.keys()[0] + time.sleep(2) + process_manager.check_children() + assert initial_pid == process_manager._process_map.keys()[0] + process_manager.kill_children() + + def test_kill(self): + def spin(): + while True: + time.sleep(1) + + process_manager = salt.utils.process.ProcessManager() + process_manager.add_process(spin) + initial_pid = process_manager._process_map.keys()[0] + # kill the child + os.kill(initial_pid, signal.SIGTERM) + # give the OS time to give the signal... + time.sleep(0.1) + process_manager.check_children() + assert initial_pid != process_manager._process_map.keys()[0] + process_manager.kill_children() + + def test_restarting(self): + ''' + Make sure that the process is alive 2s later + ''' + def die(): + time.sleep(1) + + process_manager = salt.utils.process.ProcessManager() + process_manager.add_process(die) + initial_pid = process_manager._process_map.keys()[0] + time.sleep(2) + process_manager.check_children() + assert initial_pid != process_manager._process_map.keys()[0] + process_manager.kill_children() + + def test_counter(self): + def incr(counter, num): + for x in xrange(0, num): + counter.value += 1 + counter = multiprocessing.Value('i', 0) + process_manager = salt.utils.process.ProcessManager() + process_manager.add_process(incr, args=(counter, 2)) + time.sleep(1) + process_manager.check_children() + time.sleep(1) + # we should have had 2 processes go at it + assert counter.value == 4 + process_manager.kill_children() + + +if __name__ == '__main__': + from integration import run_tests + run_tests( + [TestProcessManager], + needs_daemon=False + ) From 0e6195f7781ea1808e90663838a95c5c82e88984 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 12 Dec 2014 08:15:34 -0800 Subject: [PATCH 11/33] Add some tests for ThreadPool --- tests/unit/utils/process_test.py | 42 +++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/unit/utils/process_test.py b/tests/unit/utils/process_test.py index a9f31bb32e..bcd085edc4 100644 --- a/tests/unit/utils/process_test.py +++ b/tests/unit/utils/process_test.py @@ -78,10 +78,50 @@ class TestProcessManager(TestCase): assert counter.value == 4 process_manager.kill_children() +class TestThreadPool(TestCase): + + def test_basic(self): + ''' + Make sure the threadpool can do things + ''' + counter = multiprocessing.Value('i', 0) + def incr_counter(counter): + counter.value += 1 + + + pool = salt.utils.process.ThreadPool() + sent = pool.fire_async(incr_counter, args=(counter,)) + self.assertTrue(sent) + time.sleep(1) # Sleep to let the threads do things + self.assertEqual(counter.value, 1) + self.assertEqual(pool._job_queue.qsize(), 0) + + def test_full_queue(self): + ''' + Make sure that a full threadpool acts as we expect + ''' + counter = multiprocessing.Value('i', 0) + def incr_counter(counter): + counter.value += 1 + + # Create a pool with no workers and 1 queue size + pool = salt.utils.process.ThreadPool(0, 1) + # make sure we can put the one item in + sent = pool.fire_async(incr_counter, args=(counter,)) + self.assertTrue(sent) + # make sure we can't put more in + sent = pool.fire_async(incr_counter, args=(counter,)) + self.assertFalse(sent) + time.sleep(1) # Sleep to let the threads do things + # make sure no one updated the counter + self.assertEqual(counter.value, 0) + # make sure the queue is still full + self.assertEqual(pool._job_queue.qsize(), 1) + if __name__ == '__main__': from integration import run_tests run_tests( - [TestProcessManager], + [TestProcessManager, TestThreadPool], needs_daemon=False ) From 545400e006c72469538ee7d1531b6bad9ffc4cc0 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Fri, 12 Dec 2014 09:42:41 -0800 Subject: [PATCH 12/33] Pylint cleanup --- tests/unit/utils/process_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/utils/process_test.py b/tests/unit/utils/process_test.py index bcd085edc4..bcc3cde847 100644 --- a/tests/unit/utils/process_test.py +++ b/tests/unit/utils/process_test.py @@ -78,16 +78,16 @@ class TestProcessManager(TestCase): assert counter.value == 4 process_manager.kill_children() + class TestThreadPool(TestCase): def test_basic(self): ''' Make sure the threadpool can do things ''' - counter = multiprocessing.Value('i', 0) def incr_counter(counter): counter.value += 1 - + counter = multiprocessing.Value('i', 0) pool = salt.utils.process.ThreadPool() sent = pool.fire_async(incr_counter, args=(counter,)) @@ -100,9 +100,9 @@ class TestThreadPool(TestCase): ''' Make sure that a full threadpool acts as we expect ''' - counter = multiprocessing.Value('i', 0) def incr_counter(counter): counter.value += 1 + counter = multiprocessing.Value('i', 0) # Create a pool with no workers and 1 queue size pool = salt.utils.process.ThreadPool(0, 1) From 6df8c23c1b9b8f59b5830a301530c413caa3b124 Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Fri, 12 Dec 2014 12:45:24 -0700 Subject: [PATCH 13/33] Fixup one more bad mdadm commandline --- salt/modules/mdadm.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/salt/modules/mdadm.py b/salt/modules/mdadm.py index 6165ad3e6f..bc1273e95f 100644 --- a/salt/modules/mdadm.py +++ b/salt/modules/mdadm.py @@ -47,7 +47,8 @@ def list_(): ''' ret = {} for line in (__salt__['cmd.run_stdout'] - ('mdadm --detail --scan', python_shell=False).splitlines()): + (['mdadm','--detail','--scan'], + python_shell=False).splitlines()): if ' ' not in line: continue comps = line.split() @@ -122,12 +123,13 @@ def destroy(device): except CommandExecutionError: return False - stop_cmd = 'mdadm --stop {0}'.format(device) - zero_cmd = 'mdadm --zero-superblock {0}' + stop_cmd = ['mdadm','--stop',device] + zero_cmd = ['mdadm', '--zero-superblock'] if __salt__['cmd.retcode'](stop_cmd): for number in details['members']: - __salt__['cmd.retcode'](zero_cmd.format(number['device'])) + zero_cmd.append(number['device']) + __salt__['cmd.retcode'](zero_cmd) # Remove entry from config file: if __grains__.get('os_family') == 'Debian': From 007d59724f68abdbeca6f614267b9f360e1f2f9b Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Fri, 12 Dec 2014 12:48:38 -0700 Subject: [PATCH 14/33] Indent error --- salt/modules/mdadm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/mdadm.py b/salt/modules/mdadm.py index bc1273e95f..f71d3e641a 100644 --- a/salt/modules/mdadm.py +++ b/salt/modules/mdadm.py @@ -129,7 +129,7 @@ def destroy(device): if __salt__['cmd.retcode'](stop_cmd): for number in details['members']: zero_cmd.append(number['device']) - __salt__['cmd.retcode'](zero_cmd) + __salt__['cmd.retcode'](zero_cmd) # Remove entry from config file: if __grains__.get('os_family') == 'Debian': From 92cf0a1d360eb3fa5657a9794cb58b70c88f3b6e Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Fri, 12 Dec 2014 12:50:01 -0700 Subject: [PATCH 15/33] Lint --- salt/modules/mdadm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/mdadm.py b/salt/modules/mdadm.py index f71d3e641a..30651e3a48 100644 --- a/salt/modules/mdadm.py +++ b/salt/modules/mdadm.py @@ -47,7 +47,7 @@ def list_(): ''' ret = {} for line in (__salt__['cmd.run_stdout'] - (['mdadm','--detail','--scan'], + (['mdadm', '--detail', '--scan'], python_shell=False).splitlines()): if ' ' not in line: continue @@ -123,7 +123,7 @@ def destroy(device): except CommandExecutionError: return False - stop_cmd = ['mdadm','--stop',device] + stop_cmd = ['mdadm', '--stop', device] zero_cmd = ['mdadm', '--zero-superblock'] if __salt__['cmd.retcode'](stop_cmd): From 6db5f4e3a5b98b4458e5fc3ef6c61bcf1fb306a6 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Fri, 12 Dec 2014 23:21:54 +0000 Subject: [PATCH 16/33] The `gpgkeys` path should use `salt.syspaths` for proper multi-platform support. Fixes #18877 --- salt/renderers/gpg.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/salt/renderers/gpg.py b/salt/renderers/gpg.py index 8e01e7c7aa..dd4642ecdd 100644 --- a/salt/renderers/gpg.py +++ b/salt/renderers/gpg.py @@ -70,8 +70,10 @@ Now you can include your ciphers in your pillar data like so: -----END PGP MESSAGE----- ''' +import os import re import salt.utils +import salt.syspaths try: import gnupg HAS_GPG = True @@ -85,7 +87,7 @@ from salt.exceptions import SaltRenderError log = logging.getLogger(__name__) GPG_HEADER = re.compile(r'-----BEGIN PGP MESSAGE-----') -DEFAULT_GPG_KEYDIR = '/etc/salt/gpgkeys' +DEFAULT_GPG_KEYDIR = os.path.join(salt.syspaths.CONFIG_DIR, 'gpgkeys') def decrypt_ciphertext(c, gpg): From e7922758ae8f92a4deae4b45b873929778d8ab9b Mon Sep 17 00:00:00 2001 From: Seth House Date: Tue, 2 Dec 2014 17:20:03 -0500 Subject: [PATCH 17/33] Minor rST formatting --- doc/topics/best_practices.rst | 40 +++++++++++++++++------------------ 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/doc/topics/best_practices.rst b/doc/topics/best_practices.rst index 42ce3745f1..fb6b4c0b5b 100644 --- a/doc/topics/best_practices.rst +++ b/doc/topics/best_practices.rst @@ -145,13 +145,13 @@ for variable definitions. Each SLS file within the ``/srv/pillar/`` directory should correspond to the states which it matches. -This would mean that the apache pillar file should contain data relevant to -apache. Structuring files in this way once again ensures modularity, and +This would mean that the ``apache`` pillar file should contain data relevant to +Apache. Structuring files in this way once again ensures modularity, and creates a consistent understanding throughout our Salt environment. Users can expect that pillar variables found in an Apache state will live inside of an Apache pillar: -/srv/salt/pillar/apache.sls +``/srv/salt/pillar/apache.sls``: .. code-block:: yaml @@ -178,7 +178,7 @@ lead to extensive flexibility. Although it is possible to set variables locally, this is generally not preferred: -/srv/salt/apache/conf.sls +``/srv/salt/apache/conf.sls``: .. code-block:: yaml @@ -203,7 +203,7 @@ When generating this information it can be easily transitioned to the pillar where data can be overwritten, modified, and applied to multiple states, or locations within a single state: -/srv/pillar/apache.sls +``/srv/pillar/apache.sls``: .. code-block:: yaml @@ -213,12 +213,10 @@ locations within a single state: config: tmpl: salt://apache/files/httpd.conf -/srv/salt/apache/conf.sls +``/srv/salt/apache/conf.sls``: .. code-block:: yaml - {% from "apache/map.jinja" import apache with context %} - include: - apache @@ -244,7 +242,7 @@ state could be re-used, and what it relies on to operate. Below are several examples which will iteratively explain how a user can go from a state which is not very modular to one that is: -/srv/salt/apache/init.sls: +``/srv/salt/apache/init.sls``: .. code-block:: yaml @@ -280,7 +278,7 @@ conf file. Our second revision begins to address the referencing by using ``- name``, as opposed to direct ID references: -/srv/salt/apache/init.sls: +``/srv/salt/apache/init.sls``: .. code-block:: yaml @@ -317,7 +315,7 @@ Starting with the addition of a map.jinja file (as noted in the :ref:`Formula documentation `), and modification of static values: -/srv/salt/apache/map.jinja: +``/srv/salt/apache/map.jinja``: .. code-block:: yaml @@ -343,7 +341,7 @@ modification of static values: config: tmpl: salt://apache/files/httpd.conf -/srv/salt/apache/init.sls: +``/srv/salt/apache/init.sls``: .. code-block:: yaml @@ -376,7 +374,7 @@ configuration file, but the default apache conf. With the current state setup this is not possible. To attain this level of modularity this state will need to be broken into two states. -/srv/salt/apache/map.jinja: +``/srv/salt/apache/map.jinja``: .. code-block:: yaml @@ -393,7 +391,7 @@ to be broken into two states. }, }, merge=salt['pillar.get']('apache:lookup')) %} -/srv/pillar/apache.sls: +``/srv/pillar/apache.sls``: .. code-block:: yaml @@ -403,7 +401,7 @@ to be broken into two states. tmpl: salt://apache/files/httpd.conf -/srv/salt/apache/init.sls: +``/srv/salt/apache/init.sls``: .. code-block:: yaml @@ -418,7 +416,7 @@ to be broken into two states. - enable: True - running -/srv/salt/apache/conf.sls: +``/srv/salt/apache/conf.sls``: .. code-block:: yaml @@ -457,7 +455,7 @@ those servers which require this secure data have access to it. In this example a use can go from an insecure configuration to one which is only accessible by the appropriate hosts: -/srv/salt/mysql/testerdb.sls: +``/srv/salt/mysql/testerdb.sls``: .. code-block:: yaml @@ -466,7 +464,7 @@ accessible by the appropriate hosts: - present: - name: testerdb -/srv/salt/mysql/user.sls: +``/srv/salt/mysql/user.sls``: .. code-block:: yaml @@ -504,7 +502,7 @@ portable it may result in more work later! Fixing this issue is relatively simple, the content just needs to be moved to the associated pillar: -/srv/pillar/mysql.sls +``/srv/pillar/mysql.sls``: .. code-block:: yaml @@ -515,7 +513,7 @@ the associated pillar: user: frank host: localhost -/srv/salt/mysql/testerdb.sls: +``/srv/salt/mysql/testerdb.sls``: .. code-block:: yaml @@ -524,7 +522,7 @@ the associated pillar: - present: - name: {{ salt['pillar.get']('mysql:lookup:name') }} -/srv/salt/mysql/user.sls: +``/srv/salt/mysql/user.sls``: .. code-block:: yaml From d0f038eec05700ae56d70ed5c06767f8848f8016 Mon Sep 17 00:00:00 2001 From: Seth House Date: Tue, 2 Dec 2014 18:34:32 -0500 Subject: [PATCH 18/33] Minor clarification to not pointing directly to formulas repos --- doc/topics/best_practices.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/doc/topics/best_practices.rst b/doc/topics/best_practices.rst index fb6b4c0b5b..a38f3185d6 100644 --- a/doc/topics/best_practices.rst +++ b/doc/topics/best_practices.rst @@ -90,8 +90,11 @@ be used as often as possible. .. note:: - Formulas should never be referenced from the main repository, and should - be forked to a repo where unintended changes will not take place. + Formulas repositories on the saltstack-formulas GitHub organization should + not be pointed to directly from systems that automatically fetch new + updates such as GitFS or similar tooling. Instead formulas repositories + should be forked on GitHub or cloned locally, where unintended, automatic + changes will not take place. Structuring Pillar Files From c0567bab0f884614a675637f7987ecf075215b3b Mon Sep 17 00:00:00 2001 From: Seth House Date: Thu, 4 Dec 2014 13:32:05 -0500 Subject: [PATCH 19/33] Updated Formula Best Practices doc with several recommendations --- .../development/conventions/formulas.rst | 674 +++++++++++++++++- 1 file changed, 662 insertions(+), 12 deletions(-) diff --git a/doc/topics/development/conventions/formulas.rst b/doc/topics/development/conventions/formulas.rst index f6ea5cd97f..68c85fdb69 100644 --- a/doc/topics/development/conventions/formulas.rst +++ b/doc/topics/development/conventions/formulas.rst @@ -219,19 +219,513 @@ on GitHub. manage which repositories they are subscribed to on GitHub's watching page: https://github.com/watching. -Abstracting platform-specific data ----------------------------------- +Style +----- -It is useful to have a single source for platform-specific or other static -information that can be reused throughout a Formula. Such a file should be -named :file:`map.jinja` and live alongside the state files. +Maintainability, readability, and reusability are all marks of a good Salt sls +file. This section contains several suggestions and examples. -The following is an example from the MySQL Formula. It is a simple dictionary -that serves as a lookup table (sometimes called a hash map or a dictionary). +.. code-block:: yaml + + # Deploy the stable master branch unless version overridden by passing + # Pillar at the CLI or via the Reactor. + + deploy_myapp: + git.latest: + - name: git@github.com/myco/myapp.git + - version: {{ salt.pillar.get('myapp:version', 'master') }} + +Use a descriptive State ID +`````````````````````````` + +The ID of a state is used as a unique identifier that may be referenced via +other states in :ref:`requisites `. It must be unique across the +whole state tree (:ref:`it is a key in a dictionary `, after +all). + +In addition a state ID should be descriptive and serve as a high-level hint of +what it will do, or manage, or change. For example, ``deploy_webapp``, or +``apache``, or ``reload_firewall``. + +Use ``module.function`` notation +```````````````````````````````` + +So-called "short-declaration" notation is preferred for referencing state +modules and state functions. It provides a consistent pattern of +``module.function`` shared between Salt States, the Reactor, Overstate, Salt +Mine, the Scheduler, as well as with the CLI. + +.. code-block:: yaml + + # Do + apache: + pkg.installed: + - name: httpd + + # Don't + apache: + pkg: + - installed + - name: httpd + +Salt's state compiler will transform "short-decs" into the longer format +:ref:`when compiling the human-friendly highstate structure into the +machine-friendly lowstate structure `. + +Specify the ``name`` parameter +`````````````````````````````` + +Use a unique and permanent identifier for the state ID and reserve ``name`` for +data with variability. + +The :ref:`name declaration ` is a required parameter for all +state functions. The state ID will implicitly be used as ``name`` if it is not +explicitly set in the state. + +In many state functions the ``name`` parameter is used for data that varies +such as OS-specific package names, OS-specific file system paths, repository +addresses, etc. Any time the ID of a state changes all references to that ID +must also be changed. Use a permanent ID when writing a state the first time to +future-proof that state and allow for easier refactors down the road. + +Comment state files +``````````````````` + +YAML allows comments at varying indentation levels. It is a good practice to +comment state files. Use vertical whitespace to visually separate different +concepts or actions. + +.. code-block:: yaml + + # Start with a high-level description of the current sls file. + # Explain the scope of what it will do or manage. + + # Comment individual states as necessary. + update_a_config_file: + # Provide details on why an unusual choice was made. For example: + # + # This template is fetched from a third-party and does not fit our + # company norm of using Jinja. This must be processed using Mako. + file.managed: + - name: /path/to/file.cfg + - source: salt://path/to/file.cfg.template + - template: mako + + # Provide a description or explanation that did not fit within the state + # ID. For example: + # + # Update the application's last-deployed timestamp. + # This is a workaround until Bob configures Jenkins to automate RPM + # builds of the app. + cmd.run: + # FIXME: Joe needs this to run on Windows by next quarter. Switch these + # from shell commands to Salt's file.managed and file.replace state + # modules. + - name: | + touch /path/to/file_last_updated + sed -e 's/foo/bar/g' /path/to/file_environment + - onchanges: + - file: a_config_file + +Be careful to use Jinja comments for commenting Jinja code and YAML comments +for commenting YAML code. + +.. code-block:: jinja + + # BAD EXAMPLE + # The Jinja in this YAML comment is still executed! + # {% set apache_is_installed = 'apache' in salt.pkg.list_pkgs() %} + + # GOOD EXAMPLE + # The Jinja in this Jinja comment will not be executed. + {# {% set apache_is_installed = 'apache' in salt.pkg.list_pkgs() %} #} + +Easy on the Jinja! +------------------ + +Jinja templating provides vast flexibility and power when building Salt sls +files. It can also create an unmaintainable tangle of logic and data. Speaking +broadly, Jinja is best used when kept apart from the states (as much as is +possible). + +Below are guidelines and examples of how Jinja can be used effectively. + +Know the evaluation and execution order +``````````````````````````````````````` + +High-level knowledge of how Salt states are compiled and run is useful when +writing states. + +The default :conf_minion:`renderer` setting in Salt is Jinja piped to YAML. +Each is a separate step. Each step is not aware of the previous or following +step. Jinja is not YAML aware, YAML is not Jinja aware; they cannot share +variables or interact. + +* Whatever the Jinja step produces must be valid YAML. +* Whatever the YAML step produces must be a valid :ref:`highstate data + structure `. (This is also true of the final step + for :ref:`any of the alternate renderers ` in Salt.) +* Highstate can be thought of as a human-friendly data structure; easy to write + and easy to read. +* Salt's state compiler validates the highstate and compiles it to low state. +* Low state can be thought of as a machine-friendly data structure. It is a + list of dictionaries that each map directly to a function call. +* Salt's state system finally starts and executes on each "chunk" in the low + state. Remember that requisites are evaluated at runtime. +* The return for each function call is added to the "running" dictionary which + is the final output at the end of the state run. + +The full evaluation and execution order:: + + Jinja -> YAML -> Highstate -> low state -> execution + +Avoid changing the underlying system with Jinja +``````````````````````````````````````````````` + +Avoid calling commands from Jinja that change the underlying system. Commands +run via Jinja do not respect Salt's dry-run mode (``test=True``)! This is +usually in conflict with the idempotent nature of Salt states unless the +command being run is also idempotent. + +Inspect the local system +```````````````````````` + +A common use for Jinja in Salt states is to gather information about the +underlying system. The ``grains`` dictionary available in the Jinja context is +a great example of common data points that Salt itself has already gathered. +Less common values are often found by running commands. For example: + +.. code-block:: jinja + + {% set is_selinux_enabled = salt.cmd.run('sestatus') == '1' %} + +This is usually best done with a variable assignment in order to separate the +data from the state that will make use of the data. + +Gather external data +```````````````````` + +One of the most common uses for Jinja is to pull external data into the state +file. External data can come from anywhere like API calls or database queries, +but it most commonly comes from flat files on the file system or Pillar data +from the Salt Master. For example: + +.. code-block:: jinja + + {% set some_data = salt.pillar.get('some_data', {'sane default': True}) %} + + {# or #} + + {% load_json 'path/to/file.json' as some_data %} + + {# or #} + + {% load_text 'path/to/ssh_key.pub' as ssh_pub_key %} + + {# or #} + + {% from 'path/to/other_file.jinja' import some_data with context %} + +This is usually best done with a variable assignment in order to separate the +data from the state that will make use of the data. + +Light conditionals and looping +`````````````````````````````` + +Jinja is extremely powerful for programatically generating Salt states. It is +also easy to overuse. As a rule of thumb, if it is hard to read it will be hard +to maintain! + +Separate Jinja control-flow statements from the states as much as is possible +to create readable states. Limit Jinja within states to simple variable +lookups. + +Below is a simple example of a readable loop: + +.. code-block:: yaml + + {% for user in salt.pillar.get('list_of_users', []) %} + + {# Ensure unique state IDs when looping. #} + {{ user.name }}-{{ loop.index }}: + user.present: + - name: {{ user.name }} + - shell: {{ user.shell }} + + {% endfor %} + +Avoid putting a Jinja conditionals within Salt states where possible. +Readability suffers and the correct YAML indentation is difficult to see in the +surrounding visual noise. Parameterization (discussed below) and variables are +both useful techniques to avoid this. For example: + +.. code-block:: yaml + + {# ---- Bad example ---- #} + + apache: + pkg.installed: + {% if grains.os_family == 'RedHat' %} + - name: httpd + {% elif grains.os_family == 'Debian' %} + - name: apache2 + {% endif %} + + {# ---- Better example ---- #} + + {% if grains.os_family == 'RedHat' %} + {% set name = 'httpd' %} + {% elif grains.os_family == 'Debian' %} + {% set name = 'apache2' %} + {% endif %} + + apache: + pkg.installed: + - name: {{ name }} + + {# ---- Good example ---- #} + + {% set name = { + 'RedHat': 'httpd', + 'Debian': 'apache2', + }.get(grains.os_family) %} + + apache: + pkg.installed: + - name: {{ name }} + +Dictionaries are useful to effectively "namespace" a collection of variables. +This is useful with parameterization (discussed below). Dictionaries are also +easily combined and merged. And they can be directly serialized into YAML which +is often easier than trying to create valid YAML through templating. For +example: + +.. code-block:: yaml + + {# ---- Bad example ---- #} + + haproxy_conf: + file.managed: + - name: /etc/haproxy/haproxy.cfg + - template: jinja + {% if 'external_loadbalancer' in grains.roles %} + - source: salt://haproxy/external_haproxy.cfg + {% elif 'internal_loadbalancer' in grains.roles %} + - source: salt://haproxy/internal_haproxy.cfg + {% endif %} + - context: + {% if 'external_loadbalancer' in grains.roles %} + ssl_termination: True + {% elif 'internal_loadbalancer' in grains.roles %} + ssl_termination: False + {% endif %} + + {# ---- Better example ---- #} + + {% load_yaml as haproxy_defaults %} + common_settings: + bind_port: 80 + + internal_loadbalancer: + source: salt://haproxy/internal_haproxy.cfg + settings: + bind_port: 8080 + ssl_termination: False + + external_loadbalancer: + source: salt://haproxy/external_haproxy.cfg + settings: + ssl_termination: True + {% endload %} + + {% if 'external_loadbalancer' in grains.roles %} + {% set haproxy = haproxy_defaults['external_loadbalancer'] %} + {% elif 'internal_loadbalancer' in grains.roles %} + {% set haproxy = haproxy_defaults['internal_loadbalancer'] %} + {% endif %} + + {% do haproxy.settings.update(haproxy_defaults.common_settings) %} + + haproxy_conf: + file.managed: + - name: /etc/haproxy/haproxy.cfg + - template: jinja + - source: {{ haproxy.source }} + - context: {{ haproxy.settings | yaml() }} + +There is still room for improvement in the above example. For example, +extracting into an external file or replacing the if-elif conditional with a +function call to filter the correct data more succinctly. However, the state +itself is simple and legible, the data is separate and also simple and legible. +And those suggested improvements can be made at some future date without +altering the state at all! + +Avoid heavy logic and programming +````````````````````````````````` + +Jinja is not Python. It was made by Python programmers and shares many +semantics and some syntax but it does not allow for abitrary Python function +calls or Python imports. Jinja is a fast and efficient templating language but +the syntax can be verbose and visually noisy. + +Once Jinja use within an sls file becomes slightly complicated -- long chains +of if-elif-elif-else statements, nested conditionals, complicated dictionary +merges, wanting to use sets -- instead consider using a different Salt +renderer, such as the Python renderer. As a rule of thumb, if it is hard to +read it will be hard to maintain -- switch to a format that is easier to read. + +Using alternate renderers is very simple to do using Salt's "she-bang" syntax +at the top of the file. The Python renderer must simply return the correct +:ref:`highstate data structure `. The following +example is a state tree of two sls files, one simple and one complicated. + +``/srv/salt/top.sls``: + +.. code-block:: yaml + + base: + '*': + - common_configuration + - roles_configuration + +``/srv/salt/common_configuration.sls``: + +.. code-block:: yaml + + common_users: + user.present: + - names: [larry, curly, moe] + +``/srv/salt/roles_configuration``: + +.. code-block:: python + + #!py + def run(): + list_of_roles = set() + + # This example has the minion id in the form 'web-03-dev'. + # Easily access the grains dictionary: + try: + app, instance_number, environment = __grains__['id'].split('-') + instance_number = int(instance_number) + except ValueError: + app, instance_number, environment = ['Unknown', 0, 'dev'] + + list_of_roles.add(app) + + if app == 'web' and environment == 'dev': + list_of_roles.add('primary') + list_of_roles.add('secondary') + elif app == 'web' and environment == 'staging': + if instance_number == 0: + list_of_roles.add('primary') + else: + list_of_roles.add('secondary') + + # Easily cross-call Salt execution modules: + if __salt__['myutils.query_valid_ec2_instance'](): + list_of_roles.add('is_ec2_instance') + + return { + 'set_roles_grains': { + 'grains.present': [ + {'name': 'roles'}, + {'value': list(list_of_roles)}, + ], + }, + } + +Jinja Macros +```````````` + +In Salt sls files Jinja macros are useful for one thing and one thing only: +creating mini templates that can be reused and rendered on demand. Do not fall +into the trap of thinking of macros as functions; Jinja is not Python (see +above). + +Macros are useful for creating reusable, parameterized states. For example: + +.. code-block:: yaml + + {% macro user_state(state_id, user_name, shell='/bin/bash', groups=[]) %} + {{ state_id }}: + user.present: + - name: {{ user_name }} + - shell: {{ shell }} + - groups: {{ groups | json() }} + {% endmacro %} + + {% for user_info in salt.pillar.get('my_users', []) %} + {{ user_state('user_number_' ~ loop.index, **user_info) }} + {% endfor %} + +Macros are also useful for creating one-off "serializers" that can accept a +data structure and write that out as a domain-specific configuration file. For +example, the following macro could be used to write a php.ini config file: + +``/srv/salt/php.sls``: + +.. code-block:: yaml + + php_ini: + file.managed: + - name: /etc/php.ini + - source: salt://php.ini.tmpl + - template: jinja + - context: + php_ini_settings: {{ salt.pillar.get('php_ini', {}) | json() }} + +``/srv/pillar/php.sls``: + +.. code-block:: yaml + + PHP: + engine: 'On' + short_open_tag: 'Off' + error_reporting: 'E_ALL & ~E_DEPRECATED & ~E_STRICT' + +``/srv/salt/php.ini.tmpl``: + +.. code-block:: jinja + + {% macro php_ini_serializer(data) %} + {% for section_name, name_val_pairs in data.items() %} + [{{ section }}] + {% for name, val in name_val_pairs.items() %} + {{ name }} = "{{ val }}" + {% endfor %} + {% endfor %} + {% endmacro %} + + ; File managed by Salt at <{{ source }}>. + ; Your changes will be overwritten. + + {{ php_ini_serializer(php_ini_settings) }} + +Abstracting static defaults into a lookup table +----------------------------------------------- + +Separate data that a state uses from the state itself to increases the +flexibility and reusability of a state. + +An obvious and common example of this is platform-specific package names and +file system paths. Another example is sane defaults for an application, or +common settings within a company or organization. Organizing such data as a +dictionary (aka hash map, lookup table, associative array) often provides a +lightweight namespacing and allows for quick and easy lookups. In addition, +using a dictionary allows for easily merging and overriding static values +within a lookup table with dynamic values fetched from Pillar. + +A strong convention in Salt Formulas is to place platform-specific data, such +as package names and file system paths, into a file named :file:`map.jinja` +that is placed alongside the state files. + +The following is an example from the MySQL Formula. The :py:func:`grains.filter_by ` function performs a lookup on that table using the ``os_family`` grain (by default). -The result is that the ``mysql`` variable is assigned to one of *subsets* of +The result is that the ``mysql`` variable is assigned to a *subset* of the lookup table for the current platform. This allows states to reference, for example, the name of a package without worrying about the underlying OS. The syntax for referencing a value is a normal dictionary lookup in Jinja, such as @@ -283,11 +777,13 @@ state file using the following syntax: Overriding values in the lookup table ````````````````````````````````````` -Any value in the lookup table may be overridden using Pillar. +Allow static values within lookup tables to be overridden. This is a simple +pattern which once again increases flexibility and reusability for state files. -The ``merge`` keyword specifies the location of a dictionary in Pillar that can -be used to override values returned from the lookup table. If the value exists -in Pillar it will take precedence. +The ``merge`` argument in :py:func:`filter_by ` +specifies the location of a dictionary in Pillar that can be used to override +values returned from the lookup table. If the value exists in Pillar it will +take precedence. This is useful when software or configuration files is installed to non-standard locations or on unsupported platforms. For example, the following @@ -299,6 +795,160 @@ Pillar would replace the ``config`` value from the call above. lookup: config: /usr/local/etc/mysql/my.cnf +The :py:func:`filter_by ` function performs a +simple dictionary lookup but also allows for fetching data from Pillar and +overriding data stored in the lookup table. That same workflow can be easily +performed without using ``filter_by``; other dictionaries besides data from +Pillar can also be used. + +.. code-block:: jinja + + {% set lookup_table = {...} %} + {% do lookup_table.update(salt.pillar.get('my:custom:data')) %} + +When to use lookup tables +````````````````````````` + +The ``map.jinja`` file is only a convention within Salt Formulas. This greater +pattern is useful for a wide variety of data in a wide variety of workflows. +This pattern is not limited to pulling data from a single file or data source. +This pattern is useful in States, Pillar, the Reactor, and Overstate as well. + +Working with a data structure instead of, say, a config file allows the data to +be cobbled together from multiple sources (local files, remote Pillar, database +queries, etc), combined, overridden, and searched. + +Below are a few examples of what lookup tables may be useful for and how they +may be used and represented. + +Platform-specific information +............................. + +An obvious pattern and one used heavily in Salt Formulas is extracting +platform-specific information such as package names and file system paths in +a file named ``map.jinja``. The pattern is explained in detail above. + +Sane defaults +............. + +Application settings can be a good fit for this pattern. Store default +settings along with the states themselves and keep overrides and sensitive +settings in Pillar. Combine both into a single dictionary and then write the +application config or settings file. + +The example below stores most of the Apache Tomcat ``server.xml`` file +alongside the Tomcat states and then allows values to be updated or augmented +via Pillar. (This example uses the BadgerFish format for transforming JSON to +XML.) + +``/srv/salt/tomcat/defaults.yaml``: + +.. code-block:: yaml + + Server: + '@port': '8005' + '@shutdown': SHUTDOWN + GlobalNamingResources: + Resource: + '@auth': Container + '@description': User database that can be updated and saved + '@factory': org.apache.catalina.users.MemoryUserDatabaseFactory + '@name': UserDatabase + '@pathname': conf/tomcat-users.xml + '@type': org.apache.catalina.UserDatabase + # <...snip...> + +``/srv/pillar/tomcat.sls``: + +.. code-block:: yaml + + appX: + server_xml_overrides: + Server: + Service: + '@name': Catalina + Connector: + '@port': '8009' + '@protocol': AJP/1.3 + '@redirectPort': '8443' + # <...snip...> + +``/srv/salt/tomcat/server_xml.sls``: + +.. code-block:: yaml + + {% load_yaml 'tomcat/defaults.yaml' as server_xml_defaults %} + {% set server_xml_final_values = salt.pillar.get( + 'appX:server_xml_overrides', + default=server_xml_defaults, + merge=True) + %} + + appX_server_xml: + file.serialize: + - name: /etc/tomcat/server.xml + - dataset: {{ server_xml_final_values | json() }} + - formatter: xml_badgerfish + +The :py:func:`file.serialize ` state can provide a +shorthand for creating some files from data structures. There are also many +examples within Salt Formulas of creating one-off "serializers" (often as Jinja +macros) that reformat a data structure to a specific config file format. For +example, `Nginx vhosts`__ or the `php.ini`__ + +__: https://github.com/saltstack-formulas/nginx-formula/blob/5cad4512/nginx/ng/vhosts_config.sls +__: https://github.com/saltstack-formulas/php-formula/blob/82e2cd3a/php/ng/files/php.ini + +Environment specific information +................................ + +A single state can be reused when it is parameterized as described in the +section below, by separating the data the state will use from the state that +performs the work. This can be the difference between deploying *Application X* +and *Application Y*, or the difference between production and development. For +example: + +``/srv/salt/app/deploy.sls``: + +.. code-block:: yaml + + {# Load the map file. #} + {% load_yaml 'app/defaults.yaml' as app_defaults %} + + {# Extract the relevant subset for the app configured on the current + machine (configured via a grain in this example). #} + {% app = app_defaults.get(salt.grains.get('role') %} + + {# Allow values from Pillar to (optionally) update values from the lookup + table. #} + {% do app_defaults.update(salt.pillar.get('myapp', {}) %} + + deploy_application: + git.latest: + - name: {{ app.repo_url }} + - version: {{ app.version }} + - target: {{ app.deploy_dir }} + + myco/myapp/deployed: + event.send: + - data: + version: {{ app.version }} + - onchanges: + - git: deploy_application + +``/srv/salt/app/defaults.yaml``: + +.. code-block:: yaml + + appX: + repo_url: git@github.com/myco/appX.git + target: /var/www/appX + version: master + appY: + repo_url: git@github.com/myco/appY.git + target: /var/www/appY + version: v1.2.3.4 + Single-purpose SLS files ------------------------ From ba38050a20cf2708bd37e789ad18211487182327 Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Fri, 12 Dec 2014 10:46:07 -0800 Subject: [PATCH 20/33] Each line can have any number of optional parameters, we use the location of the seperator field to determine the location of the elements after it. On remount, the remount option was ending up in the /etc/fstab. Ensuring that it is removed from the options. Some mount options end up in the superopts so we should look for them there too. --- salt/modules/mount.py | 12 +++++++++--- salt/states/mount.py | 6 +++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/salt/modules/mount.py b/salt/modules/mount.py index 749244d1ba..c5a622fe5e 100644 --- a/salt/modules/mount.py +++ b/salt/modules/mount.py @@ -52,7 +52,13 @@ def _active_mountinfo(ret): for line in ifile: comps = line.split() device = comps[2].split(':') - device_name = comps[8] + # each line can have any number of + # optional parameters, we use the + # location of the seperator field to + # determine the location of the elements + # after it. + _sep = comps.index('-') + device_name = comps[_sep + 2] device_uuid = None if device_name: device_uuid = blkid_info.get(device_name, {}).get('UUID') @@ -63,10 +69,10 @@ def _active_mountinfo(ret): 'minor': device[1], 'root': comps[3], 'opts': comps[5].split(','), - 'fstype': comps[7], + 'fstype': comps[_sep + 1], 'device': device_name, 'alt_device': _list.get(comps[4], None), - 'superopts': comps[9].split(','), + 'superopts': comps[_sep + 3].split(','), 'device_uuid': device_uuid} return ret diff --git a/salt/states/mount.py b/salt/states/mount.py index 12168a95fe..5f30365592 100644 --- a/salt/states/mount.py +++ b/salt/states/mount.py @@ -149,7 +149,7 @@ def mounted(name, comment_option = opt.split('=')[0] if comment_option == 'comment': opt = comment_option - if opt not in active[real_name]['opts'] and opt not in mount_invisible_options: + if opt not in active[real_name]['opts'] and opt not in active[real_name]['superopts'] and opt not in mount_invisible_options: if __opts__['test']: ret['result'] = None ret['comment'] = "Remount would be forced because options changed" @@ -159,6 +159,10 @@ def mounted(name, + "options changed" remount_result = __salt__['mount.remount'](real_name, device, mkmnt=mkmnt, fstype=fstype, opts=opts) ret['result'] = remount_result + # Cleanup after the remount, so we + # don't write remount into fstab + if 'remount' in opts: + opts.remove('remount') if real_device not in device_list: # name matches but device doesn't - need to umount if __opts__['test']: From 51fa87c446477dd040b7b8400408bd4898a34239 Mon Sep 17 00:00:00 2001 From: Seth House Date: Fri, 12 Dec 2014 23:29:48 -0700 Subject: [PATCH 21/33] Change all state examples to use short-dec format for consistency Closes #12419. The command used to search the docs for state examples is below. This generates a file in Vim's quickfix format that can be loaded with the -q flag. find . -type f \( -name '*.rst' -o -name '*.py' \) -print0 | xargs -0 -P10 -n1 awk ' BEGIN { RS=""; FS="\n" } { linenr = linect; linect += (NF + 1) } /^\s+[a-zA-Z0-9'\''_-]+:\n\s+[a-zA-Z0-9'\''_-]+:\n\s+- / { print FILENAME "|" linenr + 2 "|", $1 } ' > ./salt-states.quickfix --- doc/faq.rst | 16 ++---- doc/ref/states/compiler_ordering.rst | 57 +++++++++++-------- doc/ref/states/highstate.rst | 19 +++---- doc/ref/states/index.rst | 36 ++++++------ doc/ref/states/ordering.rst | 18 ++---- doc/ref/states/requisites.rst | 32 ++++------- doc/topics/best_practices.rst | 54 ++++++------------ .../development/conventions/formulas.rst | 24 +++----- doc/topics/installation/ubuntu.rst | 9 ++- doc/topics/mine/index.rst | 3 +- doc/topics/pillar/index.rst | 6 +- doc/topics/tutorials/cloud_controller.rst | 18 ++---- doc/topics/tutorials/pillar.rst | 6 +- doc/topics/tutorials/starting_states.rst | 18 ++---- doc/topics/tutorials/states_pt2.rst | 21 +++---- doc/topics/tutorials/states_pt3.rst | 3 +- doc/topics/tutorials/walkthrough.rst | 8 +-- salt/client/ssh/wrapper/grains.py | 6 +- salt/modules/daemontools.py | 3 +- salt/modules/grains.py | 6 +- salt/states/archive.py | 6 +- salt/states/blockdev.py | 3 +- salt/states/cmd.py | 9 +-- salt/states/glusterfs.py | 3 +- salt/states/npm.py | 6 +- salt/states/pkgng.py | 3 +- salt/states/powerpath.py | 3 +- salt/states/rabbitmq_plugin.py | 3 +- salt/states/rvm.py | 9 ++- salt/states/service.py | 9 +-- salt/states/ssh_auth.py | 12 ++-- salt/states/supervisord.py | 3 +- salt/states/tomcat.py | 9 +-- salt/states/win_system.py | 6 +- 34 files changed, 173 insertions(+), 274 deletions(-) diff --git a/doc/faq.rst b/doc/faq.rst index ad6f982ade..fa6ccb862d 100644 --- a/doc/faq.rst +++ b/doc/faq.rst @@ -203,8 +203,7 @@ Linux/Unix .. code-block:: yaml salt-minion-reload: - cmd: - - run + cmd.run: - name: echo service salt-minion restart | at now + 1 minute - order: last @@ -215,10 +214,9 @@ distro the minion is running, in case they differ from the example below. .. code-block:: yaml at: - pkg: - - installed - service: - - running + pkg.installed: + - name: at + service.running: - name: atd - enable: True @@ -240,13 +238,11 @@ Windows .. code-block:: yaml schedule-start: - cmd: - - run + cmd.run: - name: at (Get-Date).AddMinutes(1).ToString("HH:mm") cmd /c "net start salt-minion" - shell: powershell - order: last - service: - - dead + service.dead: - name: salt-minion - require: - cmd: schedule-start diff --git a/doc/ref/states/compiler_ordering.rst b/doc/ref/states/compiler_ordering.rst index a11e00511d..01fbfea640 100644 --- a/doc/ref/states/compiler_ordering.rst +++ b/doc/ref/states/compiler_ordering.rst @@ -57,16 +57,17 @@ As an example, a state written thusly: .. code-block:: yaml apache: - pkg: - - installed - service: - - running + pkg.installed: + - name: httpd + service.running: + - name: httpd - watch: - - file: /etc/httpd/conf.d/httpd.conf + - file: apache_conf - pkg: apache - /etc/httpd/conf.d/httpd.conf: - file: - - managed + + apache_conf: + file.managed: + - name: /etc/httpd/conf.d/httpd.conf - source: salt://apache/httpd.conf Will have High Data which looks like this represented in json: @@ -76,41 +77,50 @@ Will have High Data which looks like this represented in json: { "apache": { "pkg": [ + { + "name": "httpd" + }, "installed", { "order": 10000 } ], "service": [ - "running", + { + "name": "httpd" + }, { "watch": [ { - "file": "/etc/httpd/conf.d/httpd.conf" + "file": "apache_conf" }, { "pkg": "apache" } ] }, + "running", { "order": 10001 } ], - "__sls__": "apache", + "__sls__": "blah", "__env__": "base" }, - "/etc/httpd/conf.d/httpd.conf": { + "apache_conf": { "file": [ - "managed", + { + "name": "/etc/httpd/conf.d/httpd.conf" + }, { "source": "salt://apache/httpd.conf" }, + "managed", { "order": 10002 } ], - "__sls__": "apache", + "__sls__": "blah", "__env__": "base" } } @@ -121,19 +131,19 @@ The subsequent Low Data will look like this: [ { - "name": "apache", + "name": "httpd", "state": "pkg", "__id__": "apache", "fun": "installed", "__env__": "base", - "__sls__": "apache", + "__sls__": "blah", "order": 10000 }, { - "name": "apache", + "name": "httpd", "watch": [ { - "file": "/etc/httpd/conf.d/httpd.conf" + "file": "apache_conf" }, { "pkg": "apache" @@ -143,22 +153,21 @@ The subsequent Low Data will look like this: "__id__": "apache", "fun": "running", "__env__": "base", - "__sls__": "apache", + "__sls__": "blah", "order": 10001 }, { "name": "/etc/httpd/conf.d/httpd.conf", "source": "salt://apache/httpd.conf", "state": "file", - "__id__": "/etc/httpd/conf.d/httpd.conf", + "__id__": "apache_conf", "fun": "managed", "__env__": "base", - "__sls__": "apache", + "__sls__": "blah", "order": 10002 } ] - This tutorial discusses the Low Data evaluation and the state runtime. Ordering Layers @@ -235,8 +244,8 @@ ordering can be explicitly overridden using the `order` flag in states: .. code-block:: yaml apache: - pkg: - - installed + pkg.installed: + - name: httpd - order: 1 This order flag will over ride the definition order, this makes it very diff --git a/doc/ref/states/highstate.rst b/doc/ref/states/highstate.rst index 4fbb557147..3617f0010d 100644 --- a/doc/ref/states/highstate.rst +++ b/doc/ref/states/highstate.rst @@ -103,8 +103,8 @@ declaration that will restart Apache whenever the Apache configuration file, - file: mywebsite mywebsite: - file: - - managed + file.managed: + - name: /var/www/mysite .. seealso:: watch_in and require_in @@ -168,10 +168,10 @@ For example, the following state declaration calls the :mod:`installed .. code-block:: yaml httpd: - pkg.installed + pkg.installed: [] -The function can be declared inline with the state as a shortcut, but -the actual data structure is better referenced in this form: +The function can be declared inline with the state as a shortcut. +The actual data structure is compiled to this form: .. code-block:: yaml @@ -203,10 +203,8 @@ VALID: .. code-block:: yaml httpd: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: [] Occurs as the only index in the :ref:`state-declaration` list. @@ -280,8 +278,7 @@ easier to specify ``mywebsite`` than to specify - file: mywebsite apache2: - service: - - running + service.running: - watch: - file: mywebsite diff --git a/doc/ref/states/index.rst b/doc/ref/states/index.rst index 6ebadcc6c9..e637123600 100644 --- a/doc/ref/states/index.rst +++ b/doc/ref/states/index.rst @@ -113,19 +113,17 @@ Here is an example of a Salt State: .. code-block:: yaml vim: - pkg: - - installed + pkg.installed: [] salt: - pkg: - - latest + pkg.latest: + - name: salt service.running: - - require: - - file: /etc/salt/minion - - pkg: salt - names: - salt-master - salt-minion + - require: + - pkg: salt - watch: - file: /etc/salt/minion @@ -196,15 +194,15 @@ the following state file which we'll call ``pep8.sls``: .. code-block:: yaml python-pip: - cmd: - - run + cmd.run: + - name: | + easy_install --script-dir=/usr/bin -U pip - cwd: / - - name: easy_install --script-dir=/usr/bin -U pip pep8: - pip.installed - requires: - - cmd: python-pip + pip.installed: + - require: + - cmd: python-pip The above example installs `pip`_ using ``easy_install`` from `setuptools`_ and @@ -276,16 +274,16 @@ The modified state file would now be: .. code-block:: yaml python-pip: - cmd: - - run + cmd.run: + - name: | + easy_install --script-dir=/usr/bin -U pip - cwd: / - - name: easy_install --script-dir=/usr/bin -U pip - reload_modules: true pep8: - pip.installed - requires: - - cmd: python-pip + pip.installed: + - require: + - cmd: python-pip Let's run it, once: diff --git a/doc/ref/states/ordering.rst b/doc/ref/states/ordering.rst index c6c5edf20f..bca5cd05a0 100644 --- a/doc/ref/states/ordering.rst +++ b/doc/ref/states/ordering.rst @@ -61,8 +61,7 @@ These requisite statements are applied to a specific state declaration: .. code-block:: yaml httpd: - pkg: - - installed + pkg.installed: [] file.managed: - name: /etc/httpd/conf/httpd.conf - source: salt://httpd/httpd.conf @@ -91,10 +90,8 @@ the discrete states are split or groups into separate sls files: - network httpd: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - require: - pkg: httpd - sls: network @@ -121,8 +118,7 @@ more requisites. Both requisite types can also be separately declared: .. code-block:: yaml httpd: - pkg: - - installed + pkg.installed: [] service.running: - enable: True - watch: @@ -136,10 +132,8 @@ more requisites. Both requisite types can also be separately declared: - source: salt://httpd/httpd.conf - require: - pkg: httpd - user: - - present - group: - - present + user.present: [] + group.present: [] In this example, the httpd service is only going to be started if the package, user, group and file are executed successfully. diff --git a/doc/ref/states/requisites.rst b/doc/ref/states/requisites.rst index e2e86c1aab..8f0ee870e7 100644 --- a/doc/ref/states/requisites.rst +++ b/doc/ref/states/requisites.rst @@ -20,7 +20,7 @@ the targeting state. The following example demonstrates a direct requisite: .. code-block:: yaml vim: - pkg.installed + pkg.installed: [] /etc/vimrc: file.managed: @@ -258,15 +258,13 @@ The ``onfail`` requisite is applied in the same way as ``require`` as ``watch``: .. code-block:: yaml primary_mount: - mount: - - mounted + mount.mounted: - name: /mnt/share - device: 10.0.0.45:/share - fstype: nfs backup_mount: - mount: - - mounted + mount.mounted: - name: /mnt/share - device: 192.168.40.34:/share - fstype: nfs @@ -338,10 +336,8 @@ Using ``require`` .. code-block:: yaml httpd: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - require: - pkg: httpd @@ -350,12 +346,10 @@ Using ``require_in`` .. code-block:: yaml httpd: - pkg: - - installed + pkg.installed: - require_in: - service: httpd - service: - - running + service.running: [] The ``require_in`` statement is particularly useful when assigning a require in a separate sls file. For instance it may be common for httpd to require @@ -367,10 +361,8 @@ http.sls .. code-block:: yaml httpd: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - require: - pkg: httpd @@ -382,8 +374,7 @@ php.sls - http php: - pkg: - - installed + pkg.installed: - require_in: - service: httpd @@ -395,8 +386,7 @@ mod_python.sls - http mod_python: - pkg: - - installed + pkg.installed: - require_in: - service: httpd diff --git a/doc/topics/best_practices.rst b/doc/topics/best_practices.rst index a38f3185d6..d6f17dd598 100644 --- a/doc/topics/best_practices.rst +++ b/doc/topics/best_practices.rst @@ -192,8 +192,7 @@ preferred: - apache apache_conf: - file: - - managed + file.managed: - name: {{ name }} - source: {{ tmpl }} - template: jinja @@ -224,8 +223,7 @@ locations within a single state: - apache apache_conf: - file: - - managed + file.managed: - name: {{ salt['pillar.get']('apache:lookup:name') }} - source: {{ salt['pillar.get']('apache:lookup:config:tmpl') }} - template: jinja @@ -250,15 +248,12 @@ is not very modular to one that is: .. code-block:: yaml httpd: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - enable: True /etc/httpd/httpd.conf: - file: - - managed + file.managed: - source: salt://apache/files/httpd.conf - template: jinja - watch_in: @@ -286,17 +281,14 @@ opposed to direct ID references: .. code-block:: yaml apache: - pkg: - - installed + pkg.installed: - name: httpd - service: + service.running: - name: httpd - enable: True - - running apache_conf: - file: - - managed + file.managed: - name: /etc/httpd/httpd.conf - source: salt://apache/files/httpd.conf - template: jinja @@ -351,17 +343,14 @@ modification of static values: {% from "apache/map.jinja" import apache with context %} apache: - pkg: - - installed + pkg.installed: - name: {{ apache.server }} - service: + service.running: - name: {{ apache.service }} - enable: True - - running apache_conf: - file: - - managed + file.managed: - name: {{ apache.conf }} - source: {{ salt['pillar.get']('apache:lookup:config:tmpl') }} - template: jinja @@ -411,13 +400,11 @@ to be broken into two states. {% from "apache/map.jinja" import apache with context %} apache: - pkg: - - installed + pkg.installed: - name: {{ apache.server }} - service: + service.running: - name: {{ apache.service }} - enable: True - - running ``/srv/salt/apache/conf.sls``: @@ -429,8 +416,7 @@ to be broken into two states. - apache apache_conf: - file: - - managed + file.managed: - name: {{ apache.conf }} - source: {{ salt['pillar.get']('apache:lookup:config:tmpl') }} - template: jinja @@ -463,8 +449,7 @@ accessible by the appropriate hosts: .. code-block:: yaml testdb: - mysql_database: - - present: + mysql_database.present:: - name: testerdb ``/srv/salt/mysql/user.sls``: @@ -475,8 +460,7 @@ accessible by the appropriate hosts: - mysql.testerdb testdb_user: - mysql_user: - - present + mysql_user.present: - name: frank - password: "test3rdb" - host: localhost @@ -521,8 +505,7 @@ the associated pillar: .. code-block:: yaml testdb: - mysql_database: - - present: + mysql_database.present: - name: {{ salt['pillar.get']('mysql:lookup:name') }} ``/srv/salt/mysql/user.sls``: @@ -533,8 +516,7 @@ the associated pillar: - mysql.testerdb testdb_user: - mysql_user: - - present + mysql_user.present: - name: {{ salt['pillar.get']('mysql:lookup:user') }} - password: {{ salt['pillar.get']('mysql:lookup:password') }} - host: {{ salt['pillar.get']('mysql:lookup:host') }} diff --git a/doc/topics/development/conventions/formulas.rst b/doc/topics/development/conventions/formulas.rst index 68c85fdb69..99caa59f31 100644 --- a/doc/topics/development/conventions/formulas.rst +++ b/doc/topics/development/conventions/formulas.rst @@ -116,8 +116,7 @@ package until after the EPEL repository has also been installed: - epel python26: - pkg: - - installed + pkg.installed: - require: - pkg: epel @@ -767,11 +766,9 @@ state file using the following syntax: {% from "mysql/map.jinja" import mysql with context %} mysql-server: - pkg: - - installed + pkg.installed: - name: {{ mysql.server }} - service: - - running + service.running: - name: {{ mysql.service }} Overriding values in the lookup table @@ -973,11 +970,9 @@ skips platform-specific options for brevity. See the full # apache/init.sls apache: - pkg: - - installed + pkg.installed: [...] - service: - - running + service.running: [...] # apache/mod_wsgi.sls @@ -985,8 +980,7 @@ skips platform-specific options for brevity. See the full - apache mod_wsgi: - pkg: - - installed + pkg.installed: [...] - require: - pkg: apache @@ -996,8 +990,7 @@ skips platform-specific options for brevity. See the full - apache apache_conf: - file: - - managed + file.managed: [...] - watch_in: - service: apache @@ -1088,8 +1081,7 @@ thousands of function calls across a large state tree. {% set settings = salt['pillar.get']('apache', {}) %} mod_status: - file: - - managed + file.managed: - name: {{ apache.conf_dir }} - source: {{ settings.get('mod_status_conf', 'salt://apache/mod_status.conf') }} - template: {{ settings.get('template_engine', 'jinja') }} diff --git a/doc/topics/installation/ubuntu.rst b/doc/topics/installation/ubuntu.rst index 2ba21354ac..1aa1a8853d 100644 --- a/doc/topics/installation/ubuntu.rst +++ b/doc/topics/installation/ubuntu.rst @@ -85,15 +85,14 @@ the minion: .. code-block:: yaml update_zmq: - pkg: - - latest + pkg.latest: - pkgs: - zeromq - python-zmq - order: last - cmd: - - wait - - name: echo service salt-minion restart | at now + 1 minute + cmd.wait: + - name: | + echo service salt-minion restart | at now + 1 minute - watch: - pkg: update_zmq diff --git a/doc/topics/mine/index.rst b/doc/topics/mine/index.rst index ebf68a3288..4dc0d544af 100644 --- a/doc/topics/mine/index.rst +++ b/doc/topics/mine/index.rst @@ -96,8 +96,7 @@ to add them to the pool of load balanced servers. .. code-block:: yaml haproxy_config: - file: - - managed + file.managed: - name: /etc/haproxy/config - source: salt://haproxy_config - template: jinja diff --git a/doc/topics/pillar/index.rst b/doc/topics/pillar/index.rst index 5cd78db447..1bbd324975 100644 --- a/doc/topics/pillar/index.rst +++ b/doc/topics/pillar/index.rst @@ -96,15 +96,13 @@ more via the shared pillar :ref:`dict `: .. code-block:: yaml apache: - pkg: - - installed + pkg.installed: - name: {{ pillar['apache'] }} .. code-block:: yaml git: - pkg: - - installed + pkg.installed: - name: {{ pillar['git'] }} Finally, the above states can utilize the values provided to them via Pillar. diff --git a/doc/topics/tutorials/cloud_controller.rst b/doc/topics/tutorials/cloud_controller.rst index f50b6155ad..cdff7e7114 100644 --- a/doc/topics/tutorials/cloud_controller.rst +++ b/doc/topics/tutorials/cloud_controller.rst @@ -53,20 +53,16 @@ to set up the libvirt pki keys. .. code-block:: yaml libvirt: - pkg: - - installed - file: - - managed + pkg.installed: [] + file.managed: - name: /etc/sysconfig/libvirtd - contents: 'LIBVIRTD_ARGS="--listen"' - require: - pkg: libvirt - libvirt: - - keys + libvirt.keys: - require: - pkg: libvirt - service: - - running + service.running: - name: libvirtd - require: - pkg: libvirt @@ -76,12 +72,10 @@ to set up the libvirt pki keys. - file: libvirt libvirt-python: - pkg: - - installed + pkg.installed: [] libguestfs: - pkg: - - installed + pkg.installed: - pkgs: - libguestfs - libguestfs-tools diff --git a/doc/topics/tutorials/pillar.rst b/doc/topics/tutorials/pillar.rst index 5e0080da45..7dc17914e5 100644 --- a/doc/topics/tutorials/pillar.rst +++ b/doc/topics/tutorials/pillar.rst @@ -247,8 +247,7 @@ A simple formula: .. code-block:: yaml vim: - pkg: - - installed + pkg.installed: [] /etc/vimrc: file.managed: @@ -266,8 +265,7 @@ Can be easily transformed into a powerful, parameterized formula: .. code-block:: jinja vim: - pkg: - - installed + pkg.installed: - name: {{ pillar['pkgs']['vim'] }} /etc/vimrc: diff --git a/doc/topics/tutorials/starting_states.rst b/doc/topics/tutorials/starting_states.rst index 40a398c246..1d079e5e82 100644 --- a/doc/topics/tutorials/starting_states.rst +++ b/doc/topics/tutorials/starting_states.rst @@ -67,10 +67,8 @@ A typical SLS file will often look like this in YAML: .. code-block:: yaml apache: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - require: - pkg: apache @@ -107,10 +105,8 @@ and a user and group may need to be set up. .. code-block:: yaml apache: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - watch: - pkg: apache - file: /etc/httpd/conf/httpd.conf @@ -455,11 +451,9 @@ a MooseFS distributed filesystem chunkserver: - pkg: mfs-chunkserver mfs-chunkserver: - pkg: - - installed + pkg.installed: [] mfschunkserver: - service: - - running + service.running: - require: {% for mnt in salt['cmd.run']('ls /dev/data/moose*') %} - mount: /mnt/moose{{ mnt[-1] }} diff --git a/doc/topics/tutorials/states_pt2.rst b/doc/topics/tutorials/states_pt2.rst index 855e70c72c..2cbbf36112 100644 --- a/doc/topics/tutorials/states_pt2.rst +++ b/doc/topics/tutorials/states_pt2.rst @@ -23,10 +23,8 @@ You can specify multiple :ref:`state-declaration` under an :emphasize-lines: 4,5 apache: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - require: - pkg: apache @@ -47,10 +45,8 @@ installed and running. Include the following at the bottom of your :emphasize-lines: 7,11 apache: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - require: - pkg: apache @@ -121,15 +117,12 @@ Verify that Apache is now serving your custom HTML. :emphasize-lines: 1,2,3,4,11,12 /etc/httpd/extra/httpd-vhosts.conf: - file: - - managed + file.managed: - source: salt://webserver/httpd-vhosts.conf apache: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - watch: - file: /etc/httpd/extra/httpd-vhosts.conf - require: diff --git a/doc/topics/tutorials/states_pt3.rst b/doc/topics/tutorials/states_pt3.rst index b5b777907c..33c7509bb8 100644 --- a/doc/topics/tutorials/states_pt3.rst +++ b/doc/topics/tutorials/states_pt3.rst @@ -88,8 +88,7 @@ The Salt module functions are also made available in the template context as .. code-block:: jinja moe: - user: - - present + user.present: - gid: {{ salt['file.group_to_gid']('some_group_that_exists') }} Note that for the above example to work, ``some_group_that_exists`` must exist diff --git a/doc/topics/tutorials/walkthrough.rst b/doc/topics/tutorials/walkthrough.rst index b3ddf16d36..c925a39af9 100644 --- a/doc/topics/tutorials/walkthrough.rst +++ b/doc/topics/tutorials/walkthrough.rst @@ -519,7 +519,7 @@ Now, to beef up the vim SLS formula, a ``vimrc`` can be added: .. code-block:: yaml vim: - pkg.installed + pkg.installed: [] /etc/vimrc: file.managed: @@ -553,10 +553,8 @@ make an nginx subdirectory and add an init.sls file: .. code-block:: yaml nginx: - pkg: - - installed - service: - - running + pkg.installed: [] + service.running: - require: - pkg: nginx diff --git a/salt/client/ssh/wrapper/grains.py b/salt/client/ssh/wrapper/grains.py index 94e96aa0d7..f6201225f8 100644 --- a/salt/client/ssh/wrapper/grains.py +++ b/salt/client/ssh/wrapper/grains.py @@ -158,11 +158,9 @@ def filter_by(lookup_dict, grain='os_family', merge=None): }) %} myapache: - pkg: - - installed + pkg.installed: - name: {{ apache.pkg }} - service: - - running + service.running: - name: {{ apache.srv }} Values in the lookup table may be overridden by values in Pillar. An diff --git a/salt/modules/daemontools.py b/salt/modules/daemontools.py index ccd3a78add..e853e7f490 100644 --- a/salt/modules/daemontools.py +++ b/salt/modules/daemontools.py @@ -9,8 +9,7 @@ so it can be used to maintain services using the ``provider`` argument: .. code-block:: yaml myservice: - service: - - running + service.running: - provider: daemontools ''' diff --git a/salt/modules/grains.py b/salt/modules/grains.py index 1758343a97..ea195e65fc 100644 --- a/salt/modules/grains.py +++ b/salt/modules/grains.py @@ -366,11 +366,9 @@ def filter_by(lookup_dict, grain='os_family', merge=None, default='default'): }), default='Debian' %} myapache: - pkg: - - installed + pkg.installed: - name: {{ apache.pkg }} - service: - - running + service.running: - name: {{ apache.srv }} Values in the lookup table may be overridden by values in Pillar. An diff --git a/salt/states/archive.py b/salt/states/archive.py index 0914d0c6a4..152cfc972a 100644 --- a/salt/states/archive.py +++ b/salt/states/archive.py @@ -47,8 +47,7 @@ def extracted(name, .. code-block:: yaml graylog2-server: - archive: - - extracted + archive.extracted: - name: /opt/ - source: https://github.com/downloads/Graylog2/graylog2-server/graylog2-server-0.9.6p1.tar.lzma - source_hash: md5=499ae16dcae71eeb7c3a30c75ea7a1a6 @@ -59,8 +58,7 @@ def extracted(name, .. code-block:: yaml graylog2-server: - archive: - - extracted + archive.extracted: - name: /opt/ - source: https://github.com/downloads/Graylog2/graylog2-server/graylog2-server-0.9.6p1.tar.gz - source_hash: md5=499ae16dcae71eeb7c3a30c75ea7a1a6 diff --git a/salt/states/blockdev.py b/salt/states/blockdev.py index 30025d4b29..638667fa6d 100644 --- a/salt/states/blockdev.py +++ b/salt/states/blockdev.py @@ -13,8 +13,7 @@ A state module to manage blockdevices - read-only: True master-data: - blockdev: - - tuned: + blockdev.tuned:: - name : /dev/vg/master-data - read-only: True - read-ahead: 1024 diff --git a/salt/states/cmd.py b/salt/states/cmd.py index a38fd89b81..6e35d80fba 100644 --- a/salt/states/cmd.py +++ b/salt/states/cmd.py @@ -147,17 +147,14 @@ executed when the state it is watching changes. Example: .. code-block:: yaml /usr/local/bin/postinstall.sh: - cmd: - - wait + cmd.wait: - watch: - pkg: mycustompkg - file: - - managed + file.managed: - source: salt://utils/scripts/postinstall.sh mycustompkg: - pkg: - - installed + pkg.installed: - require: - file: /usr/local/bin/postinstall.sh diff --git a/salt/states/glusterfs.py b/salt/states/glusterfs.py index 812b73bb3c..79ab7b556a 100644 --- a/salt/states/glusterfs.py +++ b/salt/states/glusterfs.py @@ -167,8 +167,7 @@ def started(name): .. code-block:: yaml mycluster: - glusterfs: - - started + glusterfs.started: [] ''' ret = {'name': name, 'changes': {}, diff --git a/salt/states/npm.py b/salt/states/npm.py index de7c1d67b7..9e8d353051 100644 --- a/salt/states/npm.py +++ b/salt/states/npm.py @@ -46,13 +46,11 @@ def installed(name, .. code-block:: yaml coffee-script: - npm: - - installed + npm.installed: - user: someuser coffee-script@1.0.1: - npm: - - installed + npm.installed: [] name The package to install diff --git a/salt/states/pkgng.py b/salt/states/pkgng.py index 15d7f54dcd..911a02a1d7 100644 --- a/salt/states/pkgng.py +++ b/salt/states/pkgng.py @@ -10,8 +10,7 @@ typically rather simple: .. code-block:: yaml pkgng_clients: - pkgng: - - update_packaging_site + pkgng.update_packaging_site: - name: "http://192.168.0.2" ''' diff --git a/salt/states/powerpath.py b/salt/states/powerpath.py index 0c3147dbc6..12ebd2a195 100644 --- a/salt/states/powerpath.py +++ b/salt/states/powerpath.py @@ -9,8 +9,7 @@ only addition/deletion of licenses is supported. .. code-block:: yaml key: - powerpath: - - license_present + powerpath.license_present: [] ''' diff --git a/salt/states/rabbitmq_plugin.py b/salt/states/rabbitmq_plugin.py index f2ad8b9f6a..95c91633d1 100644 --- a/salt/states/rabbitmq_plugin.py +++ b/salt/states/rabbitmq_plugin.py @@ -10,8 +10,7 @@ Example: .. code-block:: yaml some_plugin: - rabbitmq_plugin: - - enabled + rabbitmq_plugin.enabled: [] ''' # Import python libs diff --git a/salt/states/rvm.py b/salt/states/rvm.py index e3c226e924..e55e9bd885 100644 --- a/salt/states/rvm.py +++ b/salt/states/rvm.py @@ -15,8 +15,7 @@ configuration could look like: .. code-block:: yaml rvm: - group: - - present + group.present: [] user.present: - gid: rvm - home: /home/rvm @@ -25,7 +24,7 @@ configuration could look like: rvm-deps: pkg.installed: - - names: + - pkgs: - bash - coreutils - gzip @@ -38,7 +37,7 @@ configuration could look like: mri-deps: pkg.installed: - - names: + - pkgs: - build-essential - openssl - libreadline6 @@ -65,7 +64,7 @@ configuration could look like: jruby-deps: pkg.installed: - - names: + - pkgs: - curl - g++ - openjdk-6-jre-headless diff --git a/salt/states/service.py b/salt/states/service.py index f0c9f30145..6287e32d3f 100644 --- a/salt/states/service.py +++ b/salt/states/service.py @@ -9,16 +9,14 @@ rc scripts, services can be defined as running or dead. .. code-block:: yaml httpd: - service: - - running + service.running: [] The service can also be set to be started at runtime via the enable option: .. code-block:: yaml openvpn: - service: - - running + service.running: - enable: True By default if a service is triggered to refresh due to a watch statement the @@ -28,8 +26,7 @@ service, then set the reload value to True: .. code-block:: yaml redis: - service: - - running + service.running: - enable: True - reload: True - watch: diff --git a/salt/states/ssh_auth.py b/salt/states/ssh_auth.py index 49e35fe410..3236226a0c 100644 --- a/salt/states/ssh_auth.py +++ b/salt/states/ssh_auth.py @@ -15,27 +15,23 @@ to use a YAML 'explicit key', as demonstrated in the second example below. .. code-block:: yaml AAAAB3NzaC1kc3MAAACBAL0sQ9fJ5bYTEyY==: - ssh_auth: - - present + ssh_auth.present: - user: root - enc: ssh-dss ? AAAAB3NzaC1kc3MAAACBAL0sQ9fJ5bYTEyY==... : - ssh_auth: - - present + ssh_auth.present: - user: root - enc: ssh-dss thatch: - ssh_auth: - - present + ssh_auth.present: - user: root - source: salt://ssh_keys/thatch.id_rsa.pub sshkeys: - ssh_auth: - - present + ssh_auth.present: - user: root - enc: ssh-rsa - options: diff --git a/salt/states/supervisord.py b/salt/states/supervisord.py index b204a38628..6447fcd0bc 100644 --- a/salt/states/supervisord.py +++ b/salt/states/supervisord.py @@ -6,8 +6,7 @@ Interaction with the Supervisor daemon .. code-block:: yaml wsgi_server: - supervisord: - - running + supervisord.running: - require: - pkg: supervisor - watch: diff --git a/salt/states/tomcat.py b/salt/states/tomcat.py index b6530b2836..193e6f564d 100644 --- a/salt/states/tomcat.py +++ b/salt/states/tomcat.py @@ -187,21 +187,18 @@ def wait(name, url='http://localhost:8080/manager', timeout=180): .. code-block:: yaml tomcat-service: - service: - - running + service.running: - name: tomcat - enable: True wait-for-tomcatmanager: - tomcat: - - wait + tomcat.wait: - timeout: 300 - require: - service: tomcat-service jenkins: - tomcat: - - war_deployed + tomcat.war_deployed: - name: /ran - war: salt://jenkins-1.2.4.war - require: diff --git a/salt/states/win_system.py b/salt/states/win_system.py index b63a4aa047..d04a94d7c3 100644 --- a/salt/states/win_system.py +++ b/salt/states/win_system.py @@ -11,12 +11,10 @@ description. .. code-block:: yaml ERIK-WORKSTATION: - system: - - computer_name + system.computer_name: [] This is Erik's computer, don't touch!: - system: - - computer_desc + system.computer_desc: [] ''' # Import python libs From c6946b083678aafc75174ac8d98b3ea3023646ff Mon Sep 17 00:00:00 2001 From: Adam Mendlik Date: Sat, 13 Dec 2014 09:37:57 -0700 Subject: [PATCH 22/33] Detect a Windows VM on OpenStack and populate the 'virtual' grain --- salt/grains/core.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/salt/grains/core.py b/salt/grains/core.py index 1006891d66..2564f9abef 100644 --- a/salt/grains/core.py +++ b/salt/grains/core.py @@ -739,6 +739,8 @@ def _windows_platform_data(): grains['virtual'] = 'Xen' if 'HVM domU' in systeminfo.Model: grains['virtual_subtype'] = 'HVM domU' + elif 'OpenStack' in systeminfo.Model: + grains['virtual'] = 'OpenStack' return grains From e6bf243cbcafebc80eb397eb9f03c28e7d1d02ab Mon Sep 17 00:00:00 2001 From: rallytime Date: Sun, 14 Dec 2014 19:45:47 -0700 Subject: [PATCH 23/33] Use lists instead of tuples in modules/zypper.py --- salt/modules/zypper.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/salt/modules/zypper.py b/salt/modules/zypper.py index b452977b91..8b97842b34 100644 --- a/salt/modules/zypper.py +++ b/salt/modules/zypper.py @@ -201,13 +201,13 @@ def list_pkgs(versions_as_list=False, **kwargs): __salt__['pkg_resource.stringify'](ret) return ret - cmd = ('rpm', '-qa', '--queryformat', '%{NAME}_|-%{VERSION}_|-%{RELEASE}\\n') + cmd = ['rpm', '-qa', '--queryformat', '%{NAME}_|-%{VERSION}_|-%{RELEASE}\\n'] ret = {} out = __salt__['cmd.run']( - cmd, - output_loglevel='trace', - python_shell=False - ) + cmd, + output_loglevel='trace', + python_shell=False + ) for line in out.splitlines(): name, pkgver, rel = line.split('_|-') if rel: @@ -673,23 +673,23 @@ def install(name=None, old = list_pkgs() downgrades = [] if fromrepo: - fromrepoopt = ('--force', '--force-resolution', '--from', fromrepo) + fromrepoopt = ['--force', '--force-resolution', '--from', fromrepo] log.info('Targeting repo {0!r}'.format(fromrepo)) # Split the targets into batches of 500 packages each, so that # the maximal length of the command line is not broken while targets: cmd = ['zypper', '--non-interactive', 'install', '--name', - '--auto-agree-with-licenses'] + '--auto-agree-with-licenses'] if fromrepo: cmd.extend(fromrepoopt) cmd.extend(targets[:500]) targets = targets[500:] out = __salt__['cmd.run']( - cmd, - output_loglevel='trace', - python_shell=False - ) + cmd, + output_loglevel='trace', + python_shell=False + ) for line in out.splitlines(): match = re.match( "^The selected package '([^']+)'.+has lower version", @@ -700,7 +700,7 @@ def install(name=None, while downgrades: cmd = ['zypper', '--non-interactive', 'install', '--name', - '--auto-agree-with-licenses', '--force'] + '--auto-agree-with-licenses', '--force'] if fromrepo: cmd.extend(fromrepoopt) cmd.extend(downgrades[:500]) From 89f0f92e06e81be5f3da5235d8638cafc09985a3 Mon Sep 17 00:00:00 2001 From: "David\\ Beitey" Date: Mon, 15 Dec 2014 14:05:06 +1000 Subject: [PATCH 24/33] Avoid double-quoting of group name for yum --- salt/modules/yumpkg.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py index f9bc9c0c9d..192d489a48 100644 --- a/salt/modules/yumpkg.py +++ b/salt/modules/yumpkg.py @@ -1429,7 +1429,7 @@ def group_info(name): 'default packages': [], 'description': '' } - cmd_template = 'repoquery --plugins --group --grouppkgs={0} --list {1!r}' + cmd_template = 'repoquery --plugins --group --grouppkgs={0} --list {1}' cmd = cmd_template.format('all', _cmd_quote(name)) out = __salt__['cmd.run_stdout'](cmd, output_loglevel='trace') @@ -1453,7 +1453,7 @@ def group_info(name): # considered to be conditional packages. ret['conditional packages'] = sorted(all_pkgs) - cmd = 'repoquery --plugins --group --info {0!r}'.format(_cmd_quote(name)) + cmd = 'repoquery --plugins --group --info {0}'.format(_cmd_quote(name)) out = __salt__['cmd.run_stdout']( cmd, output_loglevel='trace' ) From 1d33faed4bc3981ef774b2952de0261fce14418c Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 15 Dec 2014 10:18:02 -0800 Subject: [PATCH 25/33] Rebasing to fix the merge conflict --- salt/states/mount.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/salt/states/mount.py b/salt/states/mount.py index 5f30365592..8db4929211 100644 --- a/salt/states/mount.py +++ b/salt/states/mount.py @@ -32,6 +32,7 @@ import re # Import salt libs from salt._compat import string_types +from salt.exceptions import SaltInvocationError def mounted(name, @@ -155,14 +156,26 @@ def mounted(name, ret['comment'] = "Remount would be forced because options changed" return ret else: - ret['changes']['umount'] = "Forced remount because " \ - + "options changed" - remount_result = __salt__['mount.remount'](real_name, device, mkmnt=mkmnt, fstype=fstype, opts=opts) - ret['result'] = remount_result - # Cleanup after the remount, so we - # don't write remount into fstab - if 'remount' in opts: - opts.remove('remount') + # nfs requires umounting and mounting if options change + # add others to list that require similiar functionality + if fstype in ['nfs']: + ret['changes']['umount'] = "Forced unmount and mount because " \ + + "options changed" + unmount_result = __salt__['mount.umount'](real_name) + if unmount_result is True: + mount_result = __salt__['mount.mount'](real_name, device, mkmnt=mkmnt, fstype=fstype, opts=opts) + ret['result'] = mount_result + else: + raise SaltInvocationError('Unable to unmount {0}: {1}.'.format(real_name, unmount_result)) + else: + ret['changes']['umount'] = "Forced remount because " \ + + "options changed" + remount_result = __salt__['mount.remount'](real_name, device, mkmnt=mkmnt, fstype=fstype, opts=opts) + ret['result'] = remount_result + # Cleanup after the remount, so we + # don't write remount into fstab + if 'remount' in opts: + opts.remove('remount') if real_device not in device_list: # name matches but device doesn't - need to umount if __opts__['test']: From 67c08f41519235cbe84036d5bc7aed53b5cbd4df Mon Sep 17 00:00:00 2001 From: "Gareth J. Greenaway" Date: Mon, 15 Dec 2014 10:40:39 -0800 Subject: [PATCH 26/33] schedule.list should return an empty dictionary, not None --- salt/modules/schedule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/schedule.py b/salt/modules/schedule.py index dab28e8b10..9267d367cd 100644 --- a/salt/modules/schedule.py +++ b/salt/modules/schedule.py @@ -90,7 +90,7 @@ def list_(show_all=False, return_yaml=True): else: return schedule else: - return None + return {'schedule': {}} def purge(**kwargs): From a6b5011cb74abb6ef7a7c55679872132cc83d7a2 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Mon, 15 Dec 2014 19:27:25 +0000 Subject: [PATCH 27/33] Typo --- tests/jenkins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/jenkins.py b/tests/jenkins.py index 83614c2963..db5db7785e 100644 --- a/tests/jenkins.py +++ b/tests/jenkins.py @@ -189,7 +189,7 @@ def download_unittest_reports(options): os.makedirs(xml_reports_path) cmds = ( - 'salt {0} archive.tar zcvf /tmp/xml-test-reports.tar.gz \'*.xml\' cwd=/tmp/xml-unitests-output/', + 'salt {0} archive.tar zcvf /tmp/xml-test-reports.tar.gz \'*.xml\' cwd=/tmp/xml-unittests-output/', 'salt {0} cp.push /tmp/xml-test-reports.tar.gz', 'mv -f /var/cache/salt/master/minions/{1}/files/tmp/xml-test-reports.tar.gz {2} && ' 'tar zxvf {2}/xml-test-reports.tar.gz -C {2}/xml-test-reports && ' From 4c47b13e488ce2c326af30f7f6e93ae8bc6adc00 Mon Sep 17 00:00:00 2001 From: Justin Findlay Date: Mon, 15 Dec 2014 13:01:47 -0700 Subject: [PATCH 28/33] split win commands in state --- salt/modules/state.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/state.py b/salt/modules/state.py index 81a96495b9..f9194743ee 100644 --- a/salt/modules/state.py +++ b/salt/modules/state.py @@ -339,7 +339,7 @@ def highstate(test=None, try: if salt.utils.is_windows(): # Make sure cache file isn't read-only - __salt__['cmd.run']('attrib -R "{0}"'.format(cache_file)) + __salt__['cmd.run'](['attrib', '-R', cache_file], python_shell=False) with salt.utils.fopen(cache_file, 'w+b') as fp_: serial.dump(ret, fp_) except (IOError, OSError): @@ -481,7 +481,7 @@ def sls(mods, try: if salt.utils.is_windows(): # Make sure cache file isn't read-only - __salt__['cmd.run']('attrib -R "{0}"'.format(cache_file)) + __salt__['cmd.run'](['attrib', '-R', cache_file], python_shell=False) with salt.utils.fopen(cache_file, 'w+b') as fp_: serial.dump(ret, fp_) except (IOError, OSError): From 22cd943e5c371edbc1b87d516449d6b30f27996b Mon Sep 17 00:00:00 2001 From: "C. R. Oldham" Date: Mon, 15 Dec 2014 15:29:27 -0700 Subject: [PATCH 29/33] Fix typo in os.walk --- salt/daemons/masterapi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/daemons/masterapi.py b/salt/daemons/masterapi.py index da8806ca2b..eadf44f5d2 100644 --- a/salt/daemons/masterapi.py +++ b/salt/daemons/masterapi.py @@ -143,7 +143,7 @@ def clean_pub_auth(opts): if not os.path.exists(auth_cache): return else: - for (dirpath, dirnames, filenames) in os.walkpath(auth_cache): + for (dirpath, dirnames, filenames) in os.walk(auth_cache): for auth_file in filenames: auth_file_path = os.path.join(dirpath, auth_file) if not os.path.isfile(auth_file_path): From 44e60acd12438dc17b0645128de94cebe4fda085 Mon Sep 17 00:00:00 2001 From: rallytime Date: Mon, 15 Dec 2014 16:45:01 -0700 Subject: [PATCH 30/33] Fix mac_user.py module --> Don't quote integers like uid and gid --- salt/modules/mac_user.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/salt/modules/mac_user.py b/salt/modules/mac_user.py index dfbe0bdb55..2a3d17fdd4 100644 --- a/salt/modules/mac_user.py +++ b/salt/modules/mac_user.py @@ -108,9 +108,8 @@ def add(name, if not isinstance(gid, int): raise SaltInvocationError('gid must be an integer') - _dscl('/Users/{0} UniqueID {1!r}'.format(_cmd_quote(name), _cmd_quote(uid))) - _dscl('/Users/{0} PrimaryGroupID {1!r}'.format(_cmd_quote(name), - _cmd_quote(gid))) + _dscl('/Users/{0} UniqueID {1!r}'.format(_cmd_quote(name), uid)) + _dscl('/Users/{0} PrimaryGroupID {1!r}'.format(_cmd_quote(name), gid)) _dscl('/Users/{0} UserShell {1!r}'.format(_cmd_quote(name), _cmd_quote(shell))) _dscl('/Users/{0} NFSHomeDirectory {1!r}'.format(_cmd_quote(name), @@ -195,7 +194,7 @@ def chuid(name, uid): return True _dscl( '/Users/{0} UniqueID {1!r} {2!r}'.format(_cmd_quote(name), - _cmd_quote(pre_info['uid']), + pre_info['uid'], uid), ctype='change' ) @@ -224,8 +223,9 @@ def chgid(name, gid): return True _dscl( '/Users/{0} PrimaryGroupID {1!r} {2!r}'.format( - _cmd_quote(name), _cmd_quote(pre_info['gid']), - _cmd_quote(gid)), + _cmd_quote(name), + pre_info['gid'], + gid), ctype='change' ) # dscl buffers changes, sleep 1 second before checking if new value @@ -307,8 +307,7 @@ def chfullname(name, fullname): if fullname == pre_info['fullname']: return True _dscl( - '/Users/{0} RealName {1!r}'.format(_cmd_quote(name), - _cmd_quote(fullname)), + '/Users/{0} RealName {1!r}'.format(_cmd_quote(name), fullname), # use a "create" command, because a "change" command would fail if # current fullname is an empty string. The "create" will just overwrite # this field. From 44f1448d019750d1580b4fe9117f9a20336cfe7a Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Mon, 15 Dec 2014 19:15:17 -0800 Subject: [PATCH 31/33] Remove "init_timeout" in RemoteFileClient.get_file This timeout seems to attempt to retry in the event that the master returns nothing-- but the channel should be responsible for that. In addition this would retry infinitely since the retry counter was re-initialized on every loop iteration. This meant you spun with an strace that looks something like: ``` read(12, "e\351\r\6y\357\331\313a0\233t\256\16qP", 16) = 16 close(12) = 0 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) write(8, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 60000) = 1 ([{fd=10, revents=POLLIN}]) poll([{fd=10, events=POLLIN}], 1, 0) = 1 ([{fd=10, revents=POLLIN}]) read(10, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) write(1, "('foo here', 'top.sls')\n", 24) = 24 select(0, NULL, NULL, NULL, {0, 20000}) = 0 (Timeout) open("/dev/urandom", O_RDONLY) = 12 read(12, "\207L\210\340\314\203\353\207\236\231B{\304$\320\227", 16) = 16 close(12) = 0 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) write(8, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 60000) = 1 ([{fd=10, revents=POLLIN}]) poll([{fd=10, events=POLLIN}], 1, 0) = 1 ([{fd=10, revents=POLLIN}]) read(10, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) write(1, "('foo here', 'top.sls')\n", 24) = 24 select(0, NULL, NULL, NULL, {0, 20000}) = 0 (Timeout) open("/dev/urandom", O_RDONLY) = 12 read(12, "\3\253\250\326w~\325\277\306\233'\355\300\225\202=", 16) = 16 close(12) = 0 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) write(8, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 60000) = 1 ([{fd=10, revents=POLLIN}]) poll([{fd=10, events=POLLIN}], 1, 0) = 1 ([{fd=10, revents=POLLIN}]) read(10, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) write(1, "('foo here', 'top.sls')\n", 24) = 24 select(0, NULL, NULL, NULL, {0, 20000}) = 0 (Timeout) open("/dev/urandom", O_RDONLY) = 12 read(12, "\250\371\205\375\310\257\30\254\376\261\250o\177\357C6", 16) = 16 close(12) = 0 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) write(8, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 60000) = 1 ([{fd=10, revents=POLLIN}]) poll([{fd=10, events=POLLIN}], 1, 0) = 1 ([{fd=10, revents=POLLIN}]) read(10, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) write(1, "('foo here', 'top.sls')\n", 24) = 24 select(0, NULL, NULL, NULL, {0, 20000}) = 0 (Timeout) open("/dev/urandom", O_RDONLY) = 12 read(12, "\322\0276\\\231\214v\5@pp\36\211\277\360\207", 16) = 16 close(12) = 0 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) write(8, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout) poll([{fd=10, events=POLLIN}], 1, 60000) = 1 ([{fd=10, revents=POLLIN}]) poll([{fd=10, events=POLLIN}], 1, 0) = 1 ([{fd=10, revents=POLLIN}]) read(10, "\1\0\0\0\0\0\0\0", 8) = 8 poll([{fd=10, events=POLLIN}], 1, 0) = 0 (Timeout ``` Lastly, this retry attempt counter did nothing in the event of 10 failures (assuming it could get one) so it should be safe to remove it all together Conflicts: salt/fileclient.py --- salt/fileclient.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/salt/fileclient.py b/salt/fileclient.py index be11041977..abc9e7ba2e 100644 --- a/salt/fileclient.py +++ b/salt/fileclient.py @@ -982,21 +982,11 @@ class RemoteClient(Client): return False fn_ = salt.utils.fopen(dest, 'wb+') while True: - init_retries = 10 if not fn_: load['loc'] = 0 else: load['loc'] = fn_.tell() - try: - channel = self._get_channel() - data = channel.send(load) - except SaltReqTimeoutError: - return '' - if not data: - if init_retries: - init_retries -= 1 - time.sleep(0.02) - continue + data = self.channel.send(load) if 'data' not in data: log.error('Data is {0}'.format(data)) if not data['data']: From a86c2e8b5a4b9e6183d8e24a0e7d263603b55130 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Mon, 15 Dec 2014 19:29:28 -0800 Subject: [PATCH 32/33] Pylint cleanup --- salt/fileclient.py | 1 - 1 file changed, 1 deletion(-) diff --git a/salt/fileclient.py b/salt/fileclient.py index abc9e7ba2e..78434217b2 100644 --- a/salt/fileclient.py +++ b/salt/fileclient.py @@ -9,7 +9,6 @@ import logging import hashlib import os import shutil -import time import requests # Import salt libs From 782f6118ccf44be93fdc70db6e84832b2d918686 Mon Sep 17 00:00:00 2001 From: Michal Galet Date: Tue, 16 Dec 2014 18:45:14 +0100 Subject: [PATCH 33/33] Fix ini_manage state - equality detection for non-string values --- salt/states/ini_manage.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/salt/states/ini_manage.py b/salt/states/ini_manage.py index 9e2d2a39a6..cf86fa8354 100644 --- a/salt/states/ini_manage.py +++ b/salt/states/ini_manage.py @@ -58,8 +58,10 @@ def options_present(name, sections=None): current_value = __salt__['ini.get_option'](name, section, key) - if current_value == sections[section][key]: + # Test if the change is necessary + if current_value == str(sections[section][key]): continue + ret['changes'] = __salt__['ini.set_option'](name, sections) if 'error' in ret['changes']: