fix(routes): Only delete routes in the Cluster CIDR #432
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We previously required that the route we were about to delete had a valid server. This does not make sense because in most cases we delete the route precisely because the server was deleted. Related to #425
We implemented a route cleanup in #238 that deletes all routes in the network that can not be mapped to the private IP of a node. This causes issues for users who create their own routes, e.g. to an alias IP. We do not need to implement this ourselves, because `kubernetes/cloud-provider` already implemented all the checks for us, with more consideration about which routes actually need to be deleted. In particular, the ccm should only delete routes that are within the configured Cluster CIDR range.
samcday
approved these changes
Apr 27, 2023
apricote
pushed a commit
that referenced
this pull request
Jun 16, 2023
🤖 I have created a release *beep* *boop* --- ## [1.16.0](v1.15.0-rc.0...v1.16.0) (2023-06-16) ### Features * **helm:** allow to manually set network name or ID ([#458](#458)) ([8410277](8410277)) ### Bug Fixes * **ci:** qemu binfmt wrappers during release ([#421](#421)) ([84a7541](84a7541)) * **routes:** Only delete routes in the Cluster CIDR ([#432](#432)) ([c35d292](c35d292)) ### Continuous Integration * setup release-please ([#437](#437)) ([bbec89e](bbec89e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Hetzner Cloud Bot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a two-part fix for #425.
Closes #425
More details can be found in this comment
Part 1: fix(routes): correctly handle delete for routes without server
We previously required that the route we were about to delete had a valid server. This does not make sense because in most cases we delete the route precisely because the server was deleted.
Part 2: fix(routes): let k/cloud-provider decide which routes to delete
We implemented a route cleanup in #238 that deletes all routes in the network that can not be mapped to the private IP of a node. This causes issues for users who create their own routes, e.g. to an alias IP.
We do not need to implement this ourselves, because
kubernetes/cloud-provideralready implemented all the checks for us, with more consideration about which routes actually need to be deleted.In particular, the ccm should only delete routes that are within the configured
--cluster-cidrrange.