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.

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

[ws-proxy] Decouple ws-proxy from ws-manager #6462

Merged
merged 8 commits into from
Nov 5, 2021
Merged

[ws-proxy] Decouple ws-proxy from ws-manager #6462

merged 8 commits into from
Nov 5, 2021

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Oct 30, 2021

Description

This refactoring removes the use of services to export workspace ports, using the pod directly instead.
Changing the default behavior, we do not rely on DNS to route traffic to the workspace (ws-..cluster.local:) anymore, and instead, we use the pod IP address. This removes issues related to DNS queries, timeous, or UDP connection tracking (iptables).
A side effect of this change is kube-dns do not store information about the workspaces (services) anymore. This will reduce the load on that component.

Changes:

  • Decouple ws-proxy from ws-manager
  • Remove GRPC from ws-proxy
  • Workspace details are extracted from pods
  • Workspace exposed ports are published on the pod itself (annotation gitpod/exposedPorts)
  • There are no more workspace services

Related Issue(s)

fixes #6441
fixes #6325

How to test

Open a workspace. The improvement of speed to expose ports should be noticeable.
Executing kubectl get svc should not return any services related to workspaces.

Things to test:

  • start a server in the workspace, expose the port and access it
  • stop the server and see the port go away
  • test local VS code and auto-tunneling
  • try experimentalNetwork: true in the .gitpod.yml

Release Notes

[ws-proxy] Decouple ws-proxy from ws-manager

@aledbf
Copy link
Member Author

aledbf commented Oct 30, 2021

/werft run

👍 started the job as gitpod-build-aledbf-wsproxy.2

@codecov
Copy link

codecov bot commented Oct 30, 2021

Codecov Report

Merging #6462 (f016151) into main (fc5ba3d) will increase coverage by 30.42%.
The diff coverage is 60.36%.

❗ Current head f016151 differs from pull request most recent head 6c31254. Consider uploading reports for the commit 6c31254 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##            main    #6462       +/-   ##
==========================================
+ Coverage   7.09%   37.51%   +30.42%     
==========================================
  Files         10       19        +9     
  Lines        930     4555     +3625     
==========================================
+ Hits          66     1709     +1643     
- Misses       862     2713     +1851     
- Partials       2      133      +131     
Flag Coverage Δ
components-supervisor-app 37.51% <60.36%> (?)
installer-raw-app ?

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

Impacted Files Coverage Δ
components/supervisor/pkg/dropwriter/dropwriter.go 73.46% <ø> (ø)
components/supervisor/pkg/ports/exposed-ports.go 0.00% <0.00%> (ø)
components/supervisor/pkg/ports/ports-config.go 80.00% <ø> (ø)
components/supervisor/pkg/ports/tunnel.go 63.63% <0.00%> (ø)
components/supervisor/pkg/supervisor/config.go 4.23% <ø> (ø)
components/supervisor/pkg/supervisor/git.go 0.00% <ø> (ø)
components/supervisor/pkg/supervisor/services.go 23.82% <0.00%> (ø)
components/supervisor/pkg/supervisor/ssh.go 0.00% <0.00%> (ø)
components/supervisor/pkg/supervisor/supervisor.go 6.19% <0.00%> (ø)
components/supervisor/pkg/supervisor/user.go 33.12% <ø> (ø)
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61ea972...6c31254. Read the comment docs.

@roboquat roboquat added the team: webapp Issue belongs to the WebApp team label Oct 30, 2021
@aledbf aledbf requested a review from csweichel October 30, 2021 17:48
@aledbf aledbf force-pushed the aledbf/wsproxy branch 5 times, most recently from 6e2d4c5 to 3b3f2d1 Compare October 30, 2021 20:09
@aledbf aledbf changed the title WIP [ws-proxy] Decouple ws-proxy from ws-manager [ws-proxy] Decouple ws-proxy from ws-manager Oct 30, 2021
@aledbf
Copy link
Member Author

aledbf commented Nov 1, 2021

/werft run

👍 started the job as gitpod-build-aledbf-wsproxy.24

components/ws-manager-api/go/exposed_ports.go Show resolved Hide resolved
Comment on lines -73 to -74
ServiceTemplate string `json:"serviceTemplate"`
PortServiceTemplate string `json:"portServiceTemplate"`
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@aledbf aledbf force-pushed the aledbf/wsproxy branch 2 times, most recently from 1f8928b to 6e5b3f8 Compare November 2, 2021 10:25
@aledbf
Copy link
Member Author

aledbf commented Nov 2, 2021

/werft run

👍 started the job as gitpod-build-aledbf-wsproxy.48

@aledbf
Copy link
Member Author

aledbf commented Nov 2, 2021

/werft run

👍 started the job as gitpod-build-aledbf-wsproxy.49

@csweichel
Copy link
Contributor

/hold cancel

@csweichel
Copy link
Contributor

csweichel commented Nov 4, 2021

Tested these things:

  • start a server in the workspace, expose the port and access it
  • stop the server and see the port go away
  • test local VS code and auto-tunneling
  • try experimentalNetwork: true in the .gitpod.yml

All worked like a treat, and much quicker than before ✅

@csweichel
Copy link
Contributor

/approve

@JanKoehnlein
Copy link
Contributor

/lgtm

Codewise, I admit I didn't test

@JanKoehnlein
Copy link
Contributor

/lgtm

@roboquat
Copy link
Contributor

roboquat commented Nov 4, 2021

LGTM label has been added.

Git tree hash: c6f3fd7c2a67d17a744a3d2f2d21278ecb55c413

@corneliusludmann
Copy link
Contributor

/approve

looks good and works like a charm! 🚀

@roboquat
Copy link
Contributor

roboquat commented Nov 5, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: corneliusludmann, csweichel, JanKoehnlein

Associated issue: #6441

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@roboquat roboquat merged commit fe5bc40 into main Nov 5, 2021
@roboquat roboquat deleted the aledbf/wsproxy branch November 5, 2021 09:33
@roboquat roboquat added the deployed: webapp Meta team change is running in production label Nov 19, 2021
@aledbf aledbf restored the aledbf/wsproxy branch October 13, 2022 17:30
@aledbf aledbf deleted the aledbf/wsproxy branch December 8, 2022 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved deployed: webapp Meta team change is running in production release-note size/XXL team: IDE team: webapp Issue belongs to the WebApp team team: workspace Issue belongs to the Workspace team
Projects
None yet
7 participants