Skip to content

Commit

Permalink
Update zipcontent port handling to allow and report dynamic port allo…
Browse files Browse the repository at this point in the history
…cation.
  • Loading branch information
rtibbles committed Apr 29, 2021
1 parent 11e52b0 commit f7d2eb8
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 31 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/no_zombies.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,5 +42,5 @@ jobs:
pip install .
# ensure kolibri stops within 20 seconds 10 times in a row
./test/ensure_kolibri_stops_within_time.sh 20 10 8082
./test/ensure_kolibri_stops_within_time.sh 20 10 8082 8083
./test/ensure_no_kolibris_running_on_port.sh 8082
15 changes: 15 additions & 0 deletions kolibri/core/content/utils/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from kolibri.core.content.errors import InvalidStorageFilenameError
from kolibri.utils import conf
from kolibri.utils.server import get_zip_port


# valid storage filenames consist of 32-char hex plus a file extension
Expand Down Expand Up @@ -253,6 +254,20 @@ def get_file_checksums_url(channel_id, baseurl, version="1"):
ZIPCONTENT = "zipcontent/"


def get_zip_content_config():
zip_content_origin = conf.OPTIONS["Deployment"]["ZIP_CONTENT_ORIGIN"]
if not zip_content_origin:
zip_content_port = str(
get_zip_port() or conf.OPTIONS["Deployment"]["ZIP_CONTENT_PORT"]
)
elif type(zip_content_origin) is int:
zip_content_port = str(zip_content_origin)
zip_content_origin = ""
else:
zip_content_port = ""
return zip_content_origin, zip_content_port


def zip_content_path_prefix():
path_prefix = conf.OPTIONS["Deployment"]["ZIP_CONTENT_URL_PATH_PREFIX"]

Expand Down
10 changes: 2 additions & 8 deletions kolibri/core/kolibri_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from kolibri.core.content.utils.paths import get_content_storage_url
from kolibri.core.content.utils.paths import get_hashi_path
from kolibri.core.content.utils.paths import get_zip_content_base_path
from kolibri.core.content.utils.paths import get_zip_content_config
from kolibri.core.device.models import ContentCacheKey
from kolibri.core.device.utils import allow_other_browsers_to_connect
from kolibri.core.hooks import NavigationHook
Expand Down Expand Up @@ -53,14 +54,7 @@ def url_tag(self):
# For some reason the js_name gets escaped going into the template
# so this was the easiest way to inject it.
).replace("__placeholder__", js_name)
zip_content_origin = OPTIONS["Deployment"]["ZIP_CONTENT_ORIGIN"]
if not zip_content_origin:
zip_content_port = str(OPTIONS["Deployment"]["ZIP_CONTENT_PORT"])
elif type(zip_content_origin) is int:
zip_content_port = str(zip_content_origin)
zip_content_origin = ""
else:
zip_content_port = ""
zip_content_origin, zip_content_port = get_zip_content_config()
return [
mark_safe(
"""<script type="text/javascript">"""
Expand Down
17 changes: 13 additions & 4 deletions kolibri/utils/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -454,17 +454,26 @@ def main(ctx):
type=int,
help="Port on which Kolibri is being served",
)
@click.option(
"--zip-port",
default=OPTIONS["Deployment"]["ZIP_CONTENT_PORT"],
type=int,
help="Port on which zip content server is being served",
)
@click.option(
"--background/--foreground",
default=True,
help="Run Kolibri as a background process",
)
def start(port, background):
def start(port, zip_port, background):
"""
Start the server on given port.
"""
server.start(
port=port, serve_http=OPTIONS["Server"]["CHERRYPY_START"], background=background
port=port,
zip_port=zip_port,
serve_http=OPTIONS["Server"]["CHERRYPY_START"],
background=background,
)


Expand Down Expand Up @@ -530,7 +539,7 @@ def status():
"--port",
default=OPTIONS["Deployment"]["HTTP_PORT"],
type=int,
help="Port on which to run Kolibri services",
help="Port on which Kolibri is running to inform services",
)
@click.option(
"--background/--foreground",
Expand All @@ -544,7 +553,7 @@ def services(port, background):

logger.info("Starting Kolibri background services")

server.start(port=port, serve_http=False, background=background)
server.start(port=port, zip_port=None, serve_http=False, background=background)


def setup_logging(debug=False, debug_database=False):
Expand Down
4 changes: 2 additions & 2 deletions kolibri/utils/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def path_list(value):


def validate_port_number(value):
if 1 <= value <= 65535:
if 0 <= value <= 65535:
return value
raise VdtValueError(value)

Expand Down Expand Up @@ -418,7 +418,7 @@ def origin_or_port(value):
},
"ZIP_CONTENT_PORT": {
"type": "port",
"default": 8765,
"default": 0,
"envvars": ("KOLIBRI_ZIP_CONTENT_PORT",),
},
"ZIP_CONTENT_URL_PATH_PREFIX": {
Expand Down
45 changes: 30 additions & 15 deletions kolibri/utils/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,15 @@ def STOP(self):


class PIDPlugin(SimplePlugin):
def __init__(self, bus, port, pid_file=PID_FILE):
def __init__(self, bus, port, zip_port, pid_file=PID_FILE):
self.bus = bus
self.port = port
self.zip_port = zip_port
self.pid_file = pid_file
# Do this during initialization to set a startup lock
self.set_pid_file(STATUS_STARTING_UP)
self.bus.subscribe("SERVING", self.SERVING)
self.bus.subscribe("ZIP_SERVING", self.ZIP_SERVING)
for bus_status, status in status_map.items():
handler = partial(self.set_pid_file, status)
handler.priority = 10
Expand All @@ -225,10 +227,17 @@ def set_pid_file(self, status):
:param: status: status of the process
"""
with open(self.pid_file, "w") as f:
f.write("{}\n{}\n{}\n".format(os.getpid(), self.port, status))
f.write(
"{}\n{}\n{}\n{}\n".format(os.getpid(), self.port, self.zip_port, status)
)

def SERVING(self, port):
self.port = port or self.port
self.set_pid_file(STATUS_RUNNING)

def ZIP_SERVING(self, zip_port):
self.zip_port = zip_port or self.zip_port
self.set_pid_file(STATUS_RUNNING)

def EXIT(self):
try:
Expand Down Expand Up @@ -304,7 +313,7 @@ def stop():
Stops the kolibri server
:raises: NotRunning
"""
pid, __, status = _read_pid_file(PID_FILE)
pid, __, __, status = _read_pid_file(PID_FILE)

if not pid:
return status
Expand Down Expand Up @@ -335,7 +344,7 @@ def stop():
return STATUS_STOPPED


def configure_http_server(port, bus):
def configure_http_server(port, zip_port, bus):
from kolibri.deployment.default.wsgi import application
from kolibri.deployment.default.alt_wsgi import alt_application

Expand All @@ -357,7 +366,7 @@ def configure_http_server(port, bus):

alt_port_addr = (
LISTEN_ADDRESS,
conf.OPTIONS["Deployment"]["ZIP_CONTENT_PORT"],
zip_port,
)

alt_port_server = ServerPlugin(
Expand Down Expand Up @@ -390,11 +399,11 @@ def check_port_availability(host, port):
sys.exit(1)


def start(port=8080, serve_http=True, background=False):
def start(port=0, zip_port=0, serve_http=True, background=False):
"""
Starts the server.
:param: port: Port number (default: 8080)
:param: port: Port number (default: 0) - assigned by free port
"""
# On Mac, Python crashes when forking the process, so prevent daemonization until we can figure out
# a better fix. See https://github.com/learningequality/kolibri/issues/4821
Expand All @@ -403,7 +412,7 @@ def start(port=8080, serve_http=True, background=False):

# Check if there are other kolibri instances running
# If there are, then we need to stop users from starting kolibri again.
pid, port, status = _read_pid_file(PID_FILE)
_, _, _, status = _read_pid_file(PID_FILE)

if status in IS_RUNNING:
logger.error(
Expand All @@ -417,7 +426,7 @@ def start(port=8080, serve_http=True, background=False):
# Setup plugin for handling PID file cleanup
# Do this first to obtain a PID file lock as soon as
# possible and reduce the risk of competing servers
pid_plugin = PIDPlugin(bus, port)
pid_plugin = PIDPlugin(bus, port, zip_port)
pid_plugin.subscribe()

if background and serve_http:
Expand All @@ -444,7 +453,7 @@ def start(port=8080, serve_http=True, background=False):
log_plugin.subscribe()

if serve_http:
configure_http_server(port, bus)
configure_http_server(port, zip_port, bus)

# Setup plugin for services
service_plugin = ServicesPlugin(bus, port)
Expand Down Expand Up @@ -478,18 +487,24 @@ def _read_pid_file(filename):
port is None.
"""
if not os.path.isfile(PID_FILE):
return None, None, STATUS_STOPPED
return None, None, None, STATUS_STOPPED

try:
pid_file_lines = open(filename, "r").readlines()
pid, port, status = pid_file_lines
pid, port, zip_port, status = pid_file_lines
pid = int(pid.strip())
port = int(port.strip()) if port.strip() else None
zip_port = int(zip_port.strip()) if zip_port.strip() else None
status = int(status.strip())
return pid, port, status
return pid, port, zip_port, status
except (TypeError, ValueError, IOError, OSError):
pass
return None, None, STATUS_PID_FILE_INVALID
return None, None, None, STATUS_PID_FILE_INVALID


def get_zip_port():
_, _, zip_port, _ = _read_pid_file(PID_FILE)
return zip_port


def get_status(): # noqa: max-complexity=16
Expand All @@ -506,7 +521,7 @@ def get_status(): # noqa: max-complexity=16
:raises: NotRunning
"""
# PID file exists and startup has finished, check if it is running
pid, port, status = _read_pid_file(PID_FILE)
pid, port, _, status = _read_pid_file(PID_FILE)

if status not in IS_RUNNING:
raise NotRunning(status)
Expand Down
2 changes: 1 addition & 1 deletion test/ensure_kolibri_stops_within_time.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
MAX=0
for i in $(seq 1 $2)
do
kolibri start --port=$3 > /dev/null 2>&1
kolibri start --port=$3 --zip-port=$4 > /dev/null 2>&1
START_TIME=$SECONDS
kolibri stop > /dev/null 2>&1
ELAPSED_TIME=$(($SECONDS - $START_TIME))
Expand Down

0 comments on commit f7d2eb8

Please sign in to comment.