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

feat(gencommand) Generate talosctl apply and upgrade commands with context of talconfig.yaml #213

Merged
merged 11 commits into from
Nov 20, 2023
Merged

feat(gencommand) Generate talosctl apply and upgrade commands with context of talconfig.yaml #213

merged 11 commits into from
Nov 20, 2023

Conversation

mircea-pavel-anton
Copy link
Collaborator

@mircea-pavel-anton mircea-pavel-anton commented Nov 18, 2023

This PR aims to implement a new gencommand subcommand which will simplify managing Talos clusters by addressing the pain points described in #214 and #215


Fixes #214
Fixes #215

@mircea-pavel-anton mircea-pavel-anton marked this pull request as draft November 18, 2023 18:08
@mircea-pavel-anton mircea-pavel-anton changed the title talhelper gencommand feat(gencommand) Generate talosctl apply and upgrade commands with context of talconfig.yaml Nov 18, 2023
cmd/gencommand.go Outdated Show resolved Hide resolved
cmd/gencommand.go Outdated Show resolved Hide resolved
@budimanjojo
Copy link
Owner

Thanks @mirceanton! I have commented on the code. And I also think that the --insecure flag in the generated talosctl apply-config command should be omitted and let the user add it themselves? For me, the IP address of the node is not 100% the value of node[].ipAddress on bootstrapping but it will most likely be the IP address of the node once bootstrapping is completed. So the generated command function is more for applying changes to the node instead of bootstrapping a node? Please give your inputs on this.

@mircea-pavel-anton
Copy link
Collaborator Author

@budimanjojo with commit 05b8e40 I added the ability to inject flags into the generated command(s). This way, --insecure is no longer hardcoded but can be injected manually by the user if it so happens that in their setup the node[].ipAddress is the actual ip at which the node is found during the bootstrap process.

Thus, by default the command will handle applying changes:

root@talhelper-devbox:/workspace# talhelper gencommand --apply  
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.195 --file=./clusterconfig/home-cluster-cp-01.dev.k8s.mirceanton.com.yaml ;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.194 --file=./clusterconfig/home-cluster-cp-02.dev.k8s.mirceanton.com.yaml ;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.193 --file=./clusterconfig/home-cluster-cp-03.dev.k8s.mirceanton.com.yaml ;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.192 --file=./clusterconfig/home-cluster-wkr-01.dev.k8s.mirceanton.com.yaml ;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.191 --file=./clusterconfig/home-cluster-wkr-02.dev.k8s.mirceanton.com.yaml ;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.190 --file=./clusterconfig/home-cluster-wkr-03.dev.k8s.mirceanton.com.yaml ;

But it can be configured to "bootstrap" as well:

root@talhelper-devbox:/workspace# talhelper gencommand --apply --extra-flags=--insecure                           
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.195 --file=./clusterconfig/home-cluster-cp-01.dev.k8s.mirceanton.com.yaml --insecure;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.194 --file=./clusterconfig/home-cluster-cp-02.dev.k8s.mirceanton.com.yaml --insecure;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.193 --file=./clusterconfig/home-cluster-cp-03.dev.k8s.mirceanton.com.yaml --insecure;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.192 --file=./clusterconfig/home-cluster-wkr-01.dev.k8s.mirceanton.com.yaml --insecure;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.191 --file=./clusterconfig/home-cluster-wkr-02.dev.k8s.mirceanton.com.yaml --insecure;
talosctl apply-config --talosconfig=./clusterconfig/talosconfig --nodes=10.0.10.190 --file=./clusterconfig/home-cluster-wkr-03.dev.k8s.mirceanton.com.yaml --insecure;

I think this way we can cover both scenarios in one shot

@budimanjojo
Copy link
Owner

@mirceanton looks good! one more minor change and I'll merge this. Thanks!

@mircea-pavel-anton
Copy link
Collaborator Author

@budimanjojo I resolved all of the comments. I think this should be ok now.

Furthermore, I noticed that we would have the exact same chunk of code repeated ~3 times identically, so I thought in 5072810 that we may be better off extracting it to a parse function. Not sure if this is the best idea, naming or even place/package to put the file in, so feel free to revert the last commit if you don't agree with these changes!

@mircea-pavel-anton mircea-pavel-anton marked this pull request as ready for review November 20, 2023 09:43
@budimanjojo
Copy link
Owner

@mirceanton Yeah that config reading code is being repeated a lot and I have plan to make it exported like u did here but still thinking on how to structure it nicely. I'll merge this now. Thanks for the contribution ❤️

@budimanjojo budimanjojo merged commit b11566e into budimanjojo:master Nov 20, 2023
2 checks passed
@mircea-pavel-anton mircea-pavel-anton deleted the feat/gencommand branch November 20, 2023 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SImplify Talos upgrades Simplify applying the generated configuration
2 participants