From 4c9d773c8871650f7b5d3279d19970408c27b51a Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Wed, 28 Jan 2026 20:43:20 +0200 Subject: [PATCH 01/11] PT-2368 - full refactor of a config library This refactor includes new config lib based on kong module. This lib is now follows an official percona specs, for parsing default and specified configs. --- src/go/lib/config/config.go | 362 ++++++++++++++----- src/go/lib/config/config_test.go | 485 +++++++++++++++++++++++--- src/go/pt-k8s-debug-collector/main.go | 65 ++-- src/go/tests/lib/sample-config1.conf | 4 +- src/go/tests/lib/sample-config2.conf | 4 +- 5 files changed, 761 insertions(+), 159 deletions(-) diff --git a/src/go/lib/config/config.go b/src/go/lib/config/config.go index 820fc892b..002349bca 100644 --- a/src/go/lib/config/config.go +++ b/src/go/lib/config/config.go @@ -2,146 +2,336 @@ package config import ( "bufio" + "bytes" + "errors" + "fmt" + "io" "os" "os/user" - "strconv" + "path/filepath" + "reflect" "strings" + "sync" + + "github.com/alecthomas/kong" ) -type Config struct { - options map[string]interface{} -} +// BoolYN represents a boolean flag that accepts multiple textual representations. +// Supported true values: 1, true, yes, y, on, "" (empty) +// Supported false values: 0, false, no, n, off +type BoolYN bool -func (c *Config) GetString(key string) string { - if val, ok := c.options[key]; ok { - if v, ok := val.(string); ok { - return v - } +func (b *BoolYN) Decode(ctx *kong.DecodeContext, target reflect.Value) error { + var value string + if err := ctx.Scan.PopValueInto("string", &value); err != nil { + return err } - return "" -} -func (c *Config) GetInt64(key string) int64 { - if val, ok := c.options[key]; ok { - if v, ok := val.(int64); ok { - return v - } + var result bool + switch strings.ToLower(strings.TrimSpace(value)) { + case "1", "true", "yes", "y", "on", "": + result = true + case "0", "false", "no", "n", "off": + result = false + default: + return fmt.Errorf("invalid boolean value %q (expected: 1/0, true/false, yes/no, y/n, on/off)", value) } - return 0 + + target.SetBool(result) + return nil } -func (c *Config) GetFloat64(key string) float64 { - if val, ok := c.options[key]; ok { - if v, ok := val.(float64); ok { - return v - } +// StdinRequestString represents a string that can be requested from stdin if not provided. +// via Request() method. +type StdinRequestString string + +const ASK_PLACEHOLDER = "*" + +func (p *StdinRequestString) Decode(ctx *kong.DecodeContext, target reflect.Value) error { + if ctx.Scan.Len() == 0 { + target.SetString(ASK_PLACEHOLDER) + return nil } - return 0 -} -func (c *Config) GetBool(key string) bool { - if val, ok := c.options[key]; ok { - if v, ok := val.(bool); ok { - return v - } + var s string + if err := ctx.Scan.PopValueInto("string", &s); err != nil { + return err } - return false -} -func (c *Config) HasKey(key string) bool { - _, ok := c.options[key] - return ok + target.SetString(s) + return nil } -func DefaultConfigFiles(toolName string) ([]string, error) { - user, err := user.Current() +func (p *StdinRequestString) Request(f func() (string, error)) error { + if p == nil || *p != ASK_PLACEHOLDER { + return nil + } + + resp, err := f() if err != nil { - return nil, err + return err } - files := []string{ - "/etc/percona-toolkit/percona-toolkit.conf", - "/etc/percona-toolkit/${TOOLNAME}.conf", - "${HOME}/.percona-toolkit.conf", - "${HOME}/.${TOOLNAME}.conf", + *p = StdinRequestString(resp) + return nil +} + +type PerconaResolver struct { + mu sync.RWMutex + values map[string]any +} + +// NewPerconaResolver creates a resolver containing configuration +// in Percona Toolkit format. +// +// Format rules: +// - Lines starting with # are comments +// - Empty lines are ignored +// - Format: "option" or "option=value" +// - No -- prefix needed +// - Values are literal (not quoted) +// - Lines with "no-option" set option to "false" +func NewPerconaResolver(r io.Reader) (*PerconaResolver, error) { + res := &PerconaResolver{values: make(map[string]any)} + + scanner := bufio.NewScanner(r) + lineNum := 0 + + for scanner.Scan() { + lineNum++ + line := scanner.Text() + trimmed := strings.TrimSpace(line) + + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + + var key, val string + + if idx := strings.Index(trimmed, "="); idx != -1 { + key = strings.TrimSpace(trimmed[:idx]) + val = strings.TrimSpace(trimmed[idx+1:]) + + key = strings.TrimPrefix(key, "--") + + if key == "" { + return nil, fmt.Errorf("line %d: empty option name", lineNum) + } + } else { + key = strings.TrimPrefix(trimmed, "--") + val = "true" + } + + if strings.HasPrefix(key, "no-") { + actualKey := strings.TrimPrefix(key, "no-") + res.values[actualKey] = "false" + } else { + res.values[key] = val + } } - for i := 0; i < len(files); i++ { - files[i] = strings.Replace(files[i], "${TOOLNAME}", toolName, -1) - files[i] = strings.Replace(files[i], "${HOME}", user.HomeDir, -1) + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error reading config: %w", err) } - return files, nil + return res, nil } -func DefaultConfig(toolname string) *Config { - files, _ := DefaultConfigFiles(toolname) - return NewConfig(files...) +func (p *PerconaResolver) Validate(app *kong.Application) error { + return nil } -func NewConfig(files ...string) *Config { - config := &Config{ - options: make(map[string]interface{}), - } - for _, filename := range files { - if _, err := os.Stat(filename); err == nil { - read(filename, config.options) - } - } - return config +func (p *PerconaResolver) Resolve(ctx *kong.Context, parent *kong.Path, flag *kong.Flag) (any, error) { + p.mu.RLock() + defer p.mu.RUnlock() + return p.values[flag.Name], nil } -func read(filename string, opts map[string]interface{}) error { - f, err := os.Open(filename) +type configFile struct { + options io.Reader + passthrough []string +} + +// loadConfig reads a configuration file and splits it into: +// - passthrough arguments (after "--") +func loadConfig(path string) (*configFile, error) { + f, err := os.Open(path) if err != nil { - return err + return nil, err } + defer func() { + if closeErr := f.Close(); closeErr != nil && err == nil { + err = closeErr + } + }() + var optsBuffer bytes.Buffer + var passthrough []string scanner := bufio.NewScanner(f) + foundDash := false + lineNum := 0 for scanner.Scan() { + lineNum++ line := scanner.Text() - if line == "" || strings.HasPrefix(line, "#") { + trimmed := strings.TrimSpace(line) + + if !foundDash && trimmed == "--" { + foundDash = true continue } - m := strings.SplitN(scanner.Text(), "=", 2) - key := strings.TrimSpace(m[0]) - - if len(m) == 1 { - opts[key] = true - continue + if foundDash { + if trimmed != "" && !strings.HasPrefix(trimmed, "#") { + fields := strings.Fields(trimmed) + passthrough = append(passthrough, fields...) + } + } else { + optsBuffer.WriteString(line + "\n") } + } - val := strings.TrimSpace(m[1]) - lcval := strings.ToLower(val) + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error reading config file: %w", err) + } - if lcval == "true" || lcval == "yes" { - opts[key] = true - continue - } - if lcval == "false" || lcval == "no" { - opts[key] = false + return &configFile{ + options: &optsBuffer, + passthrough: passthrough, + }, nil +} + +// Setup initializes kong parser with Percona Toolkit configuration file support. +// +// It handles: +// - --config flag as the first argument (must be separated by space) +// - Default config file locations if --config not specified +// - Multiple config files (comma-separated) +// - Passthrough arguments after "--" in config files +// - Accepting kong.Options +// +// Returns: +// - *kong.Context: parsed command-line context +// - []string: passthrough arguments from config files +// - error: any error that occurred during setup +func Setup(toolName string, cli any, options ...kong.Option) (*kong.Context, []string, error) { + rawArgs := os.Args[1:] + + configPaths, specifiedConfig, remainingArgs, err := parseConfigFlag(rawArgs) + if err != nil { + return nil, nil, err + } + + if !specifiedConfig { + configPaths = getDefaultPaths(toolName) + } + + resolvers, filePassthrough, err := loadConfigFiles(configPaths, specifiedConfig) + if err != nil { + return nil, nil, err + } + + options = append(options, + kong.Name(toolName), + kong.TypeMapper(reflect.TypeOf(BoolYN(false)), new(BoolYN)), + kong.TypeMapper(reflect.TypeOf(StdinRequestString("")), new(StdinRequestString)), + kong.Resolvers(resolvers...), + ) + + parser, err := kong.New(cli, options...) + if err != nil { + return nil, nil, err + } + + ctx, err := parser.Parse(remainingArgs) + parser.FatalIfErrorf(err) + + return ctx, filePassthrough, nil +} + +func parseConfigFlag(rawArgs []string) ([]string, bool, []string, error) { + if len(rawArgs) == 0 { + return nil, false, rawArgs, nil + } + + if strings.HasPrefix(rawArgs[0], "--config=") { + return nil, false, nil, errors.New("Error: --config must not use '='. Use: --config /path/to/file") + } + + if rawArgs[0] != "--config" { + return nil, false, rawArgs, nil + } + + if len(rawArgs) < 2 { + return nil, false, nil, errors.New("Error: --config requires a value") + } + + val := rawArgs[1] + remainingArgs := rawArgs[2:] + + if val == "" || val == "''" || val == `""` { + return nil, true, remainingArgs, nil + } + + configPaths := strings.Split(val, ",") + + for i := range configPaths { + configPaths[i] = strings.TrimSpace(configPaths[i]) + } + + return configPaths, true, remainingArgs, nil +} + +func loadConfigFiles(configPaths []string, specifiedConfig bool) ([]kong.Resolver, []string, error) { + var resolvers []kong.Resolver + var filePassthrough []string + + for _, path := range configPaths { + if path == "" { continue } - f, err := strconv.ParseFloat(val, 64) + cfg, err := loadConfig(path) if err != nil { - opts[key] = strings.TrimSpace(val) // string + // If config was explicitly specified, fail on error + if specifiedConfig { + return nil, nil, fmt.Errorf("failed to open config %s: %w", path, err) + } + // Otherwise, silently skip missing default config files continue } - if f == float64(int64(f)) { - opts[key] = int64(f) // int64 - continue + resolver, err := NewPerconaResolver(cfg.options) + if err != nil { + return nil, nil, fmt.Errorf("failed to parse config %s: %w", path, err) } - opts[key] = f // float64 + resolvers = append(resolvers, resolver) + filePassthrough = append(filePassthrough, cfg.passthrough...) } - if err := scanner.Err(); err != nil { - return err + return resolvers, filePassthrough, nil +} + +// getDefaultPaths returns the default configuration file paths for a tool. +// Returns paths in order of precedence (lowest to highest): +// 1. /etc/percona-toolkit/percona-toolkit.conf +// 2. /etc/percona-toolkit/TOOL.conf +// 3. $HOME/.percona-toolkit.conf +// 4. $HOME/.TOOL.conf +func getDefaultPaths(toolName string) []string { + u, err := user.Current() + if err != nil { + return []string{ + "/etc/percona-toolkit/percona-toolkit.conf", + fmt.Sprintf("/etc/percona-toolkit/%s.conf", toolName), + } } - return nil + return []string{ + "/etc/percona-toolkit/percona-toolkit.conf", + fmt.Sprintf("/etc/percona-toolkit/%s.conf", toolName), + filepath.Join(u.HomeDir, ".percona-toolkit.conf"), + filepath.Join(u.HomeDir, fmt.Sprintf(".%s.conf", toolName)), + } } diff --git a/src/go/lib/config/config_test.go b/src/go/lib/config/config_test.go index 1eb5fd05f..e7b6149ac 100644 --- a/src/go/lib/config/config_test.go +++ b/src/go/lib/config/config_test.go @@ -1,73 +1,102 @@ package config import ( + "bytes" "fmt" + "os" "os/user" "path" + "path/filepath" "reflect" + "strings" "testing" + "github.com/alecthomas/kong" "github.com/percona/percona-toolkit/src/go/lib/tutil" ) -func TestReadConfig(t *testing.T) { +type KongFlags struct { + VersionCheck bool `name:"version-check" negatable:"" default:"true"` + TrueBoolVar bool `name:"trueboolvar" help:"test"` + YesBoolVar BoolYN `name:"yesboolvar" help:"test"` + FalseBoolVar bool `name:"falseboolvar" help:"test"` + NoBoolVar BoolYN `name:"noboolvar" help:"test"` + IntVar int `name:"intvar" default:"0"` + FloatVar float64 `name:"floatvar" default:"0.0"` + StringVar string `name:"stringvar"` + NewString string `name:"newstring" short:"n"` + AnotherInt int `name:"anotherint" default:"0" short:"a"` + IgnoredComment string `name:"ignoredcomment"` +} + +func TestReadConfigKong(t *testing.T) { rootPath, err := tutil.RootPath() if err != nil { t.Errorf("cannot get root path: %s", err) } file := path.Join(rootPath, "src/go/tests/lib/sample-config1.conf") - conf := NewConfig(file) + var mockArgs []string + + mockArgs = append(mockArgs, os.Args[0]) - keys := []string{"no-version-check", "trueboolvar", "yesboolvar", "noboolvar", "falseboolvar", "intvar", "floatvar", "stringvar"} - for _, key := range keys { - if !conf.HasKey(key) { - t.Errorf("missing %s key", key) - } + mockArgs = append(mockArgs, []string{"--config", file}...) + os.Args = mockArgs + + f := &KongFlags{} + toolName := "pt-tools-config-test" + + _, _, err = Setup(toolName, f) + if err != nil { + t.Error(err) } // no-version-check - if conf.GetBool("no-version-check") != true { + if f.VersionCheck { t.Error("no-version-check should be enabled") } // trueboolvar=true - if conf.GetBool("trueboolvar") != true { + if !f.TrueBoolVar { t.Error("trueboolvar should be true") } // yesboolvar=yes - if conf.GetBool("yesboolvar") != true { + if !f.YesBoolVar { t.Error("yesboolvar should be true") } // falseboolvar=false - if conf.GetBool("falseboolvar") != false { + if f.FalseBoolVar { t.Error("trueboolvar should be false") } // noboolvar=no - if conf.GetBool("noboolvar") != false { + if f.NoBoolVar { t.Error("yesboolvar should be false") } // intvar=1 - if got := conf.GetInt64("intvar"); got != 1 { - t.Errorf("intvar should be 1, got %d", got) + if f.IntVar != 1 { + t.Errorf("intvar should be 1, got %d", f.IntVar) } // floatvar=2.3 - if got := conf.GetFloat64("floatvar"); got != 2.3 { - t.Errorf("floatvar should be 2.3, got %f", got) + if f.FloatVar != 2.3 { + t.Errorf("floatvar should be 2.3, got %f", f.FloatVar) } // stringvar=some string var having = and # - if got := conf.GetString("stringvar"); got != "some string var having = and #" { - t.Errorf("string var incorrect value; got %s", got) + if f.StringVar != "some string var having = and #" { + t.Errorf("string var incorrect value; got %q", f.StringVar) + } + + if f.IgnoredComment != "" { + t.Errorf("ignoredcomment should be empty; got %q", f.IgnoredComment) } } -func TestOverrideConfig(t *testing.T) { +func TestOverrideConfigKong(t *testing.T) { rootPath, err := tutil.RootPath() if err != nil { t.Errorf("cannot get root path: %s", err) @@ -75,60 +104,149 @@ func TestOverrideConfig(t *testing.T) { file1 := path.Join(rootPath, "src/go/tests/lib/sample-config1.conf") file2 := path.Join(rootPath, "src/go/tests/lib/sample-config2.conf") - conf := NewConfig(file1, file2) + var mockArgs []string + + mockArgs = append(mockArgs, os.Args[0]) - keys := []string{"no-version-check", "trueboolvar", "yesboolvar", "noboolvar", "falseboolvar", "intvar", "floatvar", "stringvar"} - for _, key := range keys { - if !conf.HasKey(key) { - t.Errorf("missing %s key", key) - } + mockArgs = append(mockArgs, []string{"--config", fmt.Sprintf("%s,%s", file1, file2)}...) + os.Args = mockArgs + + f := &KongFlags{} + toolName := "pt-tools-config-test" + + _, _, err = Setup(toolName, f) + if err != nil { + t.Error(err) } // no-version-check. This option is missing in the 2nd file. // It should remain unchanged - if conf.GetBool("no-version-check") != true { + if f.VersionCheck { t.Error("no-version-check should be enabled") } - if conf.GetBool("trueboolvar") == true { + if f.TrueBoolVar { t.Error("trueboolvar should be false") } - if conf.GetBool("yesboolvar") == true { + if f.YesBoolVar { t.Error("yesboolvar should be false") } - if conf.GetBool("falseboolvar") == false { + if !f.FalseBoolVar { t.Error("trueboolvar should be true") } - if conf.GetBool("noboolvar") == false { + if !f.NoBoolVar { t.Error("yesboolvar should be true") } - if got := conf.GetInt64("intvar"); got != 4 { - t.Errorf("intvar should be 4, got %d", got) + if f.IntVar != 4 { + t.Errorf("intvar should be 4, got %d", f.IntVar) } - if got := conf.GetFloat64("floatvar"); got != 5.6 { - t.Errorf("floatvar should be 5.6, got %f", got) + if f.FloatVar != 5.6 { + t.Errorf("floatvar should be 5.6, got %f", f.FloatVar) } - if got := conf.GetString("stringvar"); got != "some other string" { - t.Errorf("string var incorrect value; got %s", got) + if f.StringVar != "some other string" { + t.Errorf("string var incorrect value; got %s", f.StringVar) } // This exists only in file2 - if got := conf.GetString("newstring"); got != "a new string" { - t.Errorf("string var incorrect value; got %s", got) + if f.NewString != "a new string" { + t.Errorf("string var incorrect value; got %s", f.NewString) } - if got := conf.GetInt64("anotherint"); got != 8 { - t.Errorf("intvar should be 8, got %d", got) + if f.AnotherInt != 8 { + t.Errorf("intvar should be 8, got %d", f.AnotherInt) + } + + if f.IgnoredComment != "" { + t.Errorf("ignoredcomment should be empty; got %q", f.IgnoredComment) } } -func TestDefaultFiles(t *testing.T) { +func TestOverrideCMDConfigKong(t *testing.T) { + rootPath, err := tutil.RootPath() + if err != nil { + t.Errorf("cannot get root path: %s", err) + } + file1 := path.Join(rootPath, "src/go/tests/lib/sample-config1.conf") + + var mockArgs []string + + mockArgs = append(mockArgs, os.Args[0]) + + mockArgs = append(mockArgs, + "--config", file1, + "--trueboolvar=false", // reset bool flag + "--yesboolvar", "no", + "--falseboolvar=true", // reset bool flag + "--noboolvar", "yes", + "--intvar", "1337", + "--floatvar", "1337.1", + "--stringvar", "hello", + "-n", "world", // test shorthand + "-a", "3", // test shorthand + ) + os.Args = mockArgs + + f := &KongFlags{} + toolName := "pt-tools-config-test" + + _, _, err = Setup(toolName, f) + if err != nil { + t.Error(err) + } + + if f.VersionCheck { + t.Error("no-version-check should be enabled") + } + + if f.TrueBoolVar { + t.Error("trueboolvar should be false") + } + + if f.YesBoolVar { + t.Error("yesboolvar should be false") + } + + if !f.FalseBoolVar { + t.Error("trueboolvar should be true") + } + + if !f.NoBoolVar { + t.Error("yesboolvar should be true") + } + + if f.IntVar != 1337 { + t.Errorf("intvar should be 1337, got %d", f.IntVar) + } + + if f.FloatVar != 1337.1 { + t.Errorf("floatvar should be 1337.1, got %f", f.FloatVar) + } + + if f.StringVar != "hello" { + t.Errorf("string var incorrect value; got %s", f.StringVar) + } + + // This exists only in file2 + if f.NewString != "world" { + t.Errorf("string var incorrect value; got %s", f.NewString) + } + + if f.AnotherInt != 3 { + t.Errorf("intvar should be 3, got %d", f.AnotherInt) + } + + if f.IgnoredComment != "" { + t.Errorf("ignoredcomment should be empty; got %q", f.IgnoredComment) + } +} + +func TestDefaultFilesKong(t *testing.T) { current, _ := user.Current() toolname := "pt-testing" @@ -139,12 +257,289 @@ func TestDefaultFiles(t *testing.T) { fmt.Sprintf("%s/.%s.conf", current.HomeDir, toolname), } - got, err := DefaultConfigFiles(toolname) - if err != nil { - t.Errorf("cannot get default config files list: %s", err) - } + got := getDefaultPaths(toolname) if !reflect.DeepEqual(got, want) { t.Errorf("got %#v\nwant: %#v\n", got, want) } } + +func TestNewPerconaResolver(t *testing.T) { + tests := []struct { + name string + input string + want map[string]any + wantErr bool + }{ + { + name: "basic_options", + input: `# Comment +variable=Threads_connected +cycles=2 +verbose`, + want: map[string]any{ + "variable": "Threads_connected", + "cycles": "2", + "verbose": "true", + }, + wantErr: false, + }, + { + name: "with_no_prefix", + input: `option=value +no-optimize`, + want: map[string]any{ + "option": "value", + "optimize": "false", + }, + wantErr: false, + }, + { + name: "with_double_dash_prefix", // Not valid according to specs but should pass + input: `--host=localhost +--port=3306`, + want: map[string]any{ + "host": "localhost", + "port": "3306", + }, + wantErr: false, + }, + { + name: "empty_lines_and_comments", + input: "\n# Comment\n\n# Another comment\n\noption=value\n\n", + want: map[string]any{ + "option": "value", + }, + wantErr: false, + }, + { + name: "spaces_around_equals", // Not valid according to specs but should pass + input: `key = value +another=test`, + want: map[string]any{ + "key": "value", + "another": "test", + }, + wantErr: false, + }, + { + name: "invalid_empty_key", + input: `=value`, + want: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reader := strings.NewReader(tt.input) + got, err := NewPerconaResolver(reader) + + if (err != nil) != tt.wantErr { + t.Errorf("NewPerconaResolver() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if tt.wantErr { + return + } + + if !reflect.DeepEqual(got.values, tt.want) { + t.Errorf("NewPerconaResolver() values = %v, want %v", got.values, tt.want) + } + }) + } +} + +func TestLoadConfig(t *testing.T) { + tests := []struct { + name string + content string + wantOptions string + wantPassthrough []string + wantErr bool + }{ + { + name: "basic_config", + content: `variable=Threads_connected +cycles=2`, + wantOptions: `variable=Threads_connected +cycles=2 +`, + wantPassthrough: nil, + wantErr: false, + }, + { + name: "with_passthrough", + content: `variable=Threads_connected +cycles=2 +-- +--user daniel +--password secret`, + wantOptions: `variable=Threads_connected +cycles=2 +`, + wantPassthrough: []string{"--user", "daniel", "--password", "secret"}, + wantErr: false, + }, + { + name: "passthrough_with_comments", + content: `option=value +-- +# This is a comment +--user root +# Another comment +--host localhost`, + wantOptions: `option=value +`, + wantPassthrough: []string{"--user", "root", "--host", "localhost"}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create temp file + tmpDir := t.TempDir() + tmpFile := filepath.Join(tmpDir, "test.conf") + if err := os.WriteFile(tmpFile, []byte(tt.content), 0644); err != nil { + t.Fatalf("Failed to create temp file: %v", err) + } + + got, err := loadConfig(tmpFile) + if (err != nil) != tt.wantErr { + t.Errorf("loadConfig() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if tt.wantErr { + return + } + + // Check options + var buf bytes.Buffer + if _, err := buf.ReadFrom(got.options); err != nil { + t.Fatalf("Failed to read options: %v", err) + } + if buf.String() != tt.wantOptions { + t.Errorf("loadConfig() options = %q, want %q", buf.String(), tt.wantOptions) + } + + // Check passthrough + if !reflect.DeepEqual(got.passthrough, tt.wantPassthrough) { + t.Errorf("loadConfig() passthrough = %v, want %v", got.passthrough, tt.wantPassthrough) + } + }) + } +} + +func TestParseConfigFlag(t *testing.T) { + tests := []struct { + name string + args []string + wantPaths []string + wantSpecified bool + wantRemainingLen int + wantErr bool + }{ + { + name: "no_config_flag", + args: []string{"--verbose", "--host=localhost"}, + wantPaths: nil, + wantSpecified: false, + wantRemainingLen: 2, + wantErr: false, + }, + { + name: "single_config", + args: []string{"--config", "/path/to/config.conf", "--verbose"}, + wantPaths: []string{"/path/to/config.conf"}, + wantSpecified: true, + wantRemainingLen: 1, + wantErr: false, + }, + { + name: "multiple_configs", + args: []string{"--config", "/etc/config.conf,~/.config.conf", "--verbose"}, + wantPaths: []string{"/etc/config.conf", "~/.config.conf"}, + wantSpecified: true, + wantRemainingLen: 1, + wantErr: false, + }, + { + name: "empty_config", + args: []string{"--config", "''", "--verbose"}, + wantPaths: nil, + wantSpecified: true, + wantRemainingLen: 1, + wantErr: false, + }, + { + name: "config_with_equals", + args: []string{"--config=/path/to/config.conf"}, + wantPaths: nil, + wantErr: true, + }, + { + name: "config_without_value", + args: []string{"--config"}, + wantPaths: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotPaths, gotSpecified, gotRemaining, err := parseConfigFlag(tt.args) + + if (err != nil) != tt.wantErr { + t.Errorf("parseConfigFlag() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if tt.wantErr { + return + } + + if !reflect.DeepEqual(gotPaths, tt.wantPaths) { + t.Errorf("parseConfigFlag() paths = %v, want %v", gotPaths, tt.wantPaths) + } + + if gotSpecified != tt.wantSpecified { + t.Errorf("parseConfigFlag() specified = %v, want %v", gotSpecified, tt.wantSpecified) + } + + if len(gotRemaining) != tt.wantRemainingLen { + t.Errorf("parseConfigFlag() remaining len = %d, want %d", len(gotRemaining), tt.wantRemainingLen) + } + }) + } +} + +type mockScanner struct { + value string + hasValue bool +} + +func (m *mockScanner) Len() int { + if m.hasValue { + return 1 + } + return 0 +} + +func (m *mockScanner) PopValueInto(name string, target interface{}) error { + if !m.hasValue { + return fmt.Errorf("no value set") + } + if s, ok := target.(*string); ok { + *s = m.value + return nil + } + return fmt.Errorf("scanner error") +} + +// Additional mock methods required by kong.Scanner interface +func (m *mockScanner) Pop() *kong.Token { return nil } +func (m *mockScanner) Peek() *kong.Token { return nil } +func (m *mockScanner) PushTyped(interface{}, *kong.Token) {} diff --git a/src/go/pt-k8s-debug-collector/main.go b/src/go/pt-k8s-debug-collector/main.go index 71d95dae2..268d6c802 100644 --- a/src/go/pt-k8s-debug-collector/main.go +++ b/src/go/pt-k8s-debug-collector/main.go @@ -1,11 +1,12 @@ package main import ( - "flag" "fmt" "log" "os" + "github.com/percona/percona-toolkit/src/go/lib/config" + "github.com/percona/percona-toolkit/src/go/lib/versioncheck" "github.com/percona/percona-toolkit/src/go/pt-k8s-debug-collector/dumper" ) @@ -21,41 +22,57 @@ var ( Commit string //nolint ) -func main() { - namespace := "" - resource := "" - clusterName := "" - kubeconfig := "" - forwardport := "" - version := false - skipPodSummary := false - - flag.StringVar(&namespace, "namespace", "", "Namespace for collecting data. If empty data will be collected from all namespaces") - flag.StringVar(&resource, "resource", "auto", "Collect data, specific to the resource. Supported values: pxc, psmdb, pg, pgv2, ps, none, auto") - flag.StringVar(&clusterName, "cluster", "", "Cluster name") - flag.StringVar(&kubeconfig, "kubeconfig", "", "Path to kubeconfig") - flag.StringVar(&forwardport, "forwardport", "", "Port to use for port forwarding") - flag.BoolVar(&version, "version", false, "Print version") - flag.BoolVar(&skipPodSummary, "skip-pod-summary", false, "Skip pod summary collection") - flag.Parse() +type cliOptions struct { + Namespace string `name:"namespace" help:"Namespace for collecting data. If empty data will be collected from all namespaces"` + Resource string `name:"resource" help:"Collect data, specific to the resource. Supported values: pxc, psmdb, pg, pgv2, ps, none, auto" default:"auto"` + ClusterName string `name:"cluster" help:"Cluster name"` + Kubeconfig string `name:"kubeconfig" help:"Path to kubeconfig"` + ForwardPort string `name:"forwardport" help:"Port to use for port forwarding"` + SkipPodSummary bool `name:"skip-pod-summary" help:"Skip pod summary collection"` + Version bool `name:"version"` + VersionCheck bool `name:"version-check" negatable:"" default:"true"` +} - if version { +func (c *cliOptions) AfterApply(args ...any) error { + if c.Version { fmt.Println(toolname) fmt.Printf("Version %s\n", Version) fmt.Printf("Build: %s using %s\n", Build, GoVersion) fmt.Printf("Commit: %s\n", Commit) + return nil + } - return + if len(c.ClusterName) > 0 { + c.Resource += "/" + c.ClusterName + } + + if c.VersionCheck { + advice, err := versioncheck.CheckUpdates(toolname, Version) + if err != nil { + log.Printf("cannot check version updates: %s", err.Error()) + } else if advice != "" { + log.Printf("%s", advice) + } } + return nil +} - if len(clusterName) > 0 { - resource += "/" + clusterName +func main() { + opts := &cliOptions{} + _, _, err := config.Setup(toolname, opts) + if err != nil { + log.Printf("cannot get parameters: %s", err.Error()) + os.Exit(1) + } + + if opts.Version { + return } - d := dumper.New("", namespace, resource, kubeconfig, forwardport, skipPodSummary) + d := dumper.New("", opts.Namespace, opts.Resource, opts.Kubeconfig, opts.ForwardPort, opts.SkipPodSummary) log.Println("Start collecting cluster data") - err := d.DumpCluster() + err = d.DumpCluster() if err != nil { log.Println("Error:", err) os.Exit(1) diff --git a/src/go/tests/lib/sample-config1.conf b/src/go/tests/lib/sample-config1.conf index 0ec00811d..f10a43a14 100644 --- a/src/go/tests/lib/sample-config1.conf +++ b/src/go/tests/lib/sample-config1.conf @@ -4,6 +4,6 @@ yesboolvar=yes falseboolvar=false noboolvar=no intvar=1 -#ignored comment +#ignoredcomment=abc floatvar=2.3 -stringvar=some string var having = and # +stringvar=some string var having = and # \ No newline at end of file diff --git a/src/go/tests/lib/sample-config2.conf b/src/go/tests/lib/sample-config2.conf index 93e5dbfbe..d1a9188a8 100644 --- a/src/go/tests/lib/sample-config2.conf +++ b/src/go/tests/lib/sample-config2.conf @@ -3,8 +3,8 @@ yesboolvar=no falseboolvar=true noboolvar=yes intvar=4 -#ignored comment +#ignoredcomment=abc floatvar=5.6 stringvar=some other string newstring=a new string - anotherint = 8 + anotherint = 8 \ No newline at end of file From ecf43ff38c1ae09103e1df4ba8983bcbe62e4e35 Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Thu, 29 Jan 2026 11:48:36 +0200 Subject: [PATCH 02/11] keep old config --- src/go/lib/config/old_config.go | 147 ++++++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) create mode 100644 src/go/lib/config/old_config.go diff --git a/src/go/lib/config/old_config.go b/src/go/lib/config/old_config.go new file mode 100644 index 000000000..820fc892b --- /dev/null +++ b/src/go/lib/config/old_config.go @@ -0,0 +1,147 @@ +package config + +import ( + "bufio" + "os" + "os/user" + "strconv" + "strings" +) + +type Config struct { + options map[string]interface{} +} + +func (c *Config) GetString(key string) string { + if val, ok := c.options[key]; ok { + if v, ok := val.(string); ok { + return v + } + } + return "" +} + +func (c *Config) GetInt64(key string) int64 { + if val, ok := c.options[key]; ok { + if v, ok := val.(int64); ok { + return v + } + } + return 0 +} + +func (c *Config) GetFloat64(key string) float64 { + if val, ok := c.options[key]; ok { + if v, ok := val.(float64); ok { + return v + } + } + return 0 +} + +func (c *Config) GetBool(key string) bool { + if val, ok := c.options[key]; ok { + if v, ok := val.(bool); ok { + return v + } + } + return false +} + +func (c *Config) HasKey(key string) bool { + _, ok := c.options[key] + return ok +} + +func DefaultConfigFiles(toolName string) ([]string, error) { + user, err := user.Current() + if err != nil { + return nil, err + } + + files := []string{ + "/etc/percona-toolkit/percona-toolkit.conf", + "/etc/percona-toolkit/${TOOLNAME}.conf", + "${HOME}/.percona-toolkit.conf", + "${HOME}/.${TOOLNAME}.conf", + } + + for i := 0; i < len(files); i++ { + files[i] = strings.Replace(files[i], "${TOOLNAME}", toolName, -1) + files[i] = strings.Replace(files[i], "${HOME}", user.HomeDir, -1) + } + + return files, nil +} + +func DefaultConfig(toolname string) *Config { + files, _ := DefaultConfigFiles(toolname) + return NewConfig(files...) +} + +func NewConfig(files ...string) *Config { + config := &Config{ + options: make(map[string]interface{}), + } + for _, filename := range files { + if _, err := os.Stat(filename); err == nil { + read(filename, config.options) + } + } + return config +} + +func read(filename string, opts map[string]interface{}) error { + f, err := os.Open(filename) + if err != nil { + return err + } + + scanner := bufio.NewScanner(f) + + for scanner.Scan() { + line := scanner.Text() + if line == "" || strings.HasPrefix(line, "#") { + continue + } + + m := strings.SplitN(scanner.Text(), "=", 2) + key := strings.TrimSpace(m[0]) + + if len(m) == 1 { + opts[key] = true + continue + } + + val := strings.TrimSpace(m[1]) + lcval := strings.ToLower(val) + + if lcval == "true" || lcval == "yes" { + opts[key] = true + continue + } + if lcval == "false" || lcval == "no" { + opts[key] = false + continue + } + + f, err := strconv.ParseFloat(val, 64) + if err != nil { + opts[key] = strings.TrimSpace(val) // string + continue + } + + if f == float64(int64(f)) { + opts[key] = int64(f) // int64 + continue + } + + opts[key] = f // float64 + } + + if err := scanner.Err(); err != nil { + return err + } + + return nil +} From 14324ff374b078089e3bbae2bdcf6e09f7c4cc35 Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Thu, 29 Jan 2026 15:22:26 +0200 Subject: [PATCH 03/11] add more tests --- src/go/lib/config/config.go | 6 +- src/go/lib/config/config_test.go | 134 +++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 2 deletions(-) diff --git a/src/go/lib/config/config.go b/src/go/lib/config/config.go index 002349bca..326d3829f 100644 --- a/src/go/lib/config/config.go +++ b/src/go/lib/config/config.go @@ -313,6 +313,8 @@ func loadConfigFiles(configPaths []string, specifiedConfig bool) ([]kong.Resolve return resolvers, filePassthrough, nil } +var GLOBAL_DEFAULT_PATH = "/etc/percona-toolkit/percona-toolkit.conf" + // getDefaultPaths returns the default configuration file paths for a tool. // Returns paths in order of precedence (lowest to highest): // 1. /etc/percona-toolkit/percona-toolkit.conf @@ -323,13 +325,13 @@ func getDefaultPaths(toolName string) []string { u, err := user.Current() if err != nil { return []string{ - "/etc/percona-toolkit/percona-toolkit.conf", + GLOBAL_DEFAULT_PATH, fmt.Sprintf("/etc/percona-toolkit/%s.conf", toolName), } } return []string{ - "/etc/percona-toolkit/percona-toolkit.conf", + GLOBAL_DEFAULT_PATH, fmt.Sprintf("/etc/percona-toolkit/%s.conf", toolName), filepath.Join(u.HomeDir, ".percona-toolkit.conf"), filepath.Join(u.HomeDir, fmt.Sprintf(".%s.conf", toolName)), diff --git a/src/go/lib/config/config_test.go b/src/go/lib/config/config_test.go index e7b6149ac..2a37251be 100644 --- a/src/go/lib/config/config_test.go +++ b/src/go/lib/config/config_test.go @@ -2,6 +2,7 @@ package config import ( "bytes" + "encoding/json" "fmt" "os" "os/user" @@ -433,6 +434,139 @@ cycles=2 } } +func TestCmdWithArgs(t *testing.T) { + tests := []struct { + name string + args []string + cli any + wantJson string + }{ + { + name: "cmd_one_arg", + args: []string{"test-cmd", "file.txt"}, + cli: &struct { + TestCmd struct { + Paths []string `arg:"" name:"paths"` + } `cmd:"" name:"test-cmd"` + }{}, + wantJson: `{"TestCmd":{"Paths":["file.txt"]}}`, + }, + { + name: "cmd_one_path_arg", + args: []string{"test-cmd", "tests/logs/upgrade/node1.log"}, + cli: &struct { + TestCmd struct { + Paths []string `arg:"" name:"paths"` + } `cmd:"" name:"test-cmd"` + }{}, + wantJson: `{"TestCmd":{"Paths":["tests/logs/upgrade/node1.log"]}}`, + }, + { + name: "cmd_many_arg", + args: []string{"test-cmd", "file.txt", "file2.txt", "file3.txt"}, + cli: &struct { + TestCmd struct { + Paths []string `arg:"" name:"paths"` + } `cmd:"" name:"test-cmd"` + }{}, + wantJson: `{"TestCmd":{"Paths":["file.txt","file2.txt","file3.txt"]}}`, + }, + } + + for _, test := range tests { + os.Args = []string{test.name} + os.Args = append(os.Args, test.args...) + t.Log(os.Args) + _, _, err := Setup(test.name, test.cli) + if err != nil { + t.Fatal(err) + } + data, err := json.Marshal(test.cli) + if err != nil { + t.Fatal(err) + } + if string(data) != test.wantJson { + t.Errorf("got %s, want %s", string(data), test.wantJson) + } + } +} + +func TestCmdWithArgsAndConfig(t *testing.T) { + tests := []struct { + name string + args []string + cli any + config string + wantJson string + }{ + { + name: "cmd_one_arg", + args: []string{"test-cmd", "file.txt"}, + config: `no-version`, + cli: &struct { + TestCmd struct { + Paths []string `arg:"" name:"paths"` + } `cmd:"" name:"test-cmd"` + Version bool `negatable:"" default:"true" name:"version"` + }{}, + wantJson: `{"TestCmd":{"Paths":["file.txt"]},"Version":false}`, + }, + { + name: "cmd_one_arg", + args: []string{"test-cmd", "file.txt"}, + config: `test-list=a,b,c`, + cli: &struct { + TestCmd struct { + Paths []string `arg:"" name:"paths"` + } `cmd:"" name:"test-cmd"` + TestList []string `name:"test-list"` + }{}, + wantJson: `{"TestCmd":{"Paths":["file.txt"]},"TestList":["a","b","c"]}`, + }, + { + name: "cmd_one_arg", + args: []string{"test-cmd", "file.txt"}, + config: `test-list=a,b,c + limit=123`, + cli: &struct { + TestCmd struct { + Paths []string `arg:"" name:"paths"` + Limit int `name:"limit"` + } `cmd:"" name:"test-cmd"` + TestList []string `name:"test-list"` + }{}, + wantJson: `{"TestCmd":{"Paths":["file.txt"],"Limit":123},"TestList":["a","b","c"]}`, + }, + } + + var oldGlobalDefaultPath = GLOBAL_DEFAULT_PATH + defer func() { + GLOBAL_DEFAULT_PATH = oldGlobalDefaultPath + }() + for _, test := range tests { + tmpDir := t.TempDir() + tmpConf := filepath.Join(tmpDir, "test.conf") + os.WriteFile(tmpConf, []byte(test.config), 0644) + + GLOBAL_DEFAULT_PATH = tmpConf + + os.Args = []string{test.name} + os.Args = append(os.Args, test.args...) + t.Log(os.Args) + _, _, err := Setup(test.name, test.cli) + if err != nil { + t.Fatal(err) + } + data, err := json.Marshal(test.cli) + if err != nil { + t.Fatal(err) + } + if string(data) != test.wantJson { + t.Errorf("got %s, want %s", string(data), test.wantJson) + } + } +} + func TestParseConfigFlag(t *testing.T) { tests := []struct { name string From 4ce6cf88582ba51069eb2106316473e266abc709 Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Thu, 29 Jan 2026 15:25:56 +0200 Subject: [PATCH 04/11] PT-2368 - migrate pt-galera-log-explainer to a new config lib This change introduces support for a new config lib. --- src/go/pt-galera-log-explainer/main.go | 60 ++++++++++++++++++-------- 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/src/go/pt-galera-log-explainer/main.go b/src/go/pt-galera-log-explainer/main.go index f872bb0c4..2e7537c85 100644 --- a/src/go/pt-galera-log-explainer/main.go +++ b/src/go/pt-galera-log-explainer/main.go @@ -6,6 +6,8 @@ import ( "time" "github.com/alecthomas/kong" + "github.com/percona/percona-toolkit/src/go/lib/config" + "github.com/percona/percona-toolkit/src/go/lib/versioncheck" "github.com/percona/percona-toolkit/src/go/pt-galera-log-explainer/regex" "github.com/percona/percona-toolkit/src/go/pt-galera-log-explainer/translate" "github.com/percona/percona-toolkit/src/go/pt-galera-log-explainer/types" @@ -26,9 +28,7 @@ var ( Commit string //nolint ) -var buildInfo = fmt.Sprintf("%s\nVersion %s\nBuild: %s using %s\nCommit: %s", toolname, Version, Build, GoVersion, Commit) - -var CLI struct { +type CliOptions struct { NoColor bool Since *time.Time `help:"Only list events after this date, format: 2023-01-23T03:53:40Z (RFC3339)"` Until *time.Time `help:"Only list events before this date"` @@ -39,29 +39,55 @@ var CLI struct { MergeByDirectory bool `help:"Instead of relying on identification, merge contexts and columns by base directory. Very useful when dealing with many small logs organized per directories."` SkipMerge bool `help:"Disable the ability to merge log files together. Can be used when every nodes have the same wsrep_node_name"` - List list `cmd:""` - Whois whois `cmd:""` - // Sed sed `cmd:""` + List list `cmd:""` + Whois whois `cmd:""` Ctx ctx `cmd:""` RegexList regexList `cmd:""` Conflicts conflicts `cmd:""` - - Version kong.VersionFlag + //Sed sed `cmd:""` GrepCmd string `help:"'grep' command path. Could need to be set to 'ggrep' for darwin systems" default:"grep"` CustomRegexes map[string]string `help:"Add custom regexes, printed in magenta. Format: (golang regex string)=[optional static message to display]. If the static message is left empty, the captured string will be printed instead. Custom regexes are separated using semi-colon. Example: --custom-regexes=\"Page cleaner took [0-9]*ms to flush [0-9]* pages=;doesn't recommend.*pxc_strict_mode=unsafe query used\""` + Version kong.VersionFlag `name:"version" help:"Show version and exit"` + VersionCheck bool `name:"version-check" negatable:"" default:"true"` } +func (c *CliOptions) AfterApply() error { + if c.VersionCheck { + advice, err := versioncheck.CheckUpdates(toolname, Version) + if err != nil { + log.Error().Msgf("cannot check version updates: %s", err.Error()) + } else if advice != "" { + log.Info().Msgf("%s", advice) + } + } + + return nil +} + +var CLI = &CliOptions{} + func main() { - kongcli := kong.Parse(&CLI, - kong.Name(toolname), + kCtx, _, err := config.Setup( + toolname, + CLI, kong.Description("An utility to merge and help analyzing Galera logs"), - kong.UsageOnError(), kong.Vars{ - "version": buildInfo, + "version": fmt.Sprintf( + "%s\nVersion %s\nBuild: %s using %s\nCommit: %s", + toolname, Version, Build, GoVersion, Commit, + ), }, ) + if err != nil { + log.Error().Msgf("cannot get parameters: %s", err.Error()) + os.Exit(1) + } + + if CLI.Version { + return + } zerolog.TimeFieldFormat = zerolog.TimeFormatUnix zerolog.SetGlobalLevel(zerolog.InfoLevel) @@ -73,10 +99,10 @@ func main() { utils.SkipColor = CLI.NoColor - err := regex.AddCustomRegexes(CLI.CustomRegexes) - kongcli.FatalIfErrorf(err) + err = regex.AddCustomRegexes(CLI.CustomRegexes) + kCtx.FatalIfErrorf(err) - for _, path := range kongcli.Path { + for _, path := range kCtx.Path { if path.Positional != nil && path.Positional.Name == "paths" { paths, ok := path.Positional.Target.Interface().([]string) if ok && !CLI.PxcOperator && !CLI.SkipOperatorDetection && areOperatorFiles(paths) { @@ -88,6 +114,6 @@ func main() { translate.AssumeIPStable = !CLI.PxcOperator - err = kongcli.Run() - kongcli.FatalIfErrorf(err) + err = kCtx.Run() + kCtx.FatalIfErrorf(err) } From 7683c460c44e2386f1ec7c3823d6b2704b61e661 Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Fri, 30 Jan 2026 22:56:17 +0200 Subject: [PATCH 05/11] quick fix --- src/go/pt-k8s-debug-collector/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/go/pt-k8s-debug-collector/main.go b/src/go/pt-k8s-debug-collector/main.go index 268d6c802..e91c17d0d 100644 --- a/src/go/pt-k8s-debug-collector/main.go +++ b/src/go/pt-k8s-debug-collector/main.go @@ -33,7 +33,7 @@ type cliOptions struct { VersionCheck bool `name:"version-check" negatable:"" default:"true"` } -func (c *cliOptions) AfterApply(args ...any) error { +func (c *cliOptions) AfterApply() error { if c.Version { fmt.Println(toolname) fmt.Printf("Version %s\n", Version) From 80e238cbc67319d21c3e0f0ebdfba711f0cadc97 Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Tue, 17 Feb 2026 19:40:53 +0200 Subject: [PATCH 06/11] Refactor --- src/go/lib/config/commonFlags.go | 30 ++++ src/go/lib/config/config.go | 54 ++++--- src/go/lib/config/config_test.go | 166 ++++++++++++++-------- src/go/pt-galera-log-explainer/README.rst | 3 + src/go/pt-galera-log-explainer/main.go | 1 + src/go/pt-k8s-debug-collector/README.rst | 4 + src/go/pt-k8s-debug-collector/main.go | 14 +- 7 files changed, 190 insertions(+), 82 deletions(-) create mode 100644 src/go/lib/config/commonFlags.go diff --git a/src/go/lib/config/commonFlags.go b/src/go/lib/config/commonFlags.go new file mode 100644 index 000000000..599da9b9d --- /dev/null +++ b/src/go/lib/config/commonFlags.go @@ -0,0 +1,30 @@ +package config + +// Config specifies a list of configuration files to read. +// Following the Percona Toolkit specification: +// 1. Position: The --config option must be the first argument on the command line. +// Specifying it elsewhere will result in an error. +// 2. Syntax: It does not support the equal sign. +// Correct: --config /path/to/file. Incorrect: --config=/path/to/file. +// 3. Multiple files: You can provide a comma-separated list of files. +// 4. Disabling configs: To prevent the tool from reading any configuration files +// at all (including system-wide and user defaults), specify an empty string: --config ”. +// 5. Precedence: If specified, only the provided files are read. If omitted, +// the tool searches for default configuration files in standard locations. +type ConfigFlag struct { + Config []string `name:"config" help:"List of Percona Toolkit configuration file(s) separated by comma without equal sign. Must be a first flag. Uses default config file locations if not specified."` +} + +// VersionFlag adds a --version flag that prints the tool version and exits. +// Embed this struct into the CLI struct to enable version reporting. +type VersionFlag struct { + Version bool `name:"version"` +} + +// VersionCheckFlag adds a --version-check / --no-version-check flag that controls +// whether the tool checks for a newer version of itself on startup. +// Enabled by default; disable with --no-version-check. +// Embed this struct into the CLI struct to enable version check control: +type VersionCheckFlag struct { + VersionCheck bool `name:"version-check" negatable:"" default:"true"` +} diff --git a/src/go/lib/config/config.go b/src/go/lib/config/config.go index 5bc9d2e70..069a50916 100644 --- a/src/go/lib/config/config.go +++ b/src/go/lib/config/config.go @@ -24,7 +24,6 @@ import ( "path/filepath" "reflect" "strings" - "sync" "github.com/alecthomas/kong" ) @@ -90,7 +89,6 @@ func (p *StdinRequestString) Request(f func() (string, error)) error { } type PerconaResolver struct { - mu sync.RWMutex values map[string]any } @@ -155,8 +153,6 @@ func (p *PerconaResolver) Validate(app *kong.Application) error { } func (p *PerconaResolver) Resolve(ctx *kong.Context, parent *kong.Path, flag *kong.Flag) (any, error) { - p.mu.RLock() - defer p.mu.RUnlock() return p.values[flag.Name], nil } @@ -217,9 +213,7 @@ func loadConfig(path string) (*configFile, error) { // Setup initializes kong parser with Percona Toolkit configuration file support. // // It handles: -// - --config flag as the first argument (must be separated by space) -// - Default config file locations if --config not specified -// - Multiple config files (comma-separated) +// - --config flag as the first argument // - Passthrough arguments after "--" in config files // - Accepting kong.Options // @@ -230,7 +224,12 @@ func loadConfig(path string) (*configFile, error) { func Setup(toolName string, cli any, options ...kong.Option) (*kong.Context, []string, error) { rawArgs := os.Args[1:] - configPaths, specifiedConfig, remainingArgs, err := parseConfigFlag(rawArgs) + err := validateConfigPosition(rawArgs) + if err != nil { + return nil, nil, err + } + + configPaths, specifiedConfig, err := parseConfigFlag(rawArgs) if err != nil { return nil, nil, err } @@ -256,34 +255,49 @@ func Setup(toolName string, cli any, options ...kong.Option) (*kong.Context, []s return nil, nil, err } - ctx, err := parser.Parse(remainingArgs) + ctx, err := parser.Parse(rawArgs) parser.FatalIfErrorf(err) return ctx, filePassthrough, nil } -func parseConfigFlag(rawArgs []string) ([]string, bool, []string, error) { - if len(rawArgs) == 0 { - return nil, false, rawArgs, nil +func validateConfigPosition(args []string) error { + if len(args) == 0 { + return nil + } + + if args[0] == "--config" { + return nil } - if strings.HasPrefix(rawArgs[0], "--config=") { - return nil, false, nil, errors.New("Error: --config must not use '='. Use: --config /path/to/file") + for i, a := range args { + if a == "--config" { + return fmt.Errorf("--config must be the first argument (found at position %d)", i+1) + } + if strings.HasPrefix(a, "--config=") { + return fmt.Errorf("--config must not use '=' syntax. Use: --config file.conf") + } + } + + return nil +} + +func parseConfigFlag(rawArgs []string) ([]string, bool, error) { + if len(rawArgs) == 0 { + return nil, false, nil } if rawArgs[0] != "--config" { - return nil, false, rawArgs, nil + return nil, false, nil } if len(rawArgs) < 2 { - return nil, false, nil, errors.New("Error: --config requires a value") + return nil, false, errors.New("Error: --config requires a value") } val := rawArgs[1] - remainingArgs := rawArgs[2:] - if val == "" || val == "''" || val == `""` { - return nil, true, remainingArgs, nil + return nil, true, nil } configPaths := strings.Split(val, ",") @@ -292,7 +306,7 @@ func parseConfigFlag(rawArgs []string) ([]string, bool, []string, error) { configPaths[i] = strings.TrimSpace(configPaths[i]) } - return configPaths, true, remainingArgs, nil + return configPaths, true, nil } func loadConfigFiles(configPaths []string, specifiedConfig bool) ([]kong.Resolver, []string, error) { diff --git a/src/go/lib/config/config_test.go b/src/go/lib/config/config_test.go index c9d743281..ff085d825 100644 --- a/src/go/lib/config/config_test.go +++ b/src/go/lib/config/config_test.go @@ -25,11 +25,11 @@ import ( "strings" "testing" - "github.com/alecthomas/kong" "github.com/percona/percona-toolkit/src/go/lib/tutil" ) type KongFlags struct { + ConfigFlag VersionCheck bool `name:"version-check" negatable:"" default:"true"` TrueBoolVar bool `name:"trueboolvar" help:"test"` YesBoolVar BoolYN `name:"yesboolvar" help:"test"` @@ -489,7 +489,6 @@ func TestCmdWithArgs(t *testing.T) { for _, test := range tests { os.Args = []string{test.name} os.Args = append(os.Args, test.args...) - t.Log(os.Args) _, _, err := Setup(test.name, test.cli) if err != nil { t.Fatal(err) @@ -504,7 +503,7 @@ func TestCmdWithArgs(t *testing.T) { } } -func TestCmdWithArgsAndConfig(t *testing.T) { +func TestCmdWithArgsAndDefaultConfig(t *testing.T) { tests := []struct { name string args []string @@ -580,7 +579,7 @@ func TestCmdWithArgsAndConfig(t *testing.T) { } } -func TestParseConfigFlag(t *testing.T) { +func TestParseAndValidateConfigFlag(t *testing.T) { tests := []struct { name string args []string @@ -590,36 +589,32 @@ func TestParseConfigFlag(t *testing.T) { wantErr bool }{ { - name: "no_config_flag", - args: []string{"--verbose", "--host=localhost"}, - wantPaths: nil, - wantSpecified: false, - wantRemainingLen: 2, - wantErr: false, + name: "no_config_flag", + args: []string{"--verbose", "--host=localhost"}, + wantPaths: nil, + wantSpecified: false, + wantErr: false, }, { - name: "single_config", - args: []string{"--config", "/path/to/config.conf", "--verbose"}, - wantPaths: []string{"/path/to/config.conf"}, - wantSpecified: true, - wantRemainingLen: 1, - wantErr: false, + name: "single_config", + args: []string{"--config", "/path/to/config.conf", "--verbose"}, + wantPaths: []string{"/path/to/config.conf"}, + wantSpecified: true, + wantErr: false, }, { - name: "multiple_configs", - args: []string{"--config", "/etc/config.conf,~/.config.conf", "--verbose"}, - wantPaths: []string{"/etc/config.conf", "~/.config.conf"}, - wantSpecified: true, - wantRemainingLen: 1, - wantErr: false, + name: "multiple_configs", + args: []string{"--config", "/etc/config.conf,~/.config.conf", "--verbose"}, + wantPaths: []string{"/etc/config.conf", "~/.config.conf"}, + wantSpecified: true, + wantErr: false, }, { - name: "empty_config", - args: []string{"--config", "''", "--verbose"}, - wantPaths: nil, - wantSpecified: true, - wantRemainingLen: 1, - wantErr: false, + name: "empty_config", + args: []string{"--config", "''", "--verbose"}, + wantPaths: nil, + wantSpecified: true, + wantErr: false, }, { name: "config_with_equals", @@ -637,9 +632,15 @@ func TestParseConfigFlag(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotPaths, gotSpecified, gotRemaining, err := parseConfigFlag(tt.args) + err := validateConfigPosition(tt.args) + if err != nil && !tt.wantErr { + t.Errorf("parseConfigFlag() error = %v, wantErr %v", err, tt.wantErr) + return + } - if (err != nil) != tt.wantErr { + gotPaths, gotSpecified, err := parseConfigFlag(tt.args) + + if err != nil && !tt.wantErr { t.Errorf("parseConfigFlag() error = %v, wantErr %v", err, tt.wantErr) return } @@ -655,38 +656,91 @@ func TestParseConfigFlag(t *testing.T) { if gotSpecified != tt.wantSpecified { t.Errorf("parseConfigFlag() specified = %v, want %v", gotSpecified, tt.wantSpecified) } - - if len(gotRemaining) != tt.wantRemainingLen { - t.Errorf("parseConfigFlag() remaining len = %d, want %d", len(gotRemaining), tt.wantRemainingLen) - } }) } } -type mockScanner struct { - value string - hasValue bool -} - -func (m *mockScanner) Len() int { - if m.hasValue { - return 1 +func TestSetupWithConfigFlag(t *testing.T) { + type TestCLI struct { + ConfigFlag + User string `name:"user" default:"guest"` + Host string `name:"host" default:"localhost"` } - return 0 -} -func (m *mockScanner) PopValueInto(name string, target interface{}) error { - if !m.hasValue { - return fmt.Errorf("no value set") + tests := []struct { + name string + args []string + globalConf string + customConf string + wantUser string + wantHost string + wantErr bool + }{ + { + name: "Explicit config overrides default and flags", + args: []string{"--config", "CUSTOM_PATH"}, + globalConf: "user=default_user", + customConf: "user=custom_user\nhost=remote", + wantUser: "custom_user", + wantHost: "remote", + }, + { + name: "Empty config flag disables all configs", + args: []string{"--config", ""}, + globalConf: "user=should_not_be_read", + customConf: "", + wantUser: "guest", + wantHost: "localhost", + }, + { + name: "Error if config is not first", + args: []string{"--user", "admin", "--config", "some.conf"}, + wantErr: true, + }, } - if s, ok := target.(*string); ok { - *s = m.value - return nil + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + + globalPath := filepath.Join(tmpDir, "global.conf") + os.WriteFile(globalPath, []byte(tt.globalConf), 0644) + + oldGlobal := GLOBAL_DEFAULT_PATH + GLOBAL_DEFAULT_PATH = globalPath + defer func() { GLOBAL_DEFAULT_PATH = oldGlobal }() + + args := append([]string{"tool"}, tt.args...) + for i, arg := range args { + if arg == "CUSTOM_PATH" { + customPath := filepath.Join(tmpDir, "custom.conf") + os.WriteFile(customPath, []byte(tt.customConf), 0644) + args[i] = customPath + } + } + + os.Args = args + cli := &TestCLI{} + + _, _, err := Setup("test-tool", cli) + + if tt.wantErr { + if err == nil { + t.Fatal("expected error but got nil") + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if cli.User != tt.wantUser { + t.Errorf("User: got %s, want %s", cli.User, tt.wantUser) + } + if cli.Host != tt.wantHost { + t.Errorf("Host: got %s, want %s", cli.Host, tt.wantHost) + } + }) } - return fmt.Errorf("scanner error") } - -// Additional mock methods required by kong.Scanner interface -func (m *mockScanner) Pop() *kong.Token { return nil } -func (m *mockScanner) Peek() *kong.Token { return nil } -func (m *mockScanner) PushTyped(interface{}, *kong.Token) {} diff --git a/src/go/pt-galera-log-explainer/README.rst b/src/go/pt-galera-log-explainer/README.rst index 0ef0652d0..c654431c9 100644 --- a/src/go/pt-galera-log-explainer/README.rst +++ b/src/go/pt-galera-log-explainer/README.rst @@ -99,6 +99,9 @@ Will print every implemented regexes: Available flags ~~~~~~~~~~~~~~~ +``--config`` + List of Percona Toolkit configuration file(s) separeted by comma without equal sign. Must be a first flag. Uses default config file locations if not specified. + ``-h``, ``--help`` Show help and exit. diff --git a/src/go/pt-galera-log-explainer/main.go b/src/go/pt-galera-log-explainer/main.go index 447d0df18..760f20c67 100644 --- a/src/go/pt-galera-log-explainer/main.go +++ b/src/go/pt-galera-log-explainer/main.go @@ -42,6 +42,7 @@ var ( ) type CliOptions struct { + config.ConfigFlag NoColor bool Since *time.Time `help:"Only list events after this date, format: 2023-01-23T03:53:40Z (RFC3339)"` Until *time.Time `help:"Only list events before this date"` diff --git a/src/go/pt-k8s-debug-collector/README.rst b/src/go/pt-k8s-debug-collector/README.rst index 3e5922e42..82a217532 100644 --- a/src/go/pt-k8s-debug-collector/README.rst +++ b/src/go/pt-k8s-debug-collector/README.rst @@ -125,6 +125,10 @@ Usage Supported Flags ================ +``--config`` + +List of Percona Toolkit configuration file(s) separeted by comma without equal sign. Must be a first flag. Uses default config file locations if not specified. + ``--resource`` Targeted custom resource name. Supported values: diff --git a/src/go/pt-k8s-debug-collector/main.go b/src/go/pt-k8s-debug-collector/main.go index d0a3551f8..5e82394db 100644 --- a/src/go/pt-k8s-debug-collector/main.go +++ b/src/go/pt-k8s-debug-collector/main.go @@ -36,14 +36,15 @@ var ( ) type cliOptions struct { + config.ConfigFlag Namespace string `name:"namespace" help:"Namespace for collecting data. If empty data will be collected from all namespaces"` Resource string `name:"resource" help:"Collect data, specific to the resource. Supported values: pxc, psmdb, pg, pgv2, ps, none, auto" default:"auto"` ClusterName string `name:"cluster" help:"Cluster name"` Kubeconfig string `name:"kubeconfig" help:"Path to kubeconfig"` ForwardPort string `name:"forwardport" help:"Port to use for port forwarding"` SkipPodSummary bool `name:"skip-pod-summary" help:"Skip pod summary collection"` - Version bool `name:"version"` - VersionCheck bool `name:"version-check" negatable:"" default:"true"` + config.VersionCheckFlag + config.VersionFlag } func (c *cliOptions) AfterApply() error { @@ -55,10 +56,6 @@ func (c *cliOptions) AfterApply() error { return nil } - if len(c.ClusterName) > 0 { - c.Resource += "/" + c.ClusterName - } - if c.VersionCheck { advice, err := versioncheck.CheckUpdates(toolname, Version) if err != nil { @@ -67,6 +64,11 @@ func (c *cliOptions) AfterApply() error { log.Printf("%s", advice) } } + + if len(c.ClusterName) > 0 { + c.Resource += "/" + c.ClusterName + } + return nil } From 1aa2018def7d468a2fd38f60dfa2b77891bfe50e Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Tue, 17 Feb 2026 19:42:57 +0200 Subject: [PATCH 07/11] Add header --- src/go/lib/config/commonFlags.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/go/lib/config/commonFlags.go b/src/go/lib/config/commonFlags.go index 599da9b9d..85042e115 100644 --- a/src/go/lib/config/commonFlags.go +++ b/src/go/lib/config/commonFlags.go @@ -1,3 +1,16 @@ +// This program is copyright 2017-2026 Percona LLC and/or its affiliates. +// +// THIS PROGRAM IS PROVIDED "AS IS" AND WITHOUT ANY EXPRESS OR IMPLIED +// WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE. +// +// This program is free software; you can redistribute it and/or modify it under +// the terms of the GNU General Public License as published by the Free Software +// Foundation, version 2. +// +// You should have received a copy of the GNU General Public License, version 2 +// along with this program; if not, see . + package config // Config specifies a list of configuration files to read. From 68973a76bc5fbcbbbfc3f83cacb47df0d8f1214b Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Mon, 2 Mar 2026 13:20:35 +0200 Subject: [PATCH 08/11] Add header --- src/go/lib/config/old_config.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/go/lib/config/old_config.go b/src/go/lib/config/old_config.go index 820fc892b..97b605d8f 100644 --- a/src/go/lib/config/old_config.go +++ b/src/go/lib/config/old_config.go @@ -1,3 +1,16 @@ +// This program is copyright 2017-2026 Percona LLC and/or its affiliates. +// +// THIS PROGRAM IS PROVIDED "AS IS" AND WITHOUT ANY EXPRESS OR IMPLIED +// WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE. +// +// This program is free software; you can redistribute it and/or modify it under +// the terms of the GNU General Public License as published by the Free Software +// Foundation, version 2. +// +// You should have received a copy of the GNU General Public License, version 2 +// along with this program; if not, see . + package config import ( From a607c22eb3b50d8fcf6e7ec38bdda6839c34bbb7 Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Mon, 2 Mar 2026 13:21:51 +0200 Subject: [PATCH 09/11] Add deprecated header --- src/go/lib/config/old_config.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/go/lib/config/old_config.go b/src/go/lib/config/old_config.go index 97b605d8f..8ee21eb8d 100644 --- a/src/go/lib/config/old_config.go +++ b/src/go/lib/config/old_config.go @@ -11,6 +11,15 @@ // You should have received a copy of the GNU General Public License, version 2 // along with this program; if not, see . +// DEPRECATED: This file is kept for backward compatibility only. +// +// This configuration implementation is considered legacy and is no longer +// actively maintained. It is read-only and should not be extended, +// modified, or used for new development. +// +// New code must use the replacement configuration mechanism instead. +// This file will be removed in a future major release. + package config import ( From 1f7a56b4c6ad4ce533213dc78edb04d8ca64b0e2 Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Wed, 4 Mar 2026 19:40:53 +0200 Subject: [PATCH 10/11] Update src/go/pt-galera-log-explainer/README.rst Co-authored-by: Sveta Smirnova --- src/go/pt-galera-log-explainer/README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/go/pt-galera-log-explainer/README.rst b/src/go/pt-galera-log-explainer/README.rst index c654431c9..d9405728a 100644 --- a/src/go/pt-galera-log-explainer/README.rst +++ b/src/go/pt-galera-log-explainer/README.rst @@ -100,7 +100,7 @@ Available flags ~~~~~~~~~~~~~~~ ``--config`` - List of Percona Toolkit configuration file(s) separeted by comma without equal sign. Must be a first flag. Uses default config file locations if not specified. + List of Percona Toolkit configuration file(s) separated by a comma without an equal sign. Must be a first flag. Uses default config file locations if not specified. ``-h``, ``--help`` Show help and exit. From c00f33343171a92d86a191a5798b34e527bc9c58 Mon Sep 17 00:00:00 2001 From: Vladyslav Yurchenko Date: Fri, 6 Mar 2026 14:44:54 +0200 Subject: [PATCH 11/11] Update src/go/pt-k8s-debug-collector/README.rst Co-authored-by: Sveta Smirnova --- src/go/pt-k8s-debug-collector/README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/go/pt-k8s-debug-collector/README.rst b/src/go/pt-k8s-debug-collector/README.rst index 82a217532..73bd8260b 100644 --- a/src/go/pt-k8s-debug-collector/README.rst +++ b/src/go/pt-k8s-debug-collector/README.rst @@ -127,7 +127,7 @@ Supported Flags ``--config`` -List of Percona Toolkit configuration file(s) separeted by comma without equal sign. Must be a first flag. Uses default config file locations if not specified. +List of Percona Toolkit configuration file(s) separated by a comma without an equal sign. Must be a first flag. Uses default config file locations if not specified. ``--resource``