Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support KubeSpawner profile_options #2937

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Adam-D-Lewis
Copy link
Member

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

Reference Issues or PRs

Kubespawner has support to configure various docker images (or other options) per profile. See profile_options in their docs. We didn't support it in our Pydantic models so this adds support. I tested it and it worked on my local deployment.

I made a companion docs PR as well

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):

Testing

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

How to test this PR?

Add this section to a nebari config.

profiles:
  jupyterlab:
  - display_name: Small Instance
    description: Stable environment with 2 cpu / 4 GB RAM
    default: true
    profile_options: &profile_options
      image:
        display_name: Image
        choices:
          base:
            display_name: jupyter/base-notebook:latest
            kubespawner_override:
              image: jupyter/base-notebook:latest
          minimal:
            display_name: quay.io/nebari/nebari-jupyterlab:latest
            default: true
            kubespawner_override:
              image: quay.io/nebari/nebari-jupyterlab:latest
    kubespawner_override:
      cpu_limit: 2
      cpu_guarantee: 1.5
      mem_limit: 4G
      mem_guarantee: 3G

  - display_name: Medium Instance
    description: Stable environment with 4 cpu / 8 GB RAM
    profile_options: *profile_options
    kubespawner_override:
      cpu_limit: 4
      cpu_guarantee: 3
      mem_limit: 8G
      mem_guarantee: 6G

then ensure the profile selection page looks something like
image

It works whether or not jhub_apps is enabled.

Any other comments?

@Adam-D-Lewis
Copy link
Member Author

Failing "Local Integration Tests" test unrelated to this PR

@dcmcand dcmcand requested a review from a team as a code owner February 7, 2025 14:24
@dcmcand dcmcand requested review from dcmcand and viniciusdc and removed request for a team February 7, 2025 14:24
Copy link
Member

@aktech aktech left a comment

Choose a reason for hiding this comment

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

This looks good. Verified it works fine with or without jhub-apps enabled. I would be nice to add a cli validation test, by adding a working config here.

Screenshot 2025-02-11 at 6 47 35 pm

For posterity:

Although one thing to note is, its not fully compatible with jhub-apps as in when you deploy app you can't chose different docker images like you can when you create a jupyterlab.

Screenshot 2025-02-11 at 6 51 13 pm

To make it fully compatible with jhub-apps server options we would need to change the UI in jhub-apps and use the newly added profile options, but since that's beyond the scope of this PR, its not a blocker.

@viniciusdc
Copy link
Contributor

viniciusdc commented Feb 13, 2025

@Adam-D-Lewis in your docs PR, can you add this as a note:

To make it fully compatible with jhub-apps server options we would need to change the UI in jhub-apps and use the newly added profile options, but since that's beyond the scope of this PR, its not a blocker.

Copy link
Contributor

@viniciusdc viniciusdc left a comment

Choose a reason for hiding this comment

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

LGMT, this will also make the notion of "profiles" and the jupyterlab docker images more clear to new users

@viniciusdc
Copy link
Contributor

@Adam-D-Lewis I included a label to not merge the PR as a freeze in new additions to Nebari until the current release is out. As soon as its out, we can merge this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New 🚦
Development

Successfully merging this pull request may close these issues.

4 participants