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

fixes 'create a notebook controller that can replace jupyterhub and uses k8 native auth' #1855

Merged
merged 62 commits into from Nov 20, 2018

Conversation

kkasravi
Copy link
Contributor

@kkasravi kkasravi commented Oct 25, 2018

fixes #1769

This PR will launch a jupyter notebook in a manner similar to jupyterhub but has a
few noteworthy improvements:

  • a Notebook CRD defines the API where the Notebook.spec.template.spec (PodTemplateSpec) replaces c.KubeSpawner.<attributes>
  • no separate authentication layer is imposed. Uses k8 authentication
  • will launch a jupyter notebook in a user's own namespace if paired with the profiles component

See the README.md for details.


This change is Reviewable

@kkasravi
Copy link
Contributor Author

kkasravi commented Oct 25, 2018

[WIP] see description

@kkasravi
Copy link
Contributor Author

/hold

@kkasravi kkasravi changed the title Notebooks [WIP] Notebooks Oct 26, 2018
@jlzhao27
Copy link

hey @kkasravi, this is the metacontroller docs right? https://metacontroller.app/

Might be worthwhile to call out in the README that the only requirements are kubectl access and an installed metacontroller, I think you mention it in the design part but it is a runtime requirement also.

@jlewi
Copy link
Contributor

jlewi commented Nov 14, 2018

Makes sense, however I'll need to add an httpserver to present the ui (similar to jupyterhub)

I'm not sure I understood this comment. What does this have to do with the ksonnet package organization of the new Jupyter CRD?

@kkasravi
Copy link
Contributor Author

hey @kkasravi, this is the metacontroller docs right? https://metacontroller.app/

Might be worthwhile to call out in the README that the only requirements are kubectl access and an installed metacontroller, I think you mention it in the design part but it is a runtime requirement also.

Sure, I'll update the README.

@kkasravi
Copy link
Contributor Author

Makes sense, however I'll need to add an httpserver to present the ui (similar to jupyterhub)

I'm not sure I understood this comment. What does this have to do with the ksonnet package organization of the new Jupyter CRD?

My misunderstanding I think ...

Were you saying that I should do I git mv kubeflow/notebooks/* kubeflow/jupyter and have 2 prototypes under kubeflow/jupyter/prototypes/ or completely replace the current kubeflow/jupyter with what's under kubeflow/notebooks?

@jlewi
Copy link
Contributor

jlewi commented Nov 15, 2018

Were you saying that I should do I git mv kubeflow/notebooks/* kubeflow/jupyter and have 2 prototypes under kubeflow/jupyter/prototypes/ or completely replace the current kubeflow/jupyter with what's under kubeflow/notebooks?

I was thinking multiple prototypes in the kubeflow/jupyter package. Is there any downside to doing this? I was thinking this would just avoid additional churn later on as I think kubeflw/jupyter is the package where we'd like them to live long term.

@kkasravi
Copy link
Contributor Author

Were you saying that I should do I git mv kubeflow/notebooks/* kubeflow/jupyter and have 2 prototypes under kubeflow/jupyter/prototypes/ or completely replace the current kubeflow/jupyter with what's under kubeflow/notebooks?

I was thinking multiple prototypes in the kubeflow/jupyter package. Is there any downside to doing this? I was thinking this would just avoid additional churn later on as I think kubeflw/jupyter is the package where we'd like them to live long term.

I agree. No downside since they both in theory could be deployed. I'll try and get this out asap.

@jlewi
Copy link
Contributor

jlewi commented Nov 15, 2018

Thanks @kkasravi !

@jlewi
Copy link
Contributor

jlewi commented Nov 19, 2018

@kkasravi Think we can get this merged this week?

@kkasravi
Copy link
Contributor Author

yes, I'll push changes today which should be most of what's required.

@kkasravi
Copy link
Contributor Author

/retest

@jlewi
Copy link
Contributor

jlewi commented Nov 20, 2018

Thanks @kkasravi

2 comments but I think these are best handled in follow on discussion and PRs as necessary.

  1. Why deployment not statefulset?

The main advantage of a statefulset is that the pod name is stable. This is mostly useful from a debugging perspective; i.e. we can tell users to look for a pod with a predictable name as opposed to filtering by labels or some other mechanism.

Since the type of resource is internal to the CRD we could always fix it later.

  1. Other question is about naming. I think notebook might refer to the ipynb file. Whereas the server is jupyter. So having the resource named "notebooks.kubeflow.org" might be confusing. It might lead users to think that the resource is tied to a single notebook.

The text on the website leaves me a bit confused; http://jupyter.org
They say "Jupyter Notebook".

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 87fdf42 into kubeflow:master Nov 20, 2018
@kkasravi kkasravi deleted the notebooks branch June 22, 2019 04:41
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
…ses k8 native auth' (kubeflow#1855)

* move to notebooks component

* add component to kfctl

* reducing what's needed in the Notebook CRD

* mapping attributes to CRD

* snapshot

* snapshot

* snapshot

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* working notebooks

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest

* /retest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

create a notebook controller that can replace jupyterhub and uses k8 native auth
5 participants