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,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)

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

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.

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)")
 }