-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Hetzner(feat): Add ability to specify a subnet for autoscaled node placement #8334
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
base: master
Are you sure you want to change the base?
Conversation
Welcome @tloesch! |
Hi @tloesch. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tloesch The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Tests / test-and-verify (pull_request) fails. EDIT: Found the issue in middle of the CI run. i fixed it by formatting files |
Hey, thanks for the contribution. I will take a look at your PR ^^ |
Hey, before I go into detail about the code I would like to discuss the overall concept. Your way of storing the IPs in the Servers labels, will cause issues with multiple clusters (including an autoscaler) in the same Hetzner Cloud project. A simpler method could be to fetch the necessary data from the API (or cache) and evaluate, which IPs are free to use before starting the Server creation process. These can then be assigned to each Server in the Create call and you should be fine. This would not clash with other clusters in the project and simply the implementation. I would like to ask you to revise your concept. If you have any questions feel free to ask them. Best Regards |
Seems more robust. I will change the implementation and mention you when i am done. EDIT: The code is a bit older and I responded a little too hastily. The IP addresses are already obtained from the API/cache. Please let me know whether I should remove the local map object or whether we should find another, better conceptual solution. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR enables the optional specification of a subnet where autoscaled nodes will be placed. Previously, it was only possible to define a network ID, which caused issues when multiple subnets existed within that network.
Without subnet specification, Hetzner would place nodes randomly in one of the available subnets, leading to unpredictable network configurations and potential connectivity problems. This feature provides more precise control over node placement.
The implementation ensures that nodes receive consistent IP addresses from the specified subnet through a reservation mechanism that prevents IP conflicts during scaling operations.
Which issue(s) this PR fixes:
Fixes #5263
Special notes for your reviewer:
Notice
I am not experienced in Golang development. Please provide me with guidance on necessary improvements. I will then try to implement them as best as possible.
Concept description:
The IP Reserver operates using a thread-safe data structure that manages all previously assigned IP addresses within a subnet. When creating a new server, the process follows a defined sequence:
cluster-autoscaler/reserved-ip
) and internally asmap[string]net.IP
.Does this PR introduce a user-facing change?
NONE