add kubelet configuration #1653
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (8)
- agents.tf: Language not supported
- autoscaler-agents.tf: Language not supported
- control_planes.tf: Language not supported
- kube.tf.example: Language not supported
- locals.tf: Language not supported
- modules/host/main.tf: Language not supported
- modules/host/variables.tf: Language not supported
- variables.tf: Language not supported
|
Thanks @Atrox, that's a better way of configuring it than what we have now with the command line options |
There was a problem hiding this comment.
Overall, this PR is close to completion but there are a couple of issues that need to be addressed:
- Mismatch in filenames for the kubelet config: in
autoscaler-agents.tf, you write the file to/tmp/kubelet.yaml, but the update script compares/tmp/kubelet-config.yamlto/etc/rancher/k3s/kubelet-config.yaml. That mismatch means the config update script will never find the newly written file. They must match. - Initial missing kubelet-config.yaml case: In the
k3s_kubelet_config_update_script, there's an unconditional backup step (cp /etc/rancher/k3s/kubelet-config.yaml …). If it's the very first time and that file doesn't exist yet,cpwill fail. Consider gracefully handling the "file does not exist" case or only backing up if it exists.
With those fixes, this should be good to go. Thanks for your work!
553eeb2 to
6d59b86
Compare
|
Fixed the issues. Thanks for the catch regarding the autoscaler. I don't use those myself so I didn't notice that typo... |
|
@Atrox Again thanks for this. There's one thing though, maybe you are on windows and using a different kind of line ending. Because the diffs change whole blocks and files, it's hard to pin point what actually really changed. Please set your file editor to edit in unix mode, that way I can better review the PR. |
|
@mysticaltech i don't think the issue in that specific block is line endings. I formatted the file, so that all the lines match up with the '='. The same way it currently is, just tried to follow the project style. Should I remove the formatting? edit: fwiw, I did the PR on unix |
|
Ah I see @Atrox . In that case, please just run |
|
@mysticaltech fmt doesn't change any file for me. |
6d59b86 to
1548db4
Compare
|
@mysticaltech did a rebase with master + terraform fmt, alignment with |
|
No worries, thanks for trying, I will test it ASAP and merge! |
| user = "root" | ||
| private_key = var.ssh_private_key | ||
| agent_identity = local.ssh_agent_identity | ||
| host = hcloud_server.server.ipv4_address |
There was a problem hiding this comment.
I believe the host should be
host = coalesce(hcloud_server.server.ipv4_address, hcloud_server.server.ipv6_address, try(one(hcloud_server.server.network).ip, null))
to make sure, also private Kubernetes nodes with public IPv4 disabled can be accessed? I derived it from the resource "null_resource" "registries" part.
Example: