From af5aea4220dcd3e3df81c6191729e71316fec25e Mon Sep 17 00:00:00 2001 From: Dylan Guedes Date: Fri, 8 Oct 2021 13:38:57 -0300 Subject: [PATCH 1/3] Override default KV store for the compactor. - Currently, the default KVstore is consul. This changes it to inmemory to make it easier for new users to run the project without dependencies. - Refactor default flag value tests. Now, it will check for a populated map instead of checking for flags during iteration. --- pkg/loki/loki.go | 21 ++++++++++++++++++++- pkg/loki/loki_test.go | 41 +++++++++++++++++++++-------------------- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/pkg/loki/loki.go b/pkg/loki/loki.go index e0d0651ce935..3ffa89d68fe4 100644 --- a/pkg/loki/loki.go +++ b/pkg/loki/loki.go @@ -88,7 +88,7 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { f.BoolVar(&c.AuthEnabled, "auth.enabled", true, "Set to false to disable auth.") c.registerServerFlagsWithChangedDefaultValues(f) - c.Distributor.RegisterFlags(f) + c.registerDistributorFlagsWithChangedDefaultValues(f) c.Querier.RegisterFlags(f) c.IngesterClient.RegisterFlags(f) c.Ingester.RegisterFlags(f) @@ -129,6 +129,25 @@ func (c *Config) registerServerFlagsWithChangedDefaultValues(fs *flag.FlagSet) { }) } +func (c *Config) registerDistributorFlagsWithChangedDefaultValues(fs *flag.FlagSet) { + throwaway := flag.NewFlagSet("throwaway", flag.PanicOnError) + + // Register to throwaway flags first. Default values are remembered during registration and cannot be changed, + // but we can take values from throwaway flag set and reregister into supplied flags with new default values. + c.Distributor.RegisterFlags(throwaway) + + throwaway.VisitAll(func(f *flag.Flag) { + // Ignore errors when setting new values. We have a test to verify that it works. + + switch f.Name { + case "distributor.ring.store": + _ = f.Value.Set("inmemory") + } + + fs.Var(f.Value, f.Name, f.Usage) + }) +} + // 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 { diff --git a/pkg/loki/loki_test.go b/pkg/loki/loki_test.go index 7142d38d2712..4f69c225e9de 100644 --- a/pkg/loki/loki_test.go +++ b/pkg/loki/loki_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -25,34 +24,36 @@ func TestFlagDefaults(t *testing.T) { const delim = '\n' - minTimeChecked := false - pingWithoutStreamChecked := false + // Populate map with parsed default flags. + // Key is the flag and value is the default text. + gotFlags := make(map[string]string) for { line, err := buf.ReadString(delim) if err == io.EOF { break } - require.NoError(t, err) - if strings.Contains(line, "-server.grpc.keepalive.min-time-between-pings") { - nextLine, err := buf.ReadString(delim) - require.NoError(t, err) - assert.Contains(t, nextLine, "(default 10s)") - minTimeChecked = true - } + nextLine, err := buf.ReadString(delim) + require.NoError(t, err) - if strings.Contains(line, "-server.grpc.keepalive.ping-without-stream-allowed") { - nextLine, err := buf.ReadString(delim) - require.NoError(t, err) - assert.Contains(t, nextLine, "(default true)") - pingWithoutStreamChecked = true - } + trimmedLine := strings.Trim(line, " \n") + splittedLine := strings.Split(trimmedLine, " ")[0] + gotFlags[splittedLine] = nextLine } - require.True(t, minTimeChecked) - require.True(t, pingWithoutStreamChecked) + flagToCheck := "-distributor.ring.store" + require.Contains(t, gotFlags, flagToCheck) + require.Equal(t, c.Distributor.DistributorRing.KVStore.Store, "inmemory") + require.Contains(t, gotFlags[flagToCheck], "(default \"inmemory\")") + + flagToCheck = "-server.grpc.keepalive.min-time-between-pings" + require.Contains(t, gotFlags, flagToCheck) + require.Equal(t, c.Server.GRPCServerMinTimeBetweenPings, 10*time.Second) + require.Contains(t, gotFlags[flagToCheck], "(default 10s)") - require.Equal(t, true, c.Server.GRPCServerPingWithoutStreamAllowed) - require.Equal(t, 10*time.Second, c.Server.GRPCServerMinTimeBetweenPings) + flagToCheck = "-server.grpc.keepalive.ping-without-stream-allowed" + require.Contains(t, gotFlags, flagToCheck) + require.Equal(t, c.Server.GRPCServerPingWithoutStreamAllowed, true) + require.Contains(t, gotFlags[flagToCheck], "(default true)") } From db01cb601c022eb63b0b320dc6e477cde48f450c Mon Sep 17 00:00:00 2001 From: Dylan Guedes Date: Fri, 8 Oct 2021 16:56:10 -0300 Subject: [PATCH 2/3] Move distributor flag override to RegisterFlags. --- pkg/distributor/distributor.go | 23 ++++++++++++++++++++--- pkg/loki/loki.go | 21 +-------------------- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/pkg/distributor/distributor.go b/pkg/distributor/distributor.go index 385fee0122af..1cfa7cb890c1 100644 --- a/pkg/distributor/distributor.go +++ b/pkg/distributor/distributor.go @@ -43,9 +43,26 @@ type Config struct { factory ring_client.PoolFactory `yaml:"-"` } -// RegisterFlags registers the flags. -func (cfg *Config) RegisterFlags(f *flag.FlagSet) { - cfg.DistributorRing.RegisterFlags(f) +// RegisterFlags registers distributor-related flags. +// +// Since they are registered through an external library, we override some of them to set +// different default values. +func (cfg *Config) RegisterFlags(fs *flag.FlagSet) { + throwaway := flag.NewFlagSet("throwaway", flag.PanicOnError) + + cfg.DistributorRing.RegisterFlags(throwaway) + + // Register to throwaway flags first. Default values are remembered during registration and cannot be changed, + // but we can take values from throwaway flag set and reregister into supplied flags with new default values. + throwaway.VisitAll(func(f *flag.Flag) { + // Ignore errors when setting new values. We have a test to verify that it works. + switch f.Name { + case "distributor.ring.store": + _ = f.Value.Set("inmemory") + } + + fs.Var(f.Value, f.Name, f.Usage) + }) } // Distributor coordinates replicates and distribution of log streams. diff --git a/pkg/loki/loki.go b/pkg/loki/loki.go index 3ffa89d68fe4..e0d0651ce935 100644 --- a/pkg/loki/loki.go +++ b/pkg/loki/loki.go @@ -88,7 +88,7 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { f.BoolVar(&c.AuthEnabled, "auth.enabled", true, "Set to false to disable auth.") c.registerServerFlagsWithChangedDefaultValues(f) - c.registerDistributorFlagsWithChangedDefaultValues(f) + c.Distributor.RegisterFlags(f) c.Querier.RegisterFlags(f) c.IngesterClient.RegisterFlags(f) c.Ingester.RegisterFlags(f) @@ -129,25 +129,6 @@ func (c *Config) registerServerFlagsWithChangedDefaultValues(fs *flag.FlagSet) { }) } -func (c *Config) registerDistributorFlagsWithChangedDefaultValues(fs *flag.FlagSet) { - throwaway := flag.NewFlagSet("throwaway", flag.PanicOnError) - - // Register to throwaway flags first. Default values are remembered during registration and cannot be changed, - // but we can take values from throwaway flag set and reregister into supplied flags with new default values. - c.Distributor.RegisterFlags(throwaway) - - throwaway.VisitAll(func(f *flag.Flag) { - // Ignore errors when setting new values. We have a test to verify that it works. - - switch f.Name { - case "distributor.ring.store": - _ = f.Value.Set("inmemory") - } - - fs.Var(f.Value, f.Name, f.Usage) - }) -} - // 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 { From d0a1a9bfae6ae3030efbeae6fd8fbfb0286935d2 Mon Sep 17 00:00:00 2001 From: Dylan Guedes Date: Fri, 8 Oct 2021 17:41:58 -0300 Subject: [PATCH 3/3] Add missing CHANGELOG and upgrading guide entries. --- CHANGELOG.md | 1 + docs/sources/upgrading/_index.md | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f15b5469dc44..7fd1079d0ea9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## Main * [4400](https://github.com/grafana/loki/pull/4400) **trevorwhitney**: Config: automatically apply memberlist config too all rings when provided * [4435](https://github.com/grafana/loki/pull/4435) **trevorwhitney**: Change default values for two GRPC settings so querier can connect to frontend/scheduler +* [4440](https://github.com/grafana/loki/pull/4440) **DylanGuedes**: Config: Override distributor's default ring KV store * [4443](https://github.com/grafana/loki/pull/4443) **DylanGuedes**: Loki: Change how push API checks for contentType # 2.3.0 (2021/08/06) diff --git a/docs/sources/upgrading/_index.md b/docs/sources/upgrading/_index.md index 901e2febcc36..e38adf4388f1 100644 --- a/docs/sources/upgrading/_index.md +++ b/docs/sources/upgrading/_index.md @@ -19,6 +19,21 @@ If possible try to stay current and do sequential updates. If you want to skip v ### Loki +#### Distributor now stores ring in memory by default instead of Consul + +PR [4440](https://github.com/grafana/loki/pull/4440) **DylanGuedes**: Config: Override distributor's default ring KV store + +This change sets `inmemory` as the new default storage for the Distributor ring (previously `consul`). +The motivation is making the Distributor easier to run with default configs, by not requiring Consul anymore. +In any case, if you prefer to use Consul as the ring storage, you can set it by using the following config: + +```yaml +distributor: + ring: + kvstore: + store: consul +``` + #### Memberlist config now automatically applies to all non-configured rings PR [4400](https://github.com/grafana/loki/pull/4400) **trevorwhitney**: Config: automatically apply memberlist config too all rings when provided @@ -84,7 +99,6 @@ Please manually provide the values of `5m` and `true` (respectively) in your con -_add changes here which are unreleased_ - ## 2.3.0 ### Loki