code review feedback

command/agent/agent.go

@@ -777,34 +777,27 @@ func (a *Agent) Stats() map[string]map[string]string {
 
 // ShouldReload determines if we should reload the configuration and agent
 // connections. If the TLS Configuration has not changed, we shouldn't reload.
-func (a *Agent) ShouldReload(newConfig *Config) (bool, bool, bool) {
-	var shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC bool
-
+func (a *Agent) ShouldReload(newConfig *Config) (agent, http, rpc bool) {
 	a.configLock.Lock()
 	defer a.configLock.Unlock()
 
-	var tlsInfoChanged bool
 	if !a.config.TLSConfig.CertificateInfoIsEqual(newConfig.TLSConfig) {
-		tlsInfoChanged = true
+		return true, true, true
 	}
 
 	// Allow the ability to only reload HTTP connections
-	if a.config.TLSConfig.EnableHTTP != newConfig.TLSConfig.EnableHTTP || tlsInfoChanged {
-		shouldReloadHTTP = true
+	if a.config.TLSConfig.EnableHTTP != newConfig.TLSConfig.EnableHTTP {
+		http = true
+		agent = true
 	}
 
 	// Allow the ability to only reload HTTP connections
-	if a.config.TLSConfig.EnableRPC != newConfig.TLSConfig.EnableRPC || tlsInfoChanged {
-		shouldReloadRPC = true
+	if a.config.TLSConfig.EnableRPC != newConfig.TLSConfig.EnableRPC {
+		rpc = true
+		agent = true
 	}
 
-	// Always reload the agent if either HTTP or RPC connections need to reload,
-	// or if the TLS configuration itself has changed
-	if shouldReloadHTTP || shouldReloadRPC || tlsInfoChanged {
-		shouldReloadAgent = true
-	}
-
-	return shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC
+	return agent, http, rpc
 }
 
 // Reload handles configuration changes for the agent. Provides a method that

command/agent/agent_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/nomad/helper"
 	sconfig "github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func tmpDir(t testing.TB) string {
@@ -779,7 +780,7 @@ func TestServer_ShouldReload_ReturnFalseForNoChanges(t *testing.T) {
 
 func TestServer_ShouldReload_ReturnTrueForOnlyHTTPChanges(t *testing.T) {
 	t.Parallel()
-	assert := assert.New(t)
+	require := require.New(t)
 
 	const (
 		cafile  = "../../helper/tlsutil/testdata/ca.pem"
@@ -819,9 +820,9 @@ func TestServer_ShouldReload_ReturnTrueForOnlyHTTPChanges(t *testing.T) {
 	}
 
 	shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC := agent.ShouldReload(sameAgentConfig)
-	assert.True(shouldReloadAgent)
-	assert.True(shouldReloadHTTP)
-	assert.False(shouldReloadRPC)
+	require.True(shouldReloadAgent)
+	require.True(shouldReloadHTTP)
+	require.False(shouldReloadRPC)
 }
 
 func TestServer_ShouldReload_ReturnTrueForOnlyRPCChanges(t *testing.T) {