diff --git a/CHANGELOG.md b/CHANGELOG.md index 880107c65ff2..f15b5469dc44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## 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 * [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 3ddc0f47b1bb..901e2febcc36 100644 --- a/docs/sources/upgrading/_index.md +++ b/docs/sources/upgrading/_index.md @@ -61,6 +61,27 @@ ruler: host: consul.namespace.svc.cluster.local:8500 ``` +#### Changed defaults for some GRPC server settings +* [4435](https://github.com/grafana/loki/pull/4435) **trevorwhitney**: Change default values for two GRPC settings so querier can connect to frontend/scheduler + +This changes two default values, `grpc_server_min_time_between_pings` and `grpc_server_ping_without_stream_allowed` used by the GRPC server. + +*Previous Values*: +``` +server: + grpc_server_min_time_between_pings: '5m' + grpc_server_ping_without_stream_allowed: false +``` + +*New Values*: +``` +server: + grpc_server_min_time_between_pings: '10s' + grpc_server_ping_without_stream_allowed: true +``` + +Please manually provide the values of `5m` and `true` (respectively) in your config if you rely on those values. + -_add changes here which are unreleased_ diff --git a/pkg/loki/loki.go b/pkg/loki/loki.go index d6c9f8959526..e0d0651ce935 100644 --- a/pkg/loki/loki.go +++ b/pkg/loki/loki.go @@ -87,7 +87,7 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { "The alias 'all' can be used in the list to load a number of core modules and will enable single-binary mode. ") f.BoolVar(&c.AuthEnabled, "auth.enabled", true, "Set to false to disable auth.") - c.Server.RegisterFlags(f) + c.registerServerFlagsWithChangedDefaultValues(f) c.Distributor.RegisterFlags(f) c.Querier.RegisterFlags(f) c.IngesterClient.RegisterFlags(f) @@ -108,6 +108,27 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) { c.QueryScheduler.RegisterFlags(f) } +func (c *Config) registerServerFlagsWithChangedDefaultValues(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.Server.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 "server.grpc.keepalive.min-time-between-pings": + _ = f.Value.Set("10s") + + case "server.grpc.keepalive.ping-without-stream-allowed": + _ = f.Value.Set("true") + } + + 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 new file mode 100644 index 000000000000..7142d38d2712 --- /dev/null +++ b/pkg/loki/loki_test.go @@ -0,0 +1,58 @@ +package loki + +import ( + "bytes" + "flag" + "io" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFlagDefaults(t *testing.T) { + c := Config{} + + f := flag.NewFlagSet("test", flag.PanicOnError) + c.RegisterFlags(f) + + buf := bytes.Buffer{} + + f.SetOutput(&buf) + f.PrintDefaults() + + const delim = '\n' + + minTimeChecked := false + pingWithoutStreamChecked := false + 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 + } + + 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 + } + } + + require.True(t, minTimeChecked) + require.True(t, pingWithoutStreamChecked) + + require.Equal(t, true, c.Server.GRPCServerPingWithoutStreamAllowed) + require.Equal(t, 10*time.Second, c.Server.GRPCServerMinTimeBetweenPings) +}