diff --git a/command/agent/command.go b/command/agent/command.go index 3acd34ea8..9b9fb9ad4 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -63,9 +63,11 @@ func (c *Command) readConfig() *Config { Client: &ClientConfig{}, Consul: &config.ConsulConfig{}, Ports: &Ports{}, - Server: &ServerConfig{}, - Vault: &config.VaultConfig{}, - ACL: &ACLConfig{}, + Server: &ServerConfig{ + ServerJoin: &ServerJoin{}, + }, + Vault: &config.VaultConfig{}, + ACL: &ACLConfig{}, } flags := flag.NewFlagSet("agent", flag.ContinueOnError) @@ -78,13 +80,16 @@ func (c *Command) readConfig() *Config { // Server-only options flags.IntVar(&cmdConfig.Server.BootstrapExpect, "bootstrap-expect", 0, "") - flags.BoolVar(&cmdConfig.Server.RejoinAfterLeave, "rejoin", false, "") - flags.Var((*flaghelper.StringFlag)(&cmdConfig.Server.StartJoin), "join", "") - flags.Var((*flaghelper.StringFlag)(&cmdConfig.Server.RetryJoin), "retry-join", "") - flags.IntVar(&cmdConfig.Server.RetryMaxAttempts, "retry-max", 0, "") - flags.StringVar(&cmdConfig.Server.RetryInterval, "retry-interval", "", "") flags.StringVar(&cmdConfig.Server.EncryptKey, "encrypt", "", "gossip encryption key") flags.IntVar(&cmdConfig.Server.RaftProtocol, "raft-protocol", 0, "") + flags.BoolVar(&cmdConfig.Server.RejoinAfterLeave, "rejoin", false, "") + flags.Var((*flaghelper.StringFlag)(&cmdConfig.Server.ServerJoin.StartJoin), "join", "") + flags.Var((*flaghelper.StringFlag)(&cmdConfig.Server.ServerJoin.RetryJoin), "retry-join", "") + flags.IntVar(&cmdConfig.Server.ServerJoin.RetryMaxAttempts, "retry-max", 0, "") + flags.Var((flaghelper.FuncDurationVar)(func(d time.Duration) error { + cmdConfig.Server.ServerJoin.RetryInterval = d + return nil + }), "retry-interval", "") // Client-only options flags.StringVar(&cmdConfig.Client.StateDir, "state-dir", "", "") @@ -267,14 +272,6 @@ 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.") @@ -547,6 +544,17 @@ func (c *Command) Run(args []string) int { logGate.Flush() // Start retry join process + if err := c.handleRetryJoin(config); err != nil { + c.Ui.Error(err.Error()) + return 1 + } + + // Wait for exit + return c.handleSignals() +} + +// handleRetryJoin is used to start retry joining if it is configured. +func (c *Command) handleRetryJoin(config *Config) error { c.retryJoinErrCh = make(chan struct{}) if config.Server.Enabled && len(config.Server.RetryJoin) != 0 { @@ -559,21 +567,30 @@ func (c *Command) Run(args []string) int { } if err := joiner.Validate(config); err != nil { - c.Ui.Error(err.Error()) - return 1 + return err } - // COMPAT: Remove in 0.10 and only use ServerJoin - serverJoinInfo := &ServerJoin{ - RetryJoin: config.Server.RetryJoin, - StartJoin: config.Server.StartJoin, - RetryMaxAttempts: config.Server.RetryMaxAttempts, - RetryInterval: config.Server.retryInterval, + // Remove the duplicate fields + if len(config.Server.RetryJoin) != 0 { + config.Server.ServerJoin.RetryJoin = config.Server.RetryJoin + config.Server.RetryJoin = nil } - go joiner.RetryJoin(serverJoinInfo) + if config.Server.RetryMaxAttempts != 0 { + config.Server.ServerJoin.RetryMaxAttempts = config.Server.RetryMaxAttempts + config.Server.RetryMaxAttempts = 0 + } + if config.Server.RetryInterval != 0 { + config.Server.ServerJoin.RetryInterval = config.Server.RetryInterval + config.Server.RetryInterval = 0 + } + + c.agent.logger.Printf("[WARN] agent: Using deprecated retry_join fields. Upgrade configuration to use server_join") } - if config.Server.Enabled && config.Server.ServerJoin != nil { + if config.Server.Enabled && + config.Server.ServerJoin != nil && + len(config.Server.ServerJoin.RetryJoin) != 0 { + joiner := retryJoiner{ discover: &discover.Discover{}, errCh: c.retryJoinErrCh, @@ -583,18 +600,15 @@ func (c *Command) Run(args []string) int { } if err := joiner.Validate(config); err != nil { - c.Ui.Error(err.Error()) - return 1 + return err } go joiner.RetryJoin(config.Server.ServerJoin) } - if config.Client.Enabled && config.Client.ServerJoin != nil { - // COMPAT: Remove in 0.10 set the default RetryInterval value, as the - // ServerJoin stanza is not part of a default config for an agent. - config.Client.ServerJoin.RetryInterval = time.Duration(30) * time.Second - + if config.Client.Enabled && + config.Client.ServerJoin != nil && + len(config.Client.ServerJoin.RetryJoin) != 0 { joiner := retryJoiner{ discover: &discover.Discover{}, errCh: c.retryJoinErrCh, @@ -604,15 +618,13 @@ func (c *Command) Run(args []string) int { } if err := joiner.Validate(config); err != nil { - c.Ui.Error(err.Error()) - return 1 + return err } go joiner.RetryJoin(config.Client.ServerJoin) } - // Wait for exit - return c.handleSignals() + return nil } // handleSignals blocks until we get an exit-causing signal @@ -885,12 +897,34 @@ func (c *Command) setupTelemetry(config *Config) (*metrics.InmemSink, error) { } func (c *Command) startupJoin(config *Config) error { - if len(config.Server.StartJoin) == 0 || !config.Server.Enabled { + // Nothing to do + if !config.Server.Enabled { return nil } + // Validate both old and new aren't being set + old := len(config.Server.StartJoin) + var new int + if config.Server.ServerJoin != nil { + new = len(config.Server.ServerJoin.StartJoin) + } + if old != 0 && new != 0 { + return fmt.Errorf("server_join and start_join cannot both be defined; prefer setting the server_join stanza") + } + + // Nothing to do + if old+new == 0 { + return nil + } + + // Combine the lists and join + joining := config.Server.StartJoin + if new != 0 { + joining = append(joining, config.Server.ServerJoin.StartJoin...) + } + c.Ui.Output("Joining cluster...") - n, err := c.agent.server.Join(config.Server.StartJoin) + n, err := c.agent.server.Join(joining) if err != nil { return err } diff --git a/command/agent/config.go b/command/agent/config.go index 07e67b526..301d31b3c 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -331,8 +331,7 @@ type ServerConfig struct { // attempts on agent start. The minimum allowed value is 1 second and // the default is 30s. // Deprecated in Nomad 0.10 - RetryInterval string `mapstructure:"retry_interval"` - retryInterval time.Duration `mapstructure:"-"` + RetryInterval time.Duration `mapstructure:"retry_interval"` // RejoinAfterLeave controls our interaction with the cluster after leave. // When set to false (default), a leave causes Consul to not rejoin @@ -661,13 +660,20 @@ func DefaultConfig() *Config { GCInodeUsageThreshold: 70, GCMaxAllocs: 50, NoHostUUID: helper.BoolToPtr(true), + ServerJoin: &ServerJoin{ + RetryJoin: []string{}, + RetryInterval: 30 * time.Second, + RetryMaxAttempts: 0, + }, }, Server: &ServerConfig{ - Enabled: false, - StartJoin: []string{}, - RetryJoin: []string{}, - RetryInterval: "30s", - RetryMaxAttempts: 0, + Enabled: false, + StartJoin: []string{}, + ServerJoin: &ServerJoin{ + RetryJoin: []string{}, + RetryInterval: 30 * time.Second, + RetryMaxAttempts: 0, + }, }, ACL: &ACLConfig{ Enabled: false, @@ -1096,9 +1102,8 @@ func (a *ServerConfig) Merge(b *ServerConfig) *ServerConfig { if b.RetryMaxAttempts != 0 { result.RetryMaxAttempts = b.RetryMaxAttempts } - if b.RetryInterval != "" { + if b.RetryInterval != 0 { result.RetryInterval = b.RetryInterval - result.retryInterval = b.retryInterval } if b.RejoinAfterLeave { result.RejoinAfterLeave = true @@ -1116,9 +1121,6 @@ func (a *ServerConfig) Merge(b *ServerConfig) *ServerConfig { result.EncryptKey = b.EncryptKey } if b.ServerJoin != nil { - // // COMPAT: Remove in 0.10 - ServerJoin is not defined by default on an - // agent config, this should be eventually moved to DefaultConfig - result.ServerJoin = getDefaultServerJoin() result.ServerJoin = result.ServerJoin.Merge(b.ServerJoin) } @@ -1138,12 +1140,6 @@ func (a *ServerConfig) Merge(b *ServerConfig) *ServerConfig { return &result } -func getDefaultServerJoin() *ServerJoin { - return &ServerJoin{ - RetryInterval: time.Duration(30) * time.Second, - } -} - // Merge is used to merge two client configs together func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig { result := *a @@ -1235,9 +1231,6 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig { } if b.ServerJoin != nil { - // // COMPAT: Remove in 0.10 - ServerJoin is not defined by default on an - // agent config, this should be eventually moved to DefaultConfig - result.ServerJoin = getDefaultServerJoin() result.ServerJoin = result.ServerJoin.Merge(b.ServerJoin) } diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index 866ed6cc5..fe8c5c685 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -104,7 +104,7 @@ func TestConfig_Parse(t *testing.T) { MaxHeartbeatsPerSecond: 11.0, RetryJoin: []string{"1.1.1.1", "2.2.2.2"}, StartJoin: []string{"1.1.1.1", "2.2.2.2"}, - RetryInterval: "15s", + RetryInterval: 15 * time.Second, RejoinAfterLeave: true, RetryMaxAttempts: 3, NonVotingServer: true, diff --git a/command/agent/config_test.go b/command/agent/config_test.go index c85ad2c03..d75691209 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -265,8 +265,7 @@ func TestConfig_Merge(t *testing.T) { RejoinAfterLeave: true, StartJoin: []string{"1.1.1.1"}, RetryJoin: []string{"1.1.1.1"}, - RetryInterval: "10s", - retryInterval: time.Second * 10, + RetryInterval: time.Second * 10, NonVotingServer: true, RedundancyZone: "bar", UpgradeVersion: "bar", diff --git a/command/agent/retry_join.go b/command/agent/retry_join.go index d2deee514..2e8735be1 100644 --- a/command/agent/retry_join.go +++ b/command/agent/retry_join.go @@ -59,7 +59,7 @@ func (r *retryJoiner) Validate(config *Config) error { // If retry_join is defined for the server, ensure that deprecated // fields and the server_join stanza are not both set - if config.Server != nil && config.Server.ServerJoin != nil { + if config.Server != nil && config.Server.ServerJoin != nil && len(config.Server.ServerJoin.RetryJoin) != 0 { if len(config.Server.RetryJoin) != 0 { return fmt.Errorf("server_join and retry_join cannot both be defined; prefer setting the server_join stanza") } @@ -69,11 +69,12 @@ func (r *retryJoiner) Validate(config *Config) error { if config.Server.RetryMaxAttempts != 0 { return fmt.Errorf("server_join and retry_max cannot both be defined; prefer setting the server_join stanza") } - if config.Server.RetryInterval != "30s" { + + if config.Server.RetryInterval != 0 { return fmt.Errorf("server_join and retry_interval cannot both be defined; prefer setting the server_join stanza") } - if len(config.Server.ServerJoin.RetryJoin) != 0 && len(config.Server.ServerJoin.StartJoin) != 0 { + if len(config.Server.ServerJoin.StartJoin) != 0 { return fmt.Errorf("retry_join and start_join cannot both be defined") } } @@ -103,6 +104,7 @@ func (r *retryJoiner) RetryJoin(serverJoin *ServerJoin) { for { var addrs []string + var n int var err error for _, addr := range serverJoin.RetryJoin { @@ -121,14 +123,14 @@ func (r *retryJoiner) RetryJoin(serverJoin *ServerJoin) { if len(addrs) > 0 { if r.serverEnabled && r.serverJoin != nil { - n, err := r.serverJoin(addrs) + n, err = r.serverJoin(addrs) if err == nil { r.logger.Printf("[INFO] agent: Join completed. Server synced with %d initial servers", n) return } } if r.clientEnabled && r.clientJoin != nil { - n, err := r.clientJoin(addrs) + n, err = r.clientJoin(addrs) if err == nil { r.logger.Printf("[INFO] agent: Join completed. Client synced with %d initial servers", n) return @@ -144,7 +146,7 @@ func (r *retryJoiner) RetryJoin(serverJoin *ServerJoin) { } if err != nil { - r.logger.Printf("[WARN] agent: Join failed: %v, retrying in %v", err, + r.logger.Printf("[WARN] agent: Join failed: %q, retrying in %v", err, serverJoin.RetryInterval) } time.Sleep(serverJoin.RetryInterval) diff --git a/command/agent/retry_join_test.go b/command/agent/retry_join_test.go index f1eb7fd35..b07848d2c 100644 --- a/command/agent/retry_join_test.go +++ b/command/agent/retry_join_test.go @@ -9,7 +9,6 @@ import ( "time" "github.com/hashicorp/nomad/testutil" - "github.com/hashicorp/nomad/version" "github.com/mitchellh/cli" "github.com/stretchr/testify/require" ) @@ -31,43 +30,37 @@ func (m *MockDiscover) Names() []string { func TestRetryJoin_Integration(t *testing.T) { t.Parallel() + + // Create two agents and have one retry join the other agent := NewTestAgent(t, t.Name(), nil) defer agent.Shutdown() - doneCh := make(chan struct{}) - shutdownCh := make(chan struct{}) - - defer func() { - close(shutdownCh) - <-doneCh - }() + agent2 := NewTestAgent(t, t.Name(), func(c *Config) { + c.NodeName = "foo" + if c.Server.ServerJoin == nil { + c.Server.ServerJoin = &ServerJoin{} + } + c.Server.ServerJoin.RetryJoin = []string{agent.Config.normalizedAddrs.Serf} + c.Server.ServerJoin.RetryInterval = 1 * time.Second + }) + defer agent2.Shutdown() + // Create a fake command and have it wrap the second agent and run the retry + // join handler cmd := &Command{ - Version: version.GetVersion(), - ShutdownCh: shutdownCh, Ui: &cli.BasicUi{ Reader: os.Stdin, Writer: os.Stdout, ErrorWriter: os.Stderr, }, + agent: agent2.Agent, } - serfAddr := agent.Config.normalizedAddrs.Serf - - args := []string{ - "-dev", - "-node", "foo", - "-retry-join", serfAddr, - "-retry-interval", "1s", + if err := cmd.handleRetryJoin(agent2.Config); err != nil { + t.Fatalf("handleRetryJoin failed: %v", err) } - go func() { - if code := cmd.Run(args); code != 0 { - t.Logf("bad: %d", code) - } - close(doneCh) - }() - + // Ensure the retry join occured. testutil.WaitForResult(func() (bool, error) { mem := agent.server.Members() if len(mem) != 2 { @@ -205,8 +198,6 @@ func TestRetryJoin_Client(t *testing.T) { func TestRetryJoin_Validate(t *testing.T) { t.Parallel() - require := require.New(t) - type validateExpect struct { config *Config isValid bool @@ -225,7 +216,7 @@ func TestRetryJoin_Validate(t *testing.T) { }, RetryJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: 0, StartJoin: []string{}, }, }, @@ -243,7 +234,7 @@ func TestRetryJoin_Validate(t *testing.T) { }, StartJoin: []string{"127.0.0.1"}, RetryMaxAttempts: 0, - RetryInterval: "0", + RetryInterval: 0, RetryJoin: []string{}, }, }, @@ -261,7 +252,7 @@ func TestRetryJoin_Validate(t *testing.T) { }, StartJoin: []string{}, RetryMaxAttempts: 1, - RetryInterval: "0", + RetryInterval: 0, RetryJoin: []string{}, }, }, @@ -279,8 +270,7 @@ func TestRetryJoin_Validate(t *testing.T) { }, StartJoin: []string{}, RetryMaxAttempts: 0, - RetryInterval: "3s", - retryInterval: time.Duration(3), + RetryInterval: 3 * time.Second, RetryJoin: []string{}, }, }, @@ -333,51 +323,26 @@ func TestRetryJoin_Validate(t *testing.T) { Server: &ServerConfig{ ServerJoin: &ServerJoin{ RetryJoin: []string{"127.0.0.1"}, - RetryMaxAttempts: 0, - RetryInterval: 0, + RetryMaxAttempts: 1, + RetryInterval: 1, StartJoin: []string{}, }, - StartJoin: []string{}, - RetryMaxAttempts: 0, - RetryInterval: "30s", - RetryJoin: []string{}, }, }, isValid: true, reason: "server server_join should be valid", }, - { - config: &Config{ - Server: &ServerConfig{ - StartJoin: []string{"127.0.0.1"}, - RetryMaxAttempts: 1, - RetryInterval: "0", - RetryJoin: []string{}, - }, - }, - isValid: true, - reason: "server deprecated retry_join configuration should be valid", - }, - { - config: &Config{ - Server: &ServerConfig{ - RetryInterval: "30s", - ServerJoin: &ServerJoin{ - RetryJoin: []string{"127.0.0.1"}, - RetryMaxAttempts: 0, - RetryInterval: time.Duration(20) * time.Second, - StartJoin: []string{}, - }, - }, - }, - isValid: true, - reason: "ignore default value for retry interval", - }, } joiner := retryJoiner{} for _, scenario := range scenarios { - err := joiner.Validate(scenario.config) - require.Equal(err == nil, scenario.isValid, scenario.reason) + t.Run(scenario.reason, func(t *testing.T) { + err := joiner.Validate(scenario.config) + if scenario.isValid { + require.NoError(t, err) + } else { + require.Error(t, err) + } + }) } }