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

[provider-local] Harmonize local VPN setup with real-world scenario #9752

Merged
merged 11 commits into from
May 29, 2024

Conversation

rfranzke
Copy link
Member

@rfranzke rfranzke commented May 14, 2024

How to categorize this PR?

/area dev-productivity
/kind enhancement

What this PR does / why we need it:
Currently, in the local scenario, some pods talk to the machine pods directly (instead of using the VPN tunnel). See the referenced issue for a more detailed description.

This PR harmonizes the local VPN setup by specifying the node network for Shoots and creating a dedicated IP pool.
With this, the VPN components correctly configure IP routes for talking to the shoot node network.
As a consequence, all traffic correctly traverses the VPN tunnel and gardenlet's tunnel health check reliably detects a broken tunnel.

Which issue(s) this PR fixes:
Part of #9604
Fixes #9020
See also: https://github.com/gardener-community/hackathon/blob/main/2024-05_Schelklingen/README.md#-harmonize-local-vpn-setup-with-real-world-scenario

Special notes for your reviewer:
/cc @timebertt

We need to do some workarounds for making the e2e upgrade tests pass for this specific version. The workarounds are only active until the next minor release.

Release note:

The `allow-shoot-networks` `NetworkPolicy` has been dropped entirely, hence, the `networking.gardener.cloud/to-shoot-networks=allowed` label has no effect anymore and should be removed.

@gardener-prow gardener-prow bot requested a review from timebertt May 14, 2024 16:09
Copy link
Contributor

gardener-prow bot commented May 14, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@gardener-prow gardener-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. area/dev-productivity Developer productivity related (how to improve development) kind/enhancement Enhancement, improvement, extension cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 14, 2024
@timebertt
Copy link
Member

/test pull-gardener-e2e-kind pull-gardener-e2e-kind-ipv6

@rfranzke rfranzke force-pushed the hackathon-provider-local-vpn branch 2 times, most recently from 928e29f to a483ac7 Compare May 14, 2024 16:58
@timebertt
Copy link
Member

/test pull-gardener-e2e-kind pull-gardener-e2e-kind-ipv6

@rfranzke rfranzke force-pushed the hackathon-provider-local-vpn branch from a483ac7 to d8873a1 Compare May 14, 2024 18:24
@timebertt
Copy link
Member

/test pull-gardener-e2e-kind pull-gardener-e2e-kind-ipv6

@timebertt timebertt force-pushed the hackathon-provider-local-vpn branch 2 times, most recently from 305949a to 6259b95 Compare May 14, 2024 20:14
@timebertt timebertt marked this pull request as ready for review May 14, 2024 21:22
@gardener-prow gardener-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 14, 2024
@timebertt
Copy link
Member

timebertt commented May 15, 2024

We figured that we must perform the NetworkPolicy-related changes (dropping the allow-shoot-networks NetworkPolicy) in the next release. Otherwise, the upgrade tests will fail because the VPN will be broken after the upgrade.

EDIT: we added workarounds to make the upgrade tests past even we perform the NetworkPolicy-related changes immediately.

@timebertt timebertt force-pushed the hackathon-provider-local-vpn branch 6 times, most recently from 90822ed to 1caa265 Compare May 16, 2024 18:38
@gardener-prow gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2024
@ScheererJ
Copy link
Contributor

/assign

Copy link
Contributor

@ScheererJ ScheererJ left a comment

Choose a reason for hiding this comment

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

Thanks for bringing the local setup closer to Gardener in the real world.

It looks like there are still some end-to-end tests failing, though.

pkg/provider-local/controller/infrastructure/actuator.go Outdated Show resolved Hide resolved
pkg/provider-local/controller/infrastructure/actuator.go Outdated Show resolved Hide resolved
pkg/provider-local/controller/infrastructure/actuator.go Outdated Show resolved Hide resolved
pkg/provider-local/controller/worker/machines.go Outdated Show resolved Hide resolved
@timebertt timebertt self-assigned this May 23, 2024
@timebertt timebertt force-pushed the hackathon-provider-local-vpn branch from 1caa265 to 993a8bd Compare May 23, 2024 06:33
@rfranzke rfranzke force-pushed the hackathon-provider-local-vpn branch from 63bca2e to dca9c39 Compare May 28, 2024 07:43
@gardener-prow gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 28, 2024
@rfranzke rfranzke force-pushed the hackathon-provider-local-vpn branch from dca9c39 to 220cc9e Compare May 28, 2024 10:45
@ScheererJ
Copy link
Contributor

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 28, 2024
Copy link
Contributor

gardener-prow bot commented May 28, 2024

LGTM label has been added.

Git tree hash: bfc26186529edb2e4164aba58322fc8b01aaf476

@rfranzke rfranzke force-pushed the hackathon-provider-local-vpn branch from 220cc9e to 7218483 Compare May 28, 2024 13:59
@gardener-prow gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label May 28, 2024
rfranzke and others added 11 commits May 28, 2024 16:13
no longer needed now that MCM-provider-local no longer deploys a `Service`
This network policy is not needed since packets to the shoot networks are always encapsulated in the VPN tunnel and never handled by the seed network policies.

Co-authored-by: Tim Ebert <[email protected]>
The shoot networks are always "contacted" via the VPN tunnel (which is established FROM the machine pods TO the `vpn-seed-server`).

Co-authored-by: Tim Ebert <[email protected]>
Co-Authored-By: Rafael Franzke <[email protected]>
Co-Authored-By: Marcel Boehm <[email protected]>
@rfranzke rfranzke force-pushed the hackathon-provider-local-vpn branch from 7218483 to e4f54b4 Compare May 28, 2024 14:13
@ScheererJ
Copy link
Contributor

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label May 28, 2024
Copy link
Contributor

gardener-prow bot commented May 28, 2024

LGTM label has been added.

Git tree hash: 3126d936a08bf1c4238dcf512c1b1fd428a9d7e4

@gardener-prow gardener-prow bot merged commit 3b61cb9 into gardener:master May 29, 2024
18 checks passed
@rfranzke rfranzke deleted the hackathon-provider-local-vpn branch May 29, 2024 06:30
@rfranzke
Copy link
Member Author

Kaum macht man's richtig, schon geht's 😉

rfranzke added a commit to rfranzke/gardener that referenced this pull request May 31, 2024
vpnachev added a commit to gardener/gardener-extension-shoot-oidc-service that referenced this pull request May 31, 2024
rfranzke added a commit to rfranzke/gardener that referenced this pull request May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/dev-productivity Developer productivity related (how to improve development) cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/enhancement Enhancement, improvement, extension lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
3 participants