From d3d63e17789483370b62c9278af422d0940e7e5c Mon Sep 17 00:00:00 2001 From: Dylan Guedes Date: Wed, 13 Oct 2021 15:31:16 -0300 Subject: [PATCH] Loki: Override distributor's default ring KV store (#4440) * 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. * Move distributor flag override to RegisterFlags. * Add missing CHANGELOG and upgrading guide entries. --- CHANGELOG.md | 1 + docs/sources/upgrading/_index.md | 16 ++++++++++++- pkg/distributor/distributor.go | 23 +++++++++++++++--- pkg/loki/loki_test.go | 41 ++++++++++++++++---------------- 4 files changed, 57 insertions(+), 24 deletions(-) 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 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_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)") }