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

feat(ws): use controller-runtime for backend #43

Conversation

thesuperzapper
Copy link
Member

@thesuperzapper thesuperzapper commented Sep 11, 2024

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:

  1. We can use the local envtest for mocking API responses locally (like we already do in the controller tests)
  2. Rather than constantly querying the API, it will use a "watch cache" based approach, so that we only list the resources once (e.g. Workspaces), and then watch for changes.
  3. For extremely fast lookup of complicated queries like "list all workspaces owned by workspace kind xxxx", we can use indexes on the cache, like we already do for the controller.

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 internal net/http server.

Other than that, this PR makes the following changes:

  1. Loads the Workspace/WorkspaceKind types using a replace github.com/kubeflow/notebooks/workspaces/controller => ../controller redirect in the go.mod
  2. Adds controller-runtime to go.mod
  3. Moves all non-api go code under internal/
  4. Makes the /api/v1/healthcheck/ path return a list of Workspace names (lol, we can revert this before merging).
  5. Renames the local variable for App from app to a

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@thesuperzapper thesuperzapper requested review from ederign and removed request for kimwnasptd September 11, 2024 06:17
@thesuperzapper thesuperzapper added the project/notebooks-v2 project - kubeflow notebooks v2 label Sep 11, 2024
@thesuperzapper
Copy link
Member Author

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:

By("setting up the controller manager")
k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{
Scheme: scheme.Scheme,
Metrics: metricsserver.Options{
BindAddress: "0", // disable metrics serving
},
})
Expect(err).NotTo(HaveOccurred())

We also will need to add the github.com/onsi/gomega testing framework as a dependency of the backend.

@ederign
Copy link
Member

ederign commented Sep 14, 2024

Rebased and updated here: #44

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
@ederign
Copy link
Member

ederign commented Sep 14, 2024

/lgtm

@thesuperzapper
Copy link
Member Author

/approve

Copy link

[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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit b0af8ae into kubeflow:notebooks-v2 Sep 14, 2024
3 checks passed
@thesuperzapper thesuperzapper deleted the backend-with-controller-runtime branch September 14, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

2 participants