diff --git a/clients/cmd/promtail/main.go b/clients/cmd/promtail/main.go index e4e1ac298bc15..665a240879f38 100644 --- a/clients/cmd/promtail/main.go +++ b/clients/cmd/promtail/main.go @@ -65,7 +65,7 @@ func (c *Config) Clone() flagext.Registerer { func main() { // Load config, merging config file and CLI flags var config Config - if err := cfg.Parse(&config); err != nil { + if err := cfg.DefaultUnmarshal(&config, os.Args[1:], flag.CommandLine); err != nil { fmt.Println("Unable to parse config:", err) os.Exit(1) } diff --git a/cmd/loki/main.go b/cmd/loki/main.go index 10f480604607d..53e17d56f02d0 100644 --- a/cmd/loki/main.go +++ b/cmd/loki/main.go @@ -7,7 +7,6 @@ import ( "reflect" "github.com/go-kit/kit/log/level" - "github.com/grafana/dskit/flagext" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/version" "github.com/weaveworks/common/logging" @@ -26,43 +25,14 @@ func init() { prometheus.MustRegister(version.NewCollector("loki")) } -type Config struct { - loki.Config `yaml:",inline"` - printVersion bool - verifyConfig bool - printConfig bool - logConfig bool - configFile string - configExpandEnv bool -} - -func (c *Config) RegisterFlags(f *flag.FlagSet) { - f.BoolVar(&c.printVersion, "version", false, "Print this builds version information") - f.BoolVar(&c.verifyConfig, "verify-config", false, "Verify config file and exits") - f.BoolVar(&c.printConfig, "print-config-stderr", false, "Dump the entire Loki config object to stderr") - f.BoolVar(&c.logConfig, "log-config-reverse-order", false, "Dump the entire Loki config object at Info log "+ - "level with the order reversed, reversing the order makes viewing the entries easier in Grafana.") - f.StringVar(&c.configFile, "config.file", "", "yaml file to load") - f.BoolVar(&c.configExpandEnv, "config.expand-env", false, "Expands ${var} in config according to the values of the environment variables.") - c.Config.RegisterFlags(f) -} - -// Clone takes advantage of pass-by-value semantics to return a distinct *Config. -// This is primarily used to parse a different flag set without mutating the original *Config. -func (c *Config) Clone() flagext.Registerer { - return func(c Config) *Config { - return &c - }(*c) -} - func main() { - var config Config + var config loki.ConfigWrapper - if err := cfg.Parse(&config); err != nil { + if err := cfg.DynamicUnmarshal(&config, os.Args[1:], flag.CommandLine); err != nil { fmt.Fprintf(os.Stderr, "failed parsing config: %v\n", err) os.Exit(1) } - if config.printVersion { + if config.PrintVersion { fmt.Println(version.Print("loki")) os.Exit(0) } @@ -87,19 +57,19 @@ func main() { os.Exit(1) } - if config.verifyConfig { + if config.VerifyConfig { level.Info(util_log.Logger).Log("msg", "config is valid") os.Exit(0) } - if config.printConfig { + if config.PrintConfig { err := logutil.PrintConfig(os.Stderr, &config) if err != nil { level.Error(util_log.Logger).Log("msg", "failed to print config to stderr", "err", err.Error()) } } - if config.logConfig { + if config.LogConfig { err := logutil.LogConfig(&config) if err != nil { level.Error(util_log.Logger).Log("msg", "failed to log config object", "err", err.Error()) diff --git a/cmd/migrate/main.go b/cmd/migrate/main.go index 694e0fd961341..038849683a8c5 100644 --- a/cmd/migrate/main.go +++ b/cmd/migrate/main.go @@ -52,7 +52,7 @@ func main() { flag.Parse() // Create a set of defaults - if err := cfg.Unmarshal(&defaultsConfig, cfg.Defaults()); err != nil { + if err := cfg.Unmarshal(&defaultsConfig, cfg.Defaults(flag.CommandLine)); err != nil { log.Println("Failed parsing defaults config:", err) os.Exit(1) } diff --git a/pkg/loki/common/common.go b/pkg/loki/common/common.go new file mode 100644 index 0000000000000..cce66321362cf --- /dev/null +++ b/pkg/loki/common/common.go @@ -0,0 +1,6 @@ +package common + +// Config holds common config that can be shared between multiple other config sections +type Config struct { + PathPrefix string `yaml:"path_prefix"` +} diff --git a/pkg/loki/config_wrapper.go b/pkg/loki/config_wrapper.go new file mode 100644 index 0000000000000..732ad5c7916fe --- /dev/null +++ b/pkg/loki/config_wrapper.go @@ -0,0 +1,74 @@ +package loki + +import ( + "flag" + "fmt" + + "github.com/grafana/dskit/flagext" + "github.com/pkg/errors" + + "github.com/grafana/loki/pkg/util/cfg" +) + +// ConfigWrapper is a struct containing the Loki config along with other values that can be set on the command line +// for interacting with the config file or the application directly. +// ConfigWrapper implements cfg.DynamicCloneable, allowing configuration to be dynamically set based +// on the logic in ApplyDynamicConfig, which receives values set in config file +type ConfigWrapper struct { + Config `yaml:",inline"` + PrintVersion bool + VerifyConfig bool + PrintConfig bool + LogConfig bool + ConfigFile string + ConfigExpandEnv bool +} + +func (c *ConfigWrapper) RegisterFlags(f *flag.FlagSet) { + f.BoolVar(&c.PrintVersion, "version", false, "Print this builds version information") + f.BoolVar(&c.VerifyConfig, "verify-config", false, "Verify config file and exits") + f.BoolVar(&c.PrintConfig, "print-config-stderr", false, "Dump the entire Loki config object to stderr") + f.BoolVar(&c.LogConfig, "log-config-reverse-order", false, "Dump the entire Loki config object at Info log "+ + "level with the order reversed, reversing the order makes viewing the entries easier in Grafana.") + f.StringVar(&c.ConfigFile, "config.file", "", "yaml file to load") + f.BoolVar(&c.ConfigExpandEnv, "config.expand-env", false, "Expands ${var} in config according to the values of the environment variables.") + c.Config.RegisterFlags(f) +} + +// Clone takes advantage of pass-by-value semantics to return a distinct *Config. +// This is primarily used to parse a different flag set without mutating the original *Config. +func (c *ConfigWrapper) Clone() flagext.Registerer { + return func(c ConfigWrapper) *ConfigWrapper { + return &c + }(*c) +} + +// ApplyDynamicConfig satisfies WithCommonCloneable interface, and applies all rules for setting Loki +// config values from the common section of the Loki config file. +// This method's purpose is to simplify Loki's config in an opinionated way so that Loki can be run +// with the minimal amount of config options for most use cases. It also aims to reduce redundancy where +// some values are set multiple times through the Loki config. +func (c *ConfigWrapper) ApplyDynamicConfig() cfg.Source { + defaults := ConfigWrapper{} + flagext.DefaultValues(&defaults) + + return func(dst cfg.Cloneable) error { + r, ok := dst.(*ConfigWrapper) + if !ok { + return errors.New("dst is not a Loki ConfigWrapper") + } + + // Apply all our custom logic here to set values in the Loki config from values in the common config + if r.Common.PathPrefix != "" { + if r.Ruler.RulePath == defaults.Ruler.RulePath { + r.Ruler.RulePath = fmt.Sprintf("%s/rules", r.Common.PathPrefix) + } + + if r.Ingester.WAL.Dir == defaults.Ingester.WAL.Dir { + r.Ingester.WAL.Dir = fmt.Sprintf("%s/wal", r.Common.PathPrefix) + } + } + + return nil + } +} diff --git a/pkg/loki/config_wrapper_test.go b/pkg/loki/config_wrapper_test.go new file mode 100644 index 0000000000000..f9db09f3120dd --- /dev/null +++ b/pkg/loki/config_wrapper_test.go @@ -0,0 +1,87 @@ +package loki + +import ( + "flag" + "io/ioutil" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/loki/pkg/util/cfg" +) + +func Test_CommonConfig(t *testing.T) { + testContext := func(configFileString string, args []string) (ConfigWrapper, ConfigWrapper) { + config := ConfigWrapper{} + fs := flag.NewFlagSet(t.Name(), flag.PanicOnError) + + file, err := ioutil.TempFile("", "config.yaml") + defer func() { + os.Remove(file.Name()) + }() + + require.NoError(t, err) + _, err = file.WriteString(configFileString) + require.NoError(t, err) + + configFileArgs := []string{"-config.file", file.Name()} + if args == nil { + args = configFileArgs + } else { + args = append(args, configFileArgs...) + } + err = cfg.DynamicUnmarshal(&config, args, fs) + require.NoError(t, err) + + defaults := ConfigWrapper{} + freshFlags := flag.NewFlagSet(t.Name(), flag.PanicOnError) + err = cfg.DefaultUnmarshal(&defaults, args, freshFlags) + require.NoError(t, err) + + return config, defaults + } + + t.Run("common path prefix config", func(t *testing.T) { + t.Run("does not override defaults for file paths when not provided", func(t *testing.T) { + configFileString := `---` + config, defaults := testContext(configFileString, nil) + + assert.EqualValues(t, defaults.Ruler.RulePath, config.Ruler.RulePath) + assert.EqualValues(t, defaults.Ingester.WAL.Dir, config.Ingester.WAL.Dir) + }) + + t.Run("when provided, rewrites all default file paths to use common prefix", func(t *testing.T) { + configFileString := `--- +common: + path_prefix: /opt/loki` + config, _ := testContext(configFileString, nil) + + assert.EqualValues(t, "/opt/loki/rules", config.Ruler.RulePath) + assert.EqualValues(t, "/opt/loki/wal", config.Ingester.WAL.Dir) + }) + + t.Run("does not rewrite custom (non-default) paths passed via config file", func(t *testing.T) { + configFileString := `--- +common: + path_prefix: /opt/loki +ruler: + rule_path: /etc/ruler/rules` + config, _ := testContext(configFileString, nil) + + assert.EqualValues(t, "/etc/ruler/rules", config.Ruler.RulePath) + assert.EqualValues(t, "/opt/loki/wal", config.Ingester.WAL.Dir) + }) + + t.Run("does not rewrite custom (non-default) paths passed via the command line", func(t *testing.T) { + configFileString := `--- +common: + path_prefix: /opt/loki` + config, _ := testContext(configFileString, []string{"-ruler.rule-path", "/etc/ruler/rules"}) + + assert.EqualValues(t, "/etc/ruler/rules", config.Ruler.RulePath) + assert.EqualValues(t, "/opt/loki/wal", config.Ingester.WAL.Dir) + }) + }) +} diff --git a/pkg/loki/loki.go b/pkg/loki/loki.go index 1f59e11d2d281..18d52802ede4a 100644 --- a/pkg/loki/loki.go +++ b/pkg/loki/loki.go @@ -34,6 +34,7 @@ import ( "github.com/grafana/loki/pkg/distributor" "github.com/grafana/loki/pkg/ingester" "github.com/grafana/loki/pkg/ingester/client" + "github.com/grafana/loki/pkg/loki/common" "github.com/grafana/loki/pkg/lokifrontend" "github.com/grafana/loki/pkg/querier" "github.com/grafana/loki/pkg/querier/queryrange" @@ -52,6 +53,7 @@ type Config struct { AuthEnabled bool `yaml:"auth_enabled,omitempty"` HTTPPrefix string `yaml:"http_prefix"` + Common common.Config `yaml:"common,omitempty"` Server server.Config `yaml:"server,omitempty"` Distributor distributor.Config `yaml:"distributor,omitempty"` Querier querier.Config `yaml:"querier,omitempty"` diff --git a/pkg/util/cfg/cfg.go b/pkg/util/cfg/cfg.go index 31ac5d9df7230..188021533522f 100644 --- a/pkg/util/cfg/cfg.go +++ b/pkg/util/cfg/cfg.go @@ -2,7 +2,6 @@ package cfg import ( "flag" - "os" "reflect" "github.com/grafana/dskit/flagext" @@ -44,27 +43,11 @@ func Unmarshal(dst Cloneable, sources ...Source) error { return nil } -// Parse is a higher level wrapper for Unmarshal that automatically parses flags and a .yaml file -func Parse(dst Cloneable) error { - return dParse(dst, - dDefaults(flag.CommandLine), - YAMLFlag(os.Args[1:], "config.file"), - Flags(), - ) -} - -// dParse is the same as Parse, but with dependency injection for testing -func dParse(dst Cloneable, defaults, yaml, flags Source) error { - // check dst is a pointer - v := reflect.ValueOf(dst) - if v.Kind() != reflect.Ptr { - return ErrNotPointer - } - - // unmarshal config +// DefaultUnmarshal is a higher level wrapper for Unmarshal that automatically parses flags and a .yaml file +func DefaultUnmarshal(dst Cloneable, args []string, fs *flag.FlagSet) error { return Unmarshal(dst, - defaults, - yaml, - flags, + Defaults(fs), + YAMLFlag(args, "config.file"), + Flags(args, fs), ) } diff --git a/pkg/util/cfg/cfg_test.go b/pkg/util/cfg/cfg_test.go index 38a283bc5fe87..a9a46306af7b1 100644 --- a/pkg/util/cfg/cfg_test.go +++ b/pkg/util/cfg/cfg_test.go @@ -22,8 +22,8 @@ tls: flagSource := dFlags(fs, []string{"-verbose", "-server.port=21"}) data := Data{} - err := dParse(&data, - dDefaults(fs), + err := Unmarshal(&data, + Defaults(fs), yamlSource, flagSource, ) @@ -55,8 +55,8 @@ tls: flagSource := dFlags(fs, []string{"-verbose", "-server.port=21"}) data := Data{} - err := dParse(&data, - dDefaults(fs), + err := Unmarshal(&data, + Defaults(fs), yamlSource, flagSource, ) diff --git a/pkg/util/cfg/dynamic.go b/pkg/util/cfg/dynamic.go new file mode 100644 index 0000000000000..a56e39b6e849c --- /dev/null +++ b/pkg/util/cfg/dynamic.go @@ -0,0 +1,36 @@ +package cfg + +import ( + "flag" +) + +// DynamicCloneable must be implemented by config structs that can be dynamically unmarshalled +type DynamicCloneable interface { + Cloneable + ApplyDynamicConfig() Source +} + +// DynamicUnmarshal handles populating a config based on the following precedence: +// 1. Defaults provided by the `RegisterFlags` interface +// 2. Sections populated by dynamic logic. Configs passed to this function must implement ApplyDynamicConfig() +// 3. Any config options specified directly in the config file +// 4. Any config options specified on the command line. +func DynamicUnmarshal(dst DynamicCloneable, args []string, fs *flag.FlagSet) error { + return Unmarshal(dst, + // First populate the config with defaults including flags from the command line + Defaults(fs), + // Next populate the config from the config file, we do this to populate the `common` + // section of the config file by taking advantage of the code in YAMLFlag which will load + // and process the config file. + YAMLFlag(args, "config.file"), + // Apply any dynamic logic to set other defaults in the config. This function is called after parsing the + // config files so that values from a common, or shared, section can be used in + // the dynamic evaluation + dst.ApplyDynamicConfig(), + // Load configs from the config file a second time, this will supersede anything set by the common + // config with values specified in the config file. + YAMLFlag(args, "config.file"), + // Load the flags again, this will supersede anything set from config file with flags from the command line. + Flags(args, fs), + ) +} diff --git a/pkg/util/cfg/dynamic_test.go b/pkg/util/cfg/dynamic_test.go new file mode 100644 index 0000000000000..d4b67bb46c972 --- /dev/null +++ b/pkg/util/cfg/dynamic_test.go @@ -0,0 +1,141 @@ +package cfg + +import ( + "flag" + "io/ioutil" + "testing" + "time" + + "github.com/grafana/dskit/flagext" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_DynamicUnmarshal(t *testing.T) { + defaultYamlConfig := `--- +server: + port: 8080 +` + + testContext := func(mockApplyDynamicConfig Source, config string, args []string) DynamicConfig { + data := NewDynamicConfig(mockApplyDynamicConfig) + fs := flag.NewFlagSet(t.Name(), flag.PanicOnError) + + file, err := ioutil.TempFile("", "config.yaml") + require.NoError(t, err) + _, err = file.WriteString(config) + require.NoError(t, err) + + configFileArgs := []string{"-config.file", file.Name()} + if args == nil { + args = configFileArgs + } else { + args = append(args, configFileArgs...) + } + + err = DynamicUnmarshal(&data, args, fs) + require.NoError(t, err) + return data + } + + t.Run("parses defaults", func(t *testing.T) { + data := testContext(nil, "", nil) + + assert.Equal(t, 80, data.Server.Port) + assert.Equal(t, 60*time.Second, data.Server.Timeout) + }) + + t.Run("parses config from config.file", func(t *testing.T) { + data := testContext(nil, defaultYamlConfig, nil) + assert.Equal(t, 8080, data.Server.Port) + }) + + t.Run("calls ApplyDynamicConfig on provided DynamicCloneable", func(t *testing.T) { + applyDynamicConfigCalled := false + mockApplyDynamicConfig := func(dst Cloneable) error { + applyDynamicConfigCalled = true + return nil + } + data := testContext(mockApplyDynamicConfig, "", nil) + assert.NotNil(t, data) + assert.True(t, applyDynamicConfigCalled) + }) + + t.Run("makes config from file available to ApplyDynamicConfig", func(t *testing.T) { + var configFromFile *DynamicConfig + mockApplyDynamicConfig := func(dst Cloneable) error { + var ok bool + configFromFile, ok = dst.(*DynamicConfig) + require.True(t, ok) + return nil + } + + data := testContext(mockApplyDynamicConfig, defaultYamlConfig, nil) + assert.NotNil(t, data) + assert.NotNil(t, configFromFile) + assert.Equal(t, 8080, configFromFile.Server.Port) + }) + + t.Run("config from file take precedence over config applied in ApplyDynamicConfig", func(t *testing.T) { + mockApplyDynamicConfig := func(dst Cloneable) error { + config, ok := dst.(*DynamicConfig) + require.True(t, ok) + config.Server.Port = 9090 + return nil + } + + data := testContext(mockApplyDynamicConfig, defaultYamlConfig, nil) + assert.Equal(t, 8080, data.Server.Port) + }) + + t.Run("config from command line takes precedence over config applied in ApplyDynamicConfig and in file", func(t *testing.T) { + mockApplyDynamicConfig := func(dst Cloneable) error { + config, ok := dst.(*DynamicConfig) + require.True(t, ok) + config.Server.Port = 9090 + return nil + } + + args := []string{ + "-server.port", "7070", + } + + data := testContext(mockApplyDynamicConfig, defaultYamlConfig, args) + assert.Equal(t, 7070, data.Server.Port) + }) +} + +type DynamicConfig struct { + Server `yaml:"server"` + ConfigFile string + applyDynamicConfig Source +} + +func NewDynamicConfig(applyDynamicConfig Source) DynamicConfig { + if applyDynamicConfig == nil { + applyDynamicConfig = func(config Cloneable) error { + return nil + } + } + return DynamicConfig{ + Server: Server{}, + ConfigFile: "", + applyDynamicConfig: applyDynamicConfig, + } +} + +func (d *DynamicConfig) Clone() flagext.Registerer { + return func(d DynamicConfig) *DynamicConfig { + return &d + }(*d) +} + +func (d *DynamicConfig) ApplyDynamicConfig() Source { + return d.applyDynamicConfig +} + +func (d *DynamicConfig) RegisterFlags(fs *flag.FlagSet) { + fs.IntVar(&d.Server.Port, "server.port", 80, "") + fs.DurationVar(&d.Server.Timeout, "server.timeout", 60*time.Second, "") + fs.StringVar(&d.ConfigFile, "config.file", "", "yaml file to load") +} diff --git a/pkg/util/cfg/flag.go b/pkg/util/cfg/flag.go index 29520ba635615..efcd9c238c1e5 100644 --- a/pkg/util/cfg/flag.go +++ b/pkg/util/cfg/flag.go @@ -3,7 +3,6 @@ package cfg import ( "flag" "fmt" - "os" "sort" "strings" @@ -11,14 +10,8 @@ import ( "github.com/pkg/errors" ) -// Defaults registers flags to the command line using dst as the -// flagext.Registerer -func Defaults() Source { - return dDefaults(flag.CommandLine) -} - -// dDefaults registers flags to the flagSet using dst as the flagext.Registerer -func dDefaults(fs *flag.FlagSet) Source { +// Defaults registers flags to the flagSet using dst as the flagext.Registerer +func Defaults(fs *flag.FlagSet) Source { return func(dst Cloneable) error { r, ok := dst.(flagext.Registerer) if !ok { @@ -33,9 +26,9 @@ func dDefaults(fs *flag.FlagSet) Source { // Flags parses the flag from the command line, setting only user-supplied // values on the flagext.Registerer passed to Defaults() -func Flags() Source { - flag.Usage = categorizedUsage(flag.CommandLine) - return dFlags(flag.CommandLine, os.Args[1:]) +func Flags(args []string, fs *flag.FlagSet) Source { + flag.Usage = categorizedUsage(fs) + return dFlags(fs, args) } // dFlags parses the flagset, applying all values set on the slice diff --git a/pkg/util/cfg/flag_test.go b/pkg/util/cfg/flag_test.go index 8c45a7846eff2..dbce7eb4e63ad 100644 --- a/pkg/util/cfg/flag_test.go +++ b/pkg/util/cfg/flag_test.go @@ -16,7 +16,7 @@ func TestDefaults(t *testing.T) { fs := flag.NewFlagSet(t.Name(), flag.PanicOnError) err := Unmarshal(&data, - dDefaults(fs), + Defaults(fs), ) require.NoError(t, err) @@ -39,7 +39,7 @@ func TestFlags(t *testing.T) { data := Data{} fs := flag.NewFlagSet(t.Name(), flag.PanicOnError) err := Unmarshal(&data, - dDefaults(fs), + Defaults(fs), dFlags(fs, []string{"-server.timeout=10h", "-verbose"}), ) require.NoError(t, err) diff --git a/pkg/util/cfg/precedence_test.go b/pkg/util/cfg/precedence_test.go index 7232b6ff7b3a5..af96c94573020 100644 --- a/pkg/util/cfg/precedence_test.go +++ b/pkg/util/cfg/precedence_test.go @@ -27,7 +27,7 @@ func TestYAMLOverDefaults(t *testing.T) { data := Data{} fs := flag.NewFlagSet(t.Name(), flag.PanicOnError) err := Unmarshal(&data, - dDefaults(fs), + Defaults(fs), dYAML([]byte(y)), ) @@ -50,7 +50,7 @@ func TestFlagOverYAML(t *testing.T) { fs := flag.NewFlagSet(t.Name(), flag.PanicOnError) err := Unmarshal(&data, - dDefaults(fs), + Defaults(fs), dYAML([]byte(y)), dFlags(fs, []string{"-verbose=false", "-tls.cert=CLI"}), )