Skip to content

Conversation

@Adam-D-Lewis
Copy link
Member

@Adam-D-Lewis Adam-D-Lewis commented Feb 5, 2025

Reference Issues or PRs

Necessary changes to support Kubespawner >= 5.0.0
Should be merged simultaneously with nebari-dev/nebari-docker-images#194

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe): Update dependencies

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

How to test this PR?

When using the quay.io/nebari/nebari-jupyterhub:jhub-fancy-profiles-df68a39-20250204 for my jupyterhub image I got errors when loading the profile_list of servers to start up, but this PR resolves them, and I can now start a server successfully with Kubespawner version 6.2.

Any other comments?

envvars_fixed = {**(profile["kubespawner_override"].get("environment", {}))}

def preserve_envvars(spawner):
# This adds in JUPYTERHUB_ANYONE/GROUP rather than overwrite all env vars,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior of Kubespawner changed in v5 so kubespawner_override's default behavior is merge instead of replace now so I'm now simplifing this part.
reference: https://jupyterhub-kubespawner.readthedocs.io/en/latest/changelog.html#id24

return profile


@gen.coroutine
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flyby: tornado coroutine to native coroutine

@Adam-D-Lewis
Copy link
Member Author

Failing tests should be able to be rerun. I'm seeing Github rate-limiter failed the request. Either authenticate or wait a couple of minutes. in failing test output.

@dcmcand dcmcand requested a review from a team as a code owner February 7, 2025 14:18
@Adam-D-Lewis
Copy link
Member Author

"Local Integration Tests" will fail until nebari-dev/nebari-docker-images#194 is merged

@Adam-D-Lewis
Copy link
Member Author

Adam-D-Lewis commented Feb 10, 2025

I tested this locally and Local Integration tests passed fine. I changed the jupyterhub image to the one created by the CI in nebari-dev/nebari-docker-images#194 in 5829038. We should revert 5829038 prior to merging, but it'll show reviewers that the tests pass first.

Comment on lines 78 to 83
# Change jupyterhub image for PR testing purposes
cat >> '${{ steps.metadata.outputs.config }}' <<- EOM
default_images:
jupyterhub: quay.io/nebari/nebari-jupyterhub:jhub-fancy-profiles
EOM
Copy link
Contributor

@viniciusdc viniciusdc Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Adam-D-Lewis , I think this was not suposed to be committed as part of this PR, is that right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NVM, just saw your comment above.

@marcelovilla marcelovilla added this to the 2025.3.1 release milestone Mar 4, 2025
Copy link
Member

@marcelovilla marcelovilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Adam-D-Lewis I'm testing this PR using the image you mentioned:

default_images:
  jupyterhub: quay.io/nebari/nebari-jupyterhub:jhub-fancy-profiles-df68a39-20250204

I'm seeing the following error:
image

Here are the logs from the hub pod:

[E 2025-03-04 13:45:22.366 JupyterHub web:1875] Uncaught exception GET /hub/spawn/mvilla (10.244.0.1)
    HTTPServerRequest(protocol='http', host='172.20.1.100', method='GET', 
                      uri='/hub/spawn/mvilla', version='HTTP/1.1', remote_ip='10.244.0.1')

Traceback (most recent call last):
  File "/opt/conda/lib/python3.9/site-packages/tornado/web.py", line 1790, in _execute
    result = await result
  File "/opt/conda/lib/python3.9/site-packages/jupyterhub/handlers/pages.py", line 206, in _get
    spawner_options_form = await spawner.get_options_form()
  File "/opt/conda/lib/python3.9/site-packages/jupyterhub/spawner.py", line 614, in get_options_form
    options_form = await maybe_future(self.options_form(self))
  File "/opt/conda/lib/python3.9/site-packages/kubespawner/spawner.py", line 2999, in _render_options_form_dynamically
    return self._render_options_form(profile_list)
  File "/opt/conda/lib/python3.9/site-packages/kubespawner/spawner.py", line 2964, in _render_options_form
    profile_list = self._get_initialized_profile_list(profile_list)
  File "/opt/conda/lib/python3.9/site-packages/kubespawner/spawner.py", line 3287, in _get_initialized_profile_list
    for option_config in profile.get('profile_options', {}).values():
AttributeError: 'NoneType' object has no attribute 'values'

Any ideas? Is there anything else I need to add to the config?

@Adam-D-Lewis
Copy link
Member Author

I reproduced what you're seeing. I'll look into it further.

@Adam-D-Lewis
Copy link
Member Author

Adam-D-Lewis commented Mar 4, 2025

So this was a bug introduced by my recently merged PR #2937. Fixed now with 4d2e89a

@Adam-D-Lewis
Copy link
Member Author

Adam-D-Lewis commented Mar 4, 2025

I'm also hoping to sneak an unrelated bugfix in with ffc77d6. In that commit, we want to write "\;", but python interprets that as an invalid escape sequence so I need to escape the backslash with "\\;".

Copy link
Member

@marcelovilla marcelovilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Adam-D-Lewis 🚀 ! This is working now.

image

I'd wait until we figure out what's going on with CI on the docker images repo to merge this PR.

@Adam-D-Lewis
Copy link
Member Author

Looks like the CI issue got figured out based on nebari-dev/nebari-docker-images@8328b3a, merging

@Adam-D-Lewis Adam-D-Lewis merged commit e3f88e9 into main Mar 4, 2025
24 of 25 checks passed
@Adam-D-Lewis Adam-D-Lewis deleted the update_kubespawner branch March 4, 2025 22:52
@github-project-automation github-project-automation bot moved this from New 🚦 to Done 💪🏾 in 🪴 Nebari Project Management Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done 💪🏾

Development

Successfully merging this pull request may close these issues.

5 participants