gardener / vpn2

Network connector between the control plane (deployed in a Seed cluster) and a Shoot cluster superseding the vpn repository.
Apache License 2.0
5 stars 21 forks source link

Rewrite bash scripts into go #84

Closed dergeberl closed 3 months ago

dergeberl commented 5 months ago

Open Tasks:

What this PR does / why we need it:

This PR rewrites the bash scripts into go. This will made it easier to adopt to new features in the future.

Co-authored-by: @MartinWeindel Co-authored-by: @hown3d Co-authored-by: @mreiger

Which issue(s) this PR fixes: Fixes #

Special notes for your reviewer: ⚠️ The pipeline jobs for building images are failing as changes in .ci/pipeline_definitions are not yet used. They become only active after merging to master branch.

⚠️ This is a breaking change and needs also changes to gardener/gardener

Release note:

Rewrite bash scripts into go. 
Log output changed. Expected ENVs from the containers changed. More memory is needed because of the change to go.
gardener-robot commented 5 months ago

@dergeberl Thank you for your contribution.

gardener-robot-ci-3 commented 5 months ago

Thank you @dergeberl for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below.

MartinWeindel commented 5 months ago

Regarding memory usage these are my major findings:

vpn-seed-server-665cf779b9-52vwb             3m           19Mi
---
vpn-shoot-f956748d-gv5h5                     5m           1Mi

Without the script, on Linux the memory consumption was ~ 6Mi

gardener-robot commented 5 months ago

@dergeberl You need rebase this pull request with latest master branch. Please check.

axel7born commented 3 months ago

With ha vpn we have the shoot-client container running in the seed in the kube-apiserver pod. This is already confusing. The path-controller is only needed in seed. Maybe, we should we use the opportunity to separate seed-client and shoot client? Or change the name from shoot-client to vpn-client?

MartinWeindel commented 3 months ago

With ha vpn we have the shoot-client container running in the seed in the kube-apiserver pod. This is already confusing. The path-controller is only needed in seed. Maybe, we should we use the opportunity to separate seed-client and shoot client? Or change the name from shoot-client to vpn-client?

As we need to rename the images anyway to be able to switch back to the old implementation, I propose to use vpn-client instead of shoot-client for package and image name and similar vpn-server instead of seed-server.

I don't see a big win in separating "seed-client" and "shoot-client", as they share many things.

axel7born commented 3 months ago

/lgtm

MartinWeindel commented 3 months ago

/lgtm