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

Add new hub 2i2c-imagebuilding-demo with binderhub-service enabled #2699

Merged
merged 17 commits into from
Jul 18, 2023

Conversation

GeorgianaElena
Copy link
Member

@GeorgianaElena GeorgianaElena commented Jun 22, 2023

This PR:

  • adds the binderhub-service chart as an optional dependency of the basehub chart
  • adds a new 2i2c hub called imagebuilding-demo, where it enables and configures binderhub-service

The binderhub-service is configured in such a way that will:

  • start a docker-api pods onto each user node
  • all the builder pods spawned by binderhub will start onto a user node
  • enable pushing the images built by the builder pods into the (existing) binder-staging-registry under the binderhub-service- prefix

Closes #2827

@github-actions

This comment was marked as resolved.

@GeorgianaElena
Copy link
Member Author

This was manually deployed on the staging hub:

$ k get pods -n staging

staging-binderhub-service-59487c6c8-nt4tt    1/1     Running   0          43m
staging-binderhub-service-docker-api-8zq6l   1/1     Running   0          43m
staging-binderhub-service-docker-api-rfzcf   1/1     Running   0          43m
$ k -n staging get svc

staging-binderhub-service   ClusterIP   10.3.253.111   <none>        8090/TCP    43m

@GeorgianaElena
Copy link
Member Author

GeorgianaElena commented Jul 11, 2023

I'm leaving here some of the issues I've encountered while deploying this:

1. Issue with the startup probe

There was an issue with the startup probe that was having an extra / which was causing it to check the / endpoint (which was removed when the build_only flag is set) and not the /versions.

This was causing the binderhub container to never start.

✅ Fixed by 2i2c-org/binderhub-service#44

2. Wrong docker config secret name

The error that was showing in the binderhub pod was:

[W 230711 11:20:18 registry:112] No docker config at config.json
[W 230711 11:20:18 registry:154] No username for docker registry at https://index.docker.io/v1
[W 230711 11:20:18 registry:179] No password for docker registry at https://index.docker.io/v1
[E 230711 11:20:18 builder:418] Failed to get image manifest for us-central1-docker.pkg.dev/two-eye-two-see/binder-staging-registry/binderhub-service-binderhub-2dci-2drepos-2dcac
hed-2dminimal-2ddockerfile-c90b2b:596b52f10efb0c9befc0c4ae850cc5175297d71c
****

✅ Fixed by 2i2c-org/binderhub-service#50

3. Puttting the builder pods on the user node pool using the config below causes errors.

KubernetesBuildExecutor:
      node_selector:
        hub.jupyter.org/node-purpose: user
MountVolume.SetUp failed for volume "docker-socket" : hostPath type check failed: /var/run/docker-api/docker-api.sock is not a socket file

Resolution

If that config is removed the builder pod starts on a core node, then everything works out. I think this is because the binderhub-service-docker-api pods also run on core nodes and this where the docker process runs.

❗ TODO:

  • We might want to schedule these on the user node pool as well?

cb6945b and c35ef31

4. When cleaning up the builder pods, the wrong namespace is checked (i.e. default)

The symptom was the following error that was showing up in the binderhub pod:

[E 230711 12:40:12 app:1094] Failed to cleanup builders
    Traceback (most recent call last): <redacted for simplicity>
    ....
    kubernetes.client.exceptions.ApiException: (403)
    Reason: Forbidden
    HTTP response headers: HTTPHeaderDict({'Audit-Id': '59ef9b35-e149-442a-9b54-4aa149383fc8', 'Cache-Control': 'no-cache, private', 'Content-Type': 'application/json', 'X-Content-Type-Options': 'nosniff', 'X-Kubernetes-Pf-Flowschema-Uid': '6076e3d8-fb9c-4af6-bc16-1828eab8bf98', 'X-Kubernetes-Pf-Prioritylevel-Uid': '3b5efc35-8cfb-4eeb-8300-d32a394811b8', 'Date': 'Tue, 11 Jul 2023 12:40:12 GMT', 'Content-Length': '300'})
    HTTP response body: {"kind":"Status","apiVersion":"v1","metadata":{},"status":"Failure","message":"pods is forbidden: User \"system:serviceaccount:staging:staging-binderhub-service\" cannot list resource \"pods\" in API group \"\" in the namespace \"default\"","reason":"Forbidden","details":{"kind":"pods"},"code":403}

For now, I've fixed this by setting c.BinderHub.build_namespace = "staging" in extraConfig.
BUT this is a deprecated config in favour of KubernetesBuildExecutor.namespace

[BinderHub] WARNING | BinderHub.build_namespace is deprecated, use KubernetesBuildExecutor.namespace

The thing is that setting KubernetesBuildExecutor.namespace doesn't fix the issue and the error is still there.

❗ I think there are two TODO issues here:

  • why does binderhub uses the wrong namespace
  • why setting KubernetesBuildExecutor.namespace doesn't have the same effect as the config it has replaced BinderHub.build_namespace

✅ fixed by 2i2c-org/binderhub-service#51

@GeorgianaElena
Copy link
Member Author

@2i2c-org/engineering, this is now ready to review.

I also manually deployed it and I managed to reproduce @consideRatio's demo in 2i2c-org/binderhub-service#19 (comment) 🎉
Screenshot 2023-07-14 at 17 30 41
Screenshot 2023-07-14 at 17 30 11

@yuvipanda
Copy link
Member

@GeorgianaElena amazing! This looks great to me, but can you put it on a new hub instead of reusing staging? Let's call it imagebuilding-demo maybe?

@arnim
Copy link

arnim commented Jul 14, 2023

Not sure if we should / want do that, but would it in principle be possible to hock this to an external dns, e.g. magebuilding-demo.xyz.org, without eating too much extra time?

@yuvipanda
Copy link
Member

@arnim let's track that as a separate item. What domain are you thinking of?

@GeorgianaElena GeorgianaElena force-pushed the opt-binderhub-service branch from c578f90 to 2ee6528 Compare July 17, 2023 09:25
@GeorgianaElena GeorgianaElena changed the title Add binderhub-service as an optional dep of basehub and try it on staging Add new hub 2i2c-imagebuilding-demo with binderhub-service enabled Jul 17, 2023
@GeorgianaElena
Copy link
Member Author

@yuvipanda, I've added the new hub: https://imagebuilding-demo.2i2c.cloud

Copy link
Member

@yuvipanda yuvipanda left a comment

Choose a reason for hiding this comment

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

Awesome, @GeorgianaElena! Let’s go!

@GeorgianaElena GeorgianaElena merged commit d300d71 into 2i2c-org:master Jul 18, 2023
@GeorgianaElena GeorgianaElena deleted the opt-binderhub-service branch July 18, 2023 08:07
@github-actions
Copy link

🎉🎉🎉🎉

Monitor the deployment of the hubs here 👉 https://github.com/2i2c-org/infrastructure/actions/runs/5584909328

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Status: Done 🎉
Development

Successfully merging this pull request may close these issues.

Deploy binderhub-service chart to a new 2i2c hub
4 participants