From 380c8b957dbe574f527938c920581ad4b8049691 Mon Sep 17 00:00:00 2001 From: James Phillips Date: Sat, 24 Jun 2017 09:36:53 -0700 Subject: [PATCH] Changes host-based node IDs from opt-out to opt-in. (#3187) --- agent/agent.go | 2 +- agent/agent_test.go | 20 ++++++++--- agent/config.go | 6 ++-- agent/config_test.go | 6 ++-- command/agent.go | 6 +++- command/agent_test.go | 42 +++++++++++++++++++++++ website/source/docs/agent/options.html.md | 6 +++- 7 files changed, 75 insertions(+), 13 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 0564b0b6c5..26f0416321 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -940,7 +940,7 @@ func (a *Agent) makeRandomID() (string, error) { // gopsutil change implementations without affecting in-place upgrades of nodes. func (a *Agent) makeNodeID() (string, error) { // If they've disabled host-based IDs then just make a random one. - if a.config.DisableHostNodeID { + if *a.config.DisableHostNodeID { return a.makeRandomID() } diff --git a/agent/agent_test.go b/agent/agent_test.go index af8dcfb633..5a4e6c5ff2 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -306,24 +306,34 @@ func TestAgent_makeNodeID(t *testing.T) { t.Fatalf("err: %v", err) } - // Calling again should yield the same ID since it's host-based. + // Calling again should yield a random ID by default. another, err := a.makeNodeID() if err != nil { t.Fatalf("err: %v", err) } - if id != another { + if id == another { t.Fatalf("bad: %s vs %s", id, another) } - // Turn off host-based IDs and try again. We should get a random ID. - a.Config.DisableHostNodeID = true - another, err = a.makeNodeID() + // Turn on host-based IDs and try again. We should get the same ID + // each time (and a different one from the random one above). + a.Config.DisableHostNodeID = Bool(false) + id, err = a.makeNodeID() if err != nil { t.Fatalf("err: %v", err) } if id == another { t.Fatalf("bad: %s vs %s", id, another) } + + // Calling again should yield the host-based ID. + another, err = a.makeNodeID() + if err != nil { + t.Fatalf("err: %v", err) + } + if id != another { + t.Fatalf("bad: %s vs %s", id, another) + } } func TestAgent_AddService(t *testing.T) { diff --git a/agent/config.go b/agent/config.go index 84a8fa90b7..380739b9b7 100644 --- a/agent/config.go +++ b/agent/config.go @@ -402,7 +402,7 @@ type Config struct { // DisableHostNodeID will prevent Consul from using information from the // host to generate a node ID, and will cause Consul to generate a // random ID instead. - DisableHostNodeID bool `mapstructure:"disable_host_node_id"` + DisableHostNodeID *bool `mapstructure:"disable_host_node_id"` // Node name is the name we use to advertise. Defaults to hostname. NodeName string `mapstructure:"node_name"` @@ -959,6 +959,8 @@ func DefaultConfig() *Config { EncryptVerifyIncoming: Bool(true), EncryptVerifyOutgoing: Bool(true), + + DisableHostNodeID: Bool(true), } } @@ -1616,7 +1618,7 @@ func MergeConfig(a, b *Config) *Config { if b.NodeID != "" { result.NodeID = b.NodeID } - if b.DisableHostNodeID == true { + if b.DisableHostNodeID != nil { result.DisableHostNodeID = b.DisableHostNodeID } if b.NodeName != "" { diff --git a/agent/config_test.go b/agent/config_test.go index a573d63987..b0dc90faf7 100644 --- a/agent/config_test.go +++ b/agent/config_test.go @@ -242,8 +242,8 @@ func TestDecodeConfig(t *testing.T) { c: &Config{DisableCoordinates: true}, }, { - in: `{"disable_host_node_id":true}`, - c: &Config{DisableHostNodeID: true}, + in: `{"disable_host_node_id":false}`, + c: &Config{DisableHostNodeID: Bool(false)}, }, { in: `{"dns_config":{"allow_stale":true}}`, @@ -1305,7 +1305,7 @@ func TestMergeConfig(t *testing.T) { Domain: "other", LogLevel: "info", NodeID: "bar", - DisableHostNodeID: true, + DisableHostNodeID: Bool(false), NodeName: "baz", ClientAddr: "127.0.0.2", BindAddr: "127.0.0.2", diff --git a/command/agent.go b/command/agent.go index 553a256236..b44f21668f 100644 --- a/command/agent.go +++ b/command/agent.go @@ -79,10 +79,13 @@ func (cmd *AgentCommand) readConfig() *agent.Config { f.StringVar((*string)(&cmdCfg.NodeID), "node-id", "", "A unique ID for this node across space and time. Defaults to a randomly-generated ID"+ " that persists in the data-dir.") - f.BoolVar(&cmdCfg.DisableHostNodeID, "disable-host-node-id", false, + + var disableHostNodeID configutil.BoolValue + f.Var(&disableHostNodeID, "disable-host-node-id", "Setting this to true will prevent Consul from using information from the"+ " host to generate a node ID, and will cause Consul to generate a"+ " random node ID instead.") + f.StringVar(&cmdCfg.Datacenter, "datacenter", "", "Datacenter of the agent.") f.StringVar(&cmdCfg.DataDir, "data-dir", "", "Path to a data directory to store agent state.") f.BoolVar(&cmdCfg.EnableUI, "ui", false, "Enables the built-in static web UI server.") @@ -238,6 +241,7 @@ func (cmd *AgentCommand) readConfig() *agent.Config { cmdCfg.DNSRecursors = append(cmdCfg.DNSRecursors, dnsRecursors...) cfg = agent.MergeConfig(cfg, &cmdCfg) + disableHostNodeID.Merge(cfg.DisableHostNodeID) if cfg.NodeName == "" { hostname, err := os.Hostname() diff --git a/command/agent_test.go b/command/agent_test.go index 3948a7b225..6c74777d4a 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -291,6 +291,48 @@ func TestReadCliConfig(t *testing.T) { } } +func TestAgent_HostBasedIDs(t *testing.T) { + t.Parallel() + tmpDir := testutil.TempDir(t, "consul") + defer os.RemoveAll(tmpDir) + + shutdownCh := make(chan struct{}) + defer close(shutdownCh) + + // Host-based IDs are disabled by default. + { + cmd := &AgentCommand{ + args: []string{ + "-data-dir", tmpDir, + }, + ShutdownCh: shutdownCh, + BaseCommand: baseCommand(cli.NewMockUi()), + } + + config := cmd.readConfig() + if *config.DisableHostNodeID != true { + t.Fatalf("expected host-based node IDs to be disabled") + } + } + + // Try enabling host-based IDs. + { + cmd := &AgentCommand{ + args: []string{ + "-data-dir", tmpDir, + "-disable-host-node-id=false", + }, + ShutdownCh: shutdownCh, + BaseCommand: baseCommand(cli.NewMockUi()), + } + + config := cmd.readConfig() + if *config.DisableHostNodeID != false { + t.Fatalf("expected host-based node IDs to be enabled") + } + } +} + func TestRetryJoinFail(t *testing.T) { t.Skip("fs: skipping tests that use cmd.Run until signal handling is fixed") t.Parallel() diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index c86c672468..7bef2f1130 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -141,7 +141,11 @@ will exit with an error at startup. * `-disable-host-node-id` - Setting this to true will prevent Consul from using information from the host to generate a deterministic node ID, and will instead generate a random node ID which will be persisted in the data directory. This is useful - when running multiple Consul agents on the same host for testing. This defaults to false. + when running multiple Consul agents on the same host for testing. This defaults to false in Consul prior + to version 0.8.5 and in 0.8.5 and later defaults to true, so you must opt-in for host-based IDs. Host-based + IDs are generated using https://github.com/shirou/gopsutil/tree/master/host, which is shared with HashiCorp's + [Nomad](https://www.nomadproject.io/), so if you opt-in to host-based IDs then Consul and Nomad will use + information on the host to automatically assign the same ID in both systems. * `-dns-port` - the DNS port to listen on. This overrides the default port 8600. This is available in Consul 0.7 and later.