From d212d40b18db1985531924f426161b6e600144c6 Mon Sep 17 00:00:00 2001 From: Alex Dadgar Date: Thu, 2 Feb 2017 11:12:07 -0800 Subject: [PATCH] Fix Consul Config Merging/Copying This PR fixes config merging/copying code. Fixes https://github.com/hashicorp/nomad/issues/2264 --- command/agent/config.go | 3 +- command/agent/config_test.go | 58 ++++++++++++++++++---------------- nomad/structs/config/consul.go | 38 ++++++++++++++++++---- 3 files changed, 62 insertions(+), 37 deletions(-) diff --git a/command/agent/config.go b/command/agent/config.go index a57237de53f..c7e107525de 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -662,8 +662,7 @@ func (c *Config) Merge(b *Config) *Config { // Apply the Consul Configuration if result.Consul == nil && b.Consul != nil { - consulConfig := *b.Consul - result.Consul = &consulConfig + result.Consul = b.Consul.Copy() } else if b.Consul != nil { result.Consul = result.Consul.Merge(b.Consul) } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 29da3ad383a..6697139ff61 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -118,20 +118,21 @@ func TestConfig_Merge(t *testing.T) { TLSServerName: "1", }, Consul: &config.ConsulConfig{ - ServerServiceName: "1", - ClientServiceName: "1", - AutoAdvertise: &falseValue, - Addr: "1", - Timeout: 1 * time.Second, - Token: "1", - Auth: "1", - EnableSSL: &falseValue, - VerifySSL: &falseValue, - CAFile: "1", - CertFile: "1", - KeyFile: "1", - ServerAutoJoin: &falseValue, - ClientAutoJoin: &falseValue, + ServerServiceName: "1", + ClientServiceName: "1", + AutoAdvertise: &falseValue, + Addr: "1", + Timeout: 1 * time.Second, + Token: "1", + Auth: "1", + EnableSSL: &falseValue, + VerifySSL: &falseValue, + CAFile: "1", + CertFile: "1", + KeyFile: "1", + ServerAutoJoin: &falseValue, + ClientAutoJoin: &falseValue, + ChecksUseAdvertise: &falseValue, }, } @@ -249,20 +250,21 @@ func TestConfig_Merge(t *testing.T) { TLSServerName: "2", }, Consul: &config.ConsulConfig{ - ServerServiceName: "2", - ClientServiceName: "2", - AutoAdvertise: &trueValue, - Addr: "2", - Timeout: 2 * time.Second, - Token: "2", - Auth: "2", - EnableSSL: &trueValue, - VerifySSL: &trueValue, - CAFile: "2", - CertFile: "2", - KeyFile: "2", - ServerAutoJoin: &trueValue, - ClientAutoJoin: &trueValue, + ServerServiceName: "2", + ClientServiceName: "2", + AutoAdvertise: &trueValue, + Addr: "2", + Timeout: 2 * time.Second, + Token: "2", + Auth: "2", + EnableSSL: &trueValue, + VerifySSL: &trueValue, + CAFile: "2", + CertFile: "2", + KeyFile: "2", + ServerAutoJoin: &trueValue, + ClientAutoJoin: &trueValue, + ChecksUseAdvertise: &trueValue, }, } diff --git a/nomad/structs/config/consul.go b/nomad/structs/config/consul.go index 19ee82c71c8..2669c645a73 100644 --- a/nomad/structs/config/consul.go +++ b/nomad/structs/config/consul.go @@ -93,7 +93,7 @@ func DefaultConsulConfig() *ConsulConfig { // Merge merges two Consul Configurations together. func (a *ConsulConfig) Merge(b *ConsulConfig) *ConsulConfig { - result := *a + result := a.Copy() if b.ServerServiceName != "" { result.ServerServiceName = b.ServerServiceName @@ -102,7 +102,7 @@ func (a *ConsulConfig) Merge(b *ConsulConfig) *ConsulConfig { result.ClientServiceName = b.ClientServiceName } if b.AutoAdvertise != nil { - result.AutoAdvertise = b.AutoAdvertise + result.AutoAdvertise = helper.BoolToPtr(*b.AutoAdvertise) } if b.Addr != "" { result.Addr = b.Addr @@ -117,10 +117,10 @@ func (a *ConsulConfig) Merge(b *ConsulConfig) *ConsulConfig { result.Auth = b.Auth } if b.EnableSSL != nil { - result.EnableSSL = b.EnableSSL + result.EnableSSL = helper.BoolToPtr(*b.EnableSSL) } if b.VerifySSL != nil { - result.VerifySSL = b.VerifySSL + result.VerifySSL = helper.BoolToPtr(*b.VerifySSL) } if b.CAFile != "" { result.CAFile = b.CAFile @@ -132,12 +132,15 @@ func (a *ConsulConfig) Merge(b *ConsulConfig) *ConsulConfig { result.KeyFile = b.KeyFile } if b.ServerAutoJoin != nil { - result.ServerAutoJoin = b.ServerAutoJoin + result.ServerAutoJoin = helper.BoolToPtr(*b.ServerAutoJoin) } if b.ClientAutoJoin != nil { - result.ClientAutoJoin = b.ServerAutoJoin + result.ClientAutoJoin = helper.BoolToPtr(*b.ClientAutoJoin) } - return &result + if b.ChecksUseAdvertise != nil { + result.ChecksUseAdvertise = helper.BoolToPtr(*b.ChecksUseAdvertise) + } + return result } // ApiConfig() returns a usable Consul config that can be passed directly to @@ -200,5 +203,26 @@ func (c *ConsulConfig) Copy() *ConsulConfig { nc := new(ConsulConfig) *nc = *c + + // Copy the bools + if nc.AutoAdvertise != nil { + nc.AutoAdvertise = helper.BoolToPtr(*nc.AutoAdvertise) + } + if nc.ChecksUseAdvertise != nil { + nc.ChecksUseAdvertise = helper.BoolToPtr(*nc.ChecksUseAdvertise) + } + if nc.EnableSSL != nil { + nc.EnableSSL = helper.BoolToPtr(*nc.EnableSSL) + } + if nc.VerifySSL != nil { + nc.VerifySSL = helper.BoolToPtr(*nc.VerifySSL) + } + if nc.ServerAutoJoin != nil { + nc.ServerAutoJoin = helper.BoolToPtr(*nc.ServerAutoJoin) + } + if nc.ClientAutoJoin != nil { + nc.ClientAutoJoin = helper.BoolToPtr(*nc.ClientAutoJoin) + } + return nc }