From f9488862bc18c5cd6fae372b29d004f12ad0bdd9 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 6 Jun 2017 19:31:43 +0200 Subject: [PATCH 01/11] Reorganize some imports in cli.py --- kolibri/utils/cli.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/kolibri/utils/cli.py b/kolibri/utils/cli.py index 045ad7eb6d3..21d720ed453 100644 --- a/kolibri/utils/cli.py +++ b/kolibri/utils/cli.py @@ -2,23 +2,21 @@ import importlib # noqa import logging # noqa +import os # noqa +import signal # noqa +import sys # noqa + # Do this before importing anything else, we need to add bundled requirements # from the distributed version in case it exists before importing anything # else. -# TODO: Do we want to manage the path at an even more fundametal place like +# TODO: Do we want to manage the path at an even more fundamental place like # kolibri.__init__ !? Load order will still matter... -import os # noqa -import signal # noqa -import sys # noqa -from logging import config as logging_config # noqa import kolibri # noqa from kolibri import dist as kolibri_dist # noqa - -# Setup path in case we are running with dependencies bundled into Kolibri -# (NOTE: This *must* come before imports below, of django etc, or whl/pex will fail) -sys.path = [os.path.realpath(os.path.dirname(kolibri_dist.__file__)) - ] + sys.path +sys.path = [ + os.path.realpath(os.path.dirname(kolibri_dist.__file__)) +] + sys.path import django # noqa from django.core.management import call_command # noqa @@ -183,7 +181,7 @@ def setup_logging(debug=False): settings.DEBUG = True LOGGING['handlers']['console']['level'] = 'DEBUG' LOGGING['loggers']['kolibri']['level'] = 'DEBUG' - logging_config.dictConfig(LOGGING) + logging.config.dictConfig(LOGGING) logger.debug("Debug mode is on!") From 342961bd5f71bd302abe72349f2a192926b34e22 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 6 Jun 2017 22:18:22 +0200 Subject: [PATCH 02/11] Adding status checks and pid behavior, no daemon logic yet --- kolibri/utils/cli.py | 148 ++++++++++++++++++++++++++---- kolibri/utils/server.py | 199 +++++++++++++++++++++++++++++++++++++--- kolibri/utils/system.py | 72 +++++++++++++++ 3 files changed, 386 insertions(+), 33 deletions(-) create mode 100644 kolibri/utils/system.py diff --git a/kolibri/utils/cli.py b/kolibri/utils/cli.py index 21d720ed453..3ac089ad527 100644 --- a/kolibri/utils/cli.py +++ b/kolibri/utils/cli.py @@ -18,6 +18,15 @@ os.path.realpath(os.path.dirname(kolibri_dist.__file__)) ] + sys.path +# Set default env +os.environ.setdefault( + "DJANGO_SETTINGS_MODULE", "kolibri.deployment.default.settings.base" +) +os.environ.setdefault( + "KOLIBRI_HOME", os.path.join(os.path.expanduser("~"), ".kolibri") +) +os.environ.setdefault("KOLIBRI_LISTEN_PORT", "8008") + import django # noqa from django.core.management import call_command # noqa from docopt import docopt # noqa @@ -98,15 +107,6 @@ """.format(usage="\n".join(map(lambda x: " " + x, USAGE.split("\n")))) -# Set default env -os.environ.setdefault( - "DJANGO_SETTINGS_MODULE", "kolibri.deployment.default.settings.base" -) -os.environ.setdefault( - "KOLIBRI_HOME", os.path.join(os.path.expanduser("~"), ".kolibri") -) -os.environ.setdefault("KOLIBRI_LISTEN_PORT", "8008") - logger = logging.getLogger(__name__) KOLIBRI_HOME = os.environ['KOLIBRI_HOME'] @@ -123,15 +123,30 @@ class PluginDoesNotExist(Exception): def initialize(debug=False): """ - Always called before running commands + Currently, always called before running commands. This may change in case + commands that conflict with this behavior show up. :param: debug: Tells initialization to setup logging etc. """ + # TODO: We'll move this to a more deliberate location whereby we can + # ensure that some parts of kolibri can run without the whole django stack + django.setup() + setup_logging(debug=debug) if not os.path.isfile(VERSION_FILE): _first_run() + else: + version = open(VERSION_FILE, "r").read() + if kolibri.__version__ != version.strip(): + logger.info( + "Version was {old}, new version: {new}".format( + old=version, + new=kolibri.__version__ + ) + ) + update() def _first_run(): @@ -150,12 +165,13 @@ def _first_run(): "to wait a bit while we create a blank database...\n\n" ) - django.setup() - from kolibri.core.settings import SKIP_AUTO_DATABASE_MIGRATION, DEFAULT_PLUGINS + # We need to migrate the database before enabling plugins, because they + # might depend on database readiness. if not SKIP_AUTO_DATABASE_MIGRATION: - call_command("migrate", interactive=False) + call_command("migrate", interactive=False, database="default") + call_command("migrate", interactive=False, database="ormq") for plugin_module in DEFAULT_PLUGINS: try: @@ -165,13 +181,105 @@ def _first_run(): logger.info("Automatically enabling applications.") + # Finally collect static assets and run migrations again + update() + + +def update(): + """ + Called whenever a version change in kolibri is detected + + TODO: We should look at version numbers of external plugins, too! + """ + logger.info("Running update routines for new version...") + call_command("collectstatic", interactive=False) + call_command("collectstatic_js_reverse", interactive=False) + + from kolibri.core.settings import SKIP_AUTO_DATABASE_MIGRATION + + if not SKIP_AUTO_DATABASE_MIGRATION: + call_command("migrate", interactive=False, database="default") + call_command("migrate", interactive=False, database="ormq") + with open(VERSION_FILE, "w") as f: f.write(kolibri.__version__) +def start(port=8080, daemon=True): + """ + Start the server on given port. + + :param: port: Port number (default: 8080) + :param: daemon: Fork to background process (default: True) + """ + + if not daemon: + logger.info("Running 'kolibri start' in foreground...\n") + else: + logger.info("Running 'kolibri start' as daemon (system service)\n") + + # TODO: moved from server.start() but not sure where it should ideally be + # located. Question is if it should be run every time the server is started + # or if it depends on some kind of state change. + from kolibri.content.utils.annotation import update_channel_metadata_cache + update_channel_metadata_cache() + + server.start(port=port) + + +def status(): + """ + Check the server's status. For possible statuses, see the status dictionary + status.codes + + Status *always* outputs the current status in the first line of stderr. + The following lines contain optional information such as the addresses where + the server is listening. + + TODO: We can't guarantee the above behavior because of the django stack + being loaded regardless + + :returns: status_code, key has description in status.codes + """ + status_code, urls = server.get_urls() + + if status_code == server.STATUS_RUNNING: + sys.stderr.write("{msg:s} (0)\n".format(msg=status.codes[0])) + sys.stderr.write("Kolibri running on:\n\n") + for addr in urls: + sys.stderr.write("\t{}\n".format(addr)) + return server.STATUS_RUNNING + else: + verbose_status = status.codes[status_code] + sys.stderr.write("{msg:s} ({code:d})\n".format( + code=status_code, msg=verbose_status)) + return status_code + + +status.codes = { + server.STATUS_RUNNING: 'OK, running', + server.STATUS_STOPPED: 'Stopped', + server.STATUS_STARTING_UP: 'Starting up', + server.STATUS_NOT_RESPONDING: 'Not responding', + server.STATUS_FAILED_TO_START: + 'Failed to start (check log file: {0})'.format(server.DAEMON_LOG), + server.STATUS_UNCLEAN_SHUTDOWN: 'Unclean shutdown', + server.STATUS_UNKNOWN_INSTANCE: 'Unknown KA Lite running on port', + server.STATUS_SERVER_CONFIGURATION_ERROR: 'KA Lite server configuration error', + server.STATUS_PID_FILE_READ_ERROR: 'Could not read PID file', + server.STATUS_PID_FILE_INVALID: 'Invalid PID file', + server.STATUS_UNKNOW: 'Could not determine status', +} + + def setup_logging(debug=False): - """Configures logging in cases where a Django environment is not supposed - to be configured""" + """ + Configures logging in cases where a Django environment is not supposed + to be configured. + + TODO: This is really confusing, importing django settings is allowed to + fail when debug=False, but if it's true it can fail? + """ try: from django.conf.settings import LOGGING except ImportError: @@ -312,9 +420,6 @@ def main(args=None): to use main() for integration tests in order to test the argument API. """ - # ensure that Django is set up before we do anything else - django.setup() - signal.signal(signal.SIGINT, signal.SIG_DFL) arguments, django_args = parse_args(args) @@ -340,7 +445,12 @@ def main(args=None): if arguments['start']: port = int(arguments['--port'] or 8080) - server.start(port=port) + start(port) + return + + if arguments['status']: + status_code = status() + sys.exit(status_code) return if arguments['language'] and arguments['setdefault']: diff --git a/kolibri/utils/server.py b/kolibri/utils/server.py index d2d188504d6..4b23331e5f4 100644 --- a/kolibri/utils/server.py +++ b/kolibri/utils/server.py @@ -4,11 +4,60 @@ import platform import cherrypy +import requests from django.conf import settings from django.core.management import call_command - from kolibri.content.utils import paths +from .system import pid_exists + +# Status codes for kolibri +STATUS_RUNNING = 0 +STATUS_STOPPED = 1 +STATUS_STARTING_UP = 4 +STATUS_NOT_RESPONDING = 5 +STATUS_FAILED_TO_START = 6 +STATUS_UNCLEAN_SHUTDOWN = 7 +STATUS_UNKNOWN_INSTANCE = 8 +STATUS_SERVER_CONFIGURATION_ERROR = 9 +STATUS_PID_FILE_READ_ERROR = 99 +STATUS_PID_FILE_INVALID = 100 +STATUS_UNKNOW = 101 + +# Used to store PID and port number (both in foreground and daemon mode) +PID_FILE = os.path.join( + os.environ['KOLIBRI_HOME'], + "server.pid" +) + +# Used to PID, port during certain exclusive startup process, before we fork +# to daemon mode +STARTUP_LOCK = os.path.join( + os.environ['KOLIBRI_HOME'], + "server.lock" +) + +# This is a special file with daemon activity. It logs ALL stderr output, some +# might not have made it to the log file! +DAEMON_LOG = os.path.join( + os.environ['KOLIBRI_HOME'], + "server.log" +) + +# Currently non-configurable until we know how to properly handle this +LISTEN_ADDRESS = "0.0.0.0" + + +class NotRunning(Exception): + """ + Raised when server was expected to run, but didn't. Contains a status + code explaining why. + """ + + def __init__(self, status_code): + self.status_code = status_code + super(NotRunning, self).__init__() + def start_background_workers(): p = multiprocessing.Process(target=call_command, args=("qcluster", )) @@ -21,25 +70,19 @@ def start_background_workers(): p.start() -def start(port=None): - - server_port = port or 8080 +def start(port=8080): + """ + Starts the server. - # TODO(aronasorman): move to install/plugin-enabling scripts, and remove from here - call_command("collectstatic", interactive=False) - call_command("collectstatic_js_reverse", interactive=False) - call_command("migrate", interactive=False, database="default") - call_command("migrate", interactive=False, database="ormq") + :param: port: Port number (default: 8080) + """ # start the qcluster process # don't run on windows; we don't run a full cluster there. if platform.system() != "Windows": start_background_workers() - from kolibri.content.utils.annotation import update_channel_metadata_cache - update_channel_metadata_cache() - - run_server(port=server_port) + run_server(port=port) def run_server(port): @@ -63,7 +106,7 @@ def run_server(port): server = cherrypy._cpserver.Server() # Configure the server - server.socket_host = "0.0.0.0" + server.socket_host = LISTEN_ADDRESS server.socket_port = port server.thread_pool = 30 @@ -83,3 +126,131 @@ def serve_static_dir(root, url): root=os.path.abspath(os.path.split(root)[0]) ) cherrypy.tree.mount(static_handler, url) + + +def _read_pid_file(filename): + """ + Reads a pid file and returns the contents. PID files have 1 or 2 lines; + - first line is always the pid + - optional second line is the port the server is listening on. + + :param filename: Path of PID to read + :return: (pid, port): with the PID in the file and the port number + if it exists. If the port number doesn't exist, then + port is None. + """ + try: + pid, port = open(filename, "r").readlines() + pid, port = int(pid), int(port) + except ValueError: + # The file only had one line + try: + pid, port = int(open(filename, "r").read()), None + except ValueError: + pid, port = None, None + return pid, port + + +def _write_pid_file(filename, port): + """ + Writes a PID file in the format Kolibri parses + + :param: filename: Path of file to write + :param: port: Listening port number which the server is assigned + """ + + with open(filename, 'w') as f: + f.write("%d\n%d" % (os.getpid(), port)) + + +def get_status(): # noqa: max-complexity=16 + """ + Tries to get the PID of a running server. + + The behavior is also quite redundant given that `kalite start` should + always create a PID file, and if its been started directly with the + runserver command, then its up to the developer to know what's happening. + :returns: (PID, address, port), where address is not currently detected in + a valid way because it's not configurable, and we might be + listening on several IPs. + :raises: NotRunning + """ + + # There is no PID file (created by server daemon) + if not os.path.isfile(PID_FILE): + # Is there a startup lock? + if os.path.isfile(STARTUP_LOCK): + try: + pid, port = _read_pid_file(STARTUP_LOCK) + # Does the PID in there still exist? + if pid_exists(pid): + raise NotRunning(STATUS_STARTING_UP) + # It's dead so assuming the startup went badly + else: + raise NotRunning(STATUS_FAILED_TO_START) + # Couldn't parse to int + except TypeError: + raise NotRunning(STATUS_STOPPED) + raise NotRunning(STATUS_STOPPED) # Stopped + + # PID file exists, check if it is running + try: + pid, port = _read_pid_file(PID_FILE) + except (ValueError, OSError): + raise NotRunning(STATUS_PID_FILE_INVALID) # Invalid PID file + + # PID file exists, but process is dead + if not pid_exists(pid): + if os.path.isfile(STARTUP_LOCK): + raise NotRunning(STATUS_FAILED_TO_START) # Failed to start + raise NotRunning(STATUS_UNCLEAN_SHUTDOWN) # Unclean shutdown + + listen_port = port + + try: + # Timeout is 3 seconds, we don't want the status command to be slow + # TODO: Using 127.0.0.1 is a hardcode default from KA Lite, it could + # be configurable + # TODO: HTTP might not be the protocol if server has SSL + response = requests.get( + "http://{}:{}".format("127.0.0.1", listen_port), + timeout=3 + ) + except (requests.exceptions.ReadTimeout, requests.exceptions.ConnectionError): + raise NotRunning(STATUS_NOT_RESPONDING) + except (requests.exceptions.RequestException): + if os.path.isfile(STARTUP_LOCK): + raise NotRunning(STATUS_STARTING_UP) # Starting up + raise NotRunning(STATUS_UNCLEAN_SHUTDOWN) + + if response.status_code == 404: + raise NotRunning(STATUS_UNKNOWN_INSTANCE) # Unknown HTTP server + + if response.status_code != 200: + # Probably a mis-configured kolibri + raise NotRunning(STATUS_SERVER_CONFIGURATION_ERROR) + + return pid, LISTEN_ADDRESS, listen_port # Correct PID ! + + # We don't detect this at present: + # Could be detected because we fetch the PID directly via HTTP, but this + # is dumb because kolibri could be running in a worker pool with different + # PID from the PID file.. + # raise NotRunning(STATUS_UNKNOWN_INSTANCE) + # This would be the fallback when we know it's not running, but we can't + # give a proper reason... + # raise NotRunning(STATUS_UNKNOW) + + +def get_urls(): + """ + This is a stub, see: https://github.com/learningequality/kolibri/issues/1595 + """ + try: + __, __, port = get_status() + urls = [] + for addr in [LISTEN_ADDRESS]: + urls.append("http://{}:{}/".format(addr, port)) + return STATUS_RUNNING, urls + except NotRunning as e: + return e.status_code, [] diff --git a/kolibri/utils/system.py b/kolibri/utils/system.py new file mode 100644 index 00000000000..f5c4768886e --- /dev/null +++ b/kolibri/utils/system.py @@ -0,0 +1,72 @@ +""" +Utilities for local system calls, everything here is cross-platform. + +We might want to refactor this into: + +system/__init__.py +system/posix.py +system/windows.py + +etc.. +""" +from __future__ import absolute_import, print_function, unicode_literals + +import os + + +def _posix_pid_exists(pid): + """Check whether PID exists in the current process table.""" + import errno + if pid < 0: + return False + try: + # Send signal 0, this is harmless + os.kill(pid, 0) + except OSError as e: + return e.errno == errno.EPERM + else: + return True + + +def _posix_kill_pid(pid): + """Kill a PID by sending a posix signal""" + import signal + try: + os.kill(pid, signal.SIGTERM) + # process does not exist + except OSError: + return + # process didn't exit cleanly, make one last effort to kill it + if pid_exists(pid): + os.kill(pid, signal.SIGKILL) + + +def _windows_pid_exists(pid): + import ctypes + kernel32 = ctypes.windll.kernel32 + SYNCHRONIZE = 0x100000 + + process = kernel32.OpenProcess(SYNCHRONIZE, 0, pid) + if process != 0: + kernel32.CloseHandle(process) + return True + else: + return False + + +def _windows_kill_pid(pid): + """Kill the proces using pywin32 and pid""" + import ctypes + PROCESS_TERMINATE = 1 + handle = ctypes.windll.kernel32.OpenProcess(PROCESS_TERMINATE, False, pid) # @UndefinedVariable + ctypes.windll.kernel32.TerminateProcess(handle, -1) # @UndefinedVariable + ctypes.windll.kernel32.CloseHandle(handle) # @UndefinedVariable + + +# Utility functions for pinging or killing PIDs +if os.name == 'posix': + pid_exists = _posix_pid_exists + kill_pid = _posix_kill_pid +else: + pid_exists = _windows_pid_exists + kill_pid = _windows_kill_pid From a42be4569cc5cb885b7e57e95d1f4200af9474a4 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 6 Jun 2017 22:38:18 +0200 Subject: [PATCH 03/11] Daemon logic (pending a stop command) --- kolibri/utils/cli.py | 14 +++++++++ kolibri/utils/server.py | 6 +++- kolibri/utils/system.py | 65 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/kolibri/utils/cli.py b/kolibri/utils/cli.py index 3ac089ad527..5f747dbbd96 100644 --- a/kolibri/utils/cli.py +++ b/kolibri/utils/cli.py @@ -32,6 +32,7 @@ from docopt import docopt # noqa from . import server # noqa +from .system import become_daemon # noqa # This was added in # https://github.com/learningequality/kolibri/pull/580 @@ -224,6 +225,19 @@ def start(port=8080, daemon=True): from kolibri.content.utils.annotation import update_channel_metadata_cache update_channel_metadata_cache() + # Daemonize at this point, no more user output is needed + if daemon: + + kwargs = {} + # Truncate the file + open(server.DAEMON_LOG, "w").truncate() + logger.info( + "Going to daemon mode, logging to {0}\n".format(server.DAEMON_LOG) + ) + kwargs['out_log'] = server.DAEMON_LOG + kwargs['err_log'] = server.DAEMON_LOG + become_daemon(**kwargs) + server.start(port=port) diff --git a/kolibri/utils/server.py b/kolibri/utils/server.py index 4b23331e5f4..4c165f8861a 100644 --- a/kolibri/utils/server.py +++ b/kolibri/utils/server.py @@ -70,7 +70,7 @@ def start_background_workers(): p.start() -def start(port=8080): +def start(port=8080, daemon=True): """ Starts the server. @@ -82,6 +82,10 @@ def start(port=8080): if platform.system() != "Windows": start_background_workers() + # Write the new PID + with open(PID_FILE, 'w') as f: + f.write("%d\n%d" % (os.getpid(), port)) + run_server(port=port) diff --git a/kolibri/utils/system.py b/kolibri/utils/system.py index f5c4768886e..718387e1a5f 100644 --- a/kolibri/utils/system.py +++ b/kolibri/utils/system.py @@ -12,6 +12,9 @@ from __future__ import absolute_import, print_function, unicode_literals import os +import sys + +import six def _posix_pid_exists(pid): @@ -63,10 +66,72 @@ def _windows_kill_pid(pid): ctypes.windll.kernel32.CloseHandle(handle) # @UndefinedVariable +buffering = int(six.PY3) # No unbuffered text I/O on Python 3 (#20815). + + +def _posix_become_daemon(our_home_dir='.', out_log='/dev/null', + err_log='/dev/null', umask=0o022): + "Robustly turn into a UNIX daemon, running in our_home_dir." + # First fork + try: + if os.fork() > 0: + sys.exit(0) # kill off parent + except OSError as e: + sys.stderr.write("fork #1 failed: (%d) %s\n" % (e.errno, e.strerror)) + sys.exit(1) + os.setsid() + os.chdir(our_home_dir) + os.umask(umask) + + # Second fork + try: + if os.fork() > 0: + os._exit(0) + except OSError as e: + sys.stderr.write("fork #2 failed: (%d) %s\n" % (e.errno, e.strerror)) + os._exit(1) + + si = open('/dev/null', 'r') + so = open(out_log, 'a+', buffering) + se = open(err_log, 'a+', buffering) + os.dup2(si.fileno(), sys.stdin.fileno()) + os.dup2(so.fileno(), sys.stdout.fileno()) + os.dup2(se.fileno(), sys.stderr.fileno()) + # Set custom file descriptors so that they get proper buffering. + sys.stdout, sys.stderr = so, se + + +def _windows_become_daemon(our_home_dir='.', out_log=None, err_log=None, umask=0o022): + """ + If we're not running under a POSIX system, just simulate the daemon + mode by doing redirections and directory changing. + """ + os.chdir(our_home_dir) + os.umask(umask) + sys.stdin.close() + sys.stdout.close() + sys.stderr.close() + if err_log: + sys.stderr = open(err_log, 'a', buffering) + else: + sys.stderr = _WindowsNullDevice() + if out_log: + sys.stdout = open(out_log, 'a', buffering) + else: + sys.stdout = _WindowsNullDevice() + +class _WindowsNullDevice: + "A writeable object that writes to nowhere -- like /dev/null." + def write(self, s): + pass + + # Utility functions for pinging or killing PIDs if os.name == 'posix': pid_exists = _posix_pid_exists kill_pid = _posix_kill_pid + become_daemon = _posix_become_daemon else: pid_exists = _windows_pid_exists kill_pid = _windows_kill_pid + become_daemon = _windows_become_daemon From 9fbaf181a7ac50db668be53c83a580d40e9027ff Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 6 Jun 2017 22:54:39 +0200 Subject: [PATCH 04/11] Remove PID file at exit, fix typo and rm unused kwarg from server.start --- kolibri/utils/cli.py | 2 +- kolibri/utils/server.py | 9 +++++++-- kolibri/utils/system.py | 3 +++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/kolibri/utils/cli.py b/kolibri/utils/cli.py index 5f747dbbd96..f3251f8d533 100644 --- a/kolibri/utils/cli.py +++ b/kolibri/utils/cli.py @@ -282,7 +282,7 @@ def status(): server.STATUS_SERVER_CONFIGURATION_ERROR: 'KA Lite server configuration error', server.STATUS_PID_FILE_READ_ERROR: 'Could not read PID file', server.STATUS_PID_FILE_INVALID: 'Invalid PID file', - server.STATUS_UNKNOW: 'Could not determine status', + server.STATUS_UNKNOWN: 'Could not determine status', } diff --git a/kolibri/utils/server.py b/kolibri/utils/server.py index 4c165f8861a..89b77521a3e 100644 --- a/kolibri/utils/server.py +++ b/kolibri/utils/server.py @@ -22,7 +22,7 @@ STATUS_SERVER_CONFIGURATION_ERROR = 9 STATUS_PID_FILE_READ_ERROR = 99 STATUS_PID_FILE_INVALID = 100 -STATUS_UNKNOW = 101 +STATUS_UNKNOWN = 101 # Used to store PID and port number (both in foreground and daemon mode) PID_FILE = os.path.join( @@ -70,7 +70,7 @@ def start_background_workers(): p.start() -def start(port=8080, daemon=True): +def start(port=8080): """ Starts the server. @@ -86,6 +86,11 @@ def start(port=8080, daemon=True): with open(PID_FILE, 'w') as f: f.write("%d\n%d" % (os.getpid(), port)) + def rm_pid_file(): + os.unlink(PID_FILE) + + atexit.register(rm_pid_file) + run_server(port=port) diff --git a/kolibri/utils/system.py b/kolibri/utils/system.py index 718387e1a5f..615c2c3ccdd 100644 --- a/kolibri/utils/system.py +++ b/kolibri/utils/system.py @@ -1,6 +1,9 @@ """ Utilities for local system calls, everything here is cross-platform. +become_daemon was originally taken from Django: +https://github.com/django/django/commit/5836a5771f2aefca83349b111f4191d6485af1d5#diff-f7d80be2ccf77f4f009d08dcac4b7736 + We might want to refactor this into: system/__init__.py From 668de3e89340bba2d09de0927de61c928604b3bf Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 6 Jun 2017 22:59:55 +0200 Subject: [PATCH 05/11] Release note for #1598 --- CHANGELOG.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5f63f65d652..9e1564bfc97 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,7 +7,10 @@ Changes are ordered reverse-chronologically. 0.5 --- + - Update all user logging related timestamps to a custom datetime field that includes timezone info + - Added daemon mode (system service) to run ``kolibri start`` in background (default!) #1548 + - Implemented ``kolibri stop`` and ``kolibri status`` #1548 0.4 From 8a394f234c9eb5efba9be4cf727223e02084ac4e Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 6 Jun 2017 23:47:48 +0200 Subject: [PATCH 06/11] Implementing the stop command --- kolibri/utils/cli.py | 48 ++++++++++++++++++++++++++++++++++++++--- kolibri/utils/server.py | 32 ++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 4 deletions(-) diff --git a/kolibri/utils/cli.py b/kolibri/utils/cli.py index f3251f8d533..3145d38077e 100644 --- a/kolibri/utils/cli.py +++ b/kolibri/utils/cli.py @@ -215,9 +215,9 @@ def start(port=8080, daemon=True): """ if not daemon: - logger.info("Running 'kolibri start' in foreground...\n") + logger.info("Running 'kolibri start' in foreground...") else: - logger.info("Running 'kolibri start' as daemon (system service)\n") + logger.info("Running 'kolibri start' as daemon (system service)") # TODO: moved from server.start() but not sure where it should ideally be # located. Question is if it should be run every time the server is started @@ -232,7 +232,7 @@ def start(port=8080, daemon=True): # Truncate the file open(server.DAEMON_LOG, "w").truncate() logger.info( - "Going to daemon mode, logging to {0}\n".format(server.DAEMON_LOG) + "Going to daemon mode, logging to {0}".format(server.DAEMON_LOG) ) kwargs['out_log'] = server.DAEMON_LOG kwargs['err_log'] = server.DAEMON_LOG @@ -241,6 +241,44 @@ def start(port=8080, daemon=True): server.start(port=port) +def stop(): + """ + Stops the server unless it isn't running + """ + try: + pid, __, __ = server.get_status() + server.stop(pid=pid) + stopped = True + except server.NotRunning as e: + verbose_status = "{msg:s} ({code:d})".format( + code=e.status_code, + msg=status.codes[e.status_code] + ) + if e.status_code == server.STATUS_STOPPED: + logger.info("Already stopped: {}".format(verbose_status)) + stopped = True + elif e.status_code == server.STATUS_STARTING_UP: + logger.error( + "Not stopped: {}".format(verbose_status) + ) + sys.exit(e.status_code) + else: + logger.error( + "During graceful shutdown, server says: {}".format( + verbose_status + ) + ) + logger.error( + "Not responding, killing with force" + ) + server.stop(force=True) + stopped = True + + if stopped: + logger.info("Server stopped") + sys.exit(0) + + def status(): """ Check the server's status. For possible statuses, see the status dictionary @@ -462,6 +500,10 @@ def main(args=None): start(port) return + if arguments['stop']: + stop() + return + if arguments['status']: status_code = status() sys.exit(status_code) diff --git a/kolibri/utils/server.py b/kolibri/utils/server.py index 89b77521a3e..bed922bb794 100644 --- a/kolibri/utils/server.py +++ b/kolibri/utils/server.py @@ -1,4 +1,5 @@ import atexit +import logging import multiprocessing import os import platform @@ -9,7 +10,9 @@ from django.core.management import call_command from kolibri.content.utils import paths -from .system import pid_exists +from .system import kill_pid, pid_exists + +logger = logging.getLogger(__name__) # Status codes for kolibri STATUS_RUNNING = 0 @@ -94,6 +97,33 @@ def rm_pid_file(): run_server(port=port) +def stop(pid=None, force=False): + """ + Stops the kalite server, either from PID or through a management command + + :param args: List of options to parse to the django management command + :raises: NotRunning + """ + + if not force: + # Kill the KA lite server + kill_pid(pid) + else: + try: + pid, __ = _read_pid_file(PID_FILE) + kill_pid(pid) + except ValueError: + logger.error("Could not find PID in .pid file\n") + except OSError: + logger.error("Could not read .pid file\n") + + # TODO: Check that server has in fact been killed, otherwise we should + # raise an error... + + # Finally, remove the PID file + os.unlink(PID_FILE) + + def run_server(port): # Mount the application From 2a0749aeca611a1d8d835fe5c438e6fdcd548064 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 6 Jun 2017 23:51:53 +0200 Subject: [PATCH 07/11] Use --foreground flag, remove --watch flag from usage as it has no effect --- kolibri/utils/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kolibri/utils/cli.py b/kolibri/utils/cli.py index 3145d38077e..0317d9483d5 100644 --- a/kolibri/utils/cli.py +++ b/kolibri/utils/cli.py @@ -49,7 +49,7 @@ www.learningequality.org Usage: - kolibri start [--foreground --watch] [--port=] [options] + kolibri start [--foreground] [--port=] [options] kolibri stop [options] kolibri restart [options] kolibri status [options] @@ -497,7 +497,7 @@ def main(args=None): if arguments['start']: port = int(arguments['--port'] or 8080) - start(port) + start(port, daemon=not arguments['--foreground']) return if arguments['stop']: From 33c0a402424ec46f9ec26f089f0e1f6e76d9fb47 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 6 Jun 2017 23:56:24 +0200 Subject: [PATCH 08/11] An attempt of adding coverage to commands run directly --- .travis.yml | 1 + tox.ini | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index a0c3e26da01..07457411cac 100644 --- a/.travis.yml +++ b/.travis.yml @@ -86,6 +86,7 @@ script: - tox -e $TOX_ENV after_success: + - coverage combine - codecov notifications: diff --git a/tox.ini b/tox.ini index 15b22574e85..f6fee5633ce 100644 --- a/tox.ini +++ b/tox.ini @@ -26,8 +26,11 @@ deps = -r{toxinidir}/requirements/test.txt commands = # Enable the plugins to ensure the configuration is read without error - kolibri plugin kolibri.plugins.learn enable - kolibri plugin kolibri.plugins.management enable + coverage run -p kolibri plugin kolibri.plugins.learn enable + coverage run -p kolibri plugin kolibri.plugins.management enable + coverage run -p kolibri start + coverage run -p kolibri stop + coverage run -p kolibri status py.test {posargs:--cov=kolibri --color=no} rm -r {env:KOLIBRI_HOME} From 779b9223a0e57a00964fa73ce3e164ececfbf4cb Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Wed, 7 Jun 2017 00:18:13 +0200 Subject: [PATCH 09/11] Use migrations because `kolibri start` was added to tox matrix --- kolibri/deployment/default/settings/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kolibri/deployment/default/settings/test.py b/kolibri/deployment/default/settings/test.py index 84d534e507f..541d5164f49 100644 --- a/kolibri/deployment/default/settings/test.py +++ b/kolibri/deployment/default/settings/test.py @@ -2,4 +2,4 @@ from .base import * # noqa -KOLIBRI_SKIP_AUTO_DATABASE_MIGRATION = True +KOLIBRI_SKIP_AUTO_DATABASE_MIGRATION = False From 902acdbd101166a4a2c0a602c8b6f7fcda445e3b Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Wed, 7 Jun 2017 00:28:25 +0200 Subject: [PATCH 10/11] Remove the status command for now to deal with non-zero exit code --- tox.ini | 1 - 1 file changed, 1 deletion(-) diff --git a/tox.ini b/tox.ini index f6fee5633ce..318a13a1da3 100644 --- a/tox.ini +++ b/tox.ini @@ -30,7 +30,6 @@ commands = coverage run -p kolibri plugin kolibri.plugins.management enable coverage run -p kolibri start coverage run -p kolibri stop - coverage run -p kolibri status py.test {posargs:--cov=kolibri --color=no} rm -r {env:KOLIBRI_HOME} From 34697f6c8e5799e1620514aefb802a01fb75fa53 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Wed, 7 Jun 2017 01:22:31 +0200 Subject: [PATCH 11/11] Do not invoke kolibri commands in postgres tox env --- tox.ini | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tox.ini b/tox.ini index 318a13a1da3..ac2085e3193 100644 --- a/tox.ini +++ b/tox.ini @@ -43,6 +43,9 @@ basepython = deps = -r{toxinidir}/requirements/test.txt -r{toxinidir}/requirements/postgres.txt +commands = + py.test {posargs:--cov=kolibri --color=no} + rm -r {env:KOLIBRI_HOME} [testenv:pythonlint] deps =