-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat(ws): use controller-runtime for backend #43
feat(ws): use controller-runtime for backend #43
Conversation
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
Also, the tests won't pass until we either remove the dependency on the client being up (in the health-check test), or do exactly what we have done in the controller, and set up the controller-manager: notebooks/workspaces/controller/internal/controller/suite_test.go Lines 98 to 105 in bc4e445
We also will need to add the |
Rebased and updated here: #44 |
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ederign, thesuperzapper 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 |
This PR sets up the Workspace Backend API to use
controller-runtime
rather than ClientSets to access the Kubernetes API.This has some significant benefits:
envtest
for mocking API responses locally (like we already do in the controller tests)The way this is implemented is a little hacky, because we are directly adding a new "Runnable" to the
controller-runtime
manager object, and this runnable encapsulates our internalnet/http
server.Other than that, this PR makes the following changes:
replace github.com/kubeflow/notebooks/workspaces/controller => ../controller
redirect in thego.mod
controller-runtime
togo.mod
internal/
/api/v1/healthcheck/
path return a list of Workspace names (lol, we can revert this before merging).App
fromapp
toa