diff --git a/command/agent/command.go b/command/agent/command.go index c8dacec4e7..d18bc02f50 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -179,15 +179,13 @@ func (c *Command) readConfig() *Config { return nil } - // Make sure SkipLeaveOnInt is set to the right default based on the - // agent's mode (client or server) + // Make sure LeaveOnTerm and SkipLeaveOnInt are set to the right + // defaults based on the agent's mode (client or server). + if config.LeaveOnTerm == nil { + config.LeaveOnTerm = Bool(!config.Server) + } if config.SkipLeaveOnInt == nil { - config.SkipLeaveOnInt = new(bool) - if config.Server { - *config.SkipLeaveOnInt = true - } else { - *config.SkipLeaveOnInt = false - } + config.SkipLeaveOnInt = Bool(config.Server) } // Ensure we have a data directory @@ -922,7 +920,7 @@ WAIT: graceful := false if sig == os.Interrupt && !(*config.SkipLeaveOnInt) { graceful = true - } else if sig == syscall.SIGTERM && config.LeaveOnTerm { + } else if sig == syscall.SIGTERM && (*config.LeaveOnTerm) { graceful = true } diff --git a/command/agent/command_test.go b/command/agent/command_test.go index 3686f24b03..6eec51ab22 100644 --- a/command/agent/command_test.go +++ b/command/agent/command_test.go @@ -137,7 +137,7 @@ func TestReadCliConfig(t *testing.T) { } } - // Test SkipLeaveOnInt default for server mode + // Test LeaveOnTerm and SkipLeaveOnInt defaults for server mode { ui := new(cli.MockUi) cmd := &Command{ @@ -157,12 +157,15 @@ func TestReadCliConfig(t *testing.T) { if config.Server != true { t.Errorf(`Expected -server to be true`) } + if (*config.LeaveOnTerm) != false { + t.Errorf(`Expected LeaveOnTerm to be false in server mode`) + } if (*config.SkipLeaveOnInt) != true { t.Errorf(`Expected SkipLeaveOnInt to be true in server mode`) } } - // Test SkipLeaveOnInt default for client mode + // Test LeaveOnTerm and SkipLeaveOnInt defaults for client mode { ui := new(cli.MockUi) cmd := &Command{ @@ -181,6 +184,9 @@ func TestReadCliConfig(t *testing.T) { if config.Server != false { t.Errorf(`Expected server to be false`) } + if (*config.LeaveOnTerm) != true { + t.Errorf(`Expected LeaveOnTerm to be true in client mode`) + } if *config.SkipLeaveOnInt != false { t.Errorf(`Expected SkipLeaveOnInt to be false in client mode`) } diff --git a/command/agent/config.go b/command/agent/config.go index d94dbb2c85..31d546e4e5 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -303,8 +303,9 @@ type Config struct { TaggedAddresses map[string]string // LeaveOnTerm controls if Serf does a graceful leave when receiving - // the TERM signal. Defaults false. This can be changed on reload. - LeaveOnTerm bool `mapstructure:"leave_on_terminate"` + // the TERM signal. Defaults true on clients, false on servers. This can + // be changed on reload. + LeaveOnTerm *bool `mapstructure:"leave_on_terminate"` // SkipLeaveOnInt controls if Serf skips a graceful leave when // receiving the INT signal. Defaults false on clients, true on @@ -1170,8 +1171,8 @@ func MergeConfig(a, b *Config) *Config { if b.Server == true { result.Server = b.Server } - if b.LeaveOnTerm == true { - result.LeaveOnTerm = true + if b.LeaveOnTerm != nil { + result.LeaveOnTerm = b.LeaveOnTerm } if b.SkipLeaveOnInt != nil { result.SkipLeaveOnInt = b.SkipLeaveOnInt diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 1da34a3db8..0937697bfe 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -78,8 +78,8 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("bad: expected nil SkipLeaveOnInt") } - if config.LeaveOnTerm != DefaultConfig().LeaveOnTerm { - t.Fatalf("bad: %#v", config) + if config.LeaveOnTerm != nil { + t.Fatalf("bad: expected nil LeaveOnTerm") } // Server bootstrap @@ -279,7 +279,7 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("err: %s", err) } - if config.LeaveOnTerm != true { + if *config.LeaveOnTerm != true { t.Fatalf("bad: %#v", config) } @@ -1382,7 +1382,7 @@ func TestMergeConfig(t *testing.T) { BindAddr: "127.0.0.1", AdvertiseAddr: "127.0.0.1", Server: false, - LeaveOnTerm: false, + LeaveOnTerm: new(bool), SkipLeaveOnInt: new(bool), EnableDebug: false, CheckUpdateIntervalRaw: "8m", @@ -1441,8 +1441,8 @@ func TestMergeConfig(t *testing.T) { HTTPS: "127.0.0.4", }, Server: true, - LeaveOnTerm: true, - SkipLeaveOnInt: new(bool), + LeaveOnTerm: Bool(true), + SkipLeaveOnInt: Bool(true), EnableDebug: true, VerifyIncoming: true, VerifyOutgoing: true, @@ -1521,7 +1521,6 @@ func TestMergeConfig(t *testing.T) { }, Reap: Bool(true), } - *b.SkipLeaveOnInt = true c := MergeConfig(a, b) diff --git a/consul/server_test.go b/consul/server_test.go index a8d65d36fa..20333e44bc 100644 --- a/consul/server_test.go +++ b/consul/server_test.go @@ -7,18 +7,17 @@ import ( "net" "os" "strings" + "sync/atomic" "testing" "time" "github.com/hashicorp/consul/testutil" ) -var nextPort = 15000 +var nextPort int32 = 15000 func getPort() int { - p := nextPort - nextPort++ - return p + return int(atomic.AddInt32(&nextPort, 1)) } func tmpDir(t *testing.T) string { diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index 318c28f5e5..6289a11346 100644 --- a/website/source/docs/agent/options.html.markdown +++ b/website/source/docs/agent/options.html.markdown @@ -567,9 +567,11 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass ``` * `leave_on_terminate` If - enabled, when the agent receives a TERM signal, - it will send a `Leave` message to the rest of the cluster and gracefully - leave. Defaults to false. + enabled, when the agent receives a TERM signal, it will send a `Leave` message to the rest + of the cluster and gracefully leave. The default behavior for this feature varies based on + whether or not the agent is running as a client or a server (prior to Consul 0.7 the default + value was unconditionally set to `false`). On agents in client-mode, this defaults to `true` + and for agents in server-mode, this defaults to `false`. * `log_level` Equivalent to the [`-log-level` command-line flag](#_log_level). @@ -581,20 +583,21 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass later, this is a nested object that allows tuning the performance of different subsystems in Consul. See the [Server Performance](/docs/guides/performance.html) guide for more details. The following parameters are available: - * `raft_multiplier` - An integer - multiplier used by Consul servers to scale key Raft timing parameters. Omitting this value - or setting it to 0 uses default timing described below. Lower values are used to tighten - timing and increase sensitivity while higher values relax timings and reduce sensitivity. - Tuning this affects the time it takes Consul to detect leader failures and to perform - leader elections, at the expense of requiring more network and CPU resources for better - performance.

