diff --git a/jupyterhub/app.py b/jupyterhub/app.py index c0ead36d..41d9c3c8 100644 --- a/jupyterhub/app.py +++ b/jupyterhub/app.py @@ -1219,7 +1219,7 @@ class JupyterHub(Application): status = yield spawner.poll() except Exception: self.log.exception("Failed to poll spawner for %s, assuming the spawner is not running.", - user.name if name else '%s|%s' % (user.name, name)) + spawner._log_name) status = -1 if status is None: @@ -1230,11 +1230,13 @@ class JupyterHub(Application): # user not running. This is expected if server is None, # but indicates the user's server died while the Hub wasn't running # if spawner.server is defined. - log = self.log.warning if spawner.server else self.log.debug - log("%s not running.", user.name) - # remove all server or servers entry from db related to the user if spawner.server: + self.log.warning("%s appears to have stopped while the Hub was down", spawner._log_name) + # remove server entry from db db.delete(spawner.orm_spawner.server) + spawner.server = None + else: + self.log.debug("%s not running", spawner._log_name) db.commit() user_summaries.append(_user_summary(user)) diff --git a/jupyterhub/orm.py b/jupyterhub/orm.py index 92a74b43..ef9482f4 100644 --- a/jupyterhub/orm.py +++ b/jupyterhub/orm.py @@ -177,7 +177,7 @@ class Spawner(Base): id = Column(Integer, primary_key=True, autoincrement=True) user_id = Column(Integer, ForeignKey('users.id', ondelete='CASCADE')) - server_id = Column(Integer, ForeignKey('servers.id')) + server_id = Column(Integer, ForeignKey('servers.id', ondelete='SET NULL')) server = relationship(Server) state = Column(JSONDict) diff --git a/jupyterhub/tests/test_app.py b/jupyterhub/tests/test_app.py index 2f9af395..d23c6b22 100644 --- a/jupyterhub/tests/test_app.py +++ b/jupyterhub/tests/test_app.py @@ -8,9 +8,11 @@ from subprocess import check_output, Popen, PIPE from tempfile import NamedTemporaryFile, TemporaryDirectory from unittest.mock import patch +from tornado import gen import pytest from .mocking import MockHub +from .test_api import add_user from .. import orm from ..app import COOKIE_SECRET_BYTES @@ -161,3 +163,57 @@ def test_load_groups(): assert gold is not None assert sorted([ u.name for u in gold.users ]) == sorted(to_load['gold']) + +@pytest.mark.gen_test +def test_resume_spawners(tmpdir, request): + if not os.getenv('JUPYTERHUB_TEST_DB_URL'): + p = patch.dict(os.environ, { + 'JUPYTERHUB_TEST_DB_URL': 'sqlite:///%s' % tmpdir.join('jupyterhub.sqlite'), + }) + p.start() + request.addfinalizer(p.stop) + @gen.coroutine + def new_hub(): + app = MockHub() + app.config.ConfigurableHTTPProxy.should_start = False + yield app.initialize([]) + return app + app = yield new_hub() + db = app.db + # spawn a user's server + name = 'kurt' + user = add_user(db, app, name=name) + yield user.spawn() + proc = user.spawner.proc + assert proc is not None + + # stop the Hub without cleaning up servers + app.cleanup_servers = False + yield app.stop() + + # proc is still running + assert proc.poll() is None + + # resume Hub, should still be running + app = yield new_hub() + db = app.db + user = app.users[name] + assert user.running + assert user.spawner.server is not None + + # stop the Hub without cleaning up servers + app.cleanup_servers = False + yield app.stop() + + # stop the server while the Hub is down. BAMF! + proc.terminate() + proc.wait(timeout=10) + assert proc.poll() is not None + + # resume Hub, should be stopped + app = yield new_hub() + db = app.db + user = app.users[name] + assert not user.running + assert user.spawner.server is None + assert list(db.query(orm.Server)) == [] diff --git a/jupyterhub/tests/test_db.py b/jupyterhub/tests/test_db.py index 66161df8..09dd4c84 100644 --- a/jupyterhub/tests/test_db.py +++ b/jupyterhub/tests/test_db.py @@ -46,4 +46,3 @@ def test_upgrade_entrypoint(tmpdir): # run tokenapp again, it should work tokenapp.start() - \ No newline at end of file diff --git a/jupyterhub/tests/test_spawner.py b/jupyterhub/tests/test_spawner.py index a7f42005..c0057dc6 100644 --- a/jupyterhub/tests/test_spawner.py +++ b/jupyterhub/tests/test_spawner.py @@ -299,7 +299,7 @@ def test_spawner_reuse_api_token(db, app): @pytest.mark.gen_test -def test_spawner_insert_api_token(db, app): +def test_spawner_insert_api_token(app): """Token provided by spawner is not in the db Insert token into db as a user-provided token. @@ -326,7 +326,7 @@ def test_spawner_insert_api_token(db, app): @pytest.mark.gen_test -def test_spawner_bad_api_token(db, app): +def test_spawner_bad_api_token(app): """Tokens are revoked when a Spawner gets another user's token""" # we need two users for this one user = add_user(app.db, app, name='antimone') @@ -346,3 +346,37 @@ def test_spawner_bad_api_token(db, app): yield user.spawn() assert orm.APIToken.find(app.db, other_token) is None assert other_user.api_tokens == [] + + +@pytest.mark.gen_test +def test_spawner_delete_server(app): + """Test deleting spawner.server + + This can occur during app startup if their server has been deleted. + """ + db = app.db + user = add_user(app.db, app, name='gaston') + spawner = user.spawner + orm_server = orm.Server() + db.add(orm_server) + db.commit() + server_id = orm_server.id + spawner.server = Server.from_orm(orm_server) + db.commit() + + assert spawner.server is not None + assert spawner.orm_spawner.server is not None + + # trigger delete via db + db.delete(spawner.orm_spawner.server) + db.commit() + assert spawner.orm_spawner.server is None + + # setting server = None also triggers delete + spawner.server = None + db.commit() + # verify that the server was actually deleted from the db + assert db.query(orm.Server).filter(orm.Server.id == server_id).first() is None + # verify that both ORM and top-level references are None + assert spawner.orm_spawner.server is None + assert spawner.server is None