-
-
Notifications
You must be signed in to change notification settings - Fork 193
feat: allow to pass robot username & password #586
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: main
Are you sure you want to change the base?
Conversation
|
I see that you're using https://github.com/hetznercloud/hcloud-cloud-controller-manager/releases/download/v1.23.0/ccm-networks.yaml We would need to use the helm chart to be able to override the value. Not sure it's easy to do with the current state of the project. Except if you have any other idea ? If not I think the PR is ready then. |
|
Hi, and thanks for the PR. In its current state it seems to only set the credentials for Robot without changing the manifest, and as you point out using the Helm chart would be required for this to work. So it seems the PR is incomplete? If we switch to the Helm chart for the CCM, we need to ensure it can work with existing clusters that were created using the manifest for the CCM. |
|
Took another, safer road. I think it's now complete! |
|
The thing is that I was planning to refactor that code so to download the manifest and process it as YAML rather than text, since I have other things I need to do with it. Do you want to take a stab at that? |
|
Done! |
|
Thanks! This is more like what I had in mind. I will need to test it a bit when I have some time before merging so please bear with me in the meantime. |
|
It worked on public only network but I realized it didn't on private networks. It's now fixed but it requires a proper CNI system. It works with Cilium, haven't tried the rest: See explanations here Also updated cloud controller to 1.24 as it improves robot support. https://github.com/hetznercloud/hcloud-cloud-controller-manager/releases/tag/v1.24.0 |
| if settings.networking.private_network.enabled | ||
| network_routes_enabled = YAML::Any.new({ | ||
| YAML::Any.new("name") => YAML::Any.new("HCLOUD_NETWORK_ROUTES_ENABLED"), | ||
| YAML::Any.new("value") => YAML::Any.new("false"), | ||
| }) | ||
| env_array << network_routes_enabled | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
support for private networks here
|
Thanks for taking this on. It needs some thorough testing, but I'm pretty swamped with work right now, so the merge will have to wait a bit. |
|
Makes sense! |
|
In the meantime added some documentation (that should help you test it) & made sure the CSI driver doesn't run on dedicated nodes. |
|
Sorry for the delay. I'm still putting this PR on hold because I'm planning to implement a more integrated and native way to add dedicated servers to a cluster with hetzner-k3s. I want it to be more like how we currently do it for other worker nodes. There's a chance I might use some code from your PR, since you've done some work in that area. However, I might close your PR if my implementation differs too much. I’ll keep you updated when I can dive into this properly. Unfortunately, my day job is keeping me really busy right now, so I don’t have much time for other things at the moment. |
|
Thanks for your feedback. I'm not exactly sure what you have in mind but I do think we need to be able to customize dedicated a lot more than what we do with cloud nodes. How we install disks, raid 0, raid 1, etc... So having that flexibility is quite important in my opinion, especially when you want to add dedicated for databases. |
|
I understand. I hadn’t really considered how much customization might be needed with dedicated nodes. Let me break down a few key points so I can get a better grasp on your changes:
Thanks! |
|
|
|
Done! Also added a note about other CNIs |
|
|
Thanks! Now, I just need to find the time to thoroughly test this. Unfortunately, my day job is keeping me too busy at the moment. :( |
|
Perfect !! |
|
Hi @Mokto, I am close to finishing some big refactoring (mostly about improving code quality) that I have been working on for a while. After that, I will start adding your PR. But there might be some conflicts in this PR when I merge my changes. Will you be okay with helping to fix those conflicts? I am sorry this is taking so long. |
|
I will try yes! |
|
Awesome! I'll try to finish the refactoring tonight if I can. |
|
I finally merged the refactoring :) |
|
Could you explain shortly what you did in the refactoring please? |
|
Hi @Mokto - it's just some work to make the code better and more organized. I reduced the complexity of some classes by moving parts of the code into separate classes. Things like that. No functionality has changed. |
|
Hi! Would you mind fixing the conflicts and adjust the changes according to the changes in v2.4.0 which I just released? Thanks! |
|
Now I'm the one that is super busy at work 👍 |
|
I merged everything and fixed the merge conflicts here: https://github.com/clouedoc/hetzner-k3s/tree/robot-username-password-merged It builds, and I'm trying to figure out how to use it now :-) |
|
I cannot get it to work yet. It sets the configuration option in the "hcloud" secret correctly: They seem to be passed to the Hetzner Cloud Controller Manager pod successfully: Yet my node is getting deleted anyway: So perhaps we're not configuring HCCM enough? We need it to list Robot servers and pick that as a list of servers to not delete. Actually I'd be happy if it didn't delete any servers from the list at all. |
|
The issue is that this code is not doing what it's supposed to: if settings.responds_to?(:robot_user) && settings.robot_user
documents.each do |doc|
next unless doc["kind"]?.try(&.as_s) == "Deployment"
next unless doc["metadata"]?.try(&.["name"]?.try(&.as_s)) == "hcloud-cloud-controller-manager"
containers_any = doc["spec"]?.try(&.["template"]?.try(&.["spec"]?.try(&.["containers"]?)))
next unless containers_any && (containers_array = containers_any.as_a?)
container_any = containers_array[0]?
next unless container_any && (container_hash = container_any.as_h?)
env_array = container_hash[YAML::Any.new("env")]?.try(&.as_a) || [] of YAML::Any
robot_enabled = YAML::Any.new({
YAML::Any.new("name") => YAML::Any.new("ROBOT_ENABLED"),
YAML::Any.new("value") => YAML::Any.new("true"),
})
env_array << robot_enabled
if settings.networking.private_network.enabled
network_routes_enabled = YAML::Any.new({
YAML::Any.new("name") => YAML::Any.new("HCLOUD_NETWORK_ROUTES_ENABLED"),
YAML::Any.new("value") => YAML::Any.new("false"),
})
env_array << network_routes_enabled
end
container_hash[YAML::Any.new("env")] = YAML::Any.new(env_array)
containers_array[0] = YAML::Any.new(container_hash)
end
end(not exactly sure where to start debugging this rather than maybe checking if it's running at all) If I patch my HCCM deployment by adding the following env vars: - name: ROBOT_ENABLED
value: "true"
- name: HCLOUD_NETWORK_ROUTES_ENABLED
value: "false"Then I finally get a promising error in the logs: |
|
... at least my Node is not getting deleted anymore, I'll try deploying some stuff on it to see if it works |
|
@Mokto Hi, will you have a chance to fix the PR or shall I close it? Thanks |



No description provided.