Closed solidDoWant closed 2 weeks ago
Thanks! The one thing that I don't like about this is the skipped validation for the templated value and the only way to satisfy my dislike is to call the labels.Validate
function again on those templated value, which looks ugly because the validation step is passed a long time ago. And there's no neat way to do it because the template data is from the already generated talos.Provider
which is already near the end of the program.
the skipped validation
While this is skipped, the tool will fail with a validation error after the config has been generated. For example, given the following config:
nodeLabels:
installerUrl: '{{ .MachineConfig.MachineInstall.InstallImage }}'
The tool will fail, prior to generating configs, with:
2024/11/12 22:05:13 failed to generate talos config: 1 error occurred:
* invalid machine node labels: 1 error occurred:
* label value length exceeds limit of 63: "factory.talos.dev/installer-secureboot/67a999813ff49ba00dc632aa48170dc866bda646b7ee4f944e2fb7dc50b522bd:v1.7.1"
This is less descriptive than the pre-generation validation, but it is still some form of validation.
While this is skipped, the tool will fail with a validation error after the config has been generated.
Ah yeah that's validation from the Talos side of Validate()
function at the end of the program.
Thanks @solidDoWant! I think everything is good now. I'll merge this once you're done with the tests and docs (I can do the docs if you don't feel like doing this). Also can you squash the commit as feat(genconfig): support templating node labels and annotations
? Thank you!
Added tests in https://github.com/budimanjojo/talhelper/pull/701/commits/fc1a8b80b375cd1c1743af1d53aabad9eb97419e. How/where would you like documentation added? I'm not quite sure which pages should have this information added, how it should be formatted, etc.
Also can you squash the commit as
feat(genconfig): support templating node labels and annotations
?
I'll be sure to squash and rename after final review.
How/where would you like documentation added? I'm not quite sure which pages should have this information added, how it should be formatted, etc
You can put it inside https://github.com/budimanjojo/talhelper/blob/master/docs/docs/guides.md. Title can be something like Templating node labels or annotations for system-upgrade-controller
with the content of example on how to use this feature to setup system-upgrade-controller
? Feel free let me know if you want me to do the documentation instead.
Added docs. Let me know what you think, and if everything is good to go, then I'll squash and fixup the commit message.
I added a link to my system-upgrade-controller deployment in the docs, as a full example of how to handle automated Talos upgrades. A full example (including scripts, health checks, scheduling, etc.) would require a bunch of page space so I thought it might be better to just show a small portion of the concept, with a link to the full thing. Not sure what your policy on this is - let me know if you want me to remove it.
Looks good! I'm running the workflows now, let me know if you're done with the commit squashing. Thank you!
@budimanjojo squashed and renamed. Thanks for your help on this!
v3.0.10 released with this feature!
This adds support for templating node labels and annotations. This is useful for passing Talos information that is node-specific to Kubernetes workloads. Each field will be provided the built, but not yet validated or templated machine config struct as the root context (
.
).Here's a simple demo (included in the example file):
I've configured the template engine to use Sprig functions, which is what other popular Golang templating tools use (like Helm, gotemplate, gomplate). I think this will cover the majority of user's needs.
If desired, it should be pretty trivial to add templating support to other fields as well. Personally I'd find a lot of value in having support for templating in
extraKernelArgs
, but I thought it'd be best to start small with labels and annotations, and get some feedback on the concept first.Prior to merge, I need to add: