call reload on agent, client, and server separately

client/client.go

@@ -369,7 +369,7 @@ func (c *Client) init() error {
 // client's TLS configuration changes from plaintext to TLS
 func (c *Client) ReloadTLSConnections(newConfig *nconfig.TLSConfig) error {
 	var tlsWrap tlsutil.RegionWrapper
-	if newConfig.EnableRPC {
+	if newConfig != nil && newConfig.EnableRPC {
 		tw, err := c.config.NewTLSConfiguration(newConfig).OutgoingTLSWrapper()
 
 		if err != nil {
@@ -378,15 +378,21 @@ func (c *Client) ReloadTLSConnections(newConfig *nconfig.TLSConfig) error {
 		tlsWrap = tw
 	}
 
-	c.connPool.ReloadTLS(tlsWrap)
-
 	c.configLock.Lock()
 	c.config.TLSConfig = newConfig
 	c.configLock.Unlock()
 
+	c.connPool.ReloadTLS(tlsWrap)
+
 	return nil
 }
 
+// Reload allows a client to reload RPC connections if the
+// client's TLS configuration changes
+func (c *Client) Reload(newConfig *config.Config) error {
+	return c.reloadTLSConnections(newConfig.TLSConfig)
+}
+
 // Leave is used to prepare the client to leave the cluster
 func (c *Client) Leave() error {
 	// TODO

client/client_test.go

@@ -1032,7 +1032,7 @@ func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) {
 		KeyFile:              fookey,
 	}
 
-	err := c1.ReloadTLSConnections(newConfig)
+	err := c1.reloadTLSConnections(newConfig)
 	assert.Nil(err)
 
 	req := structs.NodeSpecificRequest{
@@ -1085,7 +1085,7 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) {
 
 	newConfig := &nconfig.TLSConfig{}
 
-	err := c1.ReloadTLSConnections(newConfig)
+	err := c1.reloadTLSConnections(newConfig)
 	assert.Nil(err)
 
 	req := structs.NodeSpecificRequest{

command/agent/agent.go

@@ -782,24 +782,6 @@ func (a *Agent) Reload(newConfig *Config) error {
 		a.logger.Println("[INFO] agent: Upgrading from plaintext configuration to TLS")
 	}
 
-	// Reload the TLS configuration for the client or server, depending on how
-	// the agent is configured to run.
-	if s := a.Server(); s != nil {
-		err := s.ReloadTLSConnections(a.config.TLSConfig)
-		if err != nil {
-			a.logger.Printf("[WARN] agent: error reloading the server's TLS configuration: %v", err.Error())
-			return err
-		}
-	}
-	if c := a.Client(); c != nil {
-
-		err := c.ReloadTLSConnections(a.config.TLSConfig)
-		if err != nil {
-			a.logger.Printf("[WARN] agent: error reloading the server's TLS configuration: %v", err.Error())
-			return err
-		}
-	}
-
 	return nil
 }
 

command/agent/command.go

@@ -640,18 +640,11 @@ func (c *Command) handleReload() {
 
 	shouldReload := c.agent.ShouldReload(newConf)
 	if shouldReload {
-		// Reloads configuration for an agent running in both client and server mode
 		err := c.agent.Reload(newConf)
 		if err != nil {
 			c.agent.logger.Printf("[ERR] agent: failed to reload the config: %v", err)
 			return
 		}
-
-		err = c.reloadHTTPServer(newConf)
-		if err != nil {
-			c.agent.logger.Printf("[ERR] http: failed to reload the config: %v", err)
-			return
-		}
 	}
 
 	if s := c.agent.Server(); s != nil {
@@ -661,9 +654,34 @@ func (c *Command) handleReload() {
 		} else {
 			if err := s.Reload(sconf); err != nil {
 				c.agent.logger.Printf("[ERR] agent: reloading server config failed: %v", err)
+				return
 			}
 		}
 	}
+
+	if s := c.agent.Client(); s != nil {
+		clientConfig, err := c.agent.clientConfig()
+		if err != nil {
+			c.agent.logger.Printf("[ERR] agent: reloading client config failed: %v", err)
+			return
+		}
+		if err := c.agent.Client().Reload(clientConfig); err != nil {
+			c.agent.logger.Printf("[ERR] agent: reloading client config failed: %v", err)
+			return
+		}
+	}
+
+	// reload HTTP server after we have reloaded both client and server, in case
+	// we error in either of the above cases. For example, reloading the http
+	// server to a TLS connection could succeed, while reloading the server's rpc
+	// connections could fail.
+	if shouldReload {
+		err := c.reloadHTTPServer(newConf)
+		if err != nil {
+			c.agent.logger.Printf("[ERR] http: failed to reload the config: %v", err)
+			return
+		}
+	}
 }
 
 // setupTelemetry is used ot setup the telemetry sub-systems

nomad/server.go

@@ -400,8 +400,8 @@ func getTLSConf(enableRPC bool, tlsConf *tlsutil.Config) (*tls.Config, tlsutil.R
 }
 
 // ReloadTLSConnections updates a server's TLS configuration and reloads RPC
-// connections
-func (s *Server) ReloadTLSConnections(newTLSConfig *config.TLSConfig) error {
+// connections. This will handle both TLS upgrades and downgrades.
+func (s *Server) reloadTLSConnections(newTLSConfig *config.TLSConfig) error {
 	s.logger.Printf("[INFO] nomad: reloading server connections due to configuration changes")
 
 	tlsConf := s.config.newTLSConfig(newTLSConfig)
@@ -619,6 +619,12 @@ func (s *Server) Reload(newConfig *Config) error {
 		}
 	}
 
+	if !newConfig.TLSConfig.Equals(s.config.TLSConfig) {
+		if err := s.reloadTLSConnections(newConfig.TLSConfig); err != nil {
+			multierror.Append(&mErr, err)
+		}
+	}
+
 	return mErr.ErrorOrNil()
 }
 

nomad/server_test.go

@@ -309,7 +309,7 @@ func TestServer_Reload_TLSConnections_PlaintextToTLS(t *testing.T) {
 		KeyFile:              fookey,
 	}
 
-	err := s1.ReloadTLSConnections(newTLSConfig)
+	err := s1.reloadTLSConnections(newTLSConfig)
 	assert.Nil(err)
 
 	assert.True(s1.config.TLSConfig.Equals(newTLSConfig))
@@ -330,7 +330,7 @@ func TestServer_Reload_TLSConnections_PlaintextToTLS(t *testing.T) {
 
 // Tests that the server will successfully reload its network connections,
 // downgrading from TLS to plaintext if the server's TLS configuration changes.
-func TestServer_Reload_TLSConnections_TLSToPlaintext(t *testing.T) {
+func TestServer_reload_TLSConnections_TLSToPlaintext(t *testing.T) {
 	t.Parallel()
 	assert := assert.New(t)
 
@@ -357,7 +357,7 @@ func TestServer_Reload_TLSConnections_TLSToPlaintext(t *testing.T) {
 
 	newTLSConfig := &config.TLSConfig{}
 
-	err := s1.ReloadTLSConnections(newTLSConfig)
+	err := s1.reloadTLSConnections(newTLSConfig)
 	assert.Nil(err)
 	assert.True(s1.config.TLSConfig.Equals(newTLSConfig))
 

nomad/structs/config/tls.go

@@ -188,9 +188,14 @@ func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig {
 
 // Equals compares the fields of two TLS configuration objects, returning a
 // boolean indicating if they are the same.
-// NewConfig should never be nil- calling code is responsible for walways
-// passing a valid TLSConfig object
+// It is possible for either the calling TLSConfig to be nil, or the TLSConfig
+// that it is being compared against, so we need to handle both places. See
+// server.go Reload for example.
 func (t *TLSConfig) Equals(newConfig *TLSConfig) bool {
+	if t == nil || newConfig == nil {
+		return t == newConfig
+	}
+
 	return t.EnableRPC == newConfig.EnableRPC &&
 		t.CAFile == newConfig.CAFile &&
 		t.CertFile == newConfig.CertFile &&