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

Document flow improvements, expanded on Jupyter usage #120

Merged
merged 2 commits into from Jan 16, 2018

Conversation

elsonrodriguez
Copy link
Contributor

Also added more info on the inception endpoint. Still need to provide an example with an inception client.

@k8s-ci-robot
Copy link
Contributor

Hi @elsonrodriguez. Thanks for your PR.

I'm waiting for a kubernetes or google member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Contributor

@jlewi jlewi left a comment

Choose a reason for hiding this comment

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

Does #92 provide the client that you said is still missing?

@@ -95,21 +100,53 @@ http://xx.yy.zz.ww:31942

For some cloud deployments, the LoadBalancer service may take up to five minutes display an external IP address. Re-executing `kubectl get svc` repeatedly will eventually show the external IP field populated.

Once you have an external IP, you can proceed to visit that in your browser. The hub by default is configured to take any username/password combination. After entering the username and password, you can start a single-notebook server,
request any resources (memory/CPU/GPU), and then proceed to perform single node training.
Once you have an external IP, you can proceed to visit that in your browser. You should see a sign in prompt.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to change this? I don't think we should be opening an external IP by default. So I think we should tell people how to connect via kubectl until we have a recipe for connecting securely via https (#11).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The security culture for k8s examples is pretty loose, however the documentation already mentions that by default this is totally insecure.

I do feel this is a continuation of a bad precedent.

A good solution would involve getting kube-cert-manager and external-dns configured, along with adding whitelists to the Service objects via ksonnet based on the user's IP, however I'm focusing on just getting things working for now.

I can switch everything to use port-forward if needed.

Copy link
Contributor

@jlewi jlewi Jan 14, 2018

Choose a reason for hiding this comment

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

If you don't mind, I'd prefer for us to use port-forward and not promote insecure methods; thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mind if I tackle the port-forward in another PR? It'll take me a bit to mess with ksonnet and put a toggle in there for loadbalancer vs port-forward (for those who want to use LB still)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's fine; since the code is already using LoadBalancer by default and this PR is just documenting current behavior.


Paste the example into a new Python 3 Jupyter notebook and execute the code, this should result in a 0.9014 accuracy result against the test data.

Please note that when running on most cloud providers, the public IP address will be exposed to the internet and is an
Copy link
Contributor

Choose a reason for hiding this comment

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

As noted above can we change this and make the default deployment of JupyterHub use ClusterIP?

My hope is that various Cloud Providers will chime in and provide help making JupyterHub secure.

On GKE I think it makes sense to do this using IAP (#60).

One option to provide a Cloud Agnostic secure method for accessing JupyterHub would be to use JupyterHub's built in OAuth support. I think it has a GitHub authenticator. Should we open up an issue for that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Providing clean authentication is one of the goals of JupyterHub. There's a lot listed on https://github.com/jupyterhub/jupyterhub/wiki/Authenticators and more around the web.

@elsonrodriguez
Copy link
Contributor Author

Yeah, that inception client looks right! I'll give it a test within a day or two.

@jlewi
Copy link
Contributor

jlewi commented Jan 16, 2018

LGTM. Please sync and I'll merge this.

@elsonrodriguez
Copy link
Contributor Author

Rebased.

@jlewi jlewi merged commit 8f3463c into kubeflow:master Jan 16, 2018
yanniszark pushed a commit to arrikto/kubeflow that referenced this pull request Feb 15, 2021
Signed-off-by: YujiOshima <yuji.oshima0x3fd@gmail.com>
elenzio9 pushed a commit to arrikto/kubeflow that referenced this pull request Oct 31, 2022
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.

None yet

4 participants