From efd99512b8b0ec42def8bbac182e3854757304a3 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Tue, 21 Mar 2023 20:03:53 +0100 Subject: [PATCH 01/10] Ruler: validation and logging with disabled tenant federation This PR bundles two changes because they are related and small * reject creating federated rule groups when ruler tenant federation is disabled. Currently, the rule groups would be silently ignored during bucket syncs. This made the UX around creating federated rules confusing because there was no feedback as to why the groups aren't evaluating * log an info line on each sync when a federated rule group is skipped because the feature is disabled Signed-off-by: Dimitar Dimitrov --- pkg/ruler/api_test.go | 43 +++++++++++++++++++++++++++++----- pkg/ruler/manager.go | 9 +++++-- pkg/ruler/tenant_federation.go | 5 +++- 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/pkg/ruler/api_test.go b/pkg/ruler/api_test.go index f0c52168373..acf45b085bd 100644 --- a/pkg/ruler/api_test.go +++ b/pkg/ruler/api_test.go @@ -376,13 +376,14 @@ func TestRuler_PrometheusAlerts(t *testing.T) { } func TestRuler_Create(t *testing.T) { - cfg := defaultRulerConfig(t) + defaultCfg := defaultRulerConfig(t) - r := prepareRuler(t, cfg, newMockRuleStore(make(map[string]rulespb.RuleGroupList)), withStart()) - a := NewAPI(r, r.store, log.NewNopLogger()) + cfgWithTenantFederation := defaultRulerConfig(t) + cfgWithTenantFederation.TenantFederation.Enabled = true tc := []struct { name string + cfg Config input string output string err error @@ -390,12 +391,14 @@ func TestRuler_Create(t *testing.T) { }{ { name: "with an empty payload", + cfg: defaultCfg, input: "", status: 400, err: errors.New("invalid rules config: rule group name must not be empty"), }, { name: "with no rule group name", + cfg: defaultCfg, input: ` interval: 15s rules: @@ -407,6 +410,7 @@ rules: }, { name: "with no rules", + cfg: defaultCfg, input: ` name: rg_name interval: 15s @@ -415,11 +419,35 @@ interval: 15s err: errors.New("invalid rules config: rule group 'rg_name' has no rules"), }, { - name: "with a a valid rules file", + + name: "with federated rules without enabled federation", + cfg: defaultCfg, + status: 400, + input: ` +name: test +interval: 15s +source_tenants: [t1, t2] +rules: +- record: up_rule + expr: up{} +- alert: up_alert + expr: sum(up{}) > 1 + for: 30s + annotations: + test: test + labels: + test: test +`, + err: errors.New("invalid rules config: rule group 'test' is a federated rule group but rules federation is disabled. To enable it, set -ruler.tenant-federation.enabled=true as a CLI argument or ruler.tenant_federation.enabled: true in YAML or contact your service administrator"), + }, + { + name: "with valid rules with enabled federation", + cfg: cfgWithTenantFederation, status: 202, input: ` name: test interval: 15s +source_tenants: [t1, t2] rules: - record: up_rule expr: up{} @@ -431,12 +459,15 @@ rules: labels: test: test `, - output: "name: test\ninterval: 15s\nrules:\n - record: up_rule\n expr: up{}\n - alert: up_alert\n expr: sum(up{}) > 1\n for: 30s\n labels:\n test: test\n annotations:\n test: test\n", + output: "name: test\ninterval: 15s\nsource_tenants: [t1, t2]\nrules:\n - record: up_rule\n expr: up{}\n - alert: up_alert\n expr: sum(up{}) > 1\n for: 30s\n labels:\n test: test\n annotations:\n test: test\n", }, } for _, tt := range tc { t.Run(tt.name, func(t *testing.T) { + r := prepareRuler(t, tt.cfg, newMockRuleStore(make(map[string]rulespb.RuleGroupList)), withStart()) + a := NewAPI(r, r.store, log.NewNopLogger()) + router := mux.NewRouter() router.Path("/prometheus/config/v1/rules/{namespace}").Methods("POST").HandlerFunc(a.CreateRuleGroup) router.Path("/prometheus/config/v1/rules/{namespace}/{groupName}").Methods("GET").HandlerFunc(a.GetRuleGroup) @@ -454,7 +485,7 @@ rules: router.ServeHTTP(w, req) require.Equal(t, 200, w.Code) - require.Equal(t, tt.output, w.Body.String()) + require.YAMLEq(t, tt.output, w.Body.String()) } else { require.Equal(t, tt.err.Error()+"\n", w.Body.String()) } diff --git a/pkg/ruler/manager.go b/pkg/ruler/manager.go index fd82482f27c..617e83b27be 100644 --- a/pkg/ruler/manager.go +++ b/pkg/ruler/manager.go @@ -103,7 +103,7 @@ func NewDefaultMultiTenantManager(cfg Config, managerFactory ManagerFactory, reg // It's not safe to call this function concurrently. func (r *DefaultMultiTenantManager) SyncRuleGroups(ctx context.Context, ruleGroups map[string]rulespb.RuleGroupList) { if !r.cfg.TenantFederation.Enabled { - RemoveFederatedRuleGroups(ruleGroups) + removeFederatedRuleGroups(ruleGroups, r.logger) } // Sync the rules to disk and then update the user's Prometheus Rules Manager. @@ -318,7 +318,7 @@ func (r *DefaultMultiTenantManager) Stop() { r.mapper.cleanup() } -func (*DefaultMultiTenantManager) ValidateRuleGroup(g rulefmt.RuleGroup) []error { +func (r *DefaultMultiTenantManager) ValidateRuleGroup(g rulefmt.RuleGroup) []error { var errs []error if g.Name == "" { @@ -331,6 +331,11 @@ func (*DefaultMultiTenantManager) ValidateRuleGroup(g rulefmt.RuleGroup) []error return errs } + if !r.cfg.TenantFederation.Enabled && len(g.SourceTenants) > 0 { + errs = append(errs, fmt.Errorf("invalid rules config: rule group '%s' is a federated rule group but rules federation is disabled. "+ + "To enable it, set -ruler.tenant-federation.enabled=true as a CLI argument or ruler.tenant_federation.enabled: true in YAML or contact your service administrator", g.Name)) + } + for i, r := range g.Rules { for _, err := range r.Validate() { var ruleName string diff --git a/pkg/ruler/tenant_federation.go b/pkg/ruler/tenant_federation.go index 48b6b42d80f..18b88376c71 100644 --- a/pkg/ruler/tenant_federation.go +++ b/pkg/ruler/tenant_federation.go @@ -7,6 +7,8 @@ import ( "flag" "time" + "github.com/go-kit/log" + "github.com/go-kit/log/level" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/rules" "github.com/weaveworks/common/user" @@ -55,11 +57,12 @@ func TenantFederationQueryFunc(regularQueryable, federatedQueryable rules.QueryF } } -func RemoveFederatedRuleGroups(groups map[string]rulespb.RuleGroupList) { +func removeFederatedRuleGroups(groups map[string]rulespb.RuleGroupList, logger log.Logger) { for userID, groupList := range groups { amended := make(rulespb.RuleGroupList, 0, len(groupList)) for _, group := range groupList { if len(group.GetSourceTenants()) > 0 { + level.Info(logger).Log("msg", "skipping federated rule group because rule federation is disabled", "namespace", group.Namespace, "group_name", group.Name, "user", userID) continue } amended = append(amended, group) From 4657bf3f7f2535848216e3167c6aa9a5d613c1d5 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Tue, 21 Mar 2023 20:07:16 +0100 Subject: [PATCH 02/10] Add CHANGELOG.md entry Signed-off-by: Dimitar Dimitrov --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 542514fff0d..4bb7fb3e24f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [CHANGE] Query-frontend: Cached results now contain timestamp which allows Mimir to check if cached results are still valid based on current TTL configured for tenant. Results cached by previous Mimir version are used until they expire from cache, which can take up to 7 days. If you need to use per-tenant TTL sooner, please flush results cache manually. #4439 * [CHANGE] Ingester: the `cortex_ingester_tsdb_wal_replay_duration_seconds` metrics has been removed. #4465 * [CHANGE] Query-frontend: use protobuf internal query result payload format by default. This feature is no longer considered experimental. #4557 +* [CHANGE] Ruler: reject creating federated rule groups while tenant federation is disabled. Previously the rule groups would be silently dropped during bucket sync. #4555 * [FEATURE] Cache: Introduce experimental support for using Redis for results, chunks, index, and metadata caches. #4371 * [FEATURE] Vault: Introduce experimental integration with Vault to fetch secrets used to configure TLS for clients. Server TLS secrets will still be read from a file. `tls-ca-path`, `tls-cert-path` and `tls-key-path` will denote the path in Vault for the following CLI flags when `-vault.enabled` is true: #4446. * `-distributor.ha-tracker.etcd.*` From 8d102dcc51cbbe06266eec4410fd070cfb7c3745 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Wed, 22 Mar 2023 15:23:11 +0100 Subject: [PATCH 03/10] Extract ruler flag as a constant; change wording Signed-off-by: Dimitar Dimitrov --- pkg/mimir/modules.go | 2 +- pkg/ruler/api_test.go | 2 +- pkg/ruler/manager.go | 5 +++-- pkg/ruler/tenant_federation.go | 4 +++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/mimir/modules.go b/pkg/mimir/modules.go index a3b4ef85c37..42ebd8d71c5 100644 --- a/pkg/mimir/modules.go +++ b/pkg/mimir/modules.go @@ -700,7 +700,7 @@ func (t *Mimir) initRuler() (serv services.Service, err error) { if t.Cfg.Ruler.TenantFederation.Enabled { if !t.Cfg.TenantFederation.Enabled { - return nil, errors.New("-ruler.tenant-federation.enabled=true requires -tenant-federation.enabled=true") + return nil, errors.New("-" + ruler.TenantFederationFlag + "=true requires -tenant-federation.enabled=true") } // Setting bypassForSingleQuerier=false forces `tenantfederation.NewQueryable` to add // the `__tenant_id__` label on all metrics regardless if they're for a single tenant or multiple tenants. diff --git a/pkg/ruler/api_test.go b/pkg/ruler/api_test.go index acf45b085bd..ea113ff0269 100644 --- a/pkg/ruler/api_test.go +++ b/pkg/ruler/api_test.go @@ -438,7 +438,7 @@ rules: labels: test: test `, - err: errors.New("invalid rules config: rule group 'test' is a federated rule group but rules federation is disabled. To enable it, set -ruler.tenant-federation.enabled=true as a CLI argument or ruler.tenant_federation.enabled: true in YAML or contact your service administrator"), + err: errors.New("invalid rules config: rule group 'test' is a federated rule group, but rules federation is disabled. To enable the feature, configure the Mimir or GEM ruler instance with the following parameter or contact your service administrator: -ruler.tenant-federation.enabled=true as a CLI argument, ruler.tenant_federation.enabled: true in YAML"), }, { name: "with valid rules with enabled federation", diff --git a/pkg/ruler/manager.go b/pkg/ruler/manager.go index 617e83b27be..047d5bf0263 100644 --- a/pkg/ruler/manager.go +++ b/pkg/ruler/manager.go @@ -332,8 +332,9 @@ func (r *DefaultMultiTenantManager) ValidateRuleGroup(g rulefmt.RuleGroup) []err } if !r.cfg.TenantFederation.Enabled && len(g.SourceTenants) > 0 { - errs = append(errs, fmt.Errorf("invalid rules config: rule group '%s' is a federated rule group but rules federation is disabled. "+ - "To enable it, set -ruler.tenant-federation.enabled=true as a CLI argument or ruler.tenant_federation.enabled: true in YAML or contact your service administrator", g.Name)) + errs = append(errs, fmt.Errorf("invalid rules config: rule group '%s' is a federated rule group, but rules federation is disabled. "+ + "To enable the feature, configure the Mimir or GEM ruler instance with the following parameter or contact your service administrator: "+ + "set -"+TenantFederationFlag+"=true as a CLI argument, ruler.tenant_federation.enabled: true in YAML", g.Name)) } for i, r := range g.Rules { diff --git a/pkg/ruler/tenant_federation.go b/pkg/ruler/tenant_federation.go index 18b88376c71..c483ba3f2d7 100644 --- a/pkg/ruler/tenant_federation.go +++ b/pkg/ruler/tenant_federation.go @@ -22,8 +22,10 @@ type TenantFederationConfig struct { Enabled bool `yaml:"enabled"` } +const TenantFederationFlag = "ruler.tenant-federation.enabled" + func (cfg *TenantFederationConfig) RegisterFlags(f *flag.FlagSet) { - f.BoolVar(&cfg.Enabled, "ruler.tenant-federation.enabled", false, "Enable running rule groups against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations.") + f.BoolVar(&cfg.Enabled, TenantFederationFlag, false, "Enable running rule groups against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations.") } type contextKey int From bea61fa1e11605fbe5868c2125e962ecb9af892d Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Wed, 22 Mar 2023 15:23:42 +0100 Subject: [PATCH 04/10] Change skipping rule group log level to warning Signed-off-by: Dimitar Dimitrov --- pkg/ruler/tenant_federation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ruler/tenant_federation.go b/pkg/ruler/tenant_federation.go index c483ba3f2d7..97ba61fe876 100644 --- a/pkg/ruler/tenant_federation.go +++ b/pkg/ruler/tenant_federation.go @@ -64,7 +64,7 @@ func removeFederatedRuleGroups(groups map[string]rulespb.RuleGroupList, logger l amended := make(rulespb.RuleGroupList, 0, len(groupList)) for _, group := range groupList { if len(group.GetSourceTenants()) > 0 { - level.Info(logger).Log("msg", "skipping federated rule group because rule federation is disabled", "namespace", group.Namespace, "group_name", group.Name, "user", userID) + level.Warn(logger).Log("msg", "skipping federated rule group because rule federation is disabled", "namespace", group.Namespace, "group_name", group.Name, "user", userID) continue } amended = append(amended, group) From b0b2a4baed928827f424db23a08019753c5abca0 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Wed, 22 Mar 2023 15:30:31 +0100 Subject: [PATCH 05/10] Fix out of sync error message Signed-off-by: Dimitar Dimitrov --- pkg/ruler/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/ruler/api_test.go b/pkg/ruler/api_test.go index ea113ff0269..560ce3b9226 100644 --- a/pkg/ruler/api_test.go +++ b/pkg/ruler/api_test.go @@ -438,7 +438,7 @@ rules: labels: test: test `, - err: errors.New("invalid rules config: rule group 'test' is a federated rule group, but rules federation is disabled. To enable the feature, configure the Mimir or GEM ruler instance with the following parameter or contact your service administrator: -ruler.tenant-federation.enabled=true as a CLI argument, ruler.tenant_federation.enabled: true in YAML"), + err: errors.New("invalid rules config: rule group 'test' is a federated rule group, but rules federation is disabled. To enable the feature, configure the Mimir or GEM ruler instance with the following parameter or contact your service administrator: set -ruler.tenant-federation.enabled=true as a CLI argument, ruler.tenant_federation.enabled: true in YAML"), }, { name: "with valid rules with enabled federation", From c256699cb3533a4ab442dea95380cea501e48cb8 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 27 Mar 2023 10:16:40 +0200 Subject: [PATCH 06/10] Remove flags from API error message Signed-off-by: Dimitar Dimitrov --- pkg/ruler/api_test.go | 2 +- pkg/ruler/manager.go | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/ruler/api_test.go b/pkg/ruler/api_test.go index 560ce3b9226..1c46bf186ff 100644 --- a/pkg/ruler/api_test.go +++ b/pkg/ruler/api_test.go @@ -438,7 +438,7 @@ rules: labels: test: test `, - err: errors.New("invalid rules config: rule group 'test' is a federated rule group, but rules federation is disabled. To enable the feature, configure the Mimir or GEM ruler instance with the following parameter or contact your service administrator: set -ruler.tenant-federation.enabled=true as a CLI argument, ruler.tenant_federation.enabled: true in YAML"), + err: errors.New("invalid rules config: rule group 'test' is a federated rule group, but rules federation is disabled; please contact your service administrator to have it enabled"), }, { name: "with valid rules with enabled federation", diff --git a/pkg/ruler/manager.go b/pkg/ruler/manager.go index 047d5bf0263..a97e9a65a22 100644 --- a/pkg/ruler/manager.go +++ b/pkg/ruler/manager.go @@ -332,9 +332,8 @@ func (r *DefaultMultiTenantManager) ValidateRuleGroup(g rulefmt.RuleGroup) []err } if !r.cfg.TenantFederation.Enabled && len(g.SourceTenants) > 0 { - errs = append(errs, fmt.Errorf("invalid rules config: rule group '%s' is a federated rule group, but rules federation is disabled. "+ - "To enable the feature, configure the Mimir or GEM ruler instance with the following parameter or contact your service administrator: "+ - "set -"+TenantFederationFlag+"=true as a CLI argument, ruler.tenant_federation.enabled: true in YAML", g.Name)) + errs = append(errs, fmt.Errorf("invalid rules config: rule group '%s' is a federated rule group, "+ + "but rules federation is disabled; please contact your service administrator to have it enabled", g.Name)) } for i, r := range g.Rules { From 8e0cffecb61dd41a90b9b442b9f4973980df594f Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 27 Mar 2023 10:18:38 +0200 Subject: [PATCH 07/10] Add CLI flag to log line Signed-off-by: Dimitar Dimitrov --- pkg/ruler/tenant_federation.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/ruler/tenant_federation.go b/pkg/ruler/tenant_federation.go index 97ba61fe876..f7ef97272e1 100644 --- a/pkg/ruler/tenant_federation.go +++ b/pkg/ruler/tenant_federation.go @@ -64,7 +64,15 @@ func removeFederatedRuleGroups(groups map[string]rulespb.RuleGroupList, logger l amended := make(rulespb.RuleGroupList, 0, len(groupList)) for _, group := range groupList { if len(group.GetSourceTenants()) > 0 { - level.Warn(logger).Log("msg", "skipping federated rule group because rule federation is disabled", "namespace", group.Namespace, "group_name", group.Name, "user", userID) + level.Warn(logger).Log( + "msg", "skipping federated rule group because rule federation is disabled; "+ + "to enable the feature, configure the Mimir or GEM ruler instance with the following parameter "+ + "or contact your service administrator: set -"+TenantFederationFlag+"=true as a CLI argument, "+ + "ruler.tenant_federation.enabled: true in YAML", + "namespace", group.Namespace, + "group_name", group.Name, + "user", userID, + ) continue } amended = append(amended, group) From 7127c2485fe501d543ce8714fc190501419188c4 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 27 Mar 2023 10:21:43 +0200 Subject: [PATCH 08/10] Update CLI flag for federated rules Signed-off-by: Dimitar Dimitrov --- cmd/mimir/config-descriptor.json | 2 +- cmd/mimir/help-all.txt.tmpl | 2 +- cmd/mimir/help.txt.tmpl | 2 +- .../mimir/references/configuration-parameters/index.md | 8 ++++---- pkg/ruler/tenant_federation.go | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/mimir/config-descriptor.json b/cmd/mimir/config-descriptor.json index 30041480613..4c054cb05d3 100644 --- a/cmd/mimir/config-descriptor.json +++ b/cmd/mimir/config-descriptor.json @@ -10063,7 +10063,7 @@ "kind": "field", "name": "enabled", "required": false, - "desc": "Enable running rule groups against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations.", + "desc": "Enable rule groups querying against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations.", "fieldValue": null, "fieldDefaultValue": false, "fieldFlag": "ruler.tenant-federation.enabled", diff --git a/cmd/mimir/help-all.txt.tmpl b/cmd/mimir/help-all.txt.tmpl index 9ca60f490f4..66a48598d10 100644 --- a/cmd/mimir/help-all.txt.tmpl +++ b/cmd/mimir/help-all.txt.tmpl @@ -2176,7 +2176,7 @@ Usage of ./cmd/mimir/mimir: -ruler.rule-path string Directory to store temporary rule files loaded by the Prometheus rule managers. This directory is not required to be persisted between restarts. (default "./data-ruler/") -ruler.tenant-federation.enabled - Enable running rule groups against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations. + Enable rule groups querying against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations. -ruler.tenant-shard-size int The tenant's shard size when sharding is used by ruler. Value of 0 disables shuffle sharding for the tenant, and tenant rules will be sharded across all ruler replicas. -runtime-config.file comma-separated-list-of-strings diff --git a/cmd/mimir/help.txt.tmpl b/cmd/mimir/help.txt.tmpl index f7b500c08d6..2806139929d 100644 --- a/cmd/mimir/help.txt.tmpl +++ b/cmd/mimir/help.txt.tmpl @@ -594,7 +594,7 @@ Usage of ./cmd/mimir/mimir: -ruler.rule-path string Directory to store temporary rule files loaded by the Prometheus rule managers. This directory is not required to be persisted between restarts. (default "./data-ruler/") -ruler.tenant-federation.enabled - Enable running rule groups against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations. + Enable rule groups querying against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations. -ruler.tenant-shard-size int The tenant's shard size when sharding is used by ruler. Value of 0 disables shuffle sharding for the tenant, and tenant rules will be sharded across all ruler replicas. -runtime-config.file comma-separated-list-of-strings diff --git a/docs/sources/mimir/references/configuration-parameters/index.md b/docs/sources/mimir/references/configuration-parameters/index.md index bf57151cf18..787ddb27fe7 100644 --- a/docs/sources/mimir/references/configuration-parameters/index.md +++ b/docs/sources/mimir/references/configuration-parameters/index.md @@ -1596,10 +1596,10 @@ query_frontend: [query_result_response_format: | default = "json"] tenant_federation: - # Enable running rule groups against multiple tenants. The tenant IDs involved - # need to be in the rule group's 'source_tenants' field. If this flag is set - # to 'false' when there are already created federated rule groups, then these - # rules groups will be skipped during evaluations. + # Enable rule groups querying against multiple tenants. The tenant IDs + # involved need to be in the rule group's 'source_tenants' field. If this flag + # is set to 'false' when there are already created federated rule groups, then + # these rules groups will be skipped during evaluations. # CLI flag: -ruler.tenant-federation.enabled [enabled: | default = false] ``` diff --git a/pkg/ruler/tenant_federation.go b/pkg/ruler/tenant_federation.go index f7ef97272e1..04fe1fc114d 100644 --- a/pkg/ruler/tenant_federation.go +++ b/pkg/ruler/tenant_federation.go @@ -25,7 +25,7 @@ type TenantFederationConfig struct { const TenantFederationFlag = "ruler.tenant-federation.enabled" func (cfg *TenantFederationConfig) RegisterFlags(f *flag.FlagSet) { - f.BoolVar(&cfg.Enabled, TenantFederationFlag, false, "Enable running rule groups against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations.") + f.BoolVar(&cfg.Enabled, TenantFederationFlag, false, "Enable rule groups querying against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations.") } type contextKey int From 993ddf87db15d8d7073862a455fe57e9f717a67c Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 27 Mar 2023 13:00:17 +0200 Subject: [PATCH 09/10] Update ruler tenant federation flag help text Signed-off-by: Dimitar Dimitrov --- cmd/mimir/config-descriptor.json | 2 +- cmd/mimir/help-all.txt.tmpl | 2 +- cmd/mimir/help.txt.tmpl | 2 +- .../mimir/references/configuration-parameters/index.md | 6 +++--- pkg/ruler/tenant_federation.go | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/mimir/config-descriptor.json b/cmd/mimir/config-descriptor.json index 4c054cb05d3..a5cca0e674e 100644 --- a/cmd/mimir/config-descriptor.json +++ b/cmd/mimir/config-descriptor.json @@ -10063,7 +10063,7 @@ "kind": "field", "name": "enabled", "required": false, - "desc": "Enable rule groups querying against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations.", + "desc": "Enable rule groups to query against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are federated rule groups that already exist, then these rules groups will be skipped during evaluations.", "fieldValue": null, "fieldDefaultValue": false, "fieldFlag": "ruler.tenant-federation.enabled", diff --git a/cmd/mimir/help-all.txt.tmpl b/cmd/mimir/help-all.txt.tmpl index 66a48598d10..ff073b6b1f0 100644 --- a/cmd/mimir/help-all.txt.tmpl +++ b/cmd/mimir/help-all.txt.tmpl @@ -2176,7 +2176,7 @@ Usage of ./cmd/mimir/mimir: -ruler.rule-path string Directory to store temporary rule files loaded by the Prometheus rule managers. This directory is not required to be persisted between restarts. (default "./data-ruler/") -ruler.tenant-federation.enabled - Enable rule groups querying against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations. + Enable rule groups to query against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are federated rule groups that already exist, then these rules groups will be skipped during evaluations. -ruler.tenant-shard-size int The tenant's shard size when sharding is used by ruler. Value of 0 disables shuffle sharding for the tenant, and tenant rules will be sharded across all ruler replicas. -runtime-config.file comma-separated-list-of-strings diff --git a/cmd/mimir/help.txt.tmpl b/cmd/mimir/help.txt.tmpl index 2806139929d..b1cc48c317d 100644 --- a/cmd/mimir/help.txt.tmpl +++ b/cmd/mimir/help.txt.tmpl @@ -594,7 +594,7 @@ Usage of ./cmd/mimir/mimir: -ruler.rule-path string Directory to store temporary rule files loaded by the Prometheus rule managers. This directory is not required to be persisted between restarts. (default "./data-ruler/") -ruler.tenant-federation.enabled - Enable rule groups querying against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations. + Enable rule groups to query against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are federated rule groups that already exist, then these rules groups will be skipped during evaluations. -ruler.tenant-shard-size int The tenant's shard size when sharding is used by ruler. Value of 0 disables shuffle sharding for the tenant, and tenant rules will be sharded across all ruler replicas. -runtime-config.file comma-separated-list-of-strings diff --git a/docs/sources/mimir/references/configuration-parameters/index.md b/docs/sources/mimir/references/configuration-parameters/index.md index 787ddb27fe7..b5b80a3b56f 100644 --- a/docs/sources/mimir/references/configuration-parameters/index.md +++ b/docs/sources/mimir/references/configuration-parameters/index.md @@ -1596,10 +1596,10 @@ query_frontend: [query_result_response_format: | default = "json"] tenant_federation: - # Enable rule groups querying against multiple tenants. The tenant IDs + # Enable rule groups to query against multiple tenants. The tenant IDs # involved need to be in the rule group's 'source_tenants' field. If this flag - # is set to 'false' when there are already created federated rule groups, then - # these rules groups will be skipped during evaluations. + # is set to 'false' when there are federated rule groups that already exist, + # then these rules groups will be skipped during evaluations. # CLI flag: -ruler.tenant-federation.enabled [enabled: | default = false] ``` diff --git a/pkg/ruler/tenant_federation.go b/pkg/ruler/tenant_federation.go index 04fe1fc114d..1bde5a584b8 100644 --- a/pkg/ruler/tenant_federation.go +++ b/pkg/ruler/tenant_federation.go @@ -25,7 +25,7 @@ type TenantFederationConfig struct { const TenantFederationFlag = "ruler.tenant-federation.enabled" func (cfg *TenantFederationConfig) RegisterFlags(f *flag.FlagSet) { - f.BoolVar(&cfg.Enabled, TenantFederationFlag, false, "Enable rule groups querying against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are already created federated rule groups, then these rules groups will be skipped during evaluations.") + f.BoolVar(&cfg.Enabled, TenantFederationFlag, false, "Enable rule groups to query against multiple tenants. The tenant IDs involved need to be in the rule group's 'source_tenants' field. If this flag is set to 'false' when there are federated rule groups that already exist, then these rules groups will be skipped during evaluations.") } type contextKey int From 4b7e0a8100190cd6f7c8797235edff1fe0277a67 Mon Sep 17 00:00:00 2001 From: Dimitar Dimitrov Date: Mon, 27 Mar 2023 13:00:43 +0200 Subject: [PATCH 10/10] Use config instead of configuration in err message Signed-off-by: Dimitar Dimitrov --- pkg/ruler/api_test.go | 8 ++++---- pkg/ruler/manager.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/ruler/api_test.go b/pkg/ruler/api_test.go index 1c46bf186ff..2c7acb06491 100644 --- a/pkg/ruler/api_test.go +++ b/pkg/ruler/api_test.go @@ -394,7 +394,7 @@ func TestRuler_Create(t *testing.T) { cfg: defaultCfg, input: "", status: 400, - err: errors.New("invalid rules config: rule group name must not be empty"), + err: errors.New("invalid rules configuration: rule group name must not be empty"), }, { name: "with no rule group name", @@ -406,7 +406,7 @@ rules: expr: up `, status: 400, - err: errors.New("invalid rules config: rule group name must not be empty"), + err: errors.New("invalid rules configuration: rule group name must not be empty"), }, { name: "with no rules", @@ -416,7 +416,7 @@ name: rg_name interval: 15s `, status: 400, - err: errors.New("invalid rules config: rule group 'rg_name' has no rules"), + err: errors.New("invalid rules configuration: rule group 'rg_name' has no rules"), }, { @@ -438,7 +438,7 @@ rules: labels: test: test `, - err: errors.New("invalid rules config: rule group 'test' is a federated rule group, but rules federation is disabled; please contact your service administrator to have it enabled"), + err: errors.New("invalid rules configuration: rule group 'test' is a federated rule group, but rules federation is disabled; please contact your service administrator to have it enabled"), }, { name: "with valid rules with enabled federation", diff --git a/pkg/ruler/manager.go b/pkg/ruler/manager.go index a97e9a65a22..c838a36e4c6 100644 --- a/pkg/ruler/manager.go +++ b/pkg/ruler/manager.go @@ -322,17 +322,17 @@ func (r *DefaultMultiTenantManager) ValidateRuleGroup(g rulefmt.RuleGroup) []err var errs []error if g.Name == "" { - errs = append(errs, errors.New("invalid rules config: rule group name must not be empty")) + errs = append(errs, errors.New("invalid rules configuration: rule group name must not be empty")) return errs } if len(g.Rules) == 0 { - errs = append(errs, fmt.Errorf("invalid rules config: rule group '%s' has no rules", g.Name)) + errs = append(errs, fmt.Errorf("invalid rules configuration: rule group '%s' has no rules", g.Name)) return errs } if !r.cfg.TenantFederation.Enabled && len(g.SourceTenants) > 0 { - errs = append(errs, fmt.Errorf("invalid rules config: rule group '%s' is a federated rule group, "+ + errs = append(errs, fmt.Errorf("invalid rules configuration: rule group '%s' is a federated rule group, "+ "but rules federation is disabled; please contact your service administrator to have it enabled", g.Name)) }