ws-daemon: Use a pair of veths instead of slirp4netns #8955

utam0k · 2022-03-25T06:47:29Z

Description

This PR creates a pair of veth between the workspace and ring1.

Related Issue(s)

How to test

in gitpod workspace
https://to-setup-veth.staging.gitpod-dev.com/workspaces

$ docker run hello-world

maybe you have to kill slirp4netns's process.

Release Notes

Use a pair of veths instead of slirp4netns

Documentation

No

components/ws-daemon-api/workspace_daemon.proto

components/workspacekit/cmd/rings.go

components/ws-daemon/nsinsider/main.go

utam0k · 2022-03-25T12:39:25Z

I will have more drawings and other information on Monday that will show more of what I intend to do.

utam0k · 2022-03-25T12:40:09Z

/hold

components/ws-daemon/nsinsider/main.go

aledbf · 2022-03-27T19:17:19Z

components/ws-daemon/nsinsider/main.go

+						)
+					}
+
+					cmd = exec.Command(ipCmd, "netns", "exec", netns, "ip", "link", "set", "lo", "up")


Why do we need to set lo up?

@aledbf Thanks for your review!
For now, I guess slirp4netns enable lo automatically. So, when we want to remove slirp4netns, we have to enable lo manually.

iQQBot · 2022-03-28T09:54:53Z

Is it ready for review?

kylos101 · 2022-03-28T13:55:53Z

Hey @utam0k , given the commit message in 23346cf, I am going to mark this back as draft (it seems like you're unsure if netlink will work because you use the word "try" in the commit message). May I ask you to confirm if this is ready for review? If yes, feel free to mark as ready for review.

kylos101 · 2022-03-29T02:05:29Z

Hi @utam0k , while this is in draft, if you would like "an early review". that is totally okay to request. 💌 😍 See you tomorrow!

utam0k · 2022-03-29T02:35:24Z

@kylos101 Thanks for organizing the PR. This PR will be available for review soon. I had a little trouble verifying it.

utam0k · 2022-03-29T02:37:37Z

/unhold

utam0k · 2022-03-29T02:47:59Z

@princerachit @aledbf @iQQBot @kylos101 Sorry to keep you waiting. I have finished brushing up on this PR. PTAL.
I recommend taking a look at this picture before you start reviewing. This whole attempt is too large to be in one PR, so it is divided into two phases(PRs). This PR is the first these. There should be no problem if only this PR is merged and deployed.

utam0k · 2022-04-06T07:38:44Z

components/ws-daemon/seccomp-profile-installer/main.go

@@ -42,7 +42,7 @@ func main() {
 			Action: specs.ActAllow,
 		},

-		// slirp4netns requires setns, as do we for debugging
+		// Running docker on a workspace requires setns


docker is still using slirp4netns.

We disabled the docker slirp4netns use when we introduced slirp4netns for the workspace as a whole. We have however not yet removed that code from docker-up. We should do that in a follow-up PR.

utam0k · 2022-04-06T07:48:25Z

@princerachit @aledbf @iQQBot @kylos101 @csweichel This PR is ready for review again. PTAL. Load and other measurements will be done tomorrow, but the code itself is available for review.

utam0k · 2022-04-06T07:48:46Z

/hold Until the numerical measurements are completed.

utam0k · 2022-04-07T05:27:45Z

Network IO metrics

I measured the limits of traffic from workspace to ring1 using iperf3.

slirp4netns

[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  9.78 GBytes  8.40 Gbits/sec    0             sender
[  5]   0.00-10.00  sec  9.78 GBytes  8.40 Gbits/sec                  receiver

veth

[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  50.5 GBytes  43.4 Gbits/sec  284             sender
[  5]   0.00-10.00  sec  50.5 GBytes  43.4 Gbits/sec                  receiver

CPU metrics

This is the CPU utilization when 8 Gbit/sec of traffic continues to flow(workspace -> ring1)

	slirp4netns	veth
avg of user	4.46 %	2.64 %
avg of system	10.07 %	1.69%

More details can be found in the vmstat results in this spreadsheet.

utam0k · 2022-04-07T05:27:56Z

/unhold

utam0k · 2022-04-07T05:29:04Z

@csweichel I think you are probably interested in these above metrics.

csweichel

Works like a charm - love the new performance!
Really great work @utam0k.

iQQBot · 2022-04-07T19:53:41Z

great work 👍 , will have a look for tomorrow
Another thing to consider is how to deploy, supervisor / workspacekit and ws-daemon has different deploy cycle

kylos101 · 2022-04-07T22:18:41Z

Another thing to consider is how to deploy, supervisor / workspacekit and ws-daemon has different deploy cycle

@iQQBot We use installer to ship workspace-clusters. When we do this, it uses the versions.yaml to parse necessary versions. For example: docker run --rm -it eu.gcr.io/gitpod-core-dev/build/versions:main.2836 cat versions.yaml.ws-daemon and registry-facade, as well as supervisor and workspacekit, will use versions from that file. The configmap for registry-facade will specify supervisor and workspacekit versions.

For WebApp, they currently use helm, but I believe it uses a similar versions strategy, so if we merge to main, all these components should ship too. @geropl , can you confirm?

How do you deploy IDE changes? I see this job updates web app clusters using argo. So it seems like, if we merge to main, IDE should avoid doing IDE deploy until web app has deployed. WDYT?

akosyakov

i have not tried, clean up in supervisor looks good.

utam0k requested a review from a team March 25, 2022 06:47

utam0k requested review from csweichel and aledbf as code owners March 25, 2022 06:47

roboquat added the release-note label Mar 25, 2022

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

roboquat added the size/XXL label Mar 25, 2022

github-actions bot removed the size/XXL label Mar 25, 2022

roboquat added the size/XXL label Mar 25, 2022

princerachit reviewed Mar 25, 2022

View reviewed changes

components/ws-daemon-api/workspace_daemon.proto Outdated Show resolved Hide resolved

princerachit reviewed Mar 25, 2022

View reviewed changes

components/workspacekit/cmd/rings.go Show resolved Hide resolved

princerachit reviewed Mar 25, 2022

View reviewed changes

components/workspacekit/cmd/rings.go Outdated Show resolved Hide resolved

aledbf reviewed Mar 25, 2022

View reviewed changes

components/ws-daemon/nsinsider/main.go Outdated Show resolved Hide resolved

roboquat added the do-not-merge/hold label Mar 25, 2022

aledbf reviewed Mar 27, 2022

View reviewed changes

components/ws-daemon/nsinsider/main.go Outdated Show resolved Hide resolved

aledbf reviewed Mar 27, 2022

View reviewed changes

utam0k mentioned this pull request Mar 28, 2022

[workspacekit] Use veth instead of slirp4netns #8106

Closed

utam0k force-pushed the to/setup-veth branch from 2e65507 to 23346cf Compare March 28, 2022 06:55

kylos101 marked this pull request as draft March 28, 2022 13:55

roboquat added the do-not-merge/work-in-progress label Mar 28, 2022

roboquat removed the do-not-merge/hold label Mar 29, 2022

utam0k marked this pull request as ready for review March 29, 2022 02:48

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

ws-daemon: Update the workspacekit documentation.

99f4f78

utam0k commented Apr 6, 2022

View reviewed changes

utam0k changed the title ~~Enable a pair of veths between a container and a workspace~~ ws-daemon: Use a pair of veths instead of slirp4netns Apr 6, 2022

utam0k marked this pull request as ready for review April 6, 2022 07:46

utam0k requested a review from Furisto as a code owner April 6, 2022 07:46

utam0k requested a review from a team April 6, 2022 07:46

roboquat removed the do-not-merge/work-in-progress label Apr 6, 2022

github-actions bot added the team: IDE label Apr 6, 2022

roboquat added the do-not-merge/hold label Apr 6, 2022

roboquat removed the do-not-merge/hold label Apr 7, 2022

utam0k requested a review from a team April 7, 2022 05:32

csweichel approved these changes Apr 7, 2022

View reviewed changes

aledbf self-requested a review April 7, 2022 19:42

aledbf approved these changes Apr 7, 2022

View reviewed changes

akosyakov approved these changes Apr 8, 2022

View reviewed changes

roboquat merged commit a8d07cb into main Apr 8, 2022

roboquat deleted the to/setup-veth branch April 8, 2022 08:11

akosyakov mentioned this pull request Apr 8, 2022

Revert "ws-daemon: Use a pair of veths instead of slirp4netns" #9194

Merged

roboquat added deployed: workspace Workspace team change is running in production deployed: IDE IDE change is running in production deployed Change is completely running in production labels Apr 8, 2022

kylos101 mentioned this pull request Apr 12, 2022

[ws-daemon] Enable workspace IP forwarding #9270

Merged

shaal mentioned this pull request Apr 16, 2022

Issues accessing Fastly endpoints from within docker that runs in Gitpod #9354

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ws-daemon: Use a pair of veths instead of slirp4netns #8955

ws-daemon: Use a pair of veths instead of slirp4netns #8955

utam0k commented Mar 25, 2022 •

edited

Loading

utam0k commented Mar 25, 2022 •

edited

Loading

utam0k commented Mar 25, 2022

aledbf Mar 27, 2022

utam0k Mar 29, 2022

iQQBot commented Mar 28, 2022

kylos101 commented Mar 28, 2022 •

edited

Loading

kylos101 commented Mar 29, 2022

utam0k commented Mar 29, 2022

utam0k commented Mar 29, 2022

utam0k commented Mar 29, 2022 •

edited

Loading

utam0k Apr 6, 2022

csweichel Apr 7, 2022

utam0k commented Apr 6, 2022

utam0k commented Apr 6, 2022

utam0k commented Apr 7, 2022

utam0k commented Apr 7, 2022

utam0k commented Apr 7, 2022

csweichel left a comment

iQQBot commented Apr 7, 2022 •

edited by werft-gitpod-dev-com bot

Loading

kylos101 commented Apr 7, 2022

akosyakov left a comment

ws-daemon: Use a pair of veths instead of slirp4netns #8955

ws-daemon: Use a pair of veths instead of slirp4netns #8955

Conversation

utam0k commented Mar 25, 2022 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Documentation

utam0k commented Mar 25, 2022 • edited Loading

utam0k commented Mar 25, 2022

aledbf Mar 27, 2022

Choose a reason for hiding this comment

utam0k Mar 29, 2022

Choose a reason for hiding this comment

iQQBot commented Mar 28, 2022

kylos101 commented Mar 28, 2022 • edited Loading

kylos101 commented Mar 29, 2022

utam0k commented Mar 29, 2022

utam0k commented Mar 29, 2022

utam0k commented Mar 29, 2022 • edited Loading

utam0k Apr 6, 2022

Choose a reason for hiding this comment

csweichel Apr 7, 2022

Choose a reason for hiding this comment

utam0k commented Apr 6, 2022

utam0k commented Apr 6, 2022

utam0k commented Apr 7, 2022

Network IO metrics

slirp4netns

veth

CPU metrics

utam0k commented Apr 7, 2022

utam0k commented Apr 7, 2022

csweichel left a comment

Choose a reason for hiding this comment

iQQBot commented Apr 7, 2022 • edited by werft-gitpod-dev-com bot Loading

kylos101 commented Apr 7, 2022

akosyakov left a comment

Choose a reason for hiding this comment

utam0k commented Mar 25, 2022 •

edited

Loading

utam0k commented Mar 25, 2022 •

edited

Loading

kylos101 commented Mar 28, 2022 •

edited

Loading

utam0k commented Mar 29, 2022 •

edited

Loading

iQQBot commented Apr 7, 2022 •

edited by werft-gitpod-dev-com bot

Loading