Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds config validation, similar to cortex #1939

Merged
merged 1 commit into from
Apr 15, 2020

Conversation

owen-d
Copy link
Member

@owen-d owen-d commented Apr 14, 2020

Background

After the 1.0 vendoring, we're exposed to some tricky configuration errors because we instantiate our configurations differently than cortex but use cortex's underlying configs. In this case, we loaded the schema's PeriodConfig blocks via yaml, causing subsequent calls to Load to no-op (this is intended to be an idempotent method).

What

This PR introduces a Config.Validate function and calls it in the loki execution path - it mainly calls cortex validation code similar to cortex: https://github.com/cortexproject/cortex/blob/master/cmd/cortex/main.go#L83-L89

Cortex Load: https://github.com/cortexproject/cortex/blob/master/pkg/chunk/schema_config.go#L228-L239

I've tested this fix on one of our clusters and confirmed we're no longer seeing panics due to 0 (default go value) shard factors in our configs.

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@owen-d owen-d merged commit e9789f7 into grafana:master Apr 15, 2020
@owen-d owen-d deleted the fix/validate-config branch April 15, 2020 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants