From 99c33964512bf95e3e53515ae33ba5351a4400ec Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 18 Nov 2015 11:49:45 +0000 Subject: [PATCH 1/8] Parser code should live in `salt.utils.parsers` --- salt/cli/api.py | 23 ++--------------------- salt/utils/parsers.py | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/salt/cli/api.py b/salt/cli/api.py index 7c409fadb9..93eca01e7f 100644 --- a/salt/cli/api.py +++ b/salt/cli/api.py @@ -9,44 +9,25 @@ # Import Python libs from __future__ import absolute_import, print_function -import os.path import logging # Import Salt libs +import salt.client.netapi import salt.utils.parsers as parsers -import salt.version -import salt.syspaths as syspaths from salt.utils.verify import verify_log -# Import 3rd-party libs -import salt.ext.six as six log = logging.getLogger(__name__) -class SaltAPI(six.with_metaclass(parsers.OptionParserMeta, # pylint: disable=W0232 - parsers.OptionParser, parsers.ConfigDirMixIn, - parsers.LogLevelMixIn, parsers.PidfileMixin, parsers.DaemonMixIn, - parsers.MergeConfigMixIn)): +class SaltAPI(parsers.SaltAPIParser): ''' The cli parser object used to fire up the salt api system. ''' - - VERSION = salt.version.__version__ - - # ConfigDirMixIn config filename attribute - _config_filename_ = 'master' - # LogLevelMixIn attributes - _default_logging_logfile_ = os.path.join(syspaths.LOGS_DIR, 'api') - - def setup_config(self): - return salt.config.api_config(self.get_config_file_path()) - def run(self): ''' Run the api ''' - import salt.client.netapi self.parse_args() self.setup_logfile_logger() verify_log(self.config) diff --git a/salt/utils/parsers.py b/salt/utils/parsers.py index 295999b8e5..71e76556ad 100644 --- a/salt/utils/parsers.py +++ b/salt/utils/parsers.py @@ -2929,3 +2929,22 @@ class SPMParser(six.with_metaclass(OptionParserMeta, def setup_config(self): return salt.config.spm_config(self.get_config_file_path()) + + +class SaltAPIParser(six.with_metaclass(OptionParserMeta, + OptionParser, + ConfigDirMixIn, + LogLevelMixIn, + PidfileMixin, + DaemonMixIn, + MergeConfigMixIn)): + ''' + The Salt API cli parser object used to fire up the salt api system. + ''' + # ConfigDirMixIn config filename attribute + _config_filename_ = 'master' + # LogLevelMixIn attributes + _default_logging_logfile_ = os.path.join(syspaths.LOGS_DIR, 'api') + + def setup_config(self): + return salt.config.api_config(self.get_config_file_path()) # pylint: disable=no-member From 7ec3c266a469ebf99e961f945d7125be77ca26e4 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 18 Nov 2015 12:00:30 +0000 Subject: [PATCH 2/8] Make PidfileMixin not override the DaemonMixin methods --- salt/utils/parsers.py | 60 +++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/salt/utils/parsers.py b/salt/utils/parsers.py index 71e76556ad..a99779919a 100644 --- a/salt/utils/parsers.py +++ b/salt/utils/parsers.py @@ -15,6 +15,7 @@ from __future__ import absolute_import, print_function import os import sys +import types import signal import getpass import logging @@ -954,31 +955,46 @@ class PidfileMixin(six.with_metaclass(MixInMeta, object)): 'DaemonMixIn which contains the same behavior. PidfileMixin ' 'will be supported until Salt {version}.' ) - self.add_option( - '--pid-file', dest='pidfile', - default=os.path.join( - syspaths.PIDFILE_DIR, '{0}.pid'.format(self.get_prog_name()) - ), - help=('Specify the location of the pidfile. Default: %default') - ) + try: + self.add_option( + '--pid-file', dest='pidfile', + default=os.path.join( + syspaths.PIDFILE_DIR, '{0}.pid'.format(self.get_prog_name()) + ), + help=('Specify the location of the pidfile. Default: %default') + ) - def set_pidfile(self): - from salt.utils.process import set_pidfile - set_pidfile(self.config['pidfile'], self.config['user']) + # Since there was no colision with DaemonMixin, let's add the + # pidfile mixin methods. This is used using types.MethodType + # because if we had defined these at the class level, they would + # have overridden the exact same methods from the DaemonMixin. - def check_pidfile(self): - ''' - Report whether a pidfile exists - ''' - from salt.utils.process import check_pidfile - return check_pidfile(self.config['pidfile']) + def set_pidfile(self): + from salt.utils.process import set_pidfile + set_pidfile(self.config['pidfile'], self.config['user']) - def get_pidfile(self): - ''' - Return a pid contained in a pidfile - ''' - from salt.utils.process import get_pidfile - return get_pidfile(self.config['pidfile']) + self.set_pidfile = types.MethodType(set_pidfile, self) + + def check_pidfile(self): + ''' + Report whether a pidfile exists + ''' + from salt.utils.process import check_pidfile + return check_pidfile(self.config['pidfile']) + + self.check_pidfile = types.MethodType(check_pidfile, self) + + def get_pidfile(self): + ''' + Return a pid contained in a pidfile + ''' + from salt.utils.process import get_pidfile + return get_pidfile(self.config['pidfile']) + + self.get_pidfile = types.MethodType(get_pidfile, self) + except optparse.OptionConflictError: + # The option was already added by the DaemonMixin + pass class TargetOptionsMixIn(six.with_metaclass(MixInMeta, object)): From 9bd7423b50c07af96bd2948d2c446874cc31e413 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 18 Nov 2015 12:01:18 +0000 Subject: [PATCH 3/8] Stop using the deprecated mixin --- salt/utils/parsers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/salt/utils/parsers.py b/salt/utils/parsers.py index a99779919a..1ced293fd4 100644 --- a/salt/utils/parsers.py +++ b/salt/utils/parsers.py @@ -2951,7 +2951,6 @@ class SaltAPIParser(six.with_metaclass(OptionParserMeta, OptionParser, ConfigDirMixIn, LogLevelMixIn, - PidfileMixin, DaemonMixIn, MergeConfigMixIn)): ''' From 03747dd2cc9bfa8c69af792f8abc7a7a2a4663af Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 18 Nov 2015 12:10:52 +0000 Subject: [PATCH 4/8] Turn `salt.cli.api.SaltAPI` code into proper daemon code --- salt/cli/api.py | 61 +++++++++++++++++++++++++++++++++++++++++++------ salt/scripts.py | 2 +- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/salt/cli/api.py b/salt/cli/api.py index 93eca01e7f..c7d94b648a 100644 --- a/salt/cli/api.py +++ b/salt/cli/api.py @@ -9,13 +9,13 @@ # Import Python libs from __future__ import absolute_import, print_function +import os import logging # Import Salt libs import salt.client.netapi import salt.utils.parsers as parsers -from salt.utils.verify import verify_log - +from salt.utils.verify import check_user, verify_files, verify_log log = logging.getLogger(__name__) @@ -24,14 +24,61 @@ class SaltAPI(parsers.SaltAPIParser): ''' The cli parser object used to fire up the salt api system. ''' - def run(self): + + def prepare(self): ''' - Run the api + Run the preparation sequence required to start a salt-api daemon. + + If sub-classed, don't **ever** forget to run: + + super(YourSubClass, self).prepare() ''' - self.parse_args() + super(SaltAPI, self).prepare() + + try: + if self.config['verify_env']: + logfile = self.config['log_file'] + if logfile is not None and not logfile.startswith(('tcp://', + 'udp://', + 'file://')): + # Logfile is not using Syslog, verify + current_umask = os.umask(0o027) + verify_files([logfile], self.config['user']) + os.umask(current_umask) + except OSError as err: + log.exception('Failed to prepare salt environment') + self.shutdown(err.errno) + self.setup_logfile_logger() verify_log(self.config) + log.info('Setting up the Salt API') + self.api = salt.client.netapi.NetapiClient(self.config) self.daemonize_if_required() - client = salt.client.netapi.NetapiClient(self.config) self.set_pidfile() - client.run() + + def start(self): + ''' + Start the actual master. + + If sub-classed, don't **ever** forget to run: + + super(YourSubClass, self).start() + + NOTE: Run any required code before calling `super()`. + ''' + super(SaltAPI, self).start() + if check_user(self.config['user']): + log.info('The salt-api is starting up') + self.api.run() + + def shutdown(self, exitcode=0, exitmsg=None): + ''' + If sub-classed, run any shutdown operations on this method. + ''' + log.info('The salt-api is shutting down..') + msg = 'The salt-api is shutdown. ' + if exitmsg is not None: + exitmsg = msg + exitmsg + else: + exitmsg = msg.strip() + super(SaltAPI, self).shutdown(exitcode, exitmsg) diff --git a/salt/scripts.py b/salt/scripts.py index b696b388c6..76574b3623 100644 --- a/salt/scripts.py +++ b/salt/scripts.py @@ -439,7 +439,7 @@ def salt_api(): ''' import salt.cli.api sapi = salt.cli.api.SaltAPI() # pylint: disable=E1120 - sapi.run() + sapi.start() def salt_main(): From 1195a2e1b7225314c89dd673394d7456d6cf0723 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 18 Nov 2015 12:16:45 +0000 Subject: [PATCH 5/8] Rename `logger` to `log` since that's what's used throughout Salt's code --- salt/cli/daemons.py | 56 ++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/salt/cli/daemons.py b/salt/cli/daemons.py index 66ef6514e9..dd5e575d4d 100644 --- a/salt/cli/daemons.py +++ b/salt/cli/daemons.py @@ -54,7 +54,7 @@ from salt.exceptions import SaltSystemExit # Let's instantiate logger using salt.log.setup.logging.getLogger() so pylint # leaves us alone and stops complaining about an un-used import -logger = salt.log.setup.logging.getLogger(__name__) +log = salt.log.setup.logging.getLogger(__name__) class Master(parsers.MasterOptionParser): @@ -121,12 +121,12 @@ class Master(parsers.MasterOptionParser): for syndic_file in os.listdir(self.config['syndic_dir']): os.remove(os.path.join(self.config['syndic_dir'], syndic_file)) except OSError as err: - logger.exception('Failed to prepare salt environment') + log.exception('Failed to prepare salt environment') self.shutdown(err.errno) self.setup_logfile_logger() verify_log(self.config) - logger.info('Setting up the Salt Master') + log.info('Setting up the Salt Master') # TODO: AIO core is separate from transport if self.config['transport'].lower() in ('zeromq', 'tcp'): @@ -160,14 +160,14 @@ class Master(parsers.MasterOptionParser): ''' super(Master, self).start() if check_user(self.config['user']): - logger.info('The salt master is starting up') + log.info('The salt master is starting up') self.master.start() def shutdown(self, exitcode=0, exitmsg=None): ''' If sub-classed, run any shutdown operations on this method. ''' - logger.info('The salt master is shutting down..') + log.info('The salt master is shutting down..') msg = 'The salt master is shutdown. ' if exitmsg is not None: exitmsg = msg + exitmsg @@ -240,12 +240,12 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init verify_files([logfile], self.config['user']) os.umask(current_umask) except OSError as err: - logger.exception('Failed to prepare salt environment') + log.exception('Failed to prepare salt environment') self.shutdown(err.errno) self.setup_logfile_logger() verify_log(self.config) - logger.info( + log.info( 'Setting up the Salt Minion "{0}"'.format( self.config['id'] ) @@ -254,7 +254,7 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init # Bail out if we find a process running and it matches out pidfile if self.check_running(): - logger.exception('Salt minion is already running. Exiting.') + log.exception('Salt minion is already running. Exiting.') self.shutdown(1) # TODO: AIO core is separate from transport @@ -293,15 +293,15 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init super(Minion, self).start() try: if check_user(self.config['user']): - logger.info('The salt minion is starting up') + log.info('The salt minion is starting up') self.minion.tune_in() except (KeyboardInterrupt, SaltSystemExit) as exc: - logger.warn('Stopping the Salt Minion') + log.warn('Stopping the Salt Minion') if isinstance(exc, KeyboardInterrupt): - logger.warn('Exiting on Ctrl-c') + log.warn('Exiting on Ctrl-c') self.shutdown() else: - logger.error(str(exc)) + log.error(str(exc)) self.shutdown(exc.code) def call(self, cleanup_protecteds): @@ -324,19 +324,19 @@ class Minion(parsers.MinionOptionParser): # pylint: disable=no-init self.minion.opts['raet_cleanup_protecteds'] = cleanup_protecteds self.minion.call_in() except (KeyboardInterrupt, SaltSystemExit) as exc: - logger.warn('Stopping the Salt Minion') + log.warn('Stopping the Salt Minion') if isinstance(exc, KeyboardInterrupt): - logger.warn('Exiting on Ctrl-c') + log.warn('Exiting on Ctrl-c') self.shutdown() else: - logger.error(str(exc)) + log.error(str(exc)) self.shutdown(exc.code) def shutdown(self, exitcode=0, exitmsg=None): ''' If sub-classed, run any shutdown operations on this method. ''' - logger.info('The salt minion is shutting down..') + log.info('The salt minion is shutting down..') if hasattr(self, 'minion'): self.minion.destroy() msg = 'The salt minion is shutdown. ' @@ -424,12 +424,12 @@ class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init os.umask(current_umask) except OSError as err: - logger.exception('Failed to prepare salt environment') + log.exception('Failed to prepare salt environment') self.shutdown(err.errno) self.setup_logfile_logger() verify_log(self.config) - logger.info( + log.info( 'Setting up a Salt Proxy Minion "{0}"'.format( self.config['id'] ) @@ -467,15 +467,15 @@ class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init super(ProxyMinion, self).start() try: if check_user(self.config['user']): - logger.info('The proxy minion is starting up') + log.info('The proxy minion is starting up') self.minion.tune_in() except (KeyboardInterrupt, SaltSystemExit) as exc: - logger.warn('Stopping the Salt Proxy Minion') + log.warn('Stopping the Salt Proxy Minion') if isinstance(exc, KeyboardInterrupt): - logger.warn('Exiting on Ctrl-c') + log.warn('Exiting on Ctrl-c') self.shutdown() else: - logger.error(str(exc)) + log.error(str(exc)) self.shutdown(exc.code) def shutdown(self, exitcode=0, exitmsg=None): @@ -485,7 +485,7 @@ class ProxyMinion(parsers.ProxyMinionOptionParser): # pylint: disable=no-init if hasattr(self, 'minion') and 'proxymodule' in self.minion.opts: proxy_fn = self.minion.opts['proxymodule'].loaded_base_name + '.shutdown' self.minion.opts['proxymodule'][proxy_fn](self.minion.opts) - logger.info('The proxy minion is shutting down..') + log.info('The proxy minion is shutting down..') msg = 'The proxy minion is shutdown. ' if exitmsg is not None: exitmsg = msg + exitmsg @@ -531,12 +531,12 @@ class Syndic(parsers.SyndicOptionParser): verify_files([logfile], self.config['user']) os.umask(current_umask) except OSError as err: - logger.exception('Failed to prepare salt environment') + log.exception('Failed to prepare salt environment') self.shutdown(err.errno) self.setup_logfile_logger() verify_log(self.config) - logger.info( + log.info( 'Setting up the Salt Syndic Minion "{0}"'.format( self.config['id'] ) @@ -564,18 +564,18 @@ class Syndic(parsers.SyndicOptionParser): ''' super(Syndic, self).start() if check_user(self.config['user']): - logger.info('The salt syndic is starting up') + log.info('The salt syndic is starting up') try: self.syndic.tune_in() except KeyboardInterrupt: - logger.warn('Stopping the Salt Syndic Minion') + log.warn('Stopping the Salt Syndic Minion') self.shutdown() def shutdown(self, exitcode=0, exitmsg=None): ''' If sub-classed, run any shutdown operations on this method. ''' - logger.info('The salt syndic is shutting down..') + log.info('The salt syndic is shutting down..') msg = 'The salt syndic is shutdown. ' if exitmsg is not None: exitmsg = msg + exitmsg From 641261aa3fdfd9bb987e20336df5a220516d5965 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 18 Nov 2015 12:23:51 +0000 Subject: [PATCH 6/8] Proper signal handling to the salt-api daemon --- salt/cli/api.py | 8 ++++++++ salt/client/netapi.py | 27 ++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/salt/cli/api.py b/salt/cli/api.py index c7d94b648a..139ba1e299 100644 --- a/salt/cli/api.py +++ b/salt/cli/api.py @@ -82,3 +82,11 @@ class SaltAPI(parsers.SaltAPIParser): else: exitmsg = msg.strip() super(SaltAPI, self).shutdown(exitcode, exitmsg) + + def _handle_signals(self, signum, sigframe): # pylint: disable=unused-argument + # escalate signal to the process manager processes + self.api.process_manager.stop_restarting() + self.api.process_manager.send_signal_to_processes(signum) + # kill any remaining processes + self.api.process_manager.kill_children() + super(SaltAPI, self)._handle_signals(signum, sigframe) diff --git a/salt/client/netapi.py b/salt/client/netapi.py index 96fbdddd04..c41ebf4ea3 100644 --- a/salt/client/netapi.py +++ b/salt/client/netapi.py @@ -2,15 +2,16 @@ ''' The main entry point for salt-api ''' -from __future__ import absolute_import # Import python libs +from __future__ import absolute_import +import signal import logging # Import salt-api libs import salt.loader import salt.utils.process -logger = logging.getLogger(__name__) +log = logging.getLogger(__name__) class NetapiClient(object): @@ -19,7 +20,7 @@ class NetapiClient(object): ''' def __init__(self, opts): self.opts = opts - self.process_manager = salt.utils.process.ProcessManager() + self.process_manager = salt.utils.process.ProcessManager(name='NetAPIProcessManager') self.netapi = salt.loader.netapi(self.opts) def run(self): @@ -27,11 +28,27 @@ class NetapiClient(object): Load and start all available api modules ''' if not len(self.netapi): - logger.error("Did not find any netapi configurations, nothing to start") + log.error("Did not find any netapi configurations, nothing to start") for fun in self.netapi: if fun.endswith('.start'): - logger.info('Starting {0} netapi module'.format(fun)) + log.info('Starting {0} netapi module'.format(fun)) self.process_manager.add_process(self.netapi[fun]) + # Install the SIGINT/SIGTERM handlers if not done so far + if signal.getsignal(signal.SIGINT) is signal.SIG_DFL: + # No custom signal handling was added, install our own + signal.signal(signal.SIGINT, self._handle_signals) + + if signal.getsignal(signal.SIGTERM) is signal.SIG_DFL: + # No custom signal handling was added, install our own + signal.signal(signal.SIGINT, self._handle_signals) + self.process_manager.run() + + def _handle_signals(self, signum, sigframe): # pylint: disable=unused-argument + # escalate the signals to the process manager + self.process_manager.stop_restarting() + self.process_manager.send_signal_to_processes(signum) + # kill any remaining processes + self.process_manager.kill_children() From 8a04daeb1f9f230c0532bf4355f61346313a97d5 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 18 Nov 2015 16:26:45 +0000 Subject: [PATCH 7/8] Can't just rename `run()` to `start()`. Deprecate `run()`. --- salt/cli/api.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/salt/cli/api.py b/salt/cli/api.py index 139ba1e299..dae7a357df 100644 --- a/salt/cli/api.py +++ b/salt/cli/api.py @@ -56,6 +56,15 @@ class SaltAPI(parsers.SaltAPIParser): self.daemonize_if_required() self.set_pidfile() + def run(self): + salt.utils.warn_until( + 'Nitrogen', + 'Please stop calling \'SaltAPI.run()\' and instead call ' + '\'SaltAPI.start()\'. \'SaltAPI.run()\' will be supported ' + 'until Salt {version}.' + ) + self.start() + def start(self): ''' Start the actual master. From 6e574d92c3c35e618ec7d4eab543ef767569b348 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 18 Nov 2015 16:27:37 +0000 Subject: [PATCH 8/8] Add missing import --- salt/cli/api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/salt/cli/api.py b/salt/cli/api.py index dae7a357df..c5ab6d4cb0 100644 --- a/salt/cli/api.py +++ b/salt/cli/api.py @@ -57,6 +57,7 @@ class SaltAPI(parsers.SaltAPIParser): self.set_pidfile() def run(self): + import salt.utils salt.utils.warn_until( 'Nitrogen', 'Please stop calling \'SaltAPI.run()\' and instead call '