Do not modify config after creation II

Move code for finding the advertise address via a
template into consulConfig() so that the config
object is not modified after creation.
This commit is contained in:
Frank Schroeder 2017-05-03 23:47:25 +02:00 committed by Frank Schröder
parent 6b96c9ff91
commit c772cecaab
4 changed files with 84 additions and 75 deletions

View file

@ -133,8 +133,7 @@ type Agent struct {
// Create is used to create a new Agent. Returns
// the agent or potentially an error.
func Create(config *Config, logOutput io.Writer, logWriter *logger.LogWriter,
reloadCh chan chan error) (*Agent, error) {
func Create(config *Config, logOutput io.Writer, logWriter *logger.LogWriter, reloadCh chan chan error) (*Agent, error) {
// Ensure we have a log sink
if logOutput == nil {
logOutput = os.Stderr
@ -148,54 +147,6 @@ func Create(config *Config, logOutput io.Writer, logWriter *logger.LogWriter,
return nil, fmt.Errorf("Must configure a DataDir")
}
// Try to get an advertise address
if config.AdvertiseAddr != "" {
ipStr, err := parseSingleIPTemplate(config.AdvertiseAddr)
if err != nil {
return nil, fmt.Errorf("Advertise address resolution failed: %v", err)
}
config.AdvertiseAddr = ipStr
if ip := net.ParseIP(config.AdvertiseAddr); ip == nil {
return nil, fmt.Errorf("Failed to parse advertise address: %v", config.AdvertiseAddr)
}
} else if config.BindAddr != "0.0.0.0" && config.BindAddr != "" && config.BindAddr != "[::]" {
config.AdvertiseAddr = config.BindAddr
} else {
var err error
var ip net.IP
if config.BindAddr == "[::]" {
ip, err = consul.GetPublicIPv6()
} else {
ip, err = consul.GetPrivateIP()
}
if err != nil {
return nil, fmt.Errorf("Failed to get advertise address: %v", err)
}
config.AdvertiseAddr = ip.String()
}
// Try to get an advertise address for the wan
if config.AdvertiseAddrWan != "" {
ipStr, err := parseSingleIPTemplate(config.AdvertiseAddrWan)
if err != nil {
return nil, fmt.Errorf("Advertise WAN address resolution failed: %v", err)
}
config.AdvertiseAddrWan = ipStr
if ip := net.ParseIP(config.AdvertiseAddrWan); ip == nil {
return nil, fmt.Errorf("Failed to parse advertise address for wan: %v", config.AdvertiseAddrWan)
}
} else {
config.AdvertiseAddrWan = config.AdvertiseAddr
}
// Create the default set of tagged addresses.
config.TaggedAddresses = map[string]string{
"lan": config.AdvertiseAddr,
"wan": config.AdvertiseAddrWan,
}
agent := &Agent{
config: config,
logger: log.New(logOutput, "", log.LstdFlags),
@ -288,7 +239,7 @@ func Create(config *Config, logOutput io.Writer, logWriter *logger.LogWriter,
}
// consulConfig is used to return a consul configuration
func (a *Agent) consulConfig() *consul.Config {
func (a *Agent) consulConfig() (*consul.Config, error) {
// Start with the provided config or default config
base := consul.DefaultConfig()
if a.config.ConsulConfig != nil {
@ -342,6 +293,52 @@ func (a *Agent) consulConfig() *consul.Config {
if a.config.SerfWanBindAddr != "" {
base.SerfWANConfig.MemberlistConfig.BindAddr = a.config.SerfWanBindAddr
}
// Try to get an advertise address
switch {
case a.config.AdvertiseAddr != "":
ipStr, err := parseSingleIPTemplate(a.config.AdvertiseAddr)
if err != nil {
return nil, fmt.Errorf("Advertise address resolution failed: %v", err)
}
if net.ParseIP(ipStr) == nil {
return nil, fmt.Errorf("Failed to parse advertise address: %v", ipStr)
}
a.config.AdvertiseAddr = ipStr
case a.config.BindAddr != "0.0.0.0" && a.config.BindAddr != "" && a.config.BindAddr != "[::]":
a.config.AdvertiseAddr = a.config.BindAddr
default:
ip, err := consul.GetPrivateIP()
if a.config.BindAddr == "[::]" {
ip, err = consul.GetPublicIPv6()
}
if err != nil {
return nil, fmt.Errorf("Failed to get advertise address: %v", err)
}
a.config.AdvertiseAddr = ip.String()
}
// Try to get an advertise address for the wan
if a.config.AdvertiseAddrWan != "" {
ipStr, err := parseSingleIPTemplate(a.config.AdvertiseAddrWan)
if err != nil {
return nil, fmt.Errorf("Advertise WAN address resolution failed: %v", err)
}
if net.ParseIP(ipStr) == nil {
return nil, fmt.Errorf("Failed to parse advertise address for WAN: %v", ipStr)
}
a.config.AdvertiseAddrWan = ipStr
} else {
a.config.AdvertiseAddrWan = a.config.AdvertiseAddr
}
// Create the default set of tagged addresses.
a.config.TaggedAddresses = map[string]string{
"lan": a.config.AdvertiseAddr,
"wan": a.config.AdvertiseAddrWan,
}
if a.config.AdvertiseAddr != "" {
base.SerfLANConfig.MemberlistConfig.AdvertiseAddr = a.config.AdvertiseAddr
base.SerfWANConfig.MemberlistConfig.AdvertiseAddr = a.config.AdvertiseAddr
@ -476,7 +473,7 @@ func (a *Agent) consulConfig() *consul.Config {
// Setup the loggers
base.LogOutput = a.logOutput
return base
return base, nil
}
// parseSingleIPTemplate is used as a helper function to parse out a single IP
@ -588,12 +585,13 @@ func (a *Agent) resolveTmplAddrs() error {
// setupServer is used to initialize the Consul server
func (a *Agent) setupServer() error {
config := a.consulConfig()
config, err := a.consulConfig()
if err != nil {
return err
}
if err := a.setupKeyrings(config); err != nil {
return fmt.Errorf("Failed to configure keyring: %v", err)
}
server, err := consul.NewServer(config)
if err != nil {
return fmt.Errorf("Failed to start Consul server: %v", err)
@ -604,12 +602,13 @@ func (a *Agent) setupServer() error {
// setupClient is used to initialize the Consul client
func (a *Agent) setupClient() error {
config := a.consulConfig()
config, err := a.consulConfig()
if err != nil {
return err
}
if err := a.setupKeyrings(config); err != nil {
return fmt.Errorf("Failed to configure keyring: %v", err)
}
client, err := consul.NewClient(config)
if err != nil {
return fmt.Errorf("Failed to start Consul client: %v", err)

View file

@ -337,6 +337,7 @@ func TestAgent_Reload(t *testing.T) {
args := []string{
"-server",
"-advertise", "127.0.0.1",
"-data-dir", tmpDir,
"-http-port", fmt.Sprintf("%d", conf.Ports.HTTP),
"-config-file", tmpFile.Name(),

View file

@ -195,12 +195,12 @@ func TestAgent_CheckSerfBindAddrsSettings(t *testing.T) {
defer os.RemoveAll(dir)
defer agent.Shutdown()
serfWanBind := agent.consulConfig().SerfWANConfig.MemberlistConfig.BindAddr
serfWanBind := consulConfig(agent).SerfWANConfig.MemberlistConfig.BindAddr
if serfWanBind != ip {
t.Fatalf("SerfWanBindAddr is should be a non-loopback IP not %s", serfWanBind)
}
serfLanBind := agent.consulConfig().SerfLANConfig.MemberlistConfig.BindAddr
serfLanBind := consulConfig(agent).SerfLANConfig.MemberlistConfig.BindAddr
if serfLanBind != ip {
t.Fatalf("SerfLanBindAddr is should be a non-loopback IP not %s", serfWanBind)
}
@ -214,23 +214,23 @@ func TestAgent_CheckAdvertiseAddrsSettings(t *testing.T) {
defer os.RemoveAll(dir)
defer agent.Shutdown()
serfLanAddr := agent.consulConfig().SerfLANConfig.MemberlistConfig.AdvertiseAddr
serfLanAddr := consulConfig(agent).SerfLANConfig.MemberlistConfig.AdvertiseAddr
if serfLanAddr != "127.0.0.42" {
t.Fatalf("SerfLan is not properly set to '127.0.0.42': %s", serfLanAddr)
}
serfLanPort := agent.consulConfig().SerfLANConfig.MemberlistConfig.AdvertisePort
serfLanPort := consulConfig(agent).SerfLANConfig.MemberlistConfig.AdvertisePort
if serfLanPort != 1233 {
t.Fatalf("SerfLan is not properly set to '1233': %d", serfLanPort)
}
serfWanAddr := agent.consulConfig().SerfWANConfig.MemberlistConfig.AdvertiseAddr
serfWanAddr := consulConfig(agent).SerfWANConfig.MemberlistConfig.AdvertiseAddr
if serfWanAddr != "127.0.0.43" {
t.Fatalf("SerfWan is not properly set to '127.0.0.43': %s", serfWanAddr)
}
serfWanPort := agent.consulConfig().SerfWANConfig.MemberlistConfig.AdvertisePort
serfWanPort := consulConfig(agent).SerfWANConfig.MemberlistConfig.AdvertisePort
if serfWanPort != 1234 {
t.Fatalf("SerfWan is not properly set to '1234': %d", serfWanPort)
}
rpc := agent.consulConfig().RPCAdvertise
rpc := consulConfig(agent).RPCAdvertise
if rpc != c.AdvertiseAddrs.RPC {
t.Fatalf("RPC is not properly set to %v: %s", c.AdvertiseAddrs.RPC, rpc)
}
@ -253,7 +253,7 @@ func TestAgent_CheckPerformanceSettings(t *testing.T) {
defer agent.Shutdown()
raftMult := time.Duration(consul.DefaultRaftMultiplier)
r := agent.consulConfig().RaftConfig
r := consulConfig(agent).RaftConfig
def := raft.DefaultConfig()
if r.HeartbeatTimeout != raftMult*def.HeartbeatTimeout ||
r.ElectionTimeout != raftMult*def.ElectionTimeout ||
@ -271,7 +271,7 @@ func TestAgent_CheckPerformanceSettings(t *testing.T) {
defer agent.Shutdown()
const raftMult time.Duration = 99
r := agent.consulConfig().RaftConfig
r := consulConfig(agent).RaftConfig
def := raft.DefaultConfig()
if r.HeartbeatTimeout != raftMult*def.HeartbeatTimeout ||
r.ElectionTimeout != raftMult*def.ElectionTimeout ||
@ -288,12 +288,12 @@ func TestAgent_ReconnectConfigSettings(t *testing.T) {
defer os.RemoveAll(dir)
defer agent.Shutdown()
lan := agent.consulConfig().SerfLANConfig.ReconnectTimeout
lan := consulConfig(agent).SerfLANConfig.ReconnectTimeout
if lan != 3*24*time.Hour {
t.Fatalf("bad: %s", lan.String())
}
wan := agent.consulConfig().SerfWANConfig.ReconnectTimeout
wan := consulConfig(agent).SerfWANConfig.ReconnectTimeout
if wan != 3*24*time.Hour {
t.Fatalf("bad: %s", wan.String())
}
@ -307,12 +307,12 @@ func TestAgent_ReconnectConfigSettings(t *testing.T) {
defer os.RemoveAll(dir)
defer agent.Shutdown()
lan := agent.consulConfig().SerfLANConfig.ReconnectTimeout
lan := consulConfig(agent).SerfLANConfig.ReconnectTimeout
if lan != 24*time.Hour {
t.Fatalf("bad: %s", lan.String())
}
wan := agent.consulConfig().SerfWANConfig.ReconnectTimeout
wan := consulConfig(agent).SerfWANConfig.ReconnectTimeout
if wan != 36*time.Hour {
t.Fatalf("bad: %s", wan.String())
}
@ -327,7 +327,7 @@ func TestAgent_setupNodeID(t *testing.T) {
defer agent.Shutdown()
// The auto-assigned ID should be valid.
id := agent.consulConfig().NodeID
id := consulConfig(agent).NodeID
if _, err := uuid.ParseUUID(string(id)); err != nil {
t.Fatalf("err: %v", err)
}
@ -337,7 +337,7 @@ func TestAgent_setupNodeID(t *testing.T) {
if err := agent.setupNodeID(c); err != nil {
t.Fatalf("err: %v", err)
}
if newID := agent.consulConfig().NodeID; id != newID {
if newID := consulConfig(agent).NodeID; id != newID {
t.Fatalf("bad: %q vs %q", id, newID)
}
@ -357,7 +357,7 @@ func TestAgent_setupNodeID(t *testing.T) {
if err := agent.setupNodeID(c); err != nil {
t.Fatalf("err: %v", err)
}
if id := agent.consulConfig().NodeID; string(id) != newID {
if id := consulConfig(agent).NodeID; string(id) != newID {
t.Fatalf("bad: %q vs. %q", id, newID)
}
@ -380,7 +380,7 @@ func TestAgent_setupNodeID(t *testing.T) {
if err := agent.setupNodeID(c); err != nil {
t.Fatalf("err: %v", err)
}
if id := agent.consulConfig().NodeID; string(id) != "adf4238a-882b-9ddc-4a9d-5b6758e4159e" {
if id := consulConfig(agent).NodeID; string(id) != "adf4238a-882b-9ddc-4a9d-5b6758e4159e" {
t.Fatalf("bad: %q vs. %q", id, newID)
}
}
@ -1985,3 +1985,11 @@ func TestAgent_GetCoordinate(t *testing.T) {
check(true)
check(false)
}
func consulConfig(a *Agent) *consul.Config {
c, err := a.consulConfig()
if err != nil {
panic(err)
}
return c
}

View file

@ -47,6 +47,7 @@ func testServerConfig(t *testing.T, NodeName string) (string, *Config) {
IP: []byte{127, 0, 0, 1},
Port: getPort(),
}
config.RPCAdvertise = config.RPCAddr
nodeID, err := uuid.GenerateUUID()
if err != nil {
t.Fatal(err)