Implements a new generate-config subcommand that auto-generates examp…#13385
Implements a new generate-config subcommand that auto-generates examp…#13385nmn3m wants to merge 4 commits intok3s-io:mainfrom
Conversation
63e2138 to
90e4874
Compare
brandond
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
There a couple moderate changes I'd like to see here:
- Use templates and
fmt.Fprintfwhen using a stringbuilder, instead of printing to a string and then writing the string to the builder. - Do not use reflection unless absolutely necessary. urfave/cli has a bunch of interfaces to get most if not all of the information you are trying to extract from flags. Use them. Reflection should be a last resort. We manage to wrap all the K3s flags in RKE2 without needing to do any reflection; you should be able to do the same here.
| // generateYAMLWithComments creates a YAML string with comments | ||
| func generateYAMLWithComments(flags []cli.Flag, existingConfig interface{}, currentValues map[string]interface{}, configType string) string { | ||
| var sb strings.Builder | ||
|
|
||
| sb.WriteString(fmt.Sprintf("# Example k3s %s configuration file\n", configType)) | ||
| sb.WriteString("#\n") | ||
| sb.WriteString("# This file contains all available configuration options with their descriptions.\n") | ||
| sb.WriteString("# Uncomment and modify the options you want to use.\n") | ||
| sb.WriteString("#\n") | ||
| sb.WriteString(fmt.Sprintf("# Place this file at /etc/rancher/k3s/config.yaml or use --config to specify a different location.\n")) | ||
| sb.WriteString("#\n\n") | ||
|
|
||
| categories := groupFlagsByCategory(flags) |
There was a problem hiding this comment.
Make this header a template string, and render it into the stringbuilder.
| // generateYAMLWithComments creates a YAML string with comments | |
| func generateYAMLWithComments(flags []cli.Flag, existingConfig interface{}, currentValues map[string]interface{}, configType string) string { | |
| var sb strings.Builder | |
| sb.WriteString(fmt.Sprintf("# Example k3s %s configuration file\n", configType)) | |
| sb.WriteString("#\n") | |
| sb.WriteString("# This file contains all available configuration options with their descriptions.\n") | |
| sb.WriteString("# Uncomment and modify the options you want to use.\n") | |
| sb.WriteString("#\n") | |
| sb.WriteString(fmt.Sprintf("# Place this file at /etc/rancher/k3s/config.yaml or use --config to specify a different location.\n")) | |
| sb.WriteString("#\n\n") | |
| categories := groupFlagsByCategory(flags) | |
| const yamlHeader = ` | |
| {{- /* */ -}} | |
| # Example {{ .Program }} {{ .Command }} configuration file | |
| # This file contains all available configuration options with their descriptions. | |
| # Uncomment and modify the options you want to use. | |
| # Place this file at /etc/rancher/{{ .Program }}/config.yaml or use --config to specify a different location. | |
| # | |
| ` | |
| type cmdInfo struct { | |
| Program string | |
| Command string | |
| } | |
| // generateYAMLWithComments creates a YAML string with comments | |
| func generateYAMLWithComments(flags []cli.Flag, existingConfig interface{}, currentValues map[string]interface{}, configType string) (string, error) { | |
| var sb strings.Builder | |
| tmpl, err := template.New("config").Parse(yamlHeader") | |
| if err != nil { | |
| return "", err | |
| } | |
| if err = tmpl.Execute(&sb, cmdInfo{Program: version.Program, Command: configType}); err != nil { | |
| return "", err | |
| } | |
| } | |
| categories := groupFlagsByCategory(flags) |
| sb.WriteString(fmt.Sprintf("# %s\n", strings.Repeat("=", 80))) | ||
| sb.WriteString(fmt.Sprintf("# %s\n", categoryName)) | ||
| sb.WriteString(fmt.Sprintf("# %s\n\n", strings.Repeat("=", 80))) |
There was a problem hiding this comment.
Print directly into the stringbuilder - that's how its meant to be used.
| sb.WriteString(fmt.Sprintf("# %s\n", strings.Repeat("=", 80))) | |
| sb.WriteString(fmt.Sprintf("# %s\n", categoryName)) | |
| sb.WriteString(fmt.Sprintf("# %s\n\n", strings.Repeat("=", 80))) | |
| fmt.Fprintf(&sb, "# %s\n", strings.Repeat("=", 80)) | |
| fmt.Fprintf(&sb, "# %s\n", categoryName) | |
| fmt.Fprintf(&sb, "# %s\n\n", strings.Repeat("=", 80)) |
4f97322 to
286a4ce
Compare
|
@brandond , Thanks for your feedback, I solved all review comments. |
|
@nmn3m are you still working on this? I believe this can be done with no or at least much less use of reflect. |
286a4ce to
a264c81
Compare
I fixed all mentioned parts, thanks |
…le configuration files for k3s server and agent nodes Signed-off-by: Nour <nurmn3m@gmail.com>
…faces for flag helpers Signed-off-by: Nour <nurmn3m@gmail.com>
…g helpers Signed-off-by: Nour <nurmn3m@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13385 +/- ##
==========================================
+ Coverage 21.46% 21.48% +0.01%
==========================================
Files 190 192 +2
Lines 15479 15576 +97
==========================================
+ Hits 3323 3346 +23
- Misses 11703 11770 +67
- Partials 453 460 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Please address linter issues - Are you writing this code by hand, or using AI tools? |
Signed-off-by: Nour <nurmn3m@gmail.com>
I mix between writing by hand , reviewing and fixing with AI. |
brandond
left a comment
There was a problem hiding this comment.
- Looks like you need a
go mod tidyand check in the resulting changes. Are you testing this locally withmake validate? - Please add the new command to
cmd/server/main.goas well.
|
@nmn3m are you able to make the requested changes to this PR? |
Proposed Changes
This PR implements a new
generate-configsubcommand that auto-generates example configuration files for k3s server and agent nodes, addressing the need for better discoverability of configuration options.The implementation provides:
k3s generate-configcommand that generates fully commented YAML configuration files--typeflag--from-configflag--outputflagNew files:
pkg/cli/cmds/generate_config.go- Command definitionpkg/cli/generateconfig/generate_config.go- Implementation logicModified files:
pkg/cli/cmds/agent.go- ExportedAgentFlagsvariable to matchServerFlagspatterncmd/k3s/main.go- Registered command in multicall binarymain.go- Registered command in standalone binaryTypes of Changes
New Feature - Adds a new CLI subcommand for generating example configuration files
Verification
The changes can be verified by:
Generate server config to stdout:
Generate agent config to a file:
Generate config with current values from existing file:
Verify help text:
Test invalid input:
k3s generate-config --type invalid # Should return error: invalid config type "invalid", must be 'server' or 'agent'Expected output includes all available k3s configuration options organized by category with comments explaining each option's purpose and default values.
Testing
Linked Issues
Addresses #13071
User-Facing Change
Further Comments
Design Decision:
--from-configvs--configThe implementation uses
--from-configinstead of reusing the existing--configflag. This is intentional because k3s usesconfigfilearg.MustParse()which automatically parses any file specified by--configand converts YAML keys to CLI arguments. Sincegenerate-configdoesn't accept server/agent flags (like--token,--cluster-cidr, etc.), using--configwould cause parsing errors when trying to apply those flags to the generate-config command.Example of the problem if we used
--config:Using
--from-configkeeps the flags separate and allows the command to read existing configs for reference without triggering automatic parsing.Implementation Approach
The solution uses reflection to extract flag metadata (name, usage, default values) from the existing
ServerFlagsandAgentFlagsarrays. This ensures the generated configs stay automatically synchronized with any future flag additions or modifications - no manual maintenance required.To enable this,
AgentFlagswas exported inagent.goto match the existingServerFlagspattern, maintaining consistency across the codebase.