[CharlieTLe]

Follows review on #7918. That PR documented that 1.22 breaks 1.21 configs that set the query range or HA failover timeout settings in their old location. @friedrichg asked to keep the old stanzas working instead, since these are long-standing settings. This PR does that. It targets master, and I'll backport it to release-1.22 before v1.22.0.

What breaks on 1.22 today

• Make query ingester within a per tenant configuration #7160 / making query ingesters config per tenant #7323 moved query_ingesters_within, query_store_after and shuffle_sharding_ingesters_lookback_period from querier to limits, and renamed their flags from -querier.* to -limits.*. The old YAML fails with field query_store_after not found in type querier.Config, and the old flags fail with flag provided but not defined.
• feat(ha): Add per-tenant configurable failover timeout #7481 moved the ha_tracker_failover_timeout YAML field from distributor.ha_tracker to limits. The flag name did not change. The old YAML fails with field ha_tracker_failover_timeout not found in type ha.HATrackerConfig.

Change

• The old YAML fields and the old -querier.* flags come back as hidden, deprecated settings (doc:"hidden"), scheduled for removal in v1.24.0. distributor.ha_tracker.ha_tracker_failover_timeout is YAML-only, because -distributor.ha-tracker.failover-timeout already sets the limit.
• Config.Validate copies an old value that is set into its limit. This happens before the limits are validated and before they become the runtime-config defaults (SetDefaultLimitsForYAMLUnmarshalling), so tenant overrides inherit it, and the old values go through the same ValidateQueryLimits checks as the new ones.
• Precedence follows 1.21, where a flag on the command line overrides the YAML file. The old and new names are treated as two spellings of one setting:
  • A flag on the command line wins over the YAML file, whichever name each uses. -distributor.ha-tracker.failover-timeout=30s beats a deprecated distributor.ha_tracker.ha_tracker_failover_timeout: 45s, even though 30s is the default.
  • An old flag on the command line wins over a new key in the YAML file.
  • If the old and the new name are both set in the same place, both in the YAML file or both on the command line, they must agree, otherwise startup fails with limits.query_store_after (6h0m0s) and the deprecated querier.query_store_after (12h0m0s) are both set: remove querier.query_store_after, or the same with the flag names.
  • To know where each value came from, LoadConfig records the YAML keys that are set (SetYAMLKeys, next to the existing SetYAMLRootNodeStates), and main records the flags passed on the command line (SetCommandLineFlags). A Config built in code has neither, and counts a setting as set when it is not at its default value.
• The deprecated fields are omitempty, so -print.config and /config show them only when they are set.
• Each old setting that is used logs running with DEPRECATED config querier.query_store_after, use limits.query_store_after instead, it will be removed in v1.24.0 and increments deprecated_flags_inuse_total, as Formally deprecate -blocks-storage.tsdb.max-exemplars #7793 does for -blocks-storage.tsdb.max-exemplars. This happens in cortex.New, not Validate: main calls Validate before util_log.InitLogger, while util_log.Logger is still a no-op logger, so a warning logged there is dropped.
• The -limits.query-ingesters-within help text said "less than or equal to close-idle-tsdb-timeout", while making query ingesters config per tenant #7323 rejects equal. It now says "less than". config-file-reference.md and the JSON schema are regenerated; that string is their only change.
• CHANGELOG: the Make query ingester within a per tenant configuration #7160/making query ingesters config per tenant #7323 and feat(ha): Add per-tenant configurable failover timeout #7481 entries say the old names are deprecated rather than removed, and a new entry describes the query_ingesters_within < close_idle_tsdb_timeout startup check, which was approved on Backport #7919 to release-1.22 #7918 as a correctness fix.

Testing

• TestConfigValidation_DeprecatedLimits covers the old YAML, the old flags, the HA field, old and new equal, old and new different (both kinds), and an old pair that fails ValidateQueryLimits.
• go test ./pkg/cortex/ ./pkg/ha/ ./pkg/querier/ and go vet pass. Regenerated docs and schema match.
• TestFlagParsing runs main() with the HA case from review: deprecated YAML field 45s plus -distributor.ha-tracker.failover-timeout=30s now gives 30s. Reverting to the value-based rule fails it, along with the three command-line precedence unit tests.
• Compared -print.config against a v1.21.0 build, with an old YAML field 45s and the flag unset, =30s or =1m: 45s, 30s, 1m on both. With a YAML querier.query_store_after: 12h and the flag unset, -querier.query-store-after=0 or -limits.query-store-after=0: 12h, 0, 0, matching 1.21.
• Built ./cmd/cortex and started it with the three 1.21-style configs that fail on v1.22.0-rc.3: each now passes config validation and logs the deprecation warning. The equal close_idle_tsdb_timeout config is still rejected.
• golangci-lint v2.13.1, the version in build-image/Dockerfile, built with Go 1.27: 0 issues.

[SungJin1212]

Suggested change

* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). The settings move from the `querier` block to `limits`, and their flags from `-querier.*` to `-limits.*`. The old YAML fields and flags still work as the defaults for the new limits, but are deprecated and will be removed in v1.24.0: using them logs a warning and increments `deprecated_flags_inuse_total`. Setting an old and a new name to different values fails at startup. #7160 #7323 #7919

* [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). The settings move from the `querier` block to `limits`, and their flags from `-querier.*` to `-limits.*`. The old YAML fields and flags still work as the defaults for the new limits, but are deprecated and will be removed in v1.24.0: using them logs a warning and increments `deprecated_flags_inuse_total`. Setting the old field and a non-default limit to different values fails at startup. #7160 #7323 #7919

Setting an old name and a non-default new limit to different values fails at startup. Isn't it?

[CharlieTLe]

Yes, good catch: as written it only failed when the new limit was at a non-default value. After Friedrich's review the rule changed (7245514), so I reworded the entry instead of taking the suggestion as-is: a command-line flag wins over the YAML file whichever name each uses, and the old and new name have to agree only when both are set in the same place (both in YAML, or both on the command line). The #7481 entry says the same.

[friedrichg]

Not ready yet

[friedrichg]

According to docs cli flags should take precedence over configuration changes. https://cortexmetrics.io/docs/configuration/configuration-file/#:~:text=Cortex%20can%20be,YAML%20config%20file.

I tested this PR locally and I found that if I test:

distributor:
  ha_tracker:
    ha_tracker_failover_timeout: 45s

with -distributor.ha-tracker.failover-timeout=30s

1.21 uses 30s
while this PR makes it 45s.

[CharlieTLe]

Thanks for testing it, you're right. The fallback compared values, so it couldn't tell an explicit flag at its default value (30s) from no flag at all. Fixed in 7245514: LoadConfig now records which YAML keys are set and main records which flags were passed, and a command-line flag wins over the YAML file whichever name each uses, as in 1.21. Your case is a TestFlagParsing case now. I also compared -print.config against a v1.21.0 build: with the old field at 45s and the flag unset, =30s or =1m, both give 45s / 30s / 1m. The querier settings match 1.21 too (details in the PR description).

[friedrichg]

Thanks for addressing the comments