-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Loki: Implement common net interface/instance addr #4950
Conversation
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
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
// - "instance-addr", the address advertised to be used by other components. | ||
// - "instance-interface-names", a list of net interfaces used when looking for addresses. | ||
func applyInstanceConfigs(r, defaults *ConfigWrapper) { | ||
if !reflect.DeepEqual(r.Common.InstanceAddr, defaults.Common.InstanceAddr) { |
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.
If we only ever check if r.Common.InstanceAddr != defaults.Common.InstanceAddr
, is there a reason common.instance-addr
should default to 127.0.0.1
instead of ""
?
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.
Hmm I'd say there's no reason then. Let me change it to "" instead.
ec6838d
to
4796880
Compare
Enabling auto merge that should merge once conflict resolve. |
- Implement two new common configurations: instance_interface_names, which is a list of net interfaces used to discover addresses and instance_addr, used to advertise a component address.
4796880
to
6be2021
Compare
What this PR does / why we need it:
instance_interface_names
, a common net interface used by components internally to look for addresses. The list order matters, as components internally iterate over the list until they find a valid address. If the common ring section specifies aninstance_interface_names
, this configuration is only applied to the frontend but not to other components.instance_addr
, a common instance address used by components to advertise their address. Again, if the common ring section specifies aninstance_addr
, this configuration is only applied to the frontend (since it is not a ring) but not to other components.Which issue(s) this PR fixes:
Fixes #4897
Special notes for your reviewer:
Checklist
CHANGELOG.md
about the changes.