From 68132b03e3d236e48d14c6a1aae26db421976aec Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Thu, 12 Dec 2024 19:00:29 +0200 Subject: [PATCH 1/8] VPA: Add automated flag generator for all components Signed-off-by: Omer Aplatony --- .../hack/vpa-generate-flags.go | 257 ++++++++++++++++++ .../pkg/admission-controller/Makefile | 4 + .../pkg/recommender/Makefile | 4 + vertical-pod-autoscaler/pkg/updater/Makefile | 4 + 4 files changed, 269 insertions(+) create mode 100644 vertical-pod-autoscaler/hack/vpa-generate-flags.go diff --git a/vertical-pod-autoscaler/hack/vpa-generate-flags.go b/vertical-pod-autoscaler/hack/vpa-generate-flags.go new file mode 100644 index 000000000000..079ca61c9a9f --- /dev/null +++ b/vertical-pod-autoscaler/hack/vpa-generate-flags.go @@ -0,0 +1,257 @@ +/* +Copyright 2024 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package main + +import ( + "bytes" + "fmt" + "go/ast" + "go/parser" + "go/token" + "io" + "log" + "os" + "path/filepath" + "sort" + "strings" + "text/template" + "time" +) + +type flagInfo struct { + Name string + DefaultValue string + Type string + Description string + SourceFile string +} + +type templateData struct { + Flags []flagInfo + Timestamp string +} + +var componentTemplates = map[string]string{ + "recommender": `# What are the parameters to VPA recommender? +This document is auto-generated from the flag definitions in the VPA recommender code. +Last updated: {{ .Timestamp }} + +| Flag | Type | Default | Description | +|------|------|---------|-------------| +{{- range .Flags }} +| --{{ .Name }} | {{ .Type }} | {{ .DefaultValue }} | {{ .Description }} | +{{- end }} +`, + "updater": `# What are the parameters to VPA updater? +This document is auto-generated from the flag definitions in the VPA updater code. +Last updated: {{ .Timestamp }} + +| Flag | Type | Default | Description | +|------|------|---------|-------------| +{{- range .Flags }} +| --{{ .Name }} | {{ .Type }} | {{ .DefaultValue }} | {{ .Description }} | +{{- end }} +`, + "admission": `# What are the parameters to VPA admission controller? +This document is auto-generated from the flag definitions in the VPA admission controller code. +Last updated: {{ .Timestamp }} + +| Flag | Type | Default | Description | +|------|------|---------|-------------| +{{- range .Flags }} +| --{{ .Name }} | {{ .Type }} | {{ .DefaultValue }} | {{ .Description }} | +{{- end }} +`, +} + +func main() { + if len(os.Args) != 3 { + fmt.Println("Generates markdown documentation for VPA recommender flags.") + fmt.Println("usage: vpa-recommender-generate-flags ") + fmt.Println(" - Path to the directory containing the Go source files") + fmt.Println(" - Path to the output markdown file") + os.Exit(1) + } + + sourceDir := os.Args[1] + outputFile := os.Args[2] + + flags, err := collectFlags(sourceDir) + if err != nil { + log.Fatalf("Error collecting flags: %v", err) + } + + sort.Slice(flags, func(i, j int) bool { + return flags[i].Name < flags[j].Name + }) + + err = generateDocs(flags, outputFile) + if err != nil { + log.Fatalf("Error generating documentation: %v", err) + } + + fmt.Println("Successfully generated documentation in", outputFile) +} + +func extractFlagFromCall(call *ast.CallExpr, sourcePath string) *flagInfo { + if len(call.Args) < 3 { + return nil + } + + // Extract flag name + nameArg, ok := call.Args[0].(*ast.BasicLit) + if !ok { + return nil + } + name := strings.Trim(nameArg.Value, "\"") + + // Extract default value + var defaultValue string + switch v := call.Args[1].(type) { + case *ast.BasicLit: + defaultValue = strings.Trim(v.Value, "\"") + case *ast.Ident: + defaultValue = v.Name + default: + defaultValue = fmt.Sprintf("%v", v) + } + + // Extract description + descArg, ok := call.Args[2].(*ast.BasicLit) + if !ok { + return nil + } + description := strings.Trim(descArg.Value, "\"") + + return &flagInfo{ + Name: name, + DefaultValue: defaultValue, + Type: call.Fun.(*ast.SelectorExpr).Sel.Name, + Description: description, + SourceFile: sourcePath, + } +} + +func collectFlags(sourceDir string) ([]flagInfo, error) { + var flags []flagInfo + + err := filepath.Walk(sourceDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Skip non-Go files + if !strings.HasSuffix(path, ".go") || info.IsDir() { + return nil + } + + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, path, nil, parser.ParseComments) + + if err != nil { + return fmt.Errorf("error parsing file %s: %v", path, err) + } + + fileFlags := extractFlags(file, getRelativePath(sourceDir, path)) + flags = append(flags, fileFlags...) + + return nil + }) + + return flags, err +} + +func extractFlags(file *ast.File, sourcePath string) []flagInfo { + var flags []flagInfo + + ast.Inspect(file, func(n ast.Node) bool { + // Look for flag.String(), flag.Bool(), flag.Float64(), etc. + if call, ok := n.(*ast.CallExpr); ok { + if sel, ok := call.Fun.(*ast.SelectorExpr); ok { + if ident, ok := sel.X.(*ast.Ident); ok { + if ident.Name == "flag" { + if flag := extractFlagFromCall(call, sourcePath); flag != nil { + flags = append(flags, *flag) + } + } + } + } + } + return true + }) + + return flags +} + +func generateDocs(flags []flagInfo, outputFile string) error { + component := determineComponent(outputFile) + markdownTemplate, ok := componentTemplates[component] + if !ok { + return fmt.Errorf("couldn't find template for %s ", component) + } + tmpl, err := template.New("flags").Parse(markdownTemplate) + if err != nil { + return fmt.Errorf("error parsing template: %v", err) + } + + data := templateData{ + Flags: flags, + Timestamp: time.Now().UTC().Format("2006-01-02 15:04:05 UTC"), + } + + var buf bytes.Buffer + if err := tmpl.Execute(&buf, data); err != nil { + return fmt.Errorf("error executing template: %v", err) + } + + f, err := os.Create(outputFile) + if err != nil { + return fmt.Errorf("error creating output file: %v", err) + } + defer f.Close() + + _, err = io.Copy(f, &buf) + return err +} + +func getRelativePath(baseDir, fullPath string) string { + rel, err := filepath.Rel(baseDir, fullPath) + if err != nil { + return fullPath + } + return rel +} + +// We determine the component by the path of the output file. +// if the path contains "recommender", we generate recommender flags. +// if the path contains "updater", we generate updater flags. +// if the path contains "admission", we generate admission flags. +// Otherwise, we generate recommender flags. + +func determineComponent(outputPath string) string { + if strings.Contains(outputPath, "updater") { + return "updater" + } + if strings.Contains(outputPath, "admission") { + return "admission" + } + if strings.Contains(outputPath, "recommender") { + return "recommender" + } + fmt.Println("Couldn't find component. default is recommender") + return "recommender" +} diff --git a/vertical-pod-autoscaler/pkg/admission-controller/Makefile b/vertical-pod-autoscaler/pkg/admission-controller/Makefile index efc0e28c320e..ada402a2ab84 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/Makefile +++ b/vertical-pod-autoscaler/pkg/admission-controller/Makefile @@ -88,3 +88,7 @@ format: test -z "$$(find . -path ./vendor -prune -type f -o -name '*.go' -exec gofmt -s -w {} + | tee /dev/stderr)" .PHONY: all build test-unit clean format release + +.PHONY: admission-flags +admission-flags: + go run ../../hack/vpa-generate-flags.go . ../../docs/vpa-admission-flags.md \ No newline at end of file diff --git a/vertical-pod-autoscaler/pkg/recommender/Makefile b/vertical-pod-autoscaler/pkg/recommender/Makefile index a9ccc1bd9c14..7f6ad9d13876 100644 --- a/vertical-pod-autoscaler/pkg/recommender/Makefile +++ b/vertical-pod-autoscaler/pkg/recommender/Makefile @@ -90,3 +90,7 @@ format: test -z "$$(find . -path ./vendor -prune -type f -o -name '*.go' -exec gofmt -s -w {} + | tee /dev/stderr)" .PHONY: all build test-unit clean format release + +.PHONY: recommender-flags +recommender-flags: + go run ../../hack/vpa-generate-flags.go . ../../docs/vpa-recommender-flags.md \ No newline at end of file diff --git a/vertical-pod-autoscaler/pkg/updater/Makefile b/vertical-pod-autoscaler/pkg/updater/Makefile index 434c93fa866a..84b2fe6c447f 100644 --- a/vertical-pod-autoscaler/pkg/updater/Makefile +++ b/vertical-pod-autoscaler/pkg/updater/Makefile @@ -88,3 +88,7 @@ format: test -z "$$(find . -path ./vendor -prune -type f -o -name '*.go' -exec gofmt -s -w {} + | tee /dev/stderr)" .PHONY: all build test-unit clean format release + +.PHONY: updater-flags +updater-flags: + go run ../../hack/vpa-generate-flags.go . ../../docs/vpa-updater-flags.md \ No newline at end of file From 863d49f7f46682d037089b0ece6ef151a80ecabc Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Sun, 15 Dec 2024 11:19:50 +0200 Subject: [PATCH 2/8] Address issues and add merge script Signed-off-by: Omer Aplatony --- .../hack/create-flags-doc-vpa.sh | 41 +++++++++++++++++++ .../hack/vpa-generate-flags.go | 21 +++++++++- .../pkg/admission-controller/Makefile | 4 +- .../pkg/recommender/Makefile | 4 +- vertical-pod-autoscaler/pkg/updater/Makefile | 4 +- 5 files changed, 67 insertions(+), 7 deletions(-) create mode 100755 vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh diff --git a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh new file mode 100755 index 000000000000..a6b1bb780e1b --- /dev/null +++ b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh @@ -0,0 +1,41 @@ +#!/bin/bash + +# Copyright 2024 The Kubernetes Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +function generate_vpa_docs_componenets { + componenets="recommender updater admission-controller" + for component in $componenets; do + echo "generate docs for $component" + cd ../pkg/$component + make document-flags + cd - + done +} + +function move_and_merge_docs { + files="vpa-admission-flags.md vpa-recommender-flags.md vpa-updater-flags.md" + cd ../docs + rm -f flags.md + touch flags.md + for file in $files; do + echo "merge $file" + cat $file >> flags.md + echo "" >> flags.md + done + cd - +} + +generate_vpa_docs_componenets +move_and_merge_docs \ No newline at end of file diff --git a/vertical-pod-autoscaler/hack/vpa-generate-flags.go b/vertical-pod-autoscaler/hack/vpa-generate-flags.go index 079ca61c9a9f..856bdbcf8eab 100644 --- a/vertical-pod-autoscaler/hack/vpa-generate-flags.go +++ b/vertical-pod-autoscaler/hack/vpa-generate-flags.go @@ -124,10 +124,26 @@ func extractFlagFromCall(call *ast.CallExpr, sourcePath string) *flagInfo { switch v := call.Args[1].(type) { case *ast.BasicLit: defaultValue = strings.Trim(v.Value, "\"") + case *ast.BinaryExpr: + // Handle Duration expressions like "1*time.Minute" + if lit, ok := v.X.(*ast.BasicLit); ok { + if sel, ok := v.Y.(*ast.SelectorExpr); ok { + if ident, ok := sel.X.(*ast.Ident); ok && ident.Name == "time" { + number := strings.Trim(lit.Value, "\"") + unit := sel.Sel.Name + defaultValue = fmt.Sprintf("%s%s", number, unit) + } + } + } + case *ast.UnaryExpr: + // Handle negative numbers + if lit, ok := v.X.(*ast.BasicLit); ok { + defaultValue = fmt.Sprintf("%s%s", v.Op.String(), lit.Value) + } case *ast.Ident: defaultValue = v.Name default: - defaultValue = fmt.Sprintf("%v", v) + defaultValue = "0" } // Extract description @@ -136,6 +152,9 @@ func extractFlagFromCall(call *ast.CallExpr, sourcePath string) *flagInfo { return nil } description := strings.Trim(descArg.Value, "\"") + description = strings.ReplaceAll(description, "\n\t\t", " ") + description = strings.ReplaceAll(description, "\n", " ") + description = strings.TrimSpace(description) return &flagInfo{ Name: name, diff --git a/vertical-pod-autoscaler/pkg/admission-controller/Makefile b/vertical-pod-autoscaler/pkg/admission-controller/Makefile index ada402a2ab84..444aacf1e79f 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/Makefile +++ b/vertical-pod-autoscaler/pkg/admission-controller/Makefile @@ -89,6 +89,6 @@ format: .PHONY: all build test-unit clean format release -.PHONY: admission-flags -admission-flags: +.PHONY: document-flags +document-flags: go run ../../hack/vpa-generate-flags.go . ../../docs/vpa-admission-flags.md \ No newline at end of file diff --git a/vertical-pod-autoscaler/pkg/recommender/Makefile b/vertical-pod-autoscaler/pkg/recommender/Makefile index 7f6ad9d13876..e6d06a008e90 100644 --- a/vertical-pod-autoscaler/pkg/recommender/Makefile +++ b/vertical-pod-autoscaler/pkg/recommender/Makefile @@ -91,6 +91,6 @@ format: .PHONY: all build test-unit clean format release -.PHONY: recommender-flags -recommender-flags: +.PHONY: document-flags +document-flags: go run ../../hack/vpa-generate-flags.go . ../../docs/vpa-recommender-flags.md \ No newline at end of file diff --git a/vertical-pod-autoscaler/pkg/updater/Makefile b/vertical-pod-autoscaler/pkg/updater/Makefile index 84b2fe6c447f..ae9b1f78ec69 100644 --- a/vertical-pod-autoscaler/pkg/updater/Makefile +++ b/vertical-pod-autoscaler/pkg/updater/Makefile @@ -89,6 +89,6 @@ format: .PHONY: all build test-unit clean format release -.PHONY: updater-flags -updater-flags: +.PHONY: document-flags +document-flags: go run ../../hack/vpa-generate-flags.go . ../../docs/vpa-updater-flags.md \ No newline at end of file From 4743dfbd9c03ef0ffe83cd0260760a6fc7ebd869 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Sun, 15 Dec 2024 11:22:55 +0200 Subject: [PATCH 3/8] fix misspelling Signed-off-by: Omer Aplatony --- vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh index a6b1bb780e1b..79fe58851782 100755 --- a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh +++ b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh @@ -14,9 +14,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -function generate_vpa_docs_componenets { - componenets="recommender updater admission-controller" - for component in $componenets; do +function generate_vpa_docs_components { + components="recommender updater admission-controller" + for component in $components; do echo "generate docs for $component" cd ../pkg/$component make document-flags @@ -37,5 +37,5 @@ function move_and_merge_docs { cd - } -generate_vpa_docs_componenets +generate_vpa_docs_components move_and_merge_docs \ No newline at end of file From b7ac55b48eeda12fbeb04544fb85e558778c52b4 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Sun, 15 Dec 2024 11:52:56 +0200 Subject: [PATCH 4/8] Add BASH_SOURCE Signed-off-by: Omer Aplatony --- .../hack/create-flags-doc-vpa.sh | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh index 79fe58851782..5444f69b8e19 100755 --- a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh +++ b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh @@ -14,27 +14,38 @@ # See the License for the specific language governing permissions and # limitations under the License. +set -o errexit +set -o nounset +set -o pipefail + +# Get the absolute path to the script's directory +SCRIPT_DIR=$(realpath $(dirname ${BASH_SOURCE})) +# Get the repository root (parent of hack directory) +REPOSITORY_ROOT=$(realpath ${SCRIPT_DIR}/..) + function generate_vpa_docs_components { components="recommender updater admission-controller" for component in $components; do echo "generate docs for $component" - cd ../pkg/$component + # Use absolute paths based on REPOSITORY_ROOT + cd "${REPOSITORY_ROOT}/pkg/${component}" make document-flags - cd - + cd - > /dev/null # Suppress directory change messages done } function move_and_merge_docs { files="vpa-admission-flags.md vpa-recommender-flags.md vpa-updater-flags.md" - cd ../docs + # Use absolute path for docs directory + cd "${REPOSITORY_ROOT}/docs" rm -f flags.md touch flags.md for file in $files; do echo "merge $file" - cat $file >> flags.md + cat "$file" >> flags.md echo "" >> flags.md done - cd - + cd - > /dev/null # Suppress directory change messages } generate_vpa_docs_components From 1611b35b12331967739050ddafab5ec20e09e369 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Sun, 15 Dec 2024 18:25:56 +0200 Subject: [PATCH 5/8] sub shell to avoid cd - Co-authored-by: Adrian Moisey --- .../hack/create-flags-doc-vpa.sh | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh index 5444f69b8e19..dec1103eaeaf 100755 --- a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh +++ b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh @@ -37,15 +37,16 @@ function generate_vpa_docs_components { function move_and_merge_docs { files="vpa-admission-flags.md vpa-recommender-flags.md vpa-updater-flags.md" # Use absolute path for docs directory - cd "${REPOSITORY_ROOT}/docs" - rm -f flags.md - touch flags.md - for file in $files; do - echo "merge $file" - cat "$file" >> flags.md - echo "" >> flags.md - done - cd - > /dev/null # Suppress directory change messages + ( + cd "${REPOSITORY_ROOT}/docs" + rm -f flags.md + touch flags.md + for file in $files; do + echo "merge $file" + cat "$file" >> flags.md + echo "" >> flags.md + done + ) } generate_vpa_docs_components From e227e200055769905689419cf1b5673335000c78 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Sun, 15 Dec 2024 18:27:33 +0200 Subject: [PATCH 6/8] Remove the temporary file after merging Signed-off-by: Omer Aplatony --- vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh index dec1103eaeaf..cbccbd9603cf 100755 --- a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh +++ b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh @@ -45,6 +45,8 @@ function move_and_merge_docs { echo "merge $file" cat "$file" >> flags.md echo "" >> flags.md + # Remove the temporary file after merging + rm -f "$file" done ) } From 06558a4406b6e123151f99fdc28fef6bdb40266a Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Mon, 16 Dec 2024 11:13:29 +0200 Subject: [PATCH 7/8] sub shell Co-authored-by: Adrian Moisey --- vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh index cbccbd9603cf..0e2dd0fa8bd1 100755 --- a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh +++ b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh @@ -28,9 +28,10 @@ function generate_vpa_docs_components { for component in $components; do echo "generate docs for $component" # Use absolute paths based on REPOSITORY_ROOT - cd "${REPOSITORY_ROOT}/pkg/${component}" - make document-flags - cd - > /dev/null # Suppress directory change messages + ( + cd "${REPOSITORY_ROOT}/pkg/${component}" + make document-flags + ) done } From ce75c91874d2c791cd0305b1ef59756d5fdd5049 Mon Sep 17 00:00:00 2001 From: Omer Aplatony Date: Mon, 16 Dec 2024 11:16:40 +0200 Subject: [PATCH 8/8] Add final message Signed-off-by: Omer Aplatony --- vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh index 0e2dd0fa8bd1..478b7b224b8a 100755 --- a/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh +++ b/vertical-pod-autoscaler/hack/create-flags-doc-vpa.sh @@ -50,7 +50,8 @@ function move_and_merge_docs { rm -f "$file" done ) + echo "updated docs/flags.md" } generate_vpa_docs_components -move_and_merge_docs \ No newline at end of file +move_and_merge_docs