diff --git a/jupyterhub/apihandlers/users.py b/jupyterhub/apihandlers/users.py index f5ff0561..0ea6739f 100644 --- a/jupyterhub/apihandlers/users.py +++ b/jupyterhub/apihandlers/users.py @@ -242,11 +242,7 @@ class UserAPIHandler(APIHandler): await maybe_future(self.authenticator.delete_user(user)) - # allow the spawner to cleanup any persistent resources associated with the user - try: - await user.spawner.delete_forever() - except Exception as e: - self.log.error("Error cleaning up persistent resources: %s" % e) + await user.delete_spawners() # remove from registry self.users.delete(user) @@ -488,10 +484,18 @@ class UserServerAPIHandler(APIHandler): options = self.get_json_body() remove = (options or {}).get('remove', False) - def _remove_spawner(f=None): - if f and f.exception(): - return + async def _remove_spawner(f=None): + """Remove the spawner object + + only called after it stops successfully + """ + if f: + # await f, stop on error, + # leaving resources in the db in case of failure to stop + await f self.log.info("Deleting spawner %s", spawner._log_name) + await maybe_future(user._delete_spawner(spawner)) + self.db.delete(spawner.orm_spawner) user.spawners.pop(server_name, None) self.db.commit() @@ -512,7 +516,8 @@ class UserServerAPIHandler(APIHandler): self.set_header('Content-Type', 'text/plain') self.set_status(202) if remove: - spawner._stop_future.add_done_callback(_remove_spawner) + # schedule remove when stop completes + asyncio.ensure_future(_remove_spawner(spawner._stop_future)) return if spawner.pending: @@ -530,9 +535,10 @@ class UserServerAPIHandler(APIHandler): if remove: if stop_future: - stop_future.add_done_callback(_remove_spawner) + # schedule remove when stop completes + asyncio.ensure_future(_remove_spawner(spawner._stop_future)) else: - _remove_spawner() + await _remove_spawner() status = 202 if spawner._stop_pending else 204 self.set_header('Content-Type', 'text/plain') diff --git a/jupyterhub/spawner.py b/jupyterhub/spawner.py index ead537d6..c296ff86 100644 --- a/jupyterhub/spawner.py +++ b/jupyterhub/spawner.py @@ -1141,6 +1141,18 @@ class Spawner(LoggingConfigurable): """ raise NotImplementedError("Override in subclass. Must be a coroutine.") + def delete_forever(self): + """Called when a user or server is deleted. + + This can do things like request removal of resources such as persistent storage. + Only called on stopped spawners, and is usually the last action ever taken for the user. + + Will only be called once on each Spawner, immediately prior to removal. + + Stopping a server does *not* call this method. + """ + pass + def add_poll_callback(self, callback, *args, **kwargs): """Add a callback to fire when the single-user server stops""" if args or kwargs: diff --git a/jupyterhub/tests/mocking.py b/jupyterhub/tests/mocking.py index 145ef2a9..5384ae11 100644 --- a/jupyterhub/tests/mocking.py +++ b/jupyterhub/tests/mocking.py @@ -93,16 +93,6 @@ class MockSpawner(SimpleLocalProcessSpawner): def _cmd_default(self): return [sys.executable, '-m', 'jupyterhub.tests.mocksu'] - async def delete_forever(self): - """Called when a user is deleted. - - This can do things like request removal of resources such as persistent storage. - Only called on stopped spawners, and is likely the last action ever taken for the user. - - Will only be called once on the user's default Spawner. - """ - pass - use_this_api_token = None def start(self): diff --git a/jupyterhub/user.py b/jupyterhub/user.py index e75d6b5c..819c630b 100644 --- a/jupyterhub/user.py +++ b/jupyterhub/user.py @@ -252,6 +252,35 @@ class User: await self.save_auth_state(auth_state) return auth_state + async def delete_spawners(self): + """Call spawner cleanup methods + + Allows the spawner to cleanup persistent resources + """ + for name in self.orm_user.orm_spawners.keys(): + await self._delete_spawner(name) + + async def _delete_spawner(self, name_or_spawner): + """Delete a single spawner""" + # always ensure full Spawner + # this may instantiate the Spawner if it wasn't already running, + # just to delete it + if isinstance(name_or_spawner, str): + spawner = self.spawners[name_or_spawner] + else: + spawner = name_or_spawner + + if spawner.active: + raise RuntimeError( + f"Spawner {spawner._log_name} is active and cannot be deleted." + ) + try: + await maybe_future(spawner.delete_forever()) + except Exception as e: + self.log.exception( + f"Error cleaning up persistent resources on {spawner._log_name}" + ) + def all_spawners(self, include_default=True): """Generator yielding all my spawners