jupyterhub / kubespawner

Kubernetes spawner for JupyterHub
https://jupyterhub-kubespawner.readthedocs.io
BSD 3-Clause "New" or "Revised" License
531 stars 299 forks source link

Wierdness in ordering of pre_spawn_hook and load_user_options #339

Open rkdarst opened 4 years ago

rkdarst commented 4 years ago

I was just upgrading JH/kubespawner, and noticed that PR #301 caused a problem and possibly there's a bug in how it interacts with JupyterHub. #301 handles loading the user options the correct way (in a load_user_options function instead of options_from_form). Thanks for #301, by the way, that allows us to do better CI.

JH runs pre_spawn_hook first, then .start(), but .start() applies the profile options. So with the latest kubespawner the hook can't use the kubespawner_override settings (and would have to handle the raw user_options first). And kubespawner_override will also override anything that is also set in pre_spawn_hook.

I have worked around it by adding this to the top of my pre_spawn_hook, which applies the profile options, then eliminates the profile list. An false-like but not None value for profile list makes kubespawner skip applying the profile list again.

async def pre_spawn_hook(spawner):
    await spawner.load_user_options()
    spawner._profile_list = [ ]

I'm not sure what the "right" answer is, since this seems almost intrinsic to how JH is set up with options being applied in start and hook being done first. My first idea is in the default pre_spawn_hook which needs to be super()ed, but I don't know.

consideRatio commented 3 years ago

@rkdarst could you provide an update on this issue? Is this relevant still etc, and do you have a concrete action plan for this issue at this point?

benjimin commented 7 months ago

I think documentation for all the hooks should be consolidated into a section that better explains the sequencing, and that the API should also consider providing a hook that runs immediately before the pod is created.

For anyone wanting both user-specific customisations and profile choices (for example, if they want to inject the username into an environment variable, but also want the profile to set another environment variable) then they probably want the user-customisation to occur after the profile choice has been applied but before the pod manifest is produced. Any earlier and the profile clobbers the customisation; any later and manipulating spawner attributes has no effect. The problem is that there is actually no such hook.

Probably the best approach (currently) is to use modify_pod_hook but discard the pod object and instead return spawner.get_pod_manifest() (after applying final customisations to the spawner attributes).

(This seems simpler than the alternative approach of using pre_spawn_hook to load_user_options() early, then apply further user customisations, and then trying to hack the spawner so that reapplying load_user_options will take no effect, yet without breaking how the spawner will behave if the user tries to launch another server, with a different selection of choices, immediately after stopping this one.)

Otherwise, for anyone wanting to use the hooks to make customisations, they will probably need to know: