config updates/copies should be on agent

command/agent/agent.go

@@ -43,7 +43,9 @@ const (
 // scheduled to, and are responsible for interfacing with
 // servers to run allocations.
 type Agent struct {
-	config    *Config
+	config     *Config
+	configLock sync.Mutex
+
 	logger    *log.Logger
 	logOutput io.Writer
 
@@ -727,6 +729,9 @@ func (a *Agent) Stats() map[string]map[string]string {
 // Reload handles configuration changes for the agent. Provides a method that
 // is easier to unit test, as this action is invoked via SIGHUP.
 func (a *Agent) Reload(newConfig *Config) error {
+	a.configLock.Lock()
+	defer a.configLock.Unlock()
+
 	if newConfig.TLSConfig != nil {
 
 		// If the agent is already running with TLS enabled, we need to only reload
@@ -741,6 +746,15 @@ func (a *Agent) Reload(newConfig *Config) error {
 	return nil
 }
 
+// GetConfigCopy creates a replica of the agent's config, excluding locks
+func (a *Agent) GetConfigCopy() *Config {
+	a.configLock.Lock()
+	defer a.configLock.Unlock()
+
+	new := *a.config
+	return &new
+}
+
 // setupConsul creates the Consul client and starts its main Run loop.
 func (a *Agent) setupConsul(consulConfig *config.ConsulConfig) error {
 	apiConf, err := consulConfig.ApiConfig()

command/agent/agent_test.go

@@ -605,3 +605,27 @@ func TestServer_Reload_TLS_Certificate(t *testing.T) {
 	assert.Equal(agent.config.TLSConfig.CertFile, newConfig.TLSConfig.CertFile)
 	assert.Equal(agent.config.TLSConfig.KeyFile, newConfig.TLSConfig.KeyFile)
 }
+
+func Test_GetConfigCopy(t *testing.T) {
+	assert := assert.New(t)
+
+	agentConfig := &Config{
+		Telemetry:      &Telemetry{},
+		Client:         &ClientConfig{},
+		Server:         &ServerConfig{},
+		ACL:            &ACLConfig{},
+		Ports:          &Ports{},
+		Addresses:      &Addresses{},
+		AdvertiseAddrs: &AdvertiseAddrs{},
+		Vault:          &sconfig.VaultConfig{},
+		Consul:         &sconfig.ConsulConfig{},
+		Sentinel:       &sconfig.SentinelConfig{},
+	}
+
+	agent := &Agent{
+		config: agentConfig,
+	}
+
+	configCopy := agent.GetConfigCopy()
+	assert.Equal(configCopy, agentConfig)
+}

command/agent/command.go

@@ -558,7 +558,7 @@ WAIT:
 	// Check if this is a SIGHUP
 	if sig == syscall.SIGHUP {
 		if conf := c.handleReload(config); conf != nil {
-			config = conf.Copy()
+			config = c.agent.GetConfigCopy()
 		}
 		goto WAIT
 	}

command/agent/config.go

@@ -12,7 +12,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/hashicorp/go-sockaddr/template"
@@ -125,8 +124,6 @@ type Config struct {
 	// client
 	TLSConfig *config.TLSConfig `mapstructure:"tls"`
 
-	tlsConfigLock sync.Mutex
-
 	// HTTPAPIResponseHeaders allows users to configure the Nomad http agent to
 	// set arbritrary headers on API responses
 	HTTPAPIResponseHeaders map[string]string `mapstructure:"http_api_response_headers"`
@@ -135,47 +132,6 @@ type Config struct {
 	Sentinel *config.SentinelConfig `mapstructure:"sentinel"`
 }
 
-// Copy creates a replica of the original config, excluding locks
-func (c *Config) Copy() *Config {
-	if c == nil {
-		return nil
-	}
-
-	new := Config{}
-	new.Region = c.Region
-	new.Datacenter = c.Datacenter
-	new.NodeName = c.NodeName
-	new.DataDir = c.DataDir
-	new.LogLevel = c.LogLevel
-	new.BindAddr = c.BindAddr
-	new.EnableDebug = c.EnableDebug
-	new.Ports = c.Ports
-	new.Addresses = c.Addresses
-	new.normalizedAddrs = c.normalizedAddrs
-	new.AdvertiseAddrs = c.AdvertiseAddrs
-	new.Client = c.Client
-	new.Server = c.Server
-	new.ACL = c.ACL
-	new.Telemetry = c.Telemetry
-	new.LeaveOnInt = c.LeaveOnInt
-	new.LeaveOnTerm = c.LeaveOnTerm
-	new.EnableSyslog = c.EnableSyslog
-	new.SyslogFacility = c.SyslogFacility
-	new.DisableUpdateCheck = c.DisableUpdateCheck
-	new.DisableAnonymousSignature = c.DisableAnonymousSignature
-	new.Consul = c.Consul
-	new.Vault = c.Vault
-	new.NomadConfig = c.NomadConfig
-	new.ClientConfig = c.ClientConfig
-	new.DevMode = c.DevMode
-	new.Version = c.Version
-	new.Files = c.Files
-	new.TLSConfig = c.TLSConfig.Copy()
-	new.HTTPAPIResponseHeaders = c.HTTPAPIResponseHeaders
-	new.Sentinel = c.Sentinel
-	return &new
-}
-
 // ClientConfig is configuration specific to the client mode
 type ClientConfig struct {
 	// Enabled controls if we are a client
@@ -378,9 +334,6 @@ type ServerConfig struct {
 // This only allows reloading the certificate and keyfile- other TLSConfig
 // fields are ignored.
 func (c *Config) UpdateTLSConfig(newConfig *config.TLSConfig) error {
-	c.tlsConfigLock.Lock()
-	defer c.tlsConfigLock.Unlock()
-
 	if c.TLSConfig == nil {
 		return fmt.Errorf("unable to update non-existing TLSConfig")
 	}
@@ -696,7 +649,7 @@ func (c *Config) Listener(proto, addr string, port int) (net.Listener, error) {
 
 // Merge merges two configurations.
 func (c *Config) Merge(b *Config) *Config {
-	result := c.Copy()
+	result := *c
 
 	if b.Region != "" {
 		result.Region = b.Region
@@ -835,7 +788,7 @@ func (c *Config) Merge(b *Config) *Config {
 		result.HTTPAPIResponseHeaders[k] = v
 	}
 
-	return result
+	return &result
 }
 
 // normalizeAddrs normalizes Addresses and AdvertiseAddrs to always be

command/agent/config_test.go

@@ -873,26 +873,6 @@ func TestIsMissingPort(t *testing.T) {
 	}
 }
 
-func TestConfigCopy(t *testing.T) {
-	assert := assert.New(t)
-
-	c1 := &Config{
-		Telemetry:      &Telemetry{},
-		Client:         &ClientConfig{},
-		Server:         &ServerConfig{},
-		ACL:            &ACLConfig{},
-		Ports:          &Ports{},
-		Addresses:      &Addresses{},
-		AdvertiseAddrs: &AdvertiseAddrs{},
-		Vault:          &config.VaultConfig{},
-		Consul:         &config.ConsulConfig{},
-		Sentinel:       &config.SentinelConfig{},
-	}
-
-	expected := c1.Copy()
-	assert.Equal(expected, c1)
-}
-
 func Test_UpdateTLS_Config_NoTLS(t *testing.T) {
 	assert := assert.New(t)