Fix DNS SRV port not used in cluster.join-addresses #766
Conversation
|
Thanks for opening this! @tpaschalis @wildum can one of you take a look at this one? You know more about this code than I do. |
| @@ -107,6 +107,7 @@ If `--cluster.advertise-interfaces` isn't explicitly set, {{< param "PRODUCT_NAM | |||
| Since Windows doesn't use the interface names `eth0` or `en0`, Windows users must explicitly pass at least one valid network interface for `--cluster.advertise-interfaces` or a value for `--cluster.advertise-address`. | |||
|
|
|||
| The comma-separated list of addresses provided in `--cluster.join-addresses` can either be IP addresses with an optional port, or DNS records to lookup. | |||
| If the address is a DNS record, it will be looked up as an A/AAAA record if the port is explicitly provided, otherwise it will be looked up as a SRV record and the port in the SRV record will be used. | |||
There was a problem hiding this comment.
I'm not sure if the intention was different here, but we still always perform a LookupSRV.
Also, depending on how addresses are discovered (one example is a Headless kubernetes service, but it can be any number of ways), the port is not guaranteed to be present or set to the same port that nodes use as their listen addresses. So that's why I feel making use of the default port on the local node's listen address is a sensible default here.
|
Hey @devsunb, how's it going? First off, apologies for the belated response here, it shouldn't have taken so long for me to get back to you here. Could you explain more about the intentions of this PR? As I said in the previous comment, I feel this would depend on DNS SRV records that are explicitly defined for the usecase of Alloy clustering and may break usecases where a more generic record is used for discovery. |
clayton-cornell
added
the
type/docs
PR Description
Fixed an issue where DNS SRV record's port is not used in
cluster.join-addresses.Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist