[ws-daemon] Fix CPU limit annotation #8459

csweichel · 2022-02-25T14:09:10Z

Description

When we updated the CPU limiting mechanism, we broke the gitpod.io/cpuLimit annotation. This PR makes that annotation work again. It effectively controls the base and burst limits of a workspace.

How to test

Start a workspace
Set the gitpod.io/cpuLimit annotation on that workspace
Observe how the workspace's cfs quota gets set to that value

Release Notes

Made the `gitpod.io/cpuLimit` annotation work again

codecov · 2022-02-25T14:20:11Z

Codecov Report

Merging #8459 (4c931a4) into main (37444f7) will increase coverage by 21.24%.
The diff coverage is 35.29%.

❗ Current head 4c931a4 differs from pull request most recent head 3cb57ed. Consider uploading reports for the commit 3cb57ed to get more accurate results

@@             Coverage Diff             @@
##             main    #8459       +/-   ##
===========================================
+ Coverage   11.17%   32.42%   +21.24%     
===========================================
  Files          18       92       +74     
  Lines         993    15074    +14081     
===========================================
+ Hits          111     4887     +4776     
- Misses        880     9794     +8914     
- Partials        2      393      +391

Flag	Coverage Δ
components-gitpod-cli-app	`11.17% <ø> (ø)`
components-image-builder-mk3-app	`35.71% <ø> (?)`
components-supervisor-app	`35.78% <ø> (?)`
components-ws-daemon-app	`20.17% <35.29%> (?)`
components-ws-daemon-lib	`20.17% <35.29%> (?)`
components-ws-manager-app	`39.29% <ø> (?)`
components-ws-proxy-app	`68.63% <ø> (?)`
install-installer-raw-app	`4.49% <ø> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
components/ws-daemon/pkg/cpulimit/dispatch.go	`0.00% <0.00%> (ø)`
components/ws-daemon/pkg/cpulimit/cpulimit.go	`66.45% <46.15%> (ø)`
...ents/image-builder-mk3/pkg/orchestrator/metrics.go	`48.14% <0.00%> (ø)`
components/supervisor/pkg/ports/ports.go	`58.93% <0.00%> (ø)`
components/ws-manager/pkg/manager/probe.go	`0.00% <0.00%> (ø)`
components/supervisor/pkg/ports/exposed-ports.go	`0.00% <0.00%> (ø)`
...onents/ws-daemon/pkg/internal/session/workspace.go	`50.73% <0.00%> (ø)`
components/ws-daemon/pkg/daemon/daemon.go	`0.00% <0.00%> (ø)`
...installer/pkg/components/ws-manager/rolebinding.go	`0.00% <0.00%> (ø)`
components/ws-proxy/pkg/proxy/workspacerouter.go	`81.89% <0.00%> (ø)`
... and 66 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

csweichel · 2022-02-25T14:52:39Z

/werft run with-vm

👍 started the job as gitpod-build-cw-fixlimit.1

Furisto · 2022-03-02T12:06:54Z

@csweichel I cannot start a workspace. #8526 might fix it.

iQQBot · 2022-03-02T14:06:18Z

I think it this PR, #8476

iQQBot · 2022-03-02T14:08:09Z

rebase to main is good option

princerachit · 2022-03-09T05:52:36Z

Hey @csweichel Is this PR in reviewable state? Thomas was not able to start the ws. There are also some conflicting changes

csweichel · 2022-03-09T16:12:43Z

Yeah, this has sat around for too long. It should be reviewable. Will rebase.

princerachit · 2022-03-14T10:00:18Z

The build is failing can you PTAL @csweichel

Furisto · 2022-03-14T13:27:18Z

Build fails because CgroupCFSController is now CgroupV1CFSController.

princerachit · 2022-03-16T04:44:36Z

Build fails because CgroupCFSController is now CgroupV1CFSController.

Should it be rather CFSController in the code?

princerachit · 2022-03-16T04:46:54Z

I have pushed a commit with a fix

princerachit · 2022-03-16T05:17:48Z

retriggering a build as this one failed with a typescript code error. It might be intermittent.

/werft run

👍 started the job as gitpod-build-cw-fixlimit.5

Furisto · 2022-03-16T12:05:19Z

I tried it and could observe that the cpu quota was set to the annotation value.

sagor999 · 2022-04-18T19:23:14Z

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


 		// if we didn't get the max bandwidth, but were throttled last time
 		// and there's still some bandwidth left to give, let's act as if had
 		// never spent any CPU time and assume the workspace will spend their
 		// entire bandwidth at once.
 		var burst bool
 		if totalBandwidth < d.TotalBandwidth && ws.Throttled() {
-			limit = d.BurstLimiter.Limit(ws.Usage())
+			limiter := d.BurstLimiter
+			if w := wsidx[id]; w.BaseLimit > 0 {


shouldn't this check for w.BurstLimiter?

It totally should. This basically means that a workspace with a base limit (i.e. cpuLimit annotation) will only ever get that limit, and will never burst.

In practice that has no effect though because we set the base and burst limit to the same value when that annotation is applied.

csweichel requested a review from a team February 25, 2022 14:09

roboquat added release-note size/M labels Feb 25, 2022

github-actions bot added the team: workspace Issue belongs to the Workspace team label Feb 25, 2022

csweichel mentioned this pull request Feb 25, 2022

[ws-daemon] Fix CPU limit annotation #8458

Closed

csweichel marked this pull request as draft February 25, 2022 14:09

roboquat added the do-not-merge/work-in-progress label Feb 25, 2022

csweichel force-pushed the cw/fixlimit branch from 457c3d4 to 4c931a4 Compare February 28, 2022 08:57

csweichel marked this pull request as ready for review March 1, 2022 16:08

roboquat removed the do-not-merge/work-in-progress label Mar 1, 2022

kylos101 assigned csweichel Mar 1, 2022

csweichel force-pushed the cw/fixlimit branch from 4c931a4 to 57efbfa Compare March 10, 2022 14:01

csweichel and others added 2 commits March 16, 2022 05:46

[ws-daemon] Fix CPU limit annotation

2f48983

Fix variable type

3cb57ed

princerachit force-pushed the cw/fixlimit branch from 8b335dd to 3cb57ed Compare March 16, 2022 06:01

Furisto approved these changes Mar 16, 2022

View reviewed changes

roboquat merged commit ffe6f48 into main Mar 16, 2022

roboquat deleted the cw/fixlimit branch March 16, 2022 12:06

roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Mar 16, 2022

aledbf restored the cw/fixlimit branch April 18, 2022 19:12

aledbf mentioned this pull request Apr 18, 2022

Revert "[ws-daemon] Fix CPU limit annotation" #9382

Merged

sagor999 reviewed Apr 18, 2022

View reviewed changes

sagor999 mentioned this pull request Apr 18, 2022

[ws-daemon] fix typo in cpulimiter #9384

Closed

This was referenced Apr 19, 2022

Why did fixing CPU limit annotation break CPU limiting with cgroup v2? #9407

Closed

[ws-daemon] Fix CPU limit annotation #9479

Closed

princerachit deleted the cw/fixlimit branch May 16, 2022 05:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ws-daemon] Fix CPU limit annotation #8459

[ws-daemon] Fix CPU limit annotation #8459

Uh oh!

csweichel commented Feb 25, 2022

Uh oh!

codecov bot commented Feb 25, 2022 •

edited

Loading

Uh oh!

csweichel commented Feb 25, 2022 •

edited by werft-gitpod-dev-com bot

Loading

Uh oh!

Furisto commented Mar 2, 2022

Uh oh!

iQQBot commented Mar 2, 2022

Uh oh!

iQQBot commented Mar 2, 2022

Uh oh!

princerachit commented Mar 9, 2022

Uh oh!

csweichel commented Mar 9, 2022

Uh oh!

princerachit commented Mar 14, 2022

Uh oh!

Furisto commented Mar 14, 2022

Uh oh!

princerachit commented Mar 16, 2022

Uh oh!

princerachit commented Mar 16, 2022

Uh oh!

princerachit commented Mar 16, 2022 •

edited by werft-gitpod-dev-com bot

Loading

Uh oh!

Furisto commented Mar 16, 2022

Uh oh!

sagor999 Apr 18, 2022

Uh oh!

csweichel Apr 19, 2022 •

edited

Loading

Uh oh!

Uh oh!

[ws-daemon] Fix CPU limit annotation #8459

[ws-daemon] Fix CPU limit annotation #8459

Uh oh!

Conversation

csweichel commented Feb 25, 2022

Description

How to test

Release Notes

Uh oh!

codecov bot commented Feb 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

csweichel commented Feb 25, 2022 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Furisto commented Mar 2, 2022

Uh oh!

iQQBot commented Mar 2, 2022

Uh oh!

iQQBot commented Mar 2, 2022

Uh oh!

princerachit commented Mar 9, 2022

Uh oh!

csweichel commented Mar 9, 2022

Uh oh!

princerachit commented Mar 14, 2022

Uh oh!

Furisto commented Mar 14, 2022

Uh oh!

princerachit commented Mar 16, 2022

Uh oh!

princerachit commented Mar 16, 2022

Uh oh!

princerachit commented Mar 16, 2022 • edited by werft-gitpod-dev-com bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Furisto commented Mar 16, 2022

Uh oh!

sagor999 Apr 18, 2022

Choose a reason for hiding this comment

Uh oh!

csweichel Apr 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Feb 25, 2022 •

edited

Loading

csweichel commented Feb 25, 2022 •

edited by werft-gitpod-dev-com bot

Loading

princerachit commented Mar 16, 2022 •

edited by werft-gitpod-dev-com bot

Loading

csweichel Apr 19, 2022 •

edited

Loading