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

Add support for templating node labels and annotations #701

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

solidDoWant
Copy link
Contributor

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):

# Source
nodeLabels:
  rack: rack1a
  zone: us-east-1a
  secureBootEnabled: >-
    {{
      .MachineConfig.MachineInstall.InstallImage |
      contains "installer-secureboot"
    }}
nodeAnnotations:
  rack: rack1a
  installerUrlCode: '{{ .MachineConfig.MachineInstall.InstallImage }}'
# Rendered result
  nodeLabels:
    rack: rack1a
    secureBootEnabled: "true"
    zone: us-east-1a
  nodeAnnotations:
    installerUrlCode: factory.talos.dev/installer-secureboot/67a999813ff49ba00dc632aa48170dc866bda646b7ee4f944e2fb7dc50b522bd:v1.7.1
    rack: rack1a

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:

  • Tests
  • Docs
  • Example

@budimanjojo
Copy link
Owner

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.

@solidDoWant
Copy link
Contributor Author

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.

@budimanjojo
Copy link
Owner

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.

pkg/talos/nodeconfig.go Outdated Show resolved Hide resolved
pkg/templating/template.go Show resolved Hide resolved
pkg/templating/template.go Show resolved Hide resolved
pkg/templating/template.go Outdated Show resolved Hide resolved
pkg/talos/nodeconfig.go Outdated Show resolved Hide resolved
@budimanjojo
Copy link
Owner

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!

@solidDoWant
Copy link
Contributor Author

Added tests in fc1a8b8. 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.

@budimanjojo
Copy link
Owner

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.

@solidDoWant
Copy link
Contributor Author

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.

@budimanjojo
Copy link
Owner

Looks good! I'm running the workflows now, let me know if you're done with the commit squashing. Thank you!

@solidDoWant
Copy link
Contributor Author

@budimanjojo squashed and renamed. Thanks for your help on this!

@budimanjojo budimanjojo merged commit f05658a into budimanjojo:master Nov 15, 2024
4 checks passed
@budimanjojo
Copy link
Owner

v3.0.10 released with this feature!

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.

2 participants