Expose AdvertiseAddr from the clustering configuration #1063
Conversation
| swimAdvertiseAddr, err = net.ResolveTCPAddr("tcp", swimAdvertiseAddrStr) | ||
| if err != nil { | ||
| log.Fatal(4, "CLU Config: swim-advertise-addr is not a valid TCP address: %s", err.Error()) | ||
| } |
There was a problem hiding this comment.
technically, this address only needs to resolve on other peers i suppose, but i agree it makes sense to just check it on all peers.
There was a problem hiding this comment.
The memberlist implementation seems a bit fragile. Though the docs just specify that AdvertiseAddr is a "string", it actually has to be an IP address and MT will fail to start if the IP cant be parsed by net.ParseIP().
I would have thought it would make sense to have peers use a "hostname" to reach a node, but memberlist does not support that.
|
Hi @baharclerode thanks for your contribution, this looks pretty good to me. will need some tweaks to other config files but I can make those. |
cluster/config.go
Outdated
| @@ -57,6 +59,7 @@ func ConfigSetup() { | |||
| swimCfg := flag.NewFlagSet("swim", flag.ExitOnError) | |||
| swimCfg.StringVar(&swimUseConfig, "use-config", "manual", "config setting to use. If set to anything but manual, will override all other swim settings. Use manual|default-lan|default-local|default-wan. see https://godoc.org/github.com/hashicorp/memberlist#Config . Note all our swim settings correspond to default-lan") | |||
| swimCfg.StringVar(&swimBindAddrStr, "bind-addr", "0.0.0.0:7946", "binding TCP Address for UDP and TCP gossip") | |||
| swimCfg.StringVar(&swimAdvertiseAddrStr, "advertise-addr", "", "advertised TCP Address for UDP and TCP gossip NAT traversal") | |||
There was a problem hiding this comment.
The help message here should match the text used in the config file.
|
@woodsaj i don't want casual contributors to be bogged down by our specifics re synchronizing config files etc, I'll take care of that. does this look good to you functionality/business logic wise? |
|
LGTM |
|
thanks for contributing, @baharclerode ! |
This exposes the raft advertise IP/port settings in the metrictank configuration file so that metrictank instances that are behind a NAT (such as when ports are exposed from a docker container). If a custom advertisement address is not provided, it falls back to using the bind IP/port (previous behavior).