From 05e0e91d6e04d7a06dd6831750d2692e09e4ec98 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Thu, 6 Aug 2020 11:00:44 -0400 Subject: [PATCH] Fix/promtail yaml config (#2471) * promtail config cloneable * config parsing compile time checks against cloneable --- cmd/loki/main.go | 4 ++-- cmd/promtail/main.go | 43 ++++++++++++++++++++++++++++++--------- pkg/cfg/cfg.go | 18 +++++++++++----- pkg/cfg/data_test.go | 10 +++++++++ pkg/cfg/files.go | 21 ++++++------------- pkg/cfg/flag.go | 10 ++------- pkg/logcli/query/query.go | 5 ++--- pkg/loki/loki.go | 9 ++++++++ 8 files changed, 77 insertions(+), 43 deletions(-) diff --git a/cmd/loki/main.go b/cmd/loki/main.go index f7866c713a080..1151d52c1c898 100644 --- a/cmd/loki/main.go +++ b/cmd/loki/main.go @@ -50,9 +50,9 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { // 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 flagext.Registerer(func(c Config) *Config { + return func(c Config) *Config { return &c - }(*c)) + }(*c) } func main() { diff --git a/cmd/promtail/main.go b/cmd/promtail/main.go index 13a5aed9c0cf0..891f201f54998 100644 --- a/cmd/promtail/main.go +++ b/cmd/promtail/main.go @@ -9,6 +9,7 @@ import ( "k8s.io/klog" "github.com/cortexproject/cortex/pkg/util" + "github.com/cortexproject/cortex/pkg/util/flagext" "github.com/go-kit/kit/log/level" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/common/version" @@ -26,22 +27,44 @@ func init() { prometheus.MustRegister(version.NewCollector("promtail")) } -func main() { - printVersion := flag.Bool("version", false, "Print this builds version information") - dryRun := flag.Bool("dry-run", false, "Start Promtail but print entries instead of sending them to Loki.") - printConfig := flag.Bool("print-config-stderr", false, "Dump the entire Loki config object to stderr") - logConfig := flag.Bool("log-config-reverse-order", false, "Dump the entire Loki config object at Info log "+ +type Config struct { + config.Config `yaml:",inline"` + printVersion bool + printConfig bool + logConfig bool + dryRun bool + configFile string +} + +func (c *Config) RegisterFlags(f *flag.FlagSet) { + f.BoolVar(&c.printVersion, "version", false, "Print this builds version information") + 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.BoolVar(&c.dryRun, "dry-run", false, "Start Promtail but print entries instead of sending them to Loki.") + f.StringVar(&c.configFile, "config.file", "", "yaml file to load") + 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() { // Load config, merging config file and CLI flags - var config config.Config + var config Config if err := cfg.Parse(&config); err != nil { fmt.Println("Unable to parse config:", err) os.Exit(1) } // Handle -version CLI flag - if *printVersion { + if config.printVersion { fmt.Println(version.Print("promtail")) os.Exit(0) } @@ -62,21 +85,21 @@ func main() { stages.Debug = true } - if *printConfig { + if config.printConfig { err := logutil.PrintConfig(os.Stderr, &config) if err != nil { level.Error(util.Logger).Log("msg", "failed to print config to stderr", "err", err.Error()) } } - if *logConfig { + if config.logConfig { err := logutil.LogConfig(&config) if err != nil { level.Error(util.Logger).Log("msg", "failed to log config object", "err", err.Error()) } } - p, err := promtail.New(config, *dryRun) + p, err := promtail.New(config.Config, config.dryRun) if err != nil { level.Error(util.Logger).Log("msg", "error creating promtail", "error", err) os.Exit(1) diff --git a/pkg/cfg/cfg.go b/pkg/cfg/cfg.go index 1e89fee2f7afb..0cdc5ac7e35af 100644 --- a/pkg/cfg/cfg.go +++ b/pkg/cfg/cfg.go @@ -1,9 +1,11 @@ package cfg import ( + "flag" "os" "reflect" + "github.com/cortexproject/cortex/pkg/util/flagext" "github.com/pkg/errors" ) @@ -12,7 +14,13 @@ import ( // destination, which will be something compatible to `json.Unmarshal`. The // obtained configuration may be written to this object, it may also contain // data from previous sources. -type Source func(interface{}) error +type Source func(Cloneable) error + +// Cloneable is a config which can be cloned into a flagext.Registerer +// Contract: the cloned value must not mutate the original. +type Cloneable interface { + Clone() flagext.Registerer +} var ( ErrNotPointer = errors.New("dst is not a pointer") @@ -20,7 +28,7 @@ var ( // Unmarshal merges the values of the various configuration sources and sets them on // `dst`. The object must be compatible with `json.Unmarshal`. -func Unmarshal(dst interface{}, sources ...Source) error { +func Unmarshal(dst Cloneable, sources ...Source) error { if len(sources) == 0 { panic("No sources supplied to cfg.Unmarshal(). This is most likely a programming issue and should never happen. Check the code!") } @@ -37,16 +45,16 @@ func Unmarshal(dst interface{}, sources ...Source) error { } // Parse is a higher level wrapper for Unmarshal that automatically parses flags and a .yaml file -func Parse(dst interface{}) error { +func Parse(dst Cloneable) error { return dParse(dst, - Defaults(), + 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 interface{}, defaults, yaml, flags Source) error { +func dParse(dst Cloneable, defaults, yaml, flags Source) error { // check dst is a pointer v := reflect.ValueOf(dst) if v.Kind() != reflect.Ptr { diff --git a/pkg/cfg/data_test.go b/pkg/cfg/data_test.go index b262c492b1a4b..037cd54722977 100644 --- a/pkg/cfg/data_test.go +++ b/pkg/cfg/data_test.go @@ -3,6 +3,8 @@ package cfg import ( "flag" "time" + + "github.com/cortexproject/cortex/pkg/util/flagext" ) // Data is a test Data structure @@ -12,6 +14,14 @@ type Data struct { TLS TLS `yaml:"tls"` } +// Clone takes advantage of pass-by-value semantics to return a distinct *Data. +// This is primarily used to parse a different flag set without mutating the original *Data. +func (d *Data) Clone() flagext.Registerer { + return func(d Data) *Data { + return &d + }(*d) +} + type Server struct { Port int `yaml:"port"` Timeout time.Duration `yaml:"timeout"` diff --git a/pkg/cfg/files.go b/pkg/cfg/files.go index 6e439add4cce1..36ee60efab0c3 100644 --- a/pkg/cfg/files.go +++ b/pkg/cfg/files.go @@ -7,14 +7,13 @@ import ( "io/ioutil" "os" - "github.com/cortexproject/cortex/pkg/util/flagext" "github.com/pkg/errors" "gopkg.in/yaml.v2" ) // JSON returns a Source that opens the supplied `.json` file and loads it. func JSON(f *string) Source { - return func(dst interface{}) error { + return func(dst Cloneable) error { if f == nil { return nil } @@ -31,14 +30,14 @@ func JSON(f *string) Source { // dJSON returns a JSON source and allows dependency injection func dJSON(y []byte) Source { - return func(dst interface{}) error { + return func(dst Cloneable) error { return json.Unmarshal(y, dst) } } // YAML returns a Source that opens the supplied `.yaml` file and loads it. func YAML(f string) Source { - return func(dst interface{}) error { + return func(dst Cloneable) error { y, err := ioutil.ReadFile(f) if err != nil { return err @@ -51,27 +50,19 @@ func YAML(f string) Source { // dYAML returns a YAML source and allows dependency injection func dYAML(y []byte) Source { - return func(dst interface{}) error { + return func(dst Cloneable) error { return yaml.UnmarshalStrict(y, dst) } } func YAMLFlag(args []string, name string) Source { - type cloneable interface { - Clone() flagext.Registerer - } - return func(dst interface{}) error { + return func(dst Cloneable) error { freshFlags := flag.NewFlagSet("config-file-loader", flag.ContinueOnError) - c, ok := dst.(cloneable) - if !ok { - return errors.New("dst does not satisfy cloneable") - } - // Ensure we register flags on a copy of the config so as to not mutate it while // parsing out the config file location. - c.Clone().RegisterFlags(freshFlags) + dst.Clone().RegisterFlags(freshFlags) usage := freshFlags.Usage freshFlags.Usage = func() { /* don't do anything by default, we will print usage ourselves, but only when requested. */ } diff --git a/pkg/cfg/flag.go b/pkg/cfg/flag.go index 05fa25e16f15c..cabf3c8ea2309 100644 --- a/pkg/cfg/flag.go +++ b/pkg/cfg/flag.go @@ -11,15 +11,9 @@ 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 { - return func(dst interface{}) error { + return func(dst Cloneable) error { r, ok := dst.(flagext.Registerer) if !ok { return errors.New("dst does not satisfy flagext.Registerer") @@ -40,7 +34,7 @@ func Flags() Source { // dFlags parses the flagset, applying all values set on the slice func dFlags(fs *flag.FlagSet, args []string) Source { - return func(dst interface{}) error { + return func(dst Cloneable) error { // parse the final flagset return fs.Parse(args) } diff --git a/pkg/logcli/query/query.go b/pkg/logcli/query/query.go index 71ba0e874c96e..dc959adc704f7 100644 --- a/pkg/logcli/query/query.go +++ b/pkg/logcli/query/query.go @@ -3,6 +3,7 @@ package query import ( "context" "errors" + "flag" "fmt" "log" "os" @@ -106,9 +107,7 @@ func (q *Query) printResult(value loghttp.ResultValue, out output.LogOutput) { func (q *Query) DoLocalQuery(out output.LogOutput, statistics bool, orgID string) error { var conf loki.Config - if err := cfg.Defaults()(&conf); err != nil { - return err - } + conf.RegisterFlags(flag.CommandLine) if q.LocalConfig == "" { return errors.New("no supplied config file") } diff --git a/pkg/loki/loki.go b/pkg/loki/loki.go index 546b5e1caad2c..4d22d8b15d660 100644 --- a/pkg/loki/loki.go +++ b/pkg/loki/loki.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" + "github.com/cortexproject/cortex/pkg/util/flagext" "github.com/cortexproject/cortex/pkg/util/modules" "github.com/prometheus/client_golang/prometheus" "github.com/weaveworks/common/signals" @@ -88,6 +89,14 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { c.Tracing.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) +} + // Validate the config and returns an error if the validation // doesn't pass func (c *Config) Validate(log log.Logger) error {