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

Generate per node factory url for genurl commands #211

Closed
budimanjojo opened this issue Nov 18, 2023 · 6 comments · Fixed by #212
Closed

Generate per node factory url for genurl commands #211

budimanjojo opened this issue Nov 18, 2023 · 6 comments · Fixed by #212

Comments

@budimanjojo
Copy link
Owner

Right now, talhelper genurl iso and talhelper genurl installer doesn't actually generate the url based on what you put in talconfig.yaml file. It makes more sense if the generated urls can be taken from config file. Let's say I have this block inside my talconfig.yaml:

talosVersion: v1.5.5
nodes:
  - hostname: machine1
    schematic:
       customization:
         extraKernelArgs:
           - net.ifnames=0
         systemExtensions:
           officialExtensions:
             - siderolabs/intel-ucode
             - siderolabs/tailscale
  - hostname: machine2

And when I run talhelper genurl installer from a path where the talconfig.yaml is found, it should return something like this:

machine1: factory.talos.dev/installer/e019fb5f6bf0a685e805a1263323cf8c38feefdc5a4cecbefbd5ba58e4383a62:v1.5.5
machine2: factory.talos.dev/installer/376567988ad370138ad8b2698212367b8edcb69b5fd68c80be1f2ec7d603b4ba:v1.5.5

But, when I run the talhelper genurl installer where there's no talconfig.yaml found, it shouldn't error out. Instead return something like this:

factory.talos.dev/installer/376567988ad370138ad8b2698212367b8edcb69b5fd68c80be1f2ec7d603b4ba:v1.5.5

This change will affect --extension and --kernel-args flags to be ignored when talconfig.yaml is found because you want to use the ones defined in the config file instead.

This will make upgrading Talos using talosctl upgrade so much easier, i.e you want to update machine1, you will run:

talosctl upgrade -n <machine1> --image factory.talos.dev/installer/e019fb5f6bf0a685e805a1263323cf8c38feefdc5a4cecbefbd5ba58e4383a62:v1.5.5

where the --image is gotten from the talhelper genurl installer command above.

@mircea-pavel-anton
Copy link
Collaborator

mircea-pavel-anton commented Nov 18, 2023

I think a flag should also be added to the talhelper genurl installer so that we can get the image name only and only for one machine. That way, we can do something like this and cut the middle man:

talosctl upgrade -n <machine1> --image $(talhelper genurl installer -n <machine1>)

Alternatively, an even nicer approach would be a talhelper gencommand upgrade which would simply output the talosctl upgrade command for a given node (using the hostname perhaps) with the right image, so we can do something like:

talhelper gencommand upgrade | bash

With a gencommand subcommand like this, talhelper will not "replace" talosctl but it will simply be, as the name implies, a helper tool to use talosctl more efficiently.

This could also be expanded to handle the apply-config by simply rendering the appropriate talosctl apply command -n <machine1> -f ./clusterconfig/<machine1>.yaml and then we can just talhelper gencommand apply | bash

I think this would simplify the workflow, allowing us to just do:

talhelper genconfig;
talhelper gencommand apply | bash

to handle the initial bootstrap and respectively

talhelper gencommand upgrade | bash

Whenever we need to upgrade the OS

@budimanjojo
Copy link
Owner Author

@mirceanton Thanks! Really good suggestion. Do u think I should do both genurl --node flag and gencommand or only one of them? Because genurl already exists as a subcommand.

@mircea-pavel-anton
Copy link
Collaborator

mircea-pavel-anton commented Nov 18, 2023

@budimanjojo Given that the gencommand will likely ouput a "bare" version of the command, there is value to have the genurl command as well if we want to use some flags for some nodes and not for others, i.e. something like this would not be doable with the gencommand:

talosctl upgrade \
  --node <machine1> \
  --image $(talhelper genurl installer --node <machine1>) \
  --wait=false \
  --force=true \
  --reboot-mode=powercycle
  
talosctl upgrade \
  --node <machine2> \
  --image $(talhelper genurl installer --node <machine2>) \
  --wait=true

I think this would be easier and more maintainable than trying to implement a way to pass in flags per-node to gencommand.

While most users will likely be fine with the gencommand output, those that want more control or have more intricate setups will probably opt for the genurl subcommand anyway to give them the most flexibility

@budimanjojo
Copy link
Owner Author

I'll add --node flag for genurl subcommand first then, probably tomorrow. Thanks for the input @mirceanton!

@mircea-pavel-anton
Copy link
Collaborator

@budimanjojo Thanks!

Should I open a new issue for the gencommand feature request?

@budimanjojo
Copy link
Owner Author

@mirceanton yes please do!

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 a pull request may close this issue.

2 participants