Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for hot reloading log levels #5996

Merged
merged 1 commit into from
Jul 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions command/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -934,10 +934,14 @@ func (a *Agent) ShouldReload(newConfig *Config) (agent, http bool) {
a.configLock.Lock()
defer a.configLock.Unlock()

if newConfig.LogLevel != "" && newConfig.LogLevel != a.config.LogLevel {
agent = true
}

isEqual, err := a.config.TLSConfig.CertificateInfoIsEqual(newConfig.TLSConfig)
if err != nil {
a.logger.Error("parsing TLS certificate", "error", err)
return false, false
return agent, false
} else if !isEqual {
return true, true
}
Expand All @@ -962,13 +966,27 @@ func (a *Agent) Reload(newConfig *Config) error {
a.configLock.Lock()
defer a.configLock.Unlock()

if newConfig == nil || newConfig.TLSConfig == nil {
updatedLogging := newConfig != nil && (newConfig.LogLevel != a.config.LogLevel)

if newConfig == nil || newConfig.TLSConfig == nil && !updatedLogging {
return fmt.Errorf("cannot reload agent with nil configuration")
}

// This is just a TLS configuration reload, we don't need to refresh
// existing network connections
if updatedLogging {
a.config.LogLevel = newConfig.LogLevel
a.logger.SetLevel(log.LevelFromString(newConfig.LogLevel))
}

fullUpdateTLSConfig := func() {
// Completely reload the agent's TLS configuration (moving from non-TLS to
// TLS, or vice versa)
// This does not handle errors in loading the new TLS configuration
a.config.TLSConfig = newConfig.TLSConfig.Copy()
}

if !a.config.TLSConfig.IsEmpty() && !newConfig.TLSConfig.IsEmpty() {
// This is just a TLS configuration reload, we don't need to refresh
// existing network connections

// Reload the certificates on the keyloader and on success store the
// updated TLS config. It is important to reuse the same keyloader
Expand All @@ -983,17 +1001,12 @@ func (a *Agent) Reload(newConfig *Config) error {
a.config.TLSConfig = newConfig.TLSConfig
a.config.TLSConfig.KeyLoader = keyloader
return nil
}

// Completely reload the agent's TLS configuration (moving from non-TLS to
// TLS, or vice versa)
// This does not handle errors in loading the new TLS configuration
a.config.TLSConfig = newConfig.TLSConfig.Copy()

if newConfig.TLSConfig.IsEmpty() {
} else if newConfig.TLSConfig.IsEmpty() && !a.config.TLSConfig.IsEmpty() {
a.logger.Warn("downgrading agent's existing TLS configuration to plaintext")
} else {
fullUpdateTLSConfig()
} else if !newConfig.TLSConfig.IsEmpty() && a.config.TLSConfig.IsEmpty() {
a.logger.Info("upgrading from plaintext configuration to TLS")
fullUpdateTLSConfig()
}

return nil
Expand Down
22 changes: 22 additions & 0 deletions command/agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,28 @@ func TestAgent_HTTPCheckPath(t *testing.T) {
}
}

// Here we validate that log levels get updated when the configuration is
// reloaded. I can't find a good way to fetch this from the logger itself, so
// we pull it only from the agents configuration struct, not the logger.
func TestAgent_Reload_LogLevel(t *testing.T) {
t.Parallel()
assert := assert.New(t)

agent := NewTestAgent(t, t.Name(), func(c *Config) {
c.LogLevel = "INFO"
})
defer agent.Shutdown()

assert.Equal("INFO", agent.GetConfig().LogLevel)

newConfig := &Config{
LogLevel: "TRACE",
}

assert.Nil(agent.Reload(newConfig))
assert.Equal("TRACE", agent.GetConfig().LogLevel)
}

// This test asserts that the keyloader embedded in the TLS config is shared
// across the Agent, Server, and Client. This is essential for certificate
// reloading to work.
Expand Down
2 changes: 1 addition & 1 deletion command/agent/log_levels.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
// levels that we use.
func LevelFilter() *logutils.LevelFilter {
return &logutils.LevelFilter{
Levels: []logutils.LogLevel{"TRACE", "DEBUG", "INFO", "WARN", "ERR"},
Levels: []logutils.LogLevel{"TRACE", "DEBUG", "INFO", "WARN", "ERROR"},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fixes an existing bug that doesn't allow specifying ERROR level logging as required by hclog (https://github.com/hashicorp/go-hclog/blob/master/logger.go#L59-L76)

MinLevel: "INFO",
Writer: ioutil.Discard,
}
Expand Down