From 597cdbdfb195c31e81065a70d771dc7ff1b8293b Mon Sep 17 00:00:00 2001 From: Chelsea Holland Komlo Date: Fri, 25 May 2018 17:12:13 -0400 Subject: [PATCH] RetryInterval should be a time.Duration --- command/agent/command.go | 10 ++++- command/agent/config.go | 5 +-- command/agent/config_parse_test.go | 4 +- command/agent/config_test.go | 12 +++--- command/agent/retry_join.go | 29 +------------- command/agent/retry_join_test.go | 62 ++++++------------------------ 6 files changed, 32 insertions(+), 90 deletions(-) diff --git a/command/agent/command.go b/command/agent/command.go index 968993bb476..4abdfcc51e4 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -267,6 +267,14 @@ func (c *Command) readConfig() *Config { } } + // COMPAT: Remove in 0.10. 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.") @@ -558,7 +566,7 @@ func (c *Command) Run(args []string) int { RetryJoin: config.Server.RetryJoin, StartJoin: config.Server.StartJoin, RetryMaxAttempts: config.Server.RetryMaxAttempts, - RetryInterval: config.Server.RetryInterval, + RetryInterval: config.Server.retryInterval, } go joiner.RetryJoin(serverJoinInfo) } diff --git a/command/agent/config.go b/command/agent/config.go index 3c7bf3b93f7..0398b5004ec 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -377,8 +377,7 @@ type ServerJoin struct { // RetryInterval specifies the amount of time to wait in between join // attempts on agent start. The minimum allowed value is 1 second and // the default is 30s. - RetryInterval string `mapstructure:"retry_interval"` - retryInterval time.Duration `mapstructure:"-"` + RetryInterval time.Duration `mapstructure:"retry_interval"` } func (s *ServerJoin) Merge(b *ServerJoin) { @@ -391,7 +390,7 @@ func (s *ServerJoin) Merge(b *ServerJoin) { if b.RetryMaxAttempts != 0 { s.RetryMaxAttempts = b.RetryMaxAttempts } - if b.RetryInterval != "" { + if b.RetryInterval != 0 { s.RetryInterval = b.RetryInterval } } diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index b25e4e8e4fe..137d0efd851 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -49,7 +49,7 @@ func TestConfig_Parse(t *testing.T) { NodeClass: "linux-medium-64bit", ServerJoin: &ServerJoin{ RetryJoin: []string{"1.1.1.1", "2.2.2.2"}, - RetryInterval: "15s", + RetryInterval: time.Duration(15) * time.Second, RetryMaxAttempts: 3, }, Meta: map[string]string{ @@ -114,7 +114,7 @@ func TestConfig_Parse(t *testing.T) { ServerJoin: &ServerJoin{ RetryJoin: []string{"1.1.1.1", "2.2.2.2"}, StartJoin: []string{"1.1.1.1", "2.2.2.2"}, - RetryInterval: "15s", + RetryInterval: time.Duration(15) * time.Second, RetryMaxAttempts: 3, }, }, diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 33ccd8d26a4..6c3337d44ba 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -916,13 +916,13 @@ func TestMergeServerJoin(t *testing.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" + retryInterval := time.Duration(0) a := &ServerJoin{ RetryJoin: retryJoin, StartJoin: startJoin, RetryMaxAttempts: retryMaxAttempts, - RetryInterval: retryInterval, + RetryInterval: time.Duration(retryInterval), } b := &ServerJoin{} @@ -936,14 +936,14 @@ func TestMergeServerJoin(t *testing.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" + retryInterval := time.Duration(0) a := &ServerJoin{} b := &ServerJoin{ RetryJoin: retryJoin, StartJoin: startJoin, RetryMaxAttempts: retryMaxAttempts, - RetryInterval: retryInterval, + RetryInterval: time.Duration(retryInterval), } a.Merge(b) @@ -956,7 +956,7 @@ func TestMergeServerJoin(t *testing.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" + retryInterval := time.Duration(0) a := &ServerJoin{ RetryJoin: retryJoin, @@ -964,7 +964,7 @@ func TestMergeServerJoin(t *testing.T) { } b := &ServerJoin{ RetryMaxAttempts: retryMaxAttempts, - RetryInterval: retryInterval, + RetryInterval: time.Duration(retryInterval), } a.Merge(b) diff --git a/command/agent/retry_join.go b/command/agent/retry_join.go index 315fe52650a..3600ba7b488 100644 --- a/command/agent/retry_join.go +++ b/command/agent/retry_join.go @@ -82,33 +82,6 @@ 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 } @@ -170,6 +143,6 @@ func (r *retryJoiner) RetryJoin(serverJoin *ServerJoin) { r.logger.Printf("[WARN] agent: Join failed: %v, retrying in %v", err, serverJoin.RetryInterval) } - time.Sleep(serverJoin.retryInterval) + time.Sleep(serverJoin.RetryInterval) } } diff --git a/command/agent/retry_join_test.go b/command/agent/retry_join_test.go index a7e129aca1f..4f875772f31 100644 --- a/command/agent/retry_join_test.go +++ b/command/agent/retry_join_test.go @@ -6,6 +6,7 @@ import ( "log" "os" "testing" + "time" "github.com/hashicorp/nomad/testutil" "github.com/hashicorp/nomad/version" @@ -219,7 +220,7 @@ func TestRetryJoin_Validate(t *testing.T) { ServerJoin: &ServerJoin{ RetryJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: 0, StartJoin: []string{}, }, RetryJoin: []string{"127.0.0.1"}, @@ -237,7 +238,7 @@ func TestRetryJoin_Validate(t *testing.T) { ServerJoin: &ServerJoin{ RetryJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: 0, StartJoin: []string{}, }, StartJoin: []string{"127.0.0.1"}, @@ -255,7 +256,7 @@ func TestRetryJoin_Validate(t *testing.T) { ServerJoin: &ServerJoin{ RetryJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: 0, StartJoin: []string{}, }, StartJoin: []string{}, @@ -273,12 +274,13 @@ func TestRetryJoin_Validate(t *testing.T) { ServerJoin: &ServerJoin{ RetryJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: time.Duration(1), StartJoin: []string{}, }, StartJoin: []string{}, RetryMaxAttempts: 0, - RetryInterval: "1", + RetryInterval: "3s", + retryInterval: time.Duration(3), RetryJoin: []string{}, }, }, @@ -291,7 +293,7 @@ func TestRetryJoin_Validate(t *testing.T) { ServerJoin: &ServerJoin{ RetryJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: 0, StartJoin: []string{"127.0.0.1"}, }, }, @@ -305,7 +307,7 @@ func TestRetryJoin_Validate(t *testing.T) { ServerJoin: &ServerJoin{ RetryJoin: []string{}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: 0, StartJoin: []string{"127.0.0.1"}, }, }, @@ -319,7 +321,7 @@ func TestRetryJoin_Validate(t *testing.T) { ServerJoin: &ServerJoin{ RetryJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: 0, }, }, }, @@ -332,7 +334,7 @@ func TestRetryJoin_Validate(t *testing.T) { ServerJoin: &ServerJoin{ RetryJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: 0, StartJoin: []string{}, }, StartJoin: []string{}, @@ -349,53 +351,13 @@ func TestRetryJoin_Validate(t *testing.T) { Server: &ServerConfig{ StartJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 1, - RetryInterval: "3s", + RetryInterval: "0", 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{}