-
Notifications
You must be signed in to change notification settings - Fork 66
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
[New Hub prototype] Binderhub UI demo #4119
[New Hub prototype] Binderhub UI demo #4119
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
ef2f16c
to
bd17d24
Compare
We were thinking about this stuff below, and how binderhub_config.py in the official binderhub chart includes some auth stuff that isn't in binderhub-service. - name: JUPYTERHUB_SERVICE_NAME
value: binder
- name: JUPYTERHUB_API_URL
value: {{ (print (.Values.config.BinderHub.hub_url_local | default .Values.config.BinderHub.hub_url | trimSuffix "/") "/hub/api/") }}
- name: JUPYTERHUB_BASE_URL
value: {{ .Values.jupyterhub.hub.baseUrl | quote }}
- name: JUPYTERHUB_CLIENT_ID
value: {{ .Values.jupyterhub.hub.services.binder.oauth_client_id | quote }}
- name: JUPYTERHUB_OAUTH_CALLBACK_URL
value: {{ .Values.jupyterhub.hub.services.binder.oauth_redirect_uri | quote }} |
3eb8acb
to
ecdd191
Compare
…ation of internal's hub url and the /hub/api base url happens
There's almost a functional binderhub running at https://binderhub-ui-demo.2i2c.cloud. Last bit that's missing is launching that's currently failing due to a 403 related to scopes. Also last two commits were hacks to quickly fix a couple of issues I had with registry credentials. They weren't properly investigated yet. |
Looking at this now - we haven't provided permissions to the jupyterhub registered service "binder" to launch servers. |
This was only required for configuring the official binderhub chart which overrides singleuser.cmd, but we haven't done that in the first place so we don't need to re-set it to jupyterhub-singleuser.
@GeorgianaElena amazing effort into this!! I got a launch to work all the way with small final tweaks, the tweaks where:
Related to |
Yuhooooo 🚀 |
extraConfig: | ||
# FIXME: set KubernetesBuildExecutor.push_secret again | ||
# without this for some reason the build pods | ||
# search after the wrong secret name (i.e. the default name) | ||
# set by binderhub in KubernetesBuildExecutor.push_secret | ||
01-binderhub-service-set-push-secret: | | ||
import os | ||
c.KubernetesBuildExecutor.push_secret = os.environ["PUSH_SECRET_NAME"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I have no idea what happened here. I tried investigated and started by removing this hack and checked again and this time it worked. Maybe there's something relevant that didn't get properly cleared during the redeploy?
Anyway, I will drop the commit that added this, but leaving this here for posterity in case there is indeed something going on:
This is what the build pod showed before this hack, yesterday
Warning FailedMount 6m4s (x114 over 3h41m) kubelet MountVolume.SetUp failed for volume "docker-config" : secret "binder-build-docker-config" not found
Volumes:
docker-socket:
Type: HostPath (bare host directory volume)
Path: /var/run/docker.sock
HostPathType: Socket
docker-config:
Type: Secret (a volume populated by a Secret)
SecretName: binder-build-docker-config
Optional: false
kube-api-access-687gt:
Type: Projected (a volume that contains injected data from multiple sources)
TokenExpirationSeconds: 3607
ConfigMapName: kube-root-ca.crt
ConfigMapOptional: <nil>
DownwardAPI: true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every time I work with binderhub, the push_secret
config is a big confusion =/
TL;DR - I was confused, and remain confused why you observed that.
Here are reasons for why its confusing observed in the last ~5 minutes looking:
push_secret
should be configured to a k8s secret name having aconfig.json
key providing a docker configuration including credentials to work against a registry (its not the typical k8s imagePullSecret or similar, its a docker client config)binder-build-docker-config
is a default value forpush_secret
, but its also hardcoded i the official BinderHub helm chart deployment, but in binderhub-service deployment it isn't hardcoded - insteadpush_secret
is set by aextraConfig
entry based on an env variable etc - we shouldn't touchpush_secret
config as users of binderhub-service I think.- In binderhub-service chart, we configure credentials that goes into the k8s Secret via chart config
buildPodsRegistryCredentials
, but in the official chart its underconfig.BinderHub.buildDockerConfig
which is confusing because it isn't a traitlet for the Python classBinderHub
as expected if listed there. - The binderhub-service deployment resource isn't automatically restarting if the credentials set via
buildPodsRegistryCredentials
is updated - it only gets restarted if there is something in the pod spec that changes, such as an annotation providing a hash of the secret. I'll fix this. EDIT: nevermind what I wrote doesn't make sense I realize.
domain: hub.binderhub-ui-demo-demo.2i2c.cloud | ||
helm_chart: basehub | ||
helm_chart_values_files: | ||
- basehub-common.values.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just enables nfs
things we won't use, should be removed I think.
fwiw, it looks to me that the deployment satisfies all the criteria (except the about message perhaps?) specified in #4133. If that's right, I'd suggest we review and get this merged, and generate follow-up tasks from here instead of keeping this PR open until it is 'all' sorted. |
I'll go for a merge to flush what we have to ensure we get the terraform state changes represented - we haven't provided announcement, and there are things to refine and think about before the pattern of deploying a configuration like this is replicated even though that hasn't yet been specified. |
🎉🎉🎉🎉 Monitor the deployment of the hubs here 👉 https://github.com/2i2c-org/infrastructure/actions/runs/9307796142 |
Related to #4086 and #4133
What happens in this PR:
jupyterhub.storage.type = none
BinderSpawnerMixin
andenable_auth
on it