-
Notifications
You must be signed in to change notification settings - Fork 621
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
fix: enable-ipv6
option is ignored
#3731
Conversation
6238946
to
cc73683
Compare
ring: | ||
# The key-value store used to share the hash ring across multiple instances. | ||
kvstore: | ||
# Backend storage to use for the ring. Supported values are: consul, etcd, | ||
# inmemory, memberlist, multi. | ||
# CLI flag: -distributor.ring.store | ||
[store: <string> | default = "memberlist"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, the distributor's ring configuration was omitted from the documentation, even though it is still available in the CLI help message. I've added it to the config spec doc.
[address: <string> | default = ""] | ||
[instance_addr: <string> | default = ""] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I bet many people have been caught by this unfortunate inconsistency: we use instance_addr
everywhere else. In the CLI, it's actually instance-addr
, what makes things even more confusing.
To prevent any future confusion, I'm changing address to instance_addr
while maintaining backward compatibility with the previous version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Our configuration includes an option to enable IPv6 interface discovery when a component's advertised address is not explicitly defined. However, for unknown reasons, this option is not currently utilized, which can be quite confusing.
This PR addresses the issue by ensuring the relevant configuration parameters are properly applied. It is a no-op in most cases but may impact deployments intended to use IPv6.