Limit resources based on workspace class #11374

Furisto · 2022-07-14T11:07:25Z

Description

This PR adds support for cpu limiting based on the workspace class of the workspace. The configuration for each workspace class contains a cpu limiting section which describes the minimal resources the workspaces gets and the burst limit. These values will be set as annotations on the workspace pod. ws-daemon will then use these annotations to distribute cpu resources

Related Issue(s)

Fixes #10981

How to test

Open workspace
Check that the workspace pod contains two annotations: gitpod.io/cpuMinLimit and gitpod.io/cpuBurstLimit
SSH into the harvester vm and find the cgroup for the workspace pod
Check that the min limit has been applied
Install stress-ng in the workspace
Run stress-ng --cpu 4
After a few seconds you should see that the burst limit has been applied to the cgroup of the workspace
Use kubectl annotate to change the annotations
The cgroup of the workspace should have been updated with the new values
Deactivate feature flag in configcat
Start workspace
Check that no limit annotations are present
Find the cgroup of the workspace
Check that the limits configured in the ws-daemon configmap are applied

Release Notes

None

Note

This relies on on a change in the werft files, that is why the main build failed. The custom build with the updated werft files (below) is successful however.

Werft options:

/werft with-preview

werft-gitpod-dev-com · 2022-07-14T11:07:42Z

started the job as gitpod-build-fo-class-daemon.1 because the annotations in the pull request description changed
(with .werft/ from main)

kylos101 · 2022-07-18T14:57:40Z

@Furisto what are your thoughts about limiting disk IO bandwidth? I ask because on XL (large) nodes, we "could" grant the workspaces a higher bandwidth because there are less workspaces and more available bandwidth.

For now, I think we should keep this as a 🛹, and continue to impose the same disk IO bandwidth limit like we do now (regardless of the node type). But, I am interested in your thoughts. 🤔 😄

Further, even when we go to PVC, we could initially impose the same disk IO bandwidth limit. And later, experiment with removing the disk IO bandwidth limit for /workspace, and assuming it doesn't introduce any performance degradation - ship it.

Furisto · 2022-07-18T15:02:48Z

@Furisto what are your thoughts about limiting disk IO bandwidth? I ask because on XL (large) nodes, we "could" grant the workspaces a higher bandwidth because there are less workspaces and more available bandwidth.

For now, I think we should keep this as a skateboard, and continue to impose the same disk IO bandwidth limit like we do now (regardless of the node type). But, I am interested in your thoughts. thinking smile

Yes, that is the plan. For the initial implementation I only included cpu but in the future we can make other limits dependent on the workspace class like disk IO or egress.

Further, even when we go to PVC, we could initially impose the same disk IO bandwidth limit. And later, experiment with removing the disk IO bandwidth limit for /workspace, and assuming it doesn't introduce any performance degradation - ship it.

👍

kylos101 · 2022-07-18T15:04:44Z

/hold

Why? This is not protected by a feature flag, yet, and it would be ideal to have one.

@Furisto is starting this work, and has socialized with @easyCZ.

cc: @sagor999 this is something we'll want to include as part of gen55, because usage based pricing depends on workspace having the ability to impose CPU limits that vary by node type / workspace class. It might delay the day we ship till Wednesday or Thursday. Prior to which, we should try to test other things in an ephemeral cluster.

kylos101 · 2022-07-27T15:25:18Z

@aledbf may we ask for your review on this PR? This way @Furisto can react to feedback from you. This file filter view may help (Only files owned by you):

@Furisto I think it would be good to leave the hold, at least until we have gen56 stable. @aledbf wdyt?

kylos101 · 2022-07-27T15:25:45Z

@Furisto as a heads up, there's a conflict with components/server/src/workspace/workspace-starter.ts

kylos101 · 2022-08-03T15:57:56Z

@aledbf can you help with this review? This is needed for gen60 and Usage based pricing. cc: @geropl

Furisto · 2022-08-04T11:23:50Z

/werft run

👍 started the job as gitpod-build-fo-class-daemon.26
(with .werft/ from main)

aledbf · 2022-08-05T13:08:12Z

components/ws-daemon/pkg/cpulimit/cpulimit.go

+}
+
+func (a annotationLimiter) Limit(wsh *WorkspaceHistory) (Bandwidth, error) {
+	value, ok := wsh.LastUpdate.Pod.Annotations[a.Annotation]


Why are we storing the Pod and not only the annotations?

It was just convenient to do and having the pod object around can be useful when we want to do limiting based on other attributes besides annotations.

Let's do that in the future. Please try to use what we need now. The Pod object can be expensive to store (annotations, status, env vars, etc)

aledbf · 2022-08-05T13:16:53Z

components/ws-manager-api/go/config/config.go

 	CPU              string `json:"cpu"`
 	Memory           string `json:"memory"`
 	EphemeralStorage string `json:"ephemeral-storage"`
 	Storage          string `json:"storage,omitempty"`
 }
+
+type ResourceLimitConfiguration struct {
+	CPU              *CpuResourceLimit `json:"cpu"`


Any reason why we don't set omitempty to all fields?

I just kept it consistent with the previous behavior. It is also nice to not have type out the fields when editing the configmap in e.g. preview environments.

Furisto · 2022-08-05T14:21:58Z

/werft run

👍 started the job as gitpod-build-fo-class-daemon.28
(with .werft/ from main)

Furisto · 2022-08-05T14:41:50Z

/unhold

roboquat added do-not-merge/work-in-progress do-not-merge/release-note-label-needed labels Jul 14, 2022

roboquat added size/M size/L size/XL and removed size/M size/L labels Jul 14, 2022

Furisto force-pushed the fo/class-daemon branch 6 times, most recently from 81a3b15 to 491cd95 Compare July 18, 2022 11:18

roboquat added release-note-none and removed do-not-merge/release-note-label-needed labels Jul 18, 2022

Furisto marked this pull request as ready for review July 18, 2022 14:01

Furisto requested a review from a team July 18, 2022 14:01

Furisto requested review from aledbf and sagor999 as code owners July 18, 2022 14:01

Furisto requested review from a team July 18, 2022 14:01

roboquat removed the do-not-merge/work-in-progress label Jul 18, 2022

github-actions bot added team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team labels Jul 18, 2022

roboquat added the do-not-merge/hold label Jul 18, 2022

nandajavarma approved these changes Jul 28, 2022

View reviewed changes

Furisto force-pushed the fo/class-daemon branch from 3de5b98 to 1596b85 Compare July 31, 2022 14:34

Furisto mentioned this pull request Aug 3, 2022

Remove fixed resources #11838

Merged

1 task

Furisto force-pushed the fo/class-daemon branch from 1596b85 to 7dd8bf6 Compare August 4, 2022 10:51

aledbf reviewed Aug 5, 2022

View reviewed changes

Furisto requested a review from a team August 5, 2022 13:38

Furisto added 5 commits August 5, 2022 13:40

[installer] Configure resource limits

6693d1b

[ws-manager] Add cpu constraint annotations

868f104

[ws-daemon] Limit cpu based on annotations

5de2a53

[ws-manager] Update tests

6c2fd3b

[ws-daemon/manager] Use feature flag

ebb9ec4

aledbf self-requested a review August 5, 2022 13:43

aledbf approved these changes Aug 5, 2022

View reviewed changes

akosyakov approved these changes Aug 5, 2022

View reviewed changes

Furisto added 2 commits August 5, 2022 14:02

[installer] Update render tests

3e41c03

[ws-daemon] Store only annotations instead of pod

a3d6f4e

Furisto force-pushed the fo/class-daemon branch from 050783d to a3d6f4e Compare August 5, 2022 14:02

roboquat removed the do-not-merge/hold label Aug 5, 2022

roboquat merged commit 225344b into main Aug 5, 2022

roboquat deleted the fo/class-daemon branch August 5, 2022 14:42

roboquat added deployed: webapp Meta team change is running in production deployed: workspace Workspace team change is running in production labels Aug 9, 2022

Furisto added the feature: workspace classes label Aug 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit resources based on workspace class #11374

Limit resources based on workspace class #11374

Furisto commented Jul 14, 2022 •

edited

Loading

werft-gitpod-dev-com bot commented Jul 14, 2022

kylos101 commented Jul 18, 2022

Furisto commented Jul 18, 2022

kylos101 commented Jul 18, 2022

kylos101 commented Jul 27, 2022

kylos101 commented Jul 27, 2022

kylos101 commented Aug 3, 2022

Furisto commented Aug 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading

aledbf Aug 5, 2022

Furisto Aug 5, 2022

aledbf Aug 5, 2022

aledbf Aug 5, 2022

Furisto Aug 5, 2022

Furisto commented Aug 5, 2022 •

edited by werft-gitpod-dev-com bot

Loading

Furisto commented Aug 5, 2022

Limit resources based on workspace class #11374

Limit resources based on workspace class #11374

Conversation

Furisto commented Jul 14, 2022 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Note

Werft options:

werft-gitpod-dev-com bot commented Jul 14, 2022

kylos101 commented Jul 18, 2022

Furisto commented Jul 18, 2022

kylos101 commented Jul 18, 2022

kylos101 commented Jul 27, 2022

kylos101 commented Jul 27, 2022

kylos101 commented Aug 3, 2022

Furisto commented Aug 4, 2022 • edited by werft-gitpod-dev-com bot Loading

aledbf Aug 5, 2022

Choose a reason for hiding this comment

Furisto Aug 5, 2022

Choose a reason for hiding this comment

aledbf Aug 5, 2022

Choose a reason for hiding this comment

aledbf Aug 5, 2022

Choose a reason for hiding this comment

Furisto Aug 5, 2022

Choose a reason for hiding this comment

Furisto commented Aug 5, 2022 • edited by werft-gitpod-dev-com bot Loading

Furisto commented Aug 5, 2022

Furisto commented Jul 14, 2022 •

edited

Loading

Furisto commented Aug 4, 2022 •

edited by werft-gitpod-dev-com bot

Loading

Furisto commented Aug 5, 2022 •

edited by werft-gitpod-dev-com bot

Loading