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.

Sign up for GitHub

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

Jump to bottom

Provide workspace resource information #10836

Merged
merged 6 commits into from
Jun 28, 2022
Merged

Provide workspace resource information #10836

merged 6 commits into from
Jun 28, 2022

Conversation

Furisto
Copy link
Member

@Furisto Furisto commented Jun 22, 2022 •
edited
Loading

Description

Adds a workspace info service to the workspace, that allows the retrieval of information about the current workspace. For now resource limits and usage are supported. Supports systems with cgroup v1 and v2.

The service itself only exposes a single (ratelimited) operation that is then delegated to IWS. This indirection was introduced to limit the access of user code to IWS.

In addition I introduced a (minimal) cgroup library as discussed in #10043 in order to support cgroup v1 and v2. Moving other code in e.g. ws-daemon over to use this library is outside of scope for this PR.

Related Issue(s)

Fixes #10839 and #10043

How to test

  • Open workspace in preview environment and execute gp top. Our preview environments are still on cgroup v1, if you want to test cgroup v2, you can use this VM that I prepared or use workspace-preview yourself to test it.

Release Notes

Provide endpoint that allows retrieving information about the workspace from within the workspace

Werft options:

  • /werft with-preview

@Furisto Furisto force-pushed the fo/ws-info branch 3 times, most recently from b8bfb89 to 3df4043 Compare June 22, 2022 18:27
utam0k
utam0k reviewed Jun 23, 2022
View reviewed changes
components/workspacekit/cmd/rings.go
}

func startInfoService(socketDir string) (func(), error) {
socketFN := filepath.Join(socketDir, "info.sock")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have to set the rps limit per the workspace(ideal) because the user can touch this uds, and attack to ws-daemon. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, one of the reasons it is still in draft 😉

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Furisto Furisto force-pushed the fo/ws-info branch 11 times, most recently from a1232a0 to e745ae1 Compare June 27, 2022 15:57
@Furisto
Copy link
Member Author

Furisto commented Jun 28, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run with-preview

👍 started the job as gitpod-build-fo-ws-info.32
(with .werft/ from main)

@Furisto Furisto marked this pull request as ready for review June 28, 2022 09:32
@Furisto Furisto requested a review from a team June 28, 2022 09:32
@Furisto Furisto requested a review from aledbf as a code owner June 28, 2022 09:32
@Furisto Furisto requested review from a team June 28, 2022 09:32
@github-actions github-actions bot added team: IDE team: delivery Issue belongs to the self-hosted team labels Jun 28, 2022
@akosyakov
Copy link
Member

@Furisto It looks good but we should do in the backward compatible way, so please keep old logic as a fallback for sometime in supervisor. We are not going to rollback supervisor if a new cluster deployment goes wrong.

@Furisto
Copy link
Member Author

Furisto commented Jun 28, 2022

Hey @gitpod-io/engineering-webapp @gitpod-io/engineering-self-hosted could you please a look at this? We need it for the deployment of gen50 next week.

@aledbf
Copy link
Member

aledbf commented Jun 28, 2022

We need it for the deployment of gen50 next week.

is today ;)

@Furisto
Copy link
Member Author

Furisto commented Jun 28, 2022

/hold

@geropl geropl self-assigned this Jun 28, 2022
geropl
geropl approved these changes Jun 28, 2022
View reviewed changes
Copy link
Member

@geropl geropl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a while to get a workspace, but... works nicely! 🎉

@Furisto
Copy link
Member Author

Furisto commented Jun 28, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-fo-ws-info.39
(with .werft/ from main)

@Furisto
Copy link
Member Author

Furisto commented Jun 28, 2022

@mrsimonemms One of the installer tests failed because the order of the elements in the list did not match up. I do not think the order of the elements in the list is important for correctness so I fixed the test by ensuring that the elements match up before the comparison and added the commit to this PR. If I am mistaken let me know.

@Furisto
Copy link
Member Author

Furisto commented Jun 28, 2022

/unhold

@Furisto
Copy link
Member Author

Furisto commented Jun 28, 2022 •
edited
Loading

@gitpod-io/engineering-ide Can you take a look and approve please 🙏

mustard-mh
mustard-mh approved these changes Jun 28, 2022
View reviewed changes
Copy link
Contributor

@mustard-mh mustard-mh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well 👍

image

@roboquat roboquat merged commit db62c35 into main Jun 28, 2022
@roboquat roboquat deleted the fo/ws-info branch June 28, 2022 17:15
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed: IDE IDE change is running in production labels Jun 28, 2022
@mrsimonemms mrsimonemms mentioned this pull request Jun 29, 2022
Merged
1 task
@Furisto Furisto mentioned this pull request Jun 29, 2022
Closed
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@utam0k utam0k utam0k left review comments

@akosyakov akosyakov akosyakov left review comments

@geropl geropl geropl approved these changes

@mustard-mh mustard-mh mustard-mh approved these changes

@mrsimonemms mrsimonemms mrsimonemms approved these changes

@aledbf aledbf aledbf approved these changes

Assignees

@geropl geropl

Labels
deployed: IDE IDE change is running in production deployed: webapp Meta team change is running in production deployed: workspace Workspace team change is running in production release-note size/XXL team: delivery Issue belongs to the self-hosted team team: IDE team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

gp top does not work with cgroup v2
8 participants
@Furisto @akosyakov @aledbf @mrsimonemms @utam0k @mustard-mh @geropl @roboquat