From 24a1f21fd2529526160a6b4f657c37c75c6535bf Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Sun, 13 Jan 2013 15:28:23 +0000 Subject: [PATCH 1/3] Simplify sub-classing `Master`, `Minion` and `Syndic` + PEP-8. * Since we no longer suffer from too early logger initiation, replace any `logging.getLogger(__name__)` calls with an initial defined `logger`(not `log` so it does not clash with `salt.log`. --- salt/__init__.py | 161 ++++++++++++++++++++++++++++++++--------------- salt/scripts.py | 6 +- 2 files changed, 113 insertions(+), 54 deletions(-) diff --git a/salt/__init__.py b/salt/__init__.py index 3fc39b17ff..c198144765 100644 --- a/salt/__init__.py +++ b/salt/__init__.py @@ -9,7 +9,7 @@ import logging # Import salt libs, the try block bypasses an issue at build time so that # modules don't cause the build to fail -from salt.version import __version__ +from salt.version import __version__ # pylint: disable-msg=W402 from salt.utils import migrations try: @@ -21,12 +21,15 @@ except ImportError as e: raise +logger = logging.getLogger(__name__) + + class Master(parsers.MasterOptionParser): ''' Creates a master server ''' - def start(self): + def run(self): ''' Run the sequence to start a salt master server ''' @@ -34,20 +37,22 @@ class Master(parsers.MasterOptionParser): try: if self.config['verify_env']: - verify_env([ - self.config['pki_dir'], - os.path.join(self.config['pki_dir'], 'minions'), - os.path.join(self.config['pki_dir'], 'minions_pre'), - os.path.join(self.config['pki_dir'], 'minions_rejected'), - self.config['cachedir'], - os.path.join(self.config['cachedir'], 'jobs'), - os.path.join(self.config['cachedir'], 'proc'), - self.config['sock_dir'], - self.config['token_dir'], - ], - self.config['user'], - permissive=self.config['permissive_pki_access'], - pki_dir=self.config['pki_dir'], + verify_env( + [ + self.config['pki_dir'], + os.path.join(self.config['pki_dir'], 'minions'), + os.path.join(self.config['pki_dir'], 'minions_pre'), + os.path.join(self.config['pki_dir'], + 'minions_rejected'), + self.config['cachedir'], + os.path.join(self.config['cachedir'], 'jobs'), + os.path.join(self.config['cachedir'], 'proc'), + self.config['sock_dir'], + self.config['token_dir'], + ], + self.config['user'], + permissive=self.config['permissive_pki_access'], + pki_dir=self.config['pki_dir'], ) if (not self.config['log_file'].startswith('tcp://') or not self.config['log_file'].startswith('udp://') or @@ -61,7 +66,7 @@ class Master(parsers.MasterOptionParser): sys.exit(err.errno) self.setup_logfile_logger() - logging.getLogger(__name__).warn('Setting up the Salt Master') + logger.warn('Setting up the Salt Master') if not verify_socket(self.config['interface'], self.config['publish_port'], @@ -71,22 +76,39 @@ class Master(parsers.MasterOptionParser): # Late import so logging works correctly import salt.master - master = salt.master.Master(self.config) + self.master = salt.master.Master(self.config) self.daemonize_if_required() self.set_pidfile() if check_user(self.config['user']): try: - master.start() + self.start() except salt.master.MasterExit: + self.stop() + finally: sys.exit() + def start(self): + ''' + Start the actual master. If sub-classed, don't **ever** forget to run: + + super(YourSubClass, self).start() + + NOTE: Run your start-up code before calling `super()`. + ''' + self.master.start() + + def stop(self): + ''' + If sub-classed, run any shutdown operations on this method. + ''' + class Minion(parsers.MinionOptionParser): ''' Create a minion server ''' - def start(self): + def run(self): ''' Execute this method to start up a minion. ''' @@ -95,18 +117,20 @@ class Minion(parsers.MinionOptionParser): try: if self.config['verify_env']: confd = os.path.join( - os.path.dirname(self.config['conf_file']), - 'minion.d') - verify_env([ - self.config['pki_dir'], - self.config['cachedir'], - self.config['sock_dir'], - self.config['extension_modules'], - confd, - ], - self.config['user'], - permissive=self.config['permissive_pki_access'], - pki_dir=self.config['pki_dir'], + os.path.dirname(self.config['conf_file']), + 'minion.d' + ) + verify_env( + [ + self.config['pki_dir'], + self.config['cachedir'], + self.config['sock_dir'], + self.config['extension_modules'], + confd, + ], + self.config['user'], + permissive=self.config['permissive_pki_access'], + pki_dir=self.config['pki_dir'], ) if (not self.config['log_file'].startswith('tcp://') or not self.config['log_file'].startswith('udp://') or @@ -123,9 +147,10 @@ class Minion(parsers.MinionOptionParser): sys.exit(err.errno) self.setup_logfile_logger() - log = logging.getLogger(__name__) - log.warn( - 'Setting up the Salt Minion "{0}"'.format( self.config['id']) + logger.warn( + 'Setting up the Salt Minion "{0}"'.format( + self.config['id'] + ) ) migrations.migrate_paths(self.config) # Late import so logging works correctly @@ -136,36 +161,54 @@ class Minion(parsers.MinionOptionParser): # This is the latest safe place to daemonize self.daemonize_if_required() try: - minion = salt.minion.Minion(self.config) + self.minion = salt.minion.Minion(self.config) self.set_pidfile() if check_user(self.config['user']): - minion.tune_in() + self.start() except KeyboardInterrupt: log.warn('Stopping the Salt Minion') + self.stop() + finally: raise SystemExit('\nExiting on Ctrl-c') + def start(self): + ''' + Start the actual minion. If sub-classed, don't **ever** forget to run: + + super(YourSubClass, self).start() + + NOTE: Run your start-up code before calling `super()`. + ''' + self.minion.tune_in() + + def stop(self): + ''' + If sub-classed, run any shutdown operations on this method. + ''' + class Syndic(parsers.SyndicOptionParser): ''' Create a syndic server ''' - def start(self): + def run(self): ''' Execute this method to start up a syndic. ''' self.parse_args() try: if self.config['verify_env']: - verify_env([ - self.config['pki_dir'], - self.config['cachedir'], - self.config['sock_dir'], - self.config['extension_modules'], - ], - self.config['user'], - permissive=self.config['permissive_pki_access'], - pki_dir=self.config['pki_dir'], + verify_env( + [ + self.config['pki_dir'], + self.config['cachedir'], + self.config['sock_dir'], + self.config['extension_modules'], + ], + self.config['user'], + permissive=self.config['permissive_pki_access'], + pki_dir=self.config['pki_dir'], ) if (not self.config['log_file'].startswith('tcp://') or not self.config['log_file'].startswith('udp://') or @@ -179,8 +222,7 @@ class Syndic(parsers.SyndicOptionParser): sys.exit(err.errno) self.setup_logfile_logger() - log = logging.getLogger(__name__) - log.warn( + logger.warn( 'Setting up the Salt Syndic Minion "{0}"'.format( self.config['id'] ) @@ -193,8 +235,25 @@ class Syndic(parsers.SyndicOptionParser): if check_user(self.config['user']): try: - syndic = salt.minion.Syndic(self.config) - syndic.tune_in() + self.syndic = salt.minion.Syndic(self.config) + self.start() except KeyboardInterrupt: log.warn('Stopping the Salt Syndic Minion') + self.stop() + finally: raise SystemExit('\nExiting on Ctrl-c') + + def start(self): + ''' + Start the actual syndic. If sub-classed, don't **ever** forget to run: + + super(YourSubClass, self).start() + + NOTE: Run your start-up code before calling `super()`. + ''' + self.syndic.tune_in() + + def stop(self): + ''' + If sub-classed, run any shutdown operations on this method. + ''' diff --git a/salt/scripts.py b/salt/scripts.py index 60d75f8c5d..5d729eac28 100644 --- a/salt/scripts.py +++ b/salt/scripts.py @@ -16,7 +16,7 @@ def salt_master(): Start the salt-master. ''' master = salt.Master() - master.start() + master.run() def salt_minion(): @@ -26,7 +26,7 @@ def salt_minion(): if '' in sys.path: sys.path.remove('') minion = salt.Minion() - minion.start() + minion.run() def salt_syndic(): @@ -36,7 +36,7 @@ def salt_syndic(): pid = os.getpid() try: syndic = salt.Syndic() - syndic.start() + syndic.run() except KeyboardInterrupt: os.kill(pid, 15) From f68b80b24b5765eacf5e55f19e645513bae3a3fc Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Mon, 14 Jan 2013 23:15:48 +0000 Subject: [PATCH 2/3] Comply with the convention and still provide the ability to subclass masters and minions. --- salt/__init__.py | 111 +++++++++++++++++++++++++++-------------------- salt/scripts.py | 4 +- 2 files changed, 66 insertions(+), 49 deletions(-) diff --git a/salt/__init__.py b/salt/__init__.py index c198144765..4aa68b9773 100644 --- a/salt/__init__.py +++ b/salt/__init__.py @@ -7,8 +7,8 @@ import os import sys import logging -# Import salt libs, the try block bypasses an issue at build time so that -# modules don't cause the build to fail +# Import salt libs, the try block below bypasses an issue at build time so +# that modules don't cause the build to fail from salt.version import __version__ # pylint: disable-msg=W402 from salt.utils import migrations @@ -29,9 +29,13 @@ class Master(parsers.MasterOptionParser): Creates a master server ''' - def run(self): + def prepare(self): ''' - Run the sequence to start a salt master server + Run the preparation sequence required to start a salt master server. + + If sub-classed, don't **ever** forget to run: + + super(YourSubClass, self).prepare() ''' self.parse_args() @@ -79,25 +83,27 @@ class Master(parsers.MasterOptionParser): self.master = salt.master.Master(self.config) self.daemonize_if_required() self.set_pidfile() - if check_user(self.config['user']): - try: - self.start() - except salt.master.MasterExit: - self.stop() - finally: - sys.exit() def start(self): ''' - Start the actual master. If sub-classed, don't **ever** forget to run: + Start the actual master. + + If sub-classed, don't **ever** forget to run: super(YourSubClass, self).start() - NOTE: Run your start-up code before calling `super()`. + NOTE: Run any required code before calling `super()`. ''' - self.master.start() + self.prepare() + if check_user(self.config['user']): + try: + self.master.start() + except salt.master.MasterExit: + self.shutdown() + finally: + sys.exit() - def stop(self): + def shutdown(self): ''' If sub-classed, run any shutdown operations on this method. ''' @@ -108,9 +114,13 @@ class Minion(parsers.MinionOptionParser): Create a minion server ''' - def run(self): + def prepare(self): ''' - Execute this method to start up a minion. + Run the preparation sequence required to start a salt minion. + + If sub-classed, don't **ever** forget to run: + + super(YourSubClass, self).prepare() ''' self.parse_args() @@ -160,28 +170,30 @@ class Minion(parsers.MinionOptionParser): # the boot process waiting for a key to be accepted on the master. # This is the latest safe place to daemonize self.daemonize_if_required() - try: - self.minion = salt.minion.Minion(self.config) - self.set_pidfile() - if check_user(self.config['user']): - self.start() - except KeyboardInterrupt: - log.warn('Stopping the Salt Minion') - self.stop() - finally: - raise SystemExit('\nExiting on Ctrl-c') + self.minion = salt.minion.Minion(self.config) + self.set_pidfile() def start(self): ''' - Start the actual minion. If sub-classed, don't **ever** forget to run: + Start the actual minion. + + If sub-classed, don't **ever** forget to run: super(YourSubClass, self).start() - NOTE: Run your start-up code before calling `super()`. + NOTE: Run any required code before calling `super()`. ''' - self.minion.tune_in() + self.prepare() + try: + if check_user(self.config['user']): + self.minion.tune_in() + except KeyboardInterrupt: + logger.warn('Stopping the Salt Minion') + self.shutdown() + finally: + raise SystemExit('\nExiting on Ctrl-c') - def stop(self): + def shutdown(self): ''' If sub-classed, run any shutdown operations on this method. ''' @@ -192,9 +204,13 @@ class Syndic(parsers.SyndicOptionParser): Create a syndic server ''' - def run(self): + def prepare(self): ''' - Execute this method to start up a syndic. + Run the preparation sequence required to start a salt syndic minion. + + If sub-classed, don't **ever** forget to run: + + super(YourSubClass, self).prepare() ''' self.parse_args() try: @@ -231,29 +247,30 @@ class Syndic(parsers.SyndicOptionParser): # Late import so logging works correctly import salt.minion self.daemonize_if_required() + self.syndic = salt.minion.Syndic(self.config) self.set_pidfile() - if check_user(self.config['user']): - try: - self.syndic = salt.minion.Syndic(self.config) - self.start() - except KeyboardInterrupt: - log.warn('Stopping the Salt Syndic Minion') - self.stop() - finally: - raise SystemExit('\nExiting on Ctrl-c') - def start(self): ''' - Start the actual syndic. If sub-classed, don't **ever** forget to run: + Start the actual syndic. + + If sub-classed, don't **ever** forget to run: super(YourSubClass, self).start() - NOTE: Run your start-up code before calling `super()`. + NOTE: Run any required code before calling `super()`. ''' - self.syndic.tune_in() + self.prepare() + if check_user(self.config['user']): + try: + self.syndic.tune_in() + except KeyboardInterrupt: + logger.warn('Stopping the Salt Syndic Minion') + self.shutdown() + finally: + raise SystemExit('\nExiting on Ctrl-c') - def stop(self): + def shutdown(self): ''' If sub-classed, run any shutdown operations on this method. ''' diff --git a/salt/scripts.py b/salt/scripts.py index 5d729eac28..603f2cf17d 100644 --- a/salt/scripts.py +++ b/salt/scripts.py @@ -16,7 +16,7 @@ def salt_master(): Start the salt-master. ''' master = salt.Master() - master.run() + master.start() def salt_minion(): @@ -26,7 +26,7 @@ def salt_minion(): if '' in sys.path: sys.path.remove('') minion = salt.Minion() - minion.run() + minion.start() def salt_syndic(): From b0209a7940456e5ce8347650563b132cc17e2c5e Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Mon, 14 Jan 2013 23:19:05 +0000 Subject: [PATCH 3/3] The syndic also `start()`'s it does not `run()`. --- salt/scripts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/scripts.py b/salt/scripts.py index 603f2cf17d..60d75f8c5d 100644 --- a/salt/scripts.py +++ b/salt/scripts.py @@ -36,7 +36,7 @@ def salt_syndic(): pid = os.getpid() try: syndic = salt.Syndic() - syndic.run() + syndic.start() except KeyboardInterrupt: os.kill(pid, 15)