From cb9c6709008251174dbf25f6329438ea653a8b0e Mon Sep 17 00:00:00 2001 From: Edward Welch Date: Fri, 17 Sep 2021 08:26:14 -0400 Subject: [PATCH 01/11] refactor the pkg/util/cfg package to simplify and make it accessable outside of this package. create a pkg/cfg which handles building the Loki config, this is created separately from pkg/util (which is Apache 2 licensed) to maintain proper separations of AGPL and Apache 2 code Update the main.go for several of the apps with the refactoring --- clients/cmd/promtail/main.go | 2 +- cmd/loki/main.go | 45 +++-------------- cmd/migrate/main.go | 2 +- pkg/cfg/cfg.go | 85 +++++++++++++++++++++++++++++++++ pkg/cfg/common/common.go | 5 ++ pkg/loki/loki.go | 2 + pkg/util/cfg/cfg.go | 24 ++-------- pkg/util/cfg/cfg_test.go | 8 ++-- pkg/util/cfg/flag.go | 10 +--- pkg/util/cfg/flag_test.go | 4 +- pkg/util/cfg/precedence_test.go | 4 +- 11 files changed, 115 insertions(+), 76 deletions(-) create mode 100644 pkg/cfg/cfg.go create mode 100644 pkg/cfg/common/common.go diff --git a/clients/cmd/promtail/main.go b/clients/cmd/promtail/main.go index e4e1ac298bc1..cb8927ee4878 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); 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 10f480604607..fe96fbbc35ca 100644 --- a/cmd/loki/main.go +++ b/cmd/loki/main.go @@ -1,22 +1,20 @@ package main import ( - "flag" "fmt" "os" "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" "github.com/weaveworks/common/tracing" + "github.com/grafana/loki/pkg/cfg" "github.com/grafana/loki/pkg/loki" logutil "github.com/grafana/loki/pkg/util" _ "github.com/grafana/loki/pkg/util/build" - "github.com/grafana/loki/pkg/util/cfg" "github.com/grafana/loki/pkg/validation" util_log "github.com/cortexproject/cortex/pkg/util/log" @@ -26,43 +24,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 cfg.ConfigWrapper - if err := cfg.Parse(&config); err != nil { + if err := cfg.Unmarshal(&config); 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 +56,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 694e0fd96134..038849683a8c 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/cfg/cfg.go b/pkg/cfg/cfg.go new file mode 100644 index 000000000000..431a8fa9c77a --- /dev/null +++ b/pkg/cfg/cfg.go @@ -0,0 +1,85 @@ +package cfg + +import ( + "flag" + "os" + + "github.com/grafana/dskit/flagext" + "github.com/pkg/errors" + + "github.com/grafana/loki/pkg/loki" + "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. +type ConfigWrapper struct { + loki.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) +} + +// Unmarshal handles populating Loki's config based on the following precedence: +// 1. Defaults provided by the `RegisterFlags` interface +// 2. Sections populated by the `common` config section of the Loki config +// 3. Any config options specified directly in the loki config file +// 4. Any config options specified on the command line. +func Unmarshal(dst cfg.Cloneable) error { + return cfg.Unmarshal(dst, + // First populate the config with defaults including flags from the command line + cfg.Defaults(flag.CommandLine), + // 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. + cfg.YAMLFlag(os.Args[1:], "config.file"), + // Apply our logic to use values from the common section to set values throughout the Loki config. + CommonConfig(), + // 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. + cfg.YAMLFlag(os.Args[1:], "config.file"), + // Load the flags again, this will supersede anything set from config file with flags from the command line. + cfg.Flags(), + ) +} + +// CommonConfig applies all rules for setting Loki config values from the common section of the Loki config file. +// This entire 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 CommonConfig() cfg.Source { + 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 + // FIXME this is just an example showing how we can use values from the common section to set values on the Loki config object + r.StorageConfig.BoltDBShipperConfig.SharedStoreType = r.Common.Store + + return nil + } +} diff --git a/pkg/cfg/common/common.go b/pkg/cfg/common/common.go new file mode 100644 index 000000000000..55d39df3d81c --- /dev/null +++ b/pkg/cfg/common/common.go @@ -0,0 +1,5 @@ +package common + +type Config struct { + Store string // This is just an example, but here we should define all the 'common' config values used to set other Loki values. +} diff --git a/pkg/loki/loki.go b/pkg/loki/loki.go index 1f59e11d2d28..4c90034c6f16 100644 --- a/pkg/loki/loki.go +++ b/pkg/loki/loki.go @@ -31,6 +31,7 @@ import ( "github.com/weaveworks/common/signals" "google.golang.org/grpc/health/grpc_health_v1" + "github.com/grafana/loki/pkg/cfg/common" "github.com/grafana/loki/pkg/distributor" "github.com/grafana/loki/pkg/ingester" "github.com/grafana/loki/pkg/ingester/client" @@ -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 31ac5d9df723..2fc70c9d40e3 100644 --- a/pkg/util/cfg/cfg.go +++ b/pkg/util/cfg/cfg.go @@ -44,27 +44,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), +// DefaultUnmarshal is a higher level wrapper for Unmarshal that automatically parses flags and a .yaml file +func DefaultUnmarshal(dst Cloneable) error { + return Unmarshal(dst, + Defaults(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 - return Unmarshal(dst, - defaults, - yaml, - flags, - ) -} diff --git a/pkg/util/cfg/cfg_test.go b/pkg/util/cfg/cfg_test.go index 38a283bc5fe8..a9a46306af7b 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/flag.go b/pkg/util/cfg/flag.go index 29520ba63561..7af2d488cb72 100644 --- a/pkg/util/cfg/flag.go +++ b/pkg/util/cfg/flag.go @@ -11,14 +11,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 { diff --git a/pkg/util/cfg/flag_test.go b/pkg/util/cfg/flag_test.go index 8c45a7846eff..dbce7eb4e63a 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 7232b6ff7b3a..af96c9457302 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"}), ) From b16004bd2b90fda774b6d784c62841f5a4ac0f40 Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Tue, 28 Sep 2021 16:45:08 -0600 Subject: [PATCH 02/11] refactor how dynamic/common config is loaded Signed-off-by: Trevor Whitney --- cmd/loki/main.go | 6 +- pkg/{cfg => loki}/common/common.go | 1 + pkg/{cfg/cfg.go => loki/conifg_wrapper.go} | 38 ++--- pkg/loki/loki.go | 2 +- pkg/util/cfg/dynamic.go | 37 +++++ pkg/util/cfg/dynamic_test.go | 158 +++++++++++++++++++++ 6 files changed, 208 insertions(+), 34 deletions(-) rename pkg/{cfg => loki}/common/common.go (65%) rename pkg/{cfg/cfg.go => loki/conifg_wrapper.go} (58%) create mode 100644 pkg/util/cfg/dynamic.go create mode 100644 pkg/util/cfg/dynamic_test.go diff --git a/cmd/loki/main.go b/cmd/loki/main.go index fe96fbbc35ca..8c915308f12e 100644 --- a/cmd/loki/main.go +++ b/cmd/loki/main.go @@ -11,10 +11,10 @@ import ( "github.com/weaveworks/common/logging" "github.com/weaveworks/common/tracing" - "github.com/grafana/loki/pkg/cfg" "github.com/grafana/loki/pkg/loki" logutil "github.com/grafana/loki/pkg/util" _ "github.com/grafana/loki/pkg/util/build" + "github.com/grafana/loki/pkg/util/cfg" "github.com/grafana/loki/pkg/validation" util_log "github.com/cortexproject/cortex/pkg/util/log" @@ -25,9 +25,9 @@ func init() { } func main() { - var config cfg.ConfigWrapper + var config loki.ConfigWrapper - if err := cfg.Unmarshal(&config); err != nil { + if err := cfg.DynamicUnmarshal(&config); err != nil { fmt.Fprintf(os.Stderr, "failed parsing config: %v\n", err) os.Exit(1) } diff --git a/pkg/cfg/common/common.go b/pkg/loki/common/common.go similarity index 65% rename from pkg/cfg/common/common.go rename to pkg/loki/common/common.go index 55d39df3d81c..a3c0973a2e45 100644 --- a/pkg/cfg/common/common.go +++ b/pkg/loki/common/common.go @@ -1,5 +1,6 @@ package common +// Config holds common config that can be shared between multiple other config sections type Config struct { Store string // This is just an example, but here we should define all the 'common' config values used to set other Loki values. } diff --git a/pkg/cfg/cfg.go b/pkg/loki/conifg_wrapper.go similarity index 58% rename from pkg/cfg/cfg.go rename to pkg/loki/conifg_wrapper.go index 431a8fa9c77a..63c4805909f4 100644 --- a/pkg/cfg/cfg.go +++ b/pkg/loki/conifg_wrapper.go @@ -1,20 +1,20 @@ -package cfg +package loki import ( "flag" - "os" "github.com/grafana/dskit/flagext" "github.com/pkg/errors" - "github.com/grafana/loki/pkg/loki" "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 { - loki.Config `yaml:",inline"` + Config `yaml:",inline"` PrintVersion bool VerifyConfig bool PrintConfig bool @@ -42,34 +42,12 @@ func (c *ConfigWrapper) Clone() flagext.Registerer { }(*c) } -// Unmarshal handles populating Loki's config based on the following precedence: -// 1. Defaults provided by the `RegisterFlags` interface -// 2. Sections populated by the `common` config section of the Loki config -// 3. Any config options specified directly in the loki config file -// 4. Any config options specified on the command line. -func Unmarshal(dst cfg.Cloneable) error { - return cfg.Unmarshal(dst, - // First populate the config with defaults including flags from the command line - cfg.Defaults(flag.CommandLine), - // 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. - cfg.YAMLFlag(os.Args[1:], "config.file"), - // Apply our logic to use values from the common section to set values throughout the Loki config. - CommonConfig(), - // 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. - cfg.YAMLFlag(os.Args[1:], "config.file"), - // Load the flags again, this will supersede anything set from config file with flags from the command line. - cfg.Flags(), - ) -} - -// CommonConfig applies all rules for setting Loki config values from the common section of the Loki config file. -// This entire method's purpose is to simplify Loki's config in an opinionated way so that Loki can be run +// 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 CommonConfig() cfg.Source { +func (c *ConfigWrapper) ApplyDynamicConfig() cfg.Source { return func(dst cfg.Cloneable) error { r, ok := dst.(*ConfigWrapper) if !ok { diff --git a/pkg/loki/loki.go b/pkg/loki/loki.go index 4c90034c6f16..18d52802ede4 100644 --- a/pkg/loki/loki.go +++ b/pkg/loki/loki.go @@ -31,10 +31,10 @@ import ( "github.com/weaveworks/common/signals" "google.golang.org/grpc/health/grpc_health_v1" - "github.com/grafana/loki/pkg/cfg/common" "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" diff --git a/pkg/util/cfg/dynamic.go b/pkg/util/cfg/dynamic.go new file mode 100644 index 000000000000..127086e228ad --- /dev/null +++ b/pkg/util/cfg/dynamic.go @@ -0,0 +1,37 @@ +package cfg + +import ( + "flag" + "os" +) + +// 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) error { + return Unmarshal(dst, + // First populate the config with defaults including flags from the command line + Defaults(flag.CommandLine), + // 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(os.Args[1:], "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(os.Args[1:], "config.file"), + // Load the flags again, this will supersede anything set from config file with flags from the command line. + Flags(), + ) +} diff --git a/pkg/util/cfg/dynamic_test.go b/pkg/util/cfg/dynamic_test.go new file mode 100644 index 000000000000..18fc7cc80ea8 --- /dev/null +++ b/pkg/util/cfg/dynamic_test.go @@ -0,0 +1,158 @@ +package cfg + +import ( + "flag" + "io/ioutil" + "os" + "testing" + "time" + + "github.com/grafana/dskit/flagext" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var defaultArgs = []string{"config.file", "foo.yaml"} + +func Test_DynamicUnmarshal(t *testing.T) { + defaultYamlConfig := `--- +server: + port: 8080 +` + + testContext := func(mockApplyDynamicConfig Source, config string, args []string) DynamicConfig { + data := NewDynamicConfig(mockApplyDynamicConfig) + flag.CommandLine = flag.NewFlagSet(t.Name(), flag.PanicOnError) + + file, err := ioutil.TempFile("", "foo.yaml") + require.NoError(t, err) + _, err = file.WriteString(config) + require.NoError(t, err) + + configFileArgs := []string{"loki", "-config.file", file.Name()} + if args == nil { + args = configFileArgs + } else { + args = append(args, configFileArgs...) + } + os.Args = args + + return data + } + + t.Run("parses defaults", func(t *testing.T) { + data := testContext(nil, "", nil) + err := DynamicUnmarshal(&data) + require.NoError(t, err) + + 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) + err := DynamicUnmarshal(&data) + require.NoError(t, err) + 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) + err := DynamicUnmarshal(&data) + + require.NoError(t, err) + 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) + err := DynamicUnmarshal(&data) + + require.NoError(t, err) + 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) + err := DynamicUnmarshal(&data) + + require.NoError(t, err) + 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{ + "loki", + "-server.port", "7070", + } + + data := testContext(mockApplyDynamicConfig, defaultYamlConfig, args) + err := DynamicUnmarshal(&data) + + require.NoError(t, err) + 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") +} From 4aaf99c7a7460a52ead355b2dccaef804d69c153 Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Wed, 29 Sep 2021 09:59:51 -0600 Subject: [PATCH 03/11] Add common config logic for path prefix Signed-off-by: Trevor Whitney --- clients/cmd/promtail/main.go | 2 +- cmd/loki/main.go | 3 +- pkg/loki/conifg_wrapper.go | 22 ++++++++- pkg/loki/conifg_wrapper_test.go | 86 +++++++++++++++++++++++++++++++++ pkg/util/cfg/cfg.go | 9 ++-- pkg/util/cfg/dynamic.go | 11 ++--- pkg/util/cfg/dynamic_test.go | 26 +++------- pkg/util/cfg/flag.go | 7 ++- 8 files changed, 127 insertions(+), 39 deletions(-) create mode 100644 pkg/loki/conifg_wrapper_test.go diff --git a/clients/cmd/promtail/main.go b/clients/cmd/promtail/main.go index cb8927ee4878..665a240879f3 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.DefaultUnmarshal(&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 8c915308f12e..53e17d56f02d 100644 --- a/cmd/loki/main.go +++ b/cmd/loki/main.go @@ -1,6 +1,7 @@ package main import ( + "flag" "fmt" "os" "reflect" @@ -27,7 +28,7 @@ func init() { func main() { var config loki.ConfigWrapper - if err := cfg.DynamicUnmarshal(&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) } diff --git a/pkg/loki/conifg_wrapper.go b/pkg/loki/conifg_wrapper.go index 63c4805909f4..79b34fa671ae 100644 --- a/pkg/loki/conifg_wrapper.go +++ b/pkg/loki/conifg_wrapper.go @@ -2,6 +2,7 @@ package loki import ( "flag" + "fmt" "github.com/grafana/dskit/flagext" "github.com/pkg/errors" @@ -48,15 +49,32 @@ func (c *ConfigWrapper) Clone() flagext.Registerer { // 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{} + freshFlags := flag.NewFlagSet("config-file-defaults-loader", flag.PanicOnError) + + //Do not need command line args to figure out defaults, so pass an empty slice here + defaultsUnmarshalError := cfg.DefaultUnmarshal(&defaults, []string{}, freshFlags) + return func(dst cfg.Cloneable) error { r, ok := dst.(*ConfigWrapper) if !ok { return errors.New("dst is not a Loki ConfigWrapper") } + if defaultsUnmarshalError != nil { + return defaultsUnmarshalError + } + // Apply all our custom logic here to set values in the Loki config from values in the common config - // FIXME this is just an example showing how we can use values from the common section to set values on the Loki config object - r.StorageConfig.BoltDBShipperConfig.SharedStoreType = r.Common.Store + 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/conifg_wrapper_test.go b/pkg/loki/conifg_wrapper_test.go new file mode 100644 index 000000000000..715b316d198e --- /dev/null +++ b/pkg/loki/conifg_wrapper_test.go @@ -0,0 +1,86 @@ +package loki + +import ( + "flag" + "io/ioutil" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/loki/pkg/util/cfg" +) + +func Test_CommonConfig(t *testing.T) { + // defaultYamlConfig := `--- + // server: + // port: 8080 + // ` + testContext := func(configFileString string, args []string) (ConfigWrapper, ConfigWrapper) { + config := ConfigWrapper{} + fs := flag.NewFlagSet(t.Name(), flag.PanicOnError) + + file, err := ioutil.TempFile("", "config.yaml") + 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...) + } + cfg.DynamicUnmarshal(&config, args, fs) + + 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 base directory 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/util/cfg/cfg.go b/pkg/util/cfg/cfg.go index 2fc70c9d40e3..188021533522 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" @@ -45,10 +44,10 @@ func Unmarshal(dst Cloneable, sources ...Source) error { } // DefaultUnmarshal is a higher level wrapper for Unmarshal that automatically parses flags and a .yaml file -func DefaultUnmarshal(dst Cloneable) error { +func DefaultUnmarshal(dst Cloneable, args []string, fs *flag.FlagSet) error { return Unmarshal(dst, - Defaults(flag.CommandLine), - YAMLFlag(os.Args[1:], "config.file"), - Flags(), + Defaults(fs), + YAMLFlag(args, "config.file"), + Flags(args, fs), ) } diff --git a/pkg/util/cfg/dynamic.go b/pkg/util/cfg/dynamic.go index 127086e228ad..a56e39b6e849 100644 --- a/pkg/util/cfg/dynamic.go +++ b/pkg/util/cfg/dynamic.go @@ -2,7 +2,6 @@ package cfg import ( "flag" - "os" ) // DynamicCloneable must be implemented by config structs that can be dynamically unmarshalled @@ -16,22 +15,22 @@ type DynamicCloneable 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) error { +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(flag.CommandLine), + 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(os.Args[1:], "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(os.Args[1:], "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(), + Flags(args, fs), ) } diff --git a/pkg/util/cfg/dynamic_test.go b/pkg/util/cfg/dynamic_test.go index 18fc7cc80ea8..62f928f76e48 100644 --- a/pkg/util/cfg/dynamic_test.go +++ b/pkg/util/cfg/dynamic_test.go @@ -3,7 +3,6 @@ package cfg import ( "flag" "io/ioutil" - "os" "testing" "time" @@ -22,9 +21,9 @@ server: testContext := func(mockApplyDynamicConfig Source, config string, args []string) DynamicConfig { data := NewDynamicConfig(mockApplyDynamicConfig) - flag.CommandLine = flag.NewFlagSet(t.Name(), flag.PanicOnError) + fs := flag.NewFlagSet(t.Name(), flag.PanicOnError) - file, err := ioutil.TempFile("", "foo.yaml") + file, err := ioutil.TempFile("", "config.yaml") require.NoError(t, err) _, err = file.WriteString(config) require.NoError(t, err) @@ -35,15 +34,14 @@ server: } else { args = append(args, configFileArgs...) } - os.Args = args + err = DynamicUnmarshal(&data, args, fs) + require.NoError(t, err) return data } t.Run("parses defaults", func(t *testing.T) { data := testContext(nil, "", nil) - err := DynamicUnmarshal(&data) - require.NoError(t, err) assert.Equal(t, 80, data.Server.Port) assert.Equal(t, 60*time.Second, data.Server.Timeout) @@ -51,8 +49,6 @@ server: t.Run("parses config from config.file", func(t *testing.T) { data := testContext(nil, defaultYamlConfig, nil) - err := DynamicUnmarshal(&data) - require.NoError(t, err) assert.Equal(t, 8080, data.Server.Port) }) @@ -63,9 +59,7 @@ server: return nil } data := testContext(mockApplyDynamicConfig, "", nil) - err := DynamicUnmarshal(&data) - - require.NoError(t, err) + assert.NotNil(t, data) assert.True(t, applyDynamicConfigCalled) }) @@ -79,9 +73,7 @@ server: } data := testContext(mockApplyDynamicConfig, defaultYamlConfig, nil) - err := DynamicUnmarshal(&data) - - require.NoError(t, err) + assert.NotNil(t, data) assert.NotNil(t, configFromFile) assert.Equal(t, 8080, configFromFile.Server.Port) }) @@ -95,9 +87,6 @@ server: } data := testContext(mockApplyDynamicConfig, defaultYamlConfig, nil) - err := DynamicUnmarshal(&data) - - require.NoError(t, err) assert.Equal(t, 8080, data.Server.Port) }) @@ -115,9 +104,6 @@ server: } data := testContext(mockApplyDynamicConfig, defaultYamlConfig, args) - err := DynamicUnmarshal(&data) - - require.NoError(t, err) assert.Equal(t, 7070, data.Server.Port) }) } diff --git a/pkg/util/cfg/flag.go b/pkg/util/cfg/flag.go index 7af2d488cb72..efcd9c238c1e 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" @@ -27,9 +26,9 @@ func Defaults(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 From 81a8b1b3240e007e10f67e0fcb86429ee4ee82cc Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Wed, 29 Sep 2021 10:01:57 -0600 Subject: [PATCH 04/11] Missed a file, update common config with prefix property Signed-off-by: Trevor Whitney --- pkg/loki/common/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/loki/common/common.go b/pkg/loki/common/common.go index a3c0973a2e45..cce66321362c 100644 --- a/pkg/loki/common/common.go +++ b/pkg/loki/common/common.go @@ -2,5 +2,5 @@ package common // Config holds common config that can be shared between multiple other config sections type Config struct { - Store string // This is just an example, but here we should define all the 'common' config values used to set other Loki values. + PathPrefix string `yaml:"path_prefix"` } From c0afa1530a951c8523e65bd676779800333f6ba0 Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Wed, 29 Sep 2021 10:05:44 -0600 Subject: [PATCH 05/11] fix test, remove loki from args Signed-off-by: Trevor Whitney --- pkg/util/cfg/dynamic_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/util/cfg/dynamic_test.go b/pkg/util/cfg/dynamic_test.go index 62f928f76e48..1292b9f0310d 100644 --- a/pkg/util/cfg/dynamic_test.go +++ b/pkg/util/cfg/dynamic_test.go @@ -28,7 +28,7 @@ server: _, err = file.WriteString(config) require.NoError(t, err) - configFileArgs := []string{"loki", "-config.file", file.Name()} + configFileArgs := []string{"-config.file", file.Name()} if args == nil { args = configFileArgs } else { @@ -99,7 +99,6 @@ server: } args := []string{ - "loki", "-server.port", "7070", } From c4c9b2c2502bbcac9c8e53f69a1fa8815c5df2e4 Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Wed, 29 Sep 2021 10:08:00 -0600 Subject: [PATCH 06/11] Clean up test names Signed-off-by: Trevor Whitney --- pkg/loki/conifg_wrapper_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/loki/conifg_wrapper_test.go b/pkg/loki/conifg_wrapper_test.go index 715b316d198e..5562d08b6f7e 100644 --- a/pkg/loki/conifg_wrapper_test.go +++ b/pkg/loki/conifg_wrapper_test.go @@ -41,7 +41,7 @@ func Test_CommonConfig(t *testing.T) { return config, defaults } - t.Run("common base directory config", func(t *testing.T) { + 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) @@ -82,5 +82,4 @@ common: assert.EqualValues(t, "/opt/loki/wal", config.Ingester.WAL.Dir) }) }) - } From 780efaf35e5130a5be86cb5f80a9a0796cc7debd Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Wed, 29 Sep 2021 10:24:19 -0600 Subject: [PATCH 07/11] fix typo in filename, make linter happy Signed-off-by: Trevor Whitney --- pkg/loki/{conifg_wrapper.go => config_wrapper.go} | 0 pkg/loki/{conifg_wrapper_test.go => config_wrapper_test.go} | 3 ++- pkg/util/cfg/dynamic_test.go | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) rename pkg/loki/{conifg_wrapper.go => config_wrapper.go} (100%) rename pkg/loki/{conifg_wrapper_test.go => config_wrapper_test.go} (97%) diff --git a/pkg/loki/conifg_wrapper.go b/pkg/loki/config_wrapper.go similarity index 100% rename from pkg/loki/conifg_wrapper.go rename to pkg/loki/config_wrapper.go diff --git a/pkg/loki/conifg_wrapper_test.go b/pkg/loki/config_wrapper_test.go similarity index 97% rename from pkg/loki/conifg_wrapper_test.go rename to pkg/loki/config_wrapper_test.go index 5562d08b6f7e..50c9cf870b1f 100644 --- a/pkg/loki/conifg_wrapper_test.go +++ b/pkg/loki/config_wrapper_test.go @@ -31,7 +31,8 @@ func Test_CommonConfig(t *testing.T) { } else { args = append(args, configFileArgs...) } - cfg.DynamicUnmarshal(&config, args, fs) + err = cfg.DynamicUnmarshal(&config, args, fs) + require.NoError(t, err) defaults := ConfigWrapper{} freshFlags := flag.NewFlagSet(t.Name(), flag.PanicOnError) diff --git a/pkg/util/cfg/dynamic_test.go b/pkg/util/cfg/dynamic_test.go index 1292b9f0310d..d4b67bb46c97 100644 --- a/pkg/util/cfg/dynamic_test.go +++ b/pkg/util/cfg/dynamic_test.go @@ -11,8 +11,6 @@ import ( "github.com/stretchr/testify/require" ) -var defaultArgs = []string{"config.file", "foo.yaml"} - func Test_DynamicUnmarshal(t *testing.T) { defaultYamlConfig := `--- server: From 317efba022772a7fe42516ad4937ab6b4f039c60 Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Thu, 30 Sep 2021 15:24:19 -0600 Subject: [PATCH 08/11] Update pkg/loki/config_wrapper.go Co-authored-by: Owen Diehl --- pkg/loki/config_wrapper.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/loki/config_wrapper.go b/pkg/loki/config_wrapper.go index 79b34fa671ae..9f58cca4e769 100644 --- a/pkg/loki/config_wrapper.go +++ b/pkg/loki/config_wrapper.go @@ -50,10 +50,7 @@ func (c *ConfigWrapper) Clone() flagext.Registerer { // some values are set multiple times through the Loki config. func (c *ConfigWrapper) ApplyDynamicConfig() cfg.Source { defaults := ConfigWrapper{} - freshFlags := flag.NewFlagSet("config-file-defaults-loader", flag.PanicOnError) - - //Do not need command line args to figure out defaults, so pass an empty slice here - defaultsUnmarshalError := cfg.DefaultUnmarshal(&defaults, []string{}, freshFlags) + flagext.DefaultValues(&defaults) return func(dst cfg.Cloneable) error { r, ok := dst.(*ConfigWrapper) From 3404c95c05a70fe285d423ad0a101f9503d368af Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Thu, 30 Sep 2021 15:24:38 -0600 Subject: [PATCH 09/11] Update pkg/loki/config_wrapper.go Co-authored-by: Owen Diehl --- pkg/loki/config_wrapper.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/pkg/loki/config_wrapper.go b/pkg/loki/config_wrapper.go index 9f58cca4e769..64bbcc3671f8 100644 --- a/pkg/loki/config_wrapper.go +++ b/pkg/loki/config_wrapper.go @@ -58,9 +58,6 @@ func (c *ConfigWrapper) ApplyDynamicConfig() cfg.Source { return errors.New("dst is not a Loki ConfigWrapper") } - if defaultsUnmarshalError != nil { - return defaultsUnmarshalError - } // Apply all our custom logic here to set values in the Loki config from values in the common config if r.Common.PathPrefix != "" { From d993c185235cc22303f6ce65149e26bb7e0f5946 Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Thu, 30 Sep 2021 15:24:44 -0600 Subject: [PATCH 10/11] Update pkg/loki/config_wrapper_test.go Co-authored-by: Owen Diehl --- pkg/loki/config_wrapper_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/loki/config_wrapper_test.go b/pkg/loki/config_wrapper_test.go index 50c9cf870b1f..43d33d36bd8c 100644 --- a/pkg/loki/config_wrapper_test.go +++ b/pkg/loki/config_wrapper_test.go @@ -12,10 +12,6 @@ import ( ) func Test_CommonConfig(t *testing.T) { - // defaultYamlConfig := `--- - // server: - // port: 8080 - // ` testContext := func(configFileString string, args []string) (ConfigWrapper, ConfigWrapper) { config := ConfigWrapper{} fs := flag.NewFlagSet(t.Name(), flag.PanicOnError) From 1140046248f2fc573a0edf7b4bcdd60d0914f052 Mon Sep 17 00:00:00 2001 From: Trevor Whitney Date: Thu, 30 Sep 2021 15:48:31 -0600 Subject: [PATCH 11/11] remove temp files in test Signed-off-by: Trevor Whitney --- pkg/loki/config_wrapper.go | 1 - pkg/loki/config_wrapper_test.go | 5 +++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/loki/config_wrapper.go b/pkg/loki/config_wrapper.go index 64bbcc3671f8..732ad5c7916f 100644 --- a/pkg/loki/config_wrapper.go +++ b/pkg/loki/config_wrapper.go @@ -58,7 +58,6 @@ func (c *ConfigWrapper) ApplyDynamicConfig() cfg.Source { 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 { diff --git a/pkg/loki/config_wrapper_test.go b/pkg/loki/config_wrapper_test.go index 43d33d36bd8c..f9db09f3120d 100644 --- a/pkg/loki/config_wrapper_test.go +++ b/pkg/loki/config_wrapper_test.go @@ -3,6 +3,7 @@ package loki import ( "flag" "io/ioutil" + "os" "testing" "github.com/stretchr/testify/assert" @@ -17,6 +18,10 @@ func Test_CommonConfig(t *testing.T) { 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)