From 087285af3c8529e9c2caf7b8cc61df834eb5dbf6 Mon Sep 17 00:00:00 2001 From: Karsten Jeschkies Date: Tue, 19 Mar 2024 14:34:00 +0100 Subject: [PATCH] refactor: Replace TenantConfig method with interface. (#12260) **What this PR does / why we need it**: This change allows the retrieval of the `TenantConfig` or the `DefaultConfig`. It also defines an interface instead of a tenant config function which makes it a little easier to grok the code. Eventually this will enable us to introduce GEL specific configurations. **Checklist** - [ ] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [ ] Documentation added - [x] Tests updated - [ ] `CHANGELOG.md` updated - [ ] If the change is worth mentioning in the release notes, add `add-to-release-notes` label - [ ] Changes that require user attention or interaction to upgrade are documented in `docs/sources/setup/upgrade/_index.md` - [ ] For Helm chart changes bump the Helm chart version in `production/helm/loki/Chart.yaml` and update `production/helm/loki/CHANGELOG.md` and `production/helm/loki/README.md`. [Example PR](https://github.com/grafana/loki/commit/d10549e3ece02120974929894ee333d07755d213) - [ ] If the change is deprecating or removing a configuration option, update the `deprecated-config.yaml` and `deleted-config.yaml` files respectively in the `tools/deprecated-config-checker` directory. [Example PR](https://github.com/grafana/loki/pull/10840/commits/0d4416a4b03739583349934b96f272fb4f685d15) --- pkg/distributor/writefailures/manager_test.go | 44 ++++++++++++------- pkg/loki/modules.go | 2 +- pkg/loki/runtime_config.go | 31 ++++++++----- pkg/loki/runtime_config_test.go | 10 ++--- pkg/runtime/config.go | 26 +++++------ 5 files changed, 67 insertions(+), 46 deletions(-) diff --git a/pkg/distributor/writefailures/manager_test.go b/pkg/distributor/writefailures/manager_test.go index 27028fc21c5e..6f5f1eee3841 100644 --- a/pkg/distributor/writefailures/manager_test.go +++ b/pkg/distributor/writefailures/manager_test.go @@ -20,18 +20,20 @@ func TestWriteFailuresLogging(t *testing.T) { buf := bytes.NewBuffer(nil) logger := log.NewLogfmtLogger(buf) - f := func(tenantID string) *runtime.Config { - if tenantID == "good-tenant" { - return &runtime.Config{ - LimitedLogPushErrors: true, + f := &providerMock{ + tenantConfig: func(tenantID string) *runtime.Config { + if tenantID == "good-tenant" { + return &runtime.Config{ + LimitedLogPushErrors: true, + } } - } - if tenantID == "bad-tenant" { - return &runtime.Config{ - LimitedLogPushErrors: false, + if tenantID == "bad-tenant" { + return &runtime.Config{ + LimitedLogPushErrors: false, + } } - } - return &runtime.Config{} + return &runtime.Config{} + }, } runtimeCfg, err := runtime.NewTenantConfigs(f) @@ -55,12 +57,14 @@ func TestWriteFailuresRateLimiting(t *testing.T) { buf := bytes.NewBuffer(nil) logger := log.NewLogfmtLogger(buf) - f := func(tenantID string) *runtime.Config { - return &runtime.Config{ - LimitedLogPushErrors: true, - } + provider := &providerMock{ + tenantConfig: func(tenantID string) *runtime.Config { + return &runtime.Config{ + LimitedLogPushErrors: true, + } + }, } - runtimeCfg, err := runtime.NewTenantConfigs(f) + runtimeCfg, err := runtime.NewTenantConfigs(provider) require.NoError(t, err) t.Run("with zero rate limiting", func(t *testing.T) { @@ -126,7 +130,7 @@ func TestWriteFailuresRateLimiting(t *testing.T) { }) t.Run("limit is per-tenant", func(t *testing.T) { - runtimeCfg, err := runtime.NewTenantConfigs(f) + runtimeCfg, err := runtime.NewTenantConfigs(provider) require.NoError(t, err) manager := NewManager(logger, prometheus.NewRegistry(), Cfg{LogRate: flagext.ByteSize(1000)}, runtimeCfg, "ingester") @@ -161,3 +165,11 @@ func TestWriteFailuresRateLimiting(t *testing.T) { require.Contains(t, content, "3z") }) } + +type providerMock struct { + tenantConfig func(string) *runtime.Config +} + +func (m *providerMock) TenantConfig(userID string) *runtime.Config { + return m.tenantConfig(userID) +} diff --git a/pkg/loki/modules.go b/pkg/loki/modules.go index f9d67ef255be..7dec8946c71d 100644 --- a/pkg/loki/modules.go +++ b/pkg/loki/modules.go @@ -306,7 +306,7 @@ func (t *Loki) initOverridesExporter() (services.Service, error) { } func (t *Loki) initTenantConfigs() (_ services.Service, err error) { - t.tenantConfigs, err = runtime.NewTenantConfigs(tenantConfigFromRuntimeConfig(t.runtimeConfig)) + t.tenantConfigs, err = runtime.NewTenantConfigs(newTenantConfigProvider(t.runtimeConfig)) // tenantConfigs are not a service, since they don't have any operational state. return nil, err } diff --git a/pkg/loki/runtime_config.go b/pkg/loki/runtime_config.go index 4b76bae70e85..3432ee1b68b8 100644 --- a/pkg/loki/runtime_config.go +++ b/pkg/loki/runtime_config.go @@ -55,6 +55,7 @@ func loadRuntimeConfig(r io.Reader) (interface{}, error) { return overrides, nil } +// tenantLimitsFromRuntimeConfig implements validation.Limits type tenantLimitsFromRuntimeConfig struct { c *runtimeconfig.Manager } @@ -85,20 +86,28 @@ func newtenantLimitsFromRuntimeConfig(c *runtimeconfig.Manager) validation.Tenan return &tenantLimitsFromRuntimeConfig{c: c} } -func tenantConfigFromRuntimeConfig(c *runtimeconfig.Manager) runtime.TenantConfig { - if c == nil { +type tenantConfigProvider struct { + c *runtimeconfig.Manager +} + +func newTenantConfigProvider(c *runtimeconfig.Manager) runtime.TenantConfigProvider { + return &tenantConfigProvider{c: c} +} + +// TenantConfig returns the user config or default config if none was defined. +func (t *tenantConfigProvider) TenantConfig(userID string) *runtime.Config { + if t.c == nil { return nil } - return func(userID string) *runtime.Config { - cfg, ok := c.GetConfig().(*runtimeConfigValues) - if !ok || cfg == nil { - return nil - } - if tenantCfg, ok := cfg.TenantConfig[userID]; ok { - return tenantCfg - } - return cfg.DefaultConfig + + cfg, ok := t.c.GetConfig().(*runtimeConfigValues) + if !ok || cfg == nil { + return nil + } + if tenantCfg, ok := cfg.TenantConfig[userID]; ok { + return tenantCfg } + return cfg.DefaultConfig } func multiClientRuntimeConfigChannel(manager *runtimeconfig.Manager) func() <-chan kv.MultiRuntimeConfig { diff --git a/pkg/loki/runtime_config_test.go b/pkg/loki/runtime_config_test.go index 26b018318a37..d0fd2ffa4103 100644 --- a/pkg/loki/runtime_config_test.go +++ b/pkg/loki/runtime_config_test.go @@ -98,9 +98,9 @@ configs: log_push_request: true `) - user1 := runtimeGetter("1") - user2 := runtimeGetter("2") - user3 := runtimeGetter("3") + user1 := runtimeGetter.TenantConfig("1") + user2 := runtimeGetter.TenantConfig("2") + user3 := runtimeGetter.TenantConfig("3") require.Equal(t, false, user1.LogPushRequest) require.Equal(t, false, user1.LimitedLogPushErrors) @@ -110,7 +110,7 @@ configs: require.Equal(t, true, user3.LogPushRequest) } -func newTestRuntimeconfig(t *testing.T, yaml string) runtime.TenantConfig { +func newTestRuntimeconfig(t *testing.T, yaml string) runtime.TenantConfigProvider { t.Helper() f, err := os.CreateTemp(t.TempDir(), "bar") require.NoError(t, err) @@ -140,7 +140,7 @@ func newTestRuntimeconfig(t *testing.T, yaml string) runtime.TenantConfig { require.NoError(t, runtimeConfig.AwaitTerminated(context.Background())) }() - return tenantConfigFromRuntimeConfig(runtimeConfig) + return newTenantConfigProvider(runtimeConfig) } func newTestOverrides(t *testing.T, yaml string) *validation.Overrides { diff --git a/pkg/runtime/config.go b/pkg/runtime/config.go index 9795277fbf13..234b5c0c5e2b 100644 --- a/pkg/runtime/config.go +++ b/pkg/runtime/config.go @@ -9,41 +9,41 @@ type Config struct { LimitedLogPushErrors bool `yaml:"limited_log_push_errors"` } -// TenantConfig is a function that returns configs for given tenant, or -// nil, if there are no tenant-specific configs. -type TenantConfig func(userID string) *Config +var EmptyConfig = &Config{} + +// TenantConfigProvider serves a tenant or default config. +type TenantConfigProvider interface { + TenantConfig(userID string) *Config +} // TenantConfigs periodically fetch a set of per-user configs, and provides convenience // functions for fetching the correct value. type TenantConfigs struct { - defaultConfig *Config - tenantConfig TenantConfig + TenantConfigProvider } // DefaultTenantConfigs creates and returns a new TenantConfigs with the defaults populated. func DefaultTenantConfigs() *TenantConfigs { return &TenantConfigs{ - defaultConfig: &Config{}, - tenantConfig: nil, + TenantConfigProvider: nil, } } // NewTenantConfig makes a new TenantConfigs -func NewTenantConfigs(tenantConfig TenantConfig) (*TenantConfigs, error) { +func NewTenantConfigs(configProvider TenantConfigProvider) (*TenantConfigs, error) { return &TenantConfigs{ - defaultConfig: DefaultTenantConfigs().defaultConfig, - tenantConfig: tenantConfig, + TenantConfigProvider: configProvider, }, nil } func (o *TenantConfigs) getOverridesForUser(userID string) *Config { - if o.tenantConfig != nil { - l := o.tenantConfig(userID) + if o.TenantConfigProvider != nil { + l := o.TenantConfigProvider.TenantConfig(userID) if l != nil { return l } } - return o.defaultConfig + return EmptyConfig } func (o *TenantConfigs) LogStreamCreation(userID string) bool {