From 023cc2c3b7704f348cf4d5677da7876cc53eeae6 Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Thu, 24 May 2018 17:19:51 -0400 Subject: [PATCH] set retryInterval and other code feedback --- command/agent/command.go | 8 ---- command/agent/config.go | 17 +++++++- command/agent/config_test.go | 67 ++++++++++++++++++++++++++++++++ command/agent/retry_join.go | 27 +++++++++++++ command/agent/retry_join_test.go | 42 +++++++++++++++++++- 5 files changed, 151 insertions(+), 10 deletions(-) diff --git a/command/agent/command.go b/command/agent/command.go index 19835fd19..309ab6c69 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -267,14 +267,6 @@ func (c *Command) readConfig() *Config { } } - // Parse the RetryInterval. - dur, err := time.ParseDuration(config.Server.RetryInterval) - if err != nil { - c.Ui.Error(fmt.Sprintf("Error parsing retry interval: %s", err)) - return nil - } - config.Server.retryInterval = dur - // Check that the server is running in at least one mode. if !(config.Server.Enabled || config.Client.Enabled) { c.Ui.Error("Must specify either server, client or dev mode for the agent.") diff --git a/command/agent/config.go b/command/agent/config.go index c744e5503..3c7bf3b93 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -381,6 +381,21 @@ type ServerJoin struct { retryInterval time.Duration `mapstructure:"-"` } +func (s *ServerJoin) Merge(b *ServerJoin) { + if len(b.StartJoin) != 0 { + s.StartJoin = b.StartJoin + } + if len(b.RetryJoin) != 0 { + s.RetryJoin = b.RetryJoin + } + if b.RetryMaxAttempts != 0 { + s.RetryMaxAttempts = b.RetryMaxAttempts + } + if b.RetryInterval != "" { + s.RetryInterval = b.RetryInterval + } +} + // EncryptBytes returns the encryption key configured. func (s *ServerConfig) EncryptBytes() ([]byte, error) { return base64.StdEncoding.DecodeString(s.EncryptKey) @@ -1089,7 +1104,7 @@ func (a *ServerConfig) Merge(b *ServerConfig) *ServerConfig { result.EncryptKey = b.EncryptKey } if b.ServerJoin != nil { - result.ServerJoin = b.ServerJoin + result.ServerJoin.Merge(b.ServerJoin) } // Add the schedulers diff --git a/command/agent/config_test.go b/command/agent/config_test.go index d0be57cff..33ccd8d26 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -14,6 +14,7 @@ import ( "github.com/hashicorp/nomad/helper" "github.com/hashicorp/nomad/nomad/structs" "github.com/hashicorp/nomad/nomad/structs/config" + "github.com/stretchr/testify/require" ) var ( @@ -907,3 +908,69 @@ func TestIsMissingPort(t *testing.T) { t.Errorf("expected no error, but got %v", err) } } + +func TestMergeServerJoin(t *testing.T) { + require := require.New(t) + + { + retryJoin := []string{"127.0.0.1", "127.0.0.2"} + startJoin := []string{"127.0.0.1", "127.0.0.2"} + retryMaxAttempts := 1 + retryInterval := "1" + + a := &ServerJoin{ + RetryJoin: retryJoin, + StartJoin: startJoin, + RetryMaxAttempts: retryMaxAttempts, + RetryInterval: retryInterval, + } + b := &ServerJoin{} + + a.Merge(b) + require.Equal(a.RetryJoin, retryJoin) + require.Equal(a.StartJoin, startJoin) + require.Equal(a.RetryMaxAttempts, retryMaxAttempts) + require.Equal(a.RetryInterval, retryInterval) + } + { + retryJoin := []string{"127.0.0.1", "127.0.0.2"} + startJoin := []string{"127.0.0.1", "127.0.0.2"} + retryMaxAttempts := 1 + retryInterval := "1" + + a := &ServerJoin{} + b := &ServerJoin{ + RetryJoin: retryJoin, + StartJoin: startJoin, + RetryMaxAttempts: retryMaxAttempts, + RetryInterval: retryInterval, + } + + a.Merge(b) + require.Equal(a.RetryJoin, retryJoin) + require.Equal(a.StartJoin, startJoin) + require.Equal(a.RetryMaxAttempts, retryMaxAttempts) + require.Equal(a.RetryInterval, retryInterval) + } + { + retryJoin := []string{"127.0.0.1", "127.0.0.2"} + startJoin := []string{"127.0.0.1", "127.0.0.2"} + retryMaxAttempts := 1 + retryInterval := "1" + + a := &ServerJoin{ + RetryJoin: retryJoin, + StartJoin: startJoin, + } + b := &ServerJoin{ + RetryMaxAttempts: retryMaxAttempts, + RetryInterval: retryInterval, + } + + a.Merge(b) + require.Equal(a.RetryJoin, retryJoin) + require.Equal(a.StartJoin, startJoin) + require.Equal(a.RetryMaxAttempts, retryMaxAttempts) + require.Equal(a.RetryInterval, retryInterval) + } +} diff --git a/command/agent/retry_join.go b/command/agent/retry_join.go index 430b7052e..315fe5265 100644 --- a/command/agent/retry_join.go +++ b/command/agent/retry_join.go @@ -82,6 +82,33 @@ func (r *retryJoiner) Validate(config *Config) error { } } + if config.Server != nil { + dur, err := time.ParseDuration(config.Server.RetryInterval) + if err != nil { + return fmt.Errorf("Error parsing server retry interval: %s", err) + } else { + config.Server.retryInterval = dur + } + + if config.Server.ServerJoin != nil { + dur, err := time.ParseDuration(config.Server.RetryInterval) + if err != nil { + return fmt.Errorf("Error parsing server retry interval: %s", err) + } else { + config.Server.ServerJoin.retryInterval = dur + } + } + } + + if config.Client != nil && config.Client.ServerJoin != nil { + dur, err := time.ParseDuration(config.Client.ServerJoin.RetryInterval) + if err != nil { + return fmt.Errorf("Error parsing retry interval: %s", err) + } else { + config.Client.ServerJoin.retryInterval = dur + } + } + return nil } diff --git a/command/agent/retry_join_test.go b/command/agent/retry_join_test.go index f4306e12b..a7e129aca 100644 --- a/command/agent/retry_join_test.go +++ b/command/agent/retry_join_test.go @@ -349,13 +349,53 @@ func TestRetryJoin_Validate(t *testing.T) { Server: &ServerConfig{ StartJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 1, - RetryInterval: "0", + RetryInterval: "3s", RetryJoin: []string{}, }, }, isValid: true, reason: "server deprecated retry_join configuration should be valid", }, + { + config: &Config{ + Server: &ServerConfig{ + StartJoin: []string{"127.0.0.1"}, + RetryMaxAttempts: 1, + RetryInterval: "invalid!TimeInterval", + RetryJoin: []string{}, + }, + }, + isValid: false, + reason: "invalid time interval", + }, + { + config: &Config{ + Server: &ServerConfig{ + ServerJoin: &ServerJoin{ + StartJoin: []string{"127.0.0.1"}, + RetryMaxAttempts: 1, + RetryInterval: "invalid!TimeInterval", + RetryJoin: []string{}, + }, + }, + }, + isValid: false, + reason: "invalid time interval", + }, + { + config: &Config{ + Client: &ClientConfig{ + ServerJoin: &ServerJoin{ + StartJoin: []string{"127.0.0.1"}, + RetryMaxAttempts: 1, + RetryInterval: "invalid!TimeInterval", + RetryJoin: []string{}, + }, + }, + }, + isValid: false, + reason: "invalid time interval", + }, } joiner := retryJoiner{}