ws-manager-mk2 - or what if we built it today?

[csweichel]

Description

This PR is the initial work for getting ws-manager-mk2 off the ground. It can start workspaces and do initialize the content. Communication between ws-manager, ws-daemon and ws-proxy is done through a workspace CRD. This is not a production grade implementation yet. For milestones see the milestone plan. Features not yet working will be added in subsequent PRs.

PoC: ws-manager-mk2 - Watch Video

When we built ws-manager more then three years ago, kubebuilder wasn't a thing and writing Kubernetes controllers was hard(er). Also, we didn't know too much about Kubernetes at the time.

This PR shows how ws-manager would look like if it were written as a Kubernetes controller, CRDs and all. The main takeway: it terrifyingly simplifies things. The code is orders of magnitudes easier to read and maintain compared to what we have today.

If we just look at the lines of code (a weak, but easily quantifiable proxy for complexity):

# ws-manager-mk2: 7906 SLOC
$ find ws-manager/pkg/manager -name '*.go' | xargs cat | wc -l
7906

# ws-manager-mk2: 2625 SLOC
$ find ws-manager-mk2/controllers/ -name '*.go' | xargs cat | wc -l
1773
$ find ws-manager-mk2/service/ -name '*.go' | xargs cat | wc -l
852

The version in this branch is nearly happy-path feature complete:

• it is able to start and stop workspaces
• supports headless workspaces, e.g. image builds and maybe prebuilds
• it talks to ws-daemon to do content init and disposal
• it can handle ports,
• it acts as imagespec provider for registry-facade,
• it's a drop-in replacement for ws-manager; uses the exact same config an provides the very same API.

I've also extended ws-proxy to use the workspace CRs as workspace info source.

There's a bunch of things missing in this branch:

• tests of any kind really
• better non-happy path handling
• did I mention tests?
• it's not integrated into the installer yet

Ok, but why?

Other than the considerable reduction in complexity this approach gets us in ws-manager,

• it simplifies the workspace side as a whole, because we can build the in-cluster interactions using CRs rather than gRPC. Much like ws-proxy functions today, ws-daemon and image-builder could interact on the custom resource rather than speak gRPC. This greatly simplifies state management.
• it lets us implement functionality that's independent of the pod's lifecycle, e.g. image builds become much easier to do.
• it lets us implement workspaces which are not pods. Today, all workspaces must be a pod. If we ever want to move to other means of executing workspaces, we'd have a ton of work ahead (think workspaces as harvester VMs? :D - not an actual suggestion).

Do you not have better things to do?

This was primarily an evening/weekend project - less than 20h have gone into this.

How to review

• Check for adverse side effects. This PR should not affect the function of Gitpod in any way. It's merely the foundation for things to come.
• Workspaces don't actually start yet using mk2. This is expected. We need to get this changeset off a feature branch though.

Release Notes

NONE

Relates with #11416

/werft with-vm=true
/werft with-preview=true
/werft with-wsman-mk2=true

[csweichel]

/werft run

👍 started the job as gitpod-build-cw-ws-manager-mk2.41
(with .werft/ from main)

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[csweichel]

/werft run

👍 started the job as gitpod-build-cw-ws-manager-mk2.63
(with .werft/ from main)

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[kylos101]

Non-blocking, question

@Furisto why disable? Just curious.

[Furisto]

Ah was not aware of that but it makes sense. Currently the test we have (for the webhook) fails. Webhooks will be removed with the next PR which means the test will be removed as well.

[aledbf]

Webhooks will be removed with the next PR which means the test will be removed as well.

Do we have a written reason why this is being done? What's the replacement?

[Furisto]

This was a suggestion from @csweichel. Currently there is just nothing that we would want to validate.

[aledbf]

Let's talk about that in the next sync. One of the goals of the webhook is to avoid persistence with invalid values and wasting sync loops in the controller.

[kylos101]

@Furisto was the thought to use CEL, instead of webhooks to do validation? This article suggests we cannot use CEL until Kubernetes 1.25 (we're currently on 1.23, and cannot move to 1.24 yet).

[kylos101]

👍

[Furisto]

/unhold