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

Create a BinderHub helm chart in the infrastructure repo #1280

Closed
1 task done
Tracked by #919
sgibson91 opened this issue May 6, 2022 · 18 comments
Closed
1 task done
Tracked by #919

Create a BinderHub helm chart in the infrastructure repo #1280

sgibson91 opened this issue May 6, 2022 · 18 comments
Assignees

Comments

@sgibson91
Copy link
Member

sgibson91 commented May 6, 2022

  • Structurally, I'd imagine we'd make a 'binderhub' helm-chart, which has a dependency on both binderhub and dask-gateway. We can use condition to disable dask-gateway in future binderhubs that don't need this. A big problem here is the lack of composability in helm, and we will have to duplicate a bunch of things from our basehub and daskhub chart values.yaml files :( Is there any way to avoid this? We'd also need to make sure the z2jh version matches what we have in basehub, and I'm not sure how exactly to do that either.

Originally posted by @yuvipanda in #919 (comment)

Tasks

  • Scope and design a binderhub helm chart under the helm-charts folder in this repo
@sgibson91
Copy link
Member Author

I have opened a very basic PR here: #1289

@sgibson91
Copy link
Member Author

sgibson91 commented May 11, 2022

Some notes after chatting with @yuvipanda

  • We are going to want to reuse some default values for this chart from basehub/daskhub, but helm is not very composable. Potential upstream work in the future may also change how our charts compose against z2jh. Hence we need a short-term hack to deploy this chart without bringing a lot of build complexity that we would have to undo later.
    • Alternative option is to make this validation a pre-commit hook, but I thought this would be a rabbit hole for me since I haven't written one before and I'll get too excited. CI is the 'boring' option that will get me to move onto the next task more reliably 😅

@sgibson91 sgibson91 changed the title Create a binderHub helm chart in the infrastructure repo Create a BinderHub helm chart in the infrastructure repo Jun 1, 2022
@damianavila
Copy link
Contributor

Potential upstream work in the future may also change how our charts compose against z2jh

Can you drop some thoughts/details about this one? Thanks!

@sgibson91
Copy link
Member Author

Can you drop some thoughts/details about this one? Thanks!

Chris wrote about it here #1382

@damianavila
Copy link
Contributor

Chris wrote about it here #1382

I might be missing something... but I see high-level stuff on that issue (and there in proposal).
I was a little bit more interested in the technical details about:

change how our charts compose against z2jh

Feel free to point me out to existing notes/docs about that discussion if those already exist.

@sgibson91
Copy link
Member Author

sgibson91 commented Jun 7, 2022

@damianavila I think the point is that we don't know. I'm not even sure if the BinderHub chart will even continue to exist if you can use repo2docker to built images directly in z2jh 🤷🏻‍♀️ Until that issue I linked to has more technical details, I can't say for sure what the future of this chart will be.

@sgibson91
Copy link
Member Author

Issue I'm having with the binderhub helm chart:

  • We want the binderhub helm chart to have the same functionality as basehub.
  • To do this we need to reuse the templates from basehub so that we can set up scratch buckets, etc.
  • @yuvipanda and I discussed having a symbolic link for the templates from basehub to binderhub.
  • This doesn't actually work because:
    • The templates are configured to look for .Values.jupyterhub.custom
    • The binderhub helm chart actually has .Values.binderhub.jupyterhub.custom

How to resolve?

  • Instead of symbolic links, actually copy the templates and find/replace all .Values.jupyterhub instances with .Values.binderhub.jupyterhub. Provide a validation check on the template files like we will be doing with the values files.
  • Something else?

@yuvipanda would really appreciate your input here

@sgibson91
Copy link
Member Author

sgibson91 commented Jun 14, 2022

@choldgraf and I chatted today and we will open a post on Pangeo's Discourse forum to get input on the urgency of a Binder on GCP in the community. If it's not that urgent, it will probably be better to redirect the operational toil into #1382 instead of developing a million hacks to get a Pangeo Binder up now that will only have to change once that work is underway.

@yuvipanda
Copy link
Member

@sgibson91 I think just copying them over is ok for now too.

Alternatively, we just completely ignore basehub and just copy over whatever is in https://github.com/jupyterhub/mybinder.org-deploy, remove what isn't needed and get that going.

@sgibson91
Copy link
Member Author

sgibson91 commented Jun 15, 2022

@yuvipanda and I had a quick chat about moving this forward in a quicker manner. Plan is the following:

@sgibson91
Copy link
Member Author

sgibson91 commented Jun 20, 2022

  • Terraform update to deploy GCR and push images there (instead of my personal Docker Hub account)

@yuvipanda I noticed that our GCP terraform config is already setup to create a GAR, so maybe we should use that instead of GCR?

Transition notes: https://cloud.google.com/artifact-registry/docs/transition/transition-from-gcr

@yuvipanda
Copy link
Member

@sgibson91 oooh yes using GAR would be perfect.

@sgibson91
Copy link
Member Author

@yuvipanda I requested your review on #1442

@sgibson91
Copy link
Member Author

I believe #1404 is now ready to be reviewed/merged

@sgibson91
Copy link
Member Author

I also now have config that successfully pushes the built images to GAR instead of my Docker Hub account! But I'd like to get #1404 merged first so I can rebase without force pushing anything.

@sgibson91
Copy link
Member Author

Following PR enables the GAR. It's already deployed though!

@sgibson91
Copy link
Member Author

sgibson91 commented Jun 23, 2022

Last task to close this issue:

Associated PR:

@sgibson91
Copy link
Member Author

Woohoo! I'm calling this one closed! 🎉

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

No branches or pull requests

3 participants