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

Bug: unexpected behavior around defaults when unmarshalling config #4419

Closed
trevorwhitney opened this issue Oct 6, 2021 · 3 comments · Fixed by #4433
Closed

Bug: unexpected behavior around defaults when unmarshalling config #4419

trevorwhitney opened this issue Oct 6, 2021 · 3 comments · Fixed by #4433
Assignees

Comments

@trevorwhitney
Copy link
Collaborator

trevorwhitney commented Oct 6, 2021

Describe the bug
When parsing config files, zero values resulting from the yaml unmarshalling are taking precedence over defaults.

To Reproduce
Steps to reproduce the behavior:

  1. Start Loki with a config file without a server stanza
  2. Expect all default values for the server config to be used
  3. There is a failing test here

Expected behavior
When a block is not provided in yaml, I would expect the defaults to be used. It looks like the way we refactored parsing the default configs when introducing the dynamic config section for the common config work, that we broke this behavior, and that zero values from the yaml are being preferred over defaults.

@trevorwhitney trevorwhitney self-assigned this Oct 6, 2021
@trevorwhitney
Copy link
Collaborator Author

I'm looking into a fix now, but if anyone has expertise in this area I'm open to idea/suggestions.

@trevorwhitney
Copy link
Collaborator Author

This turns out to be a red herring. This behavior only happens when a completely empty file is used as the config file, ie:

---

I was using that as test input which was causing the oddities, but as this is unexpected use case I don't think it needs special handling, nor do I think loki will even start if given a config like this.

@trevorwhitney
Copy link
Collaborator Author

Tests proving correctness of current behavior in #4433, not a real bug :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant