From 490e70e9cef68006095037e8a5abf8567903ed99 Mon Sep 17 00:00:00 2001
From: Chelsea Holland Komlo <me@chelseakomlo.com>
Date: Fri, 23 Mar 2018 17:58:10 -0400
Subject: [PATCH] code review feedback

---
 command/agent/agent.go      | 25 +++++++++----------------
 command/agent/agent_test.go |  9 +++++----
 2 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/command/agent/agent.go b/command/agent/agent.go
index ff62f6266..4916c014a 100644
--- a/command/agent/agent.go
+++ b/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
diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go
index 2df13df26..115655822 100644
--- a/command/agent/agent_test.go
+++ b/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) {