By default, Consul will use a lower-performance timing that's suitable - for [minimal Consul servers](/docs/guides/performance.html#minumum), currently equivalent - to setting this to a value of 5 (this default may be changed in future versions of Consul, - depending if the target minimum server profile changes). Setting this to a value of 1 will - configure Raft to its highest-performance mode, equivalent to the default timing of Consul - prior to 0.7, and is recommended for [production Consul servers](/docs/guides/performance.html#production). - See the note on [last contact](/docs/guides/performance.html#last-contact) timing for more - details on tuning this parameter. The maximum allowed value is 10. + +* `raft_multiplier` - An integer + multiplier used by Consul servers to scale key Raft timing parameters. Omitting this value + or setting it to 0 uses default timing described below. Lower values are used to tighten + timing and increase sensitivity while higher values relax timings and reduce sensitivity. + Tuning this affects the time it takes Consul to detect leader failures and to perform + leader elections, at the expense of requiring more network and CPU resources for better + performance.

By default, Consul will use a lower-performance timing that's suitable + for [minimal Consul servers](/docs/guides/performance.html#minumum), currently equivalent + to setting this to a value of 5 (this default may be changed in future versions of Consul, + depending if the target minimum server profile changes). Setting this to a value of 1 will + configure Raft to its highest-performance mode, equivalent to the default timing of Consul + prior to 0.7, and is recommended for [production Consul servers](/docs/guides/performance.html#production). + See the note on [last contact](/docs/guides/performance.html#last-contact) timing for more + details on tuning this parameter. The maximum allowed value is 10. * `ports` This is a nested object that allows setting the bind ports for the following keys: diff --git a/website/source/docs/upgrade-specific.html.markdown b/website/source/docs/upgrade-specific.html.markdown index 9896631f57..77d01c2e4a 100644 --- a/website/source/docs/upgrade-specific.html.markdown +++ b/website/source/docs/upgrade-specific.html.markdown @@ -19,7 +19,7 @@ standard upgrade flow. Consul version 0.7 is a very large release with many important changes. Changes to be aware of during an upgrade are categorized below. -#### Defaults Changed for Better Performance +#### Performance Timing Defaults and Tuning Consul 0.7 now defaults the DNS configuration to allow for stale queries by defaulting [`allow_stale`](/docs/agent/options.html#allow_stale) to true for better utilization @@ -53,11 +53,22 @@ to all Consul servers when upgrading: See the [Server Performance](/docs/guides/performance.html) guide for more details. -#### Servers No Longer Default to Leave on Interrupt +#### Leave-Related Configuration Defaults -The default behavior of [`skip_leave_on_interrupt`](/docs/agent/options.html#skip_leave_on_interrupt) -is now dependent on whether or not the agent is acting as a server or client. When Consul is started as a -server the default is `true` and `false` when a client. +The default behavior of [`leave_on_terminate`](/docs/agent/options.html#leave_on_terminate) +and [`skip_leave_on_interrupt`](/docs/agent/options.html#skip_leave_on_interrupt) +are now dependent on whether or not the agent is acting as a server or client: + +* For servers, `leave_on_terminate` defaults to "false" and `skip_leave_on_interrupt` +defaults to "true". + +* For clients, `leave_on_terminate` defaults to "true" and `skip_leave_on_interrupt` +defaults to "false". + +These defaults are designed to be safer for servers so that you must explicitly +configure them to leave the cluster. This also results in a better experience for +clients, especially in cloud environments where they may be created and destroyed +often and users prefer not to wait for the 72 hour reap time for cleanup. #### Dropped Support for Protocol Version 1 @@ -69,7 +80,7 @@ to upgrade all agents to a newer version of Consul before upgrading to Consul #### Prepared Query Changes Consul version 0.7 adds a feature which allows prepared queries to store a -["Near" parameter](/docs/agent/http/query.html#near) in the query definition +[`Near` parameter](/docs/agent/http/query.html#near) in the query definition itself. This feature enables using the distance sorting features of prepared queries without explicitly providing the node to sort near in requests, but requires the agent servicing a request to send additional information about @@ -88,19 +99,19 @@ Consul version 0.7 added support for translating WAN addresses in certain and the agents need to be running version 0.7 or later in order to use this feature. -These translated addresses could break clients that are expecting local -addresses. A new [`X-Consul-Translate-Addresses`](/docs/agent/http.html#translate_header) +These translated addresses could break HTTP endpoint consumers that are +expecting local addresses, so a new [`X-Consul-Translate-Addresses`](/docs/agent/http.html#translate_header) header was added to allow clients to detect if translation is enabled for HTTP -responses, and a "lan" tag was added to `TaggedAddresses` for clients that need +responses. A "lan" tag was added to `TaggedAddresses` for clients that need the local address regardless of translation. -#### Changes to Outage Recovery and `peers.json` +#### Outage Recovery and `peers.json` Changes The `peers.json` file is no longer present by default and is only used when performing recovery. This file will be deleted after Consul starts and ingests -this file. Consul 0.7 also uses a new, automatically-created raft/peers.info file -to avoid ingesting the `peers.json` file on the first start after upgrading (it -is simply deleted on the first start after upgrading). +the file. Consul 0.7 also uses a new, automatically-created raft/peers.info file +to avoid ingesting the `peers.json` file on the first start after upgrading (the +`peers.json` file is simply deleted on the first start after upgrading). Please be sure to review the [Outage Recovery Guide](/docs/guides/outage.html) before upgrading for more details.