From ad19c0afc21f76c0aacf9d6d1a6d4c86493d61a5 Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Thu, 15 Jan 2015 17:24:15 -0800
Subject: [PATCH 1/9] agent: beginning refactor

---
 api/api.go                       |  25 +++----
 api/api_test.go                  |  18 +++--
 api/status_test.go               |  16 ++---
 command/agent/agent_test.go      |  32 +--------
 command/agent/command.go         |  20 +++---
 command/agent/config.go          | 120 ++++---------------------------
 command/agent/config_test.go     | 109 ++--------------------------
 command/agent/http.go            |  44 +++++-------
 command/agent/rpc_client.go      |  23 +++---
 command/agent/rpc_client_test.go |  17 ++---
 10 files changed, 91 insertions(+), 333 deletions(-)

diff --git a/api/api.go b/api/api.go
index 5617293e4..0ff9a22c2 100644
--- a/api/api.go
+++ b/api/api.go
@@ -120,8 +120,8 @@ func DefaultConfig() *Config {
 		HttpClient: http.DefaultClient,
 	}
 
-	if len(os.Getenv("CONSUL_HTTP_ADDR")) > 0 {
-		config.Address = os.Getenv("CONSUL_HTTP_ADDR")
+	if addr := os.Getenv("CONSUL_HTTP_ADDR"); addr != "" {
+		config.Address = addr
 	}
 
 	return config
@@ -137,11 +137,7 @@ func NewClient(config *Config) (*Client, error) {
 	// bootstrap the config
 	defConfig := DefaultConfig()
 
-	switch {
-	case len(config.Address) != 0:
-	case len(os.Getenv("CONSUL_HTTP_ADDR")) > 0:
-		config.Address = os.Getenv("CONSUL_HTTP_ADDR")
-	default:
+	if len(config.Address) == 0 {
 		config.Address = defConfig.Address
 	}
 
@@ -153,14 +149,15 @@ func NewClient(config *Config) (*Client, error) {
 		config.HttpClient = defConfig.HttpClient
 	}
 
-	if strings.HasPrefix(config.Address, "unix://") {
-		shortStr := strings.TrimPrefix(config.Address, "unix://")
-		t := &http.Transport{}
-		t.Dial = func(_, _ string) (net.Conn, error) {
-			return net.Dial("unix", shortStr)
+	if parts := strings.SplitN(config.Address, "unix://", 2); len(parts) == 2 {
+		config.HttpClient = &http.Client{
+			Transport: &http.Transport{
+				Dial: func(_, _ string) (net.Conn, error) {
+					return net.Dial("unix", parts[1])
+				},
+			},
 		}
-		config.HttpClient.Transport = t
-		config.Address = shortStr
+		config.Address = parts[1]
 	}
 
 	client := &Client{
diff --git a/api/api_test.go b/api/api_test.go
index 488fcb1ee..a8c826ed2 100644
--- a/api/api_test.go
+++ b/api/api_test.go
@@ -42,6 +42,10 @@ type testServerConfig struct {
 	Ports     testPortConfig     `json:"ports,omitempty"`
 }
 
+// Callback functions for modifying config
+type configCallback func(c *Config)
+type serverConfigCallback func(c *testServerConfig)
+
 func defaultConfig() *testServerConfig {
 	return &testServerConfig{
 		Bootstrap: true,
@@ -72,7 +76,7 @@ func newTestServer(t *testing.T) *testServer {
 	return newTestServerWithConfig(t, func(c *testServerConfig) {})
 }
 
-func newTestServerWithConfig(t *testing.T, cb func(c *testServerConfig)) *testServer {
+func newTestServerWithConfig(t *testing.T, cb serverConfigCallback) *testServer {
 	if path, err := exec.LookPath("consul"); err != nil || path == "" {
 		t.Log("consul not found on $PATH, skipping")
 		t.SkipNow()
@@ -131,15 +135,21 @@ func makeClient(t *testing.T) (*Client, *testServer) {
 	}, func(c *testServerConfig) {})
 }
 
-func makeClientWithConfig(t *testing.T, clientConfig func(c *Config), serverConfig func(c *testServerConfig)) (*Client, *testServer) {
-	server := newTestServerWithConfig(t, serverConfig)
+func makeClientWithConfig(t *testing.T, cb1 configCallback, cb2 serverConfigCallback) (*Client, *testServer) {
+	// Make client config
 	conf := DefaultConfig()
-	clientConfig(conf)
+	cb1(conf)
+	fmt.Printf("%#v\n", conf.HttpClient.Transport)
+
+	// Create client
 	client, err := NewClient(conf)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
 
+	// Create server
+	server := newTestServerWithConfig(t, cb2)
+
 	// Allow the server some time to start, and verify we have a leader.
 	testutil.WaitForResult(func() (bool, error) {
 		req := client.newRequest("GET", "/v1/catalog/nodes")
diff --git a/api/status_test.go b/api/status_test.go
index 5e7acd274..61b0a435e 100644
--- a/api/status_test.go
+++ b/api/status_test.go
@@ -1,8 +1,9 @@
 package api
 
 import (
+	"fmt"
 	"io/ioutil"
-	"os/user"
+	"os"
 	"runtime"
 	"testing"
 )
@@ -29,25 +30,20 @@ func TestStatusLeaderUnix(t *testing.T) {
 
 	tempdir, err := ioutil.TempDir("", "consul-test-")
 	if err != nil {
-		t.Fatal("Could not create a working directory")
+		t.Fatalf("err: %s", err)
 	}
-
-	socket := "unix://" + tempdir + "/unix-http-test.sock"
+	defer os.RemoveAll(tempdir)
+	socket := fmt.Sprintf("unix://%s/test.sock", tempdir)
 
 	clientConfig := func(c *Config) {
 		c.Address = socket
 	}
 
 	serverConfig := func(c *testServerConfig) {
-		user, err := user.Current()
-		if err != nil {
-			t.Fatal("Could not get current user")
-		}
-
 		if c.Addresses == nil {
 			c.Addresses = &testAddressConfig{}
 		}
-		c.Addresses.HTTP = socket + ";" + user.Uid + ";" + user.Gid + ";640"
+		c.Addresses.HTTP = socket
 	}
 
 	c, s := makeClientWithConfig(t, clientConfig, serverConfig)
diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go
index 91cb5c1ef..d7b7d9153 100644
--- a/command/agent/agent_test.go
+++ b/command/agent/agent_test.go
@@ -7,7 +7,6 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
-	"os/user"
 	"path/filepath"
 	"reflect"
 	"runtime"
@@ -125,7 +124,7 @@ func TestAgentStartStop(t *testing.T) {
 	}
 }
 
-func TestAgent_RPCPingTCP(t *testing.T) {
+func TestAgent_RPCPing(t *testing.T) {
 	dir, agent := makeAgent(t, nextConfig())
 	defer os.RemoveAll(dir)
 	defer agent.Shutdown()
@@ -136,35 +135,6 @@ func TestAgent_RPCPingTCP(t *testing.T) {
 	}
 }
 
-func TestAgent_RPCPingUnix(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.SkipNow()
-	}
-
-	nextConf := nextConfig()
-
-	tempdir, err := ioutil.TempDir("", "consul-test-")
-	if err != nil {
-		t.Fatal("Could not create a working directory")
-	}
-
-	user, err := user.Current()
-	if err != nil {
-		t.Fatal("Could not get current user")
-	}
-
-	nextConf.Addresses.RPC = "unix://" + tempdir + "/unix-rpc-test.sock;" + user.Uid + ";" + user.Gid + ";640"
-
-	dir, agent := makeAgent(t, nextConf)
-	defer os.RemoveAll(dir)
-	defer agent.Shutdown()
-
-	var out struct{}
-	if err := agent.RPC("Status.Ping", struct{}{}, &out); err != nil {
-		t.Fatalf("err: %v", err)
-	}
-}
-
 func TestAgent_AddService(t *testing.T) {
 	dir, agent := makeAgent(t, nextConfig())
 	defer os.RemoveAll(dir)
diff --git a/command/agent/command.go b/command/agent/command.go
index b9a82e19a..a677b03b9 100644
--- a/command/agent/command.go
+++ b/command/agent/command.go
@@ -295,9 +295,15 @@ func (c *Command) setupAgent(config *Config, logOutput io.Writer, logWriter *log
 		return err
 	}
 
-	if _, ok := rpcAddr.(*net.UnixAddr); ok {
-		// Remove the socket if it exists, or we'll get a bind error
-		_ = os.Remove(rpcAddr.String())
+	if path, ok := unixSocketAddr(config.Addresses.RPC); ok {
+		// Remove the socket if it exists, or we'll get a bind error. This
+		// is necessary to avoid situations where Consul cannot start if the
+		// socket file exists in case of unexpected termination.
+		if _, err := os.Stat(path); err == nil {
+			if err := os.Remove(path); err != nil {
+				return err
+			}
+		}
 	}
 
 	rpcListener, err := net.Listen(rpcAddr.Network(), rpcAddr.String())
@@ -307,14 +313,6 @@ func (c *Command) setupAgent(config *Config, logOutput io.Writer, logWriter *log
 		return err
 	}
 
-	if _, ok := rpcAddr.(*net.UnixAddr); ok {
-		if err := adjustUnixSocketPermissions(config.Addresses.RPC); err != nil {
-			agent.Shutdown()
-			c.Ui.Error(fmt.Sprintf("Error adjusting Unix socket permissions: %s", err))
-			return err
-		}
-	}
-
 	// Start the IPC layer
 	c.Ui.Output("Starting Consul agent RPC...")
 	c.rpcServer = NewAgentRPC(agent, rpcListener, logOutput, logWriter)
diff --git a/command/agent/config.go b/command/agent/config.go
index 74ed4c3ad..92b9e64d3 100644
--- a/command/agent/config.go
+++ b/command/agent/config.go
@@ -7,11 +7,8 @@ import (
 	"io"
 	"net"
 	"os"
-	"os/user"
 	"path/filepath"
-	"regexp"
 	"sort"
-	"strconv"
 	"strings"
 	"time"
 
@@ -348,89 +345,13 @@ type Config struct {
 	WatchPlans []*watch.WatchPlan `mapstructure:"-" json:"-"`
 }
 
-// UnixSocket contains the parameters for a Unix socket interface
-type UnixSocket struct {
-	// Path to the socket on-disk
-	Path string
-
-	// uid of the owner of the socket
-	Uid int
-
-	// gid of the group of the socket
-	Gid int
-
-	// Permissions for the socket file
-	Permissions os.FileMode
-}
-
-func populateUnixSocket(addr string) (*UnixSocket, error) {
+// unixSocketAddr tests if a given address describes a domain socket,
+// and returns the relevant path part of the string if it is.
+func unixSocketAddr(addr string) (string, bool) {
 	if !strings.HasPrefix(addr, "unix://") {
-		return nil, fmt.Errorf("Failed to parse Unix address, format is unix://[path];[user];[group];[mode]: %v", addr)
+		return "", false
 	}
-
-	splitAddr := strings.Split(strings.TrimPrefix(addr, "unix://"), ";")
-	if len(splitAddr) != 4 {
-		return nil, fmt.Errorf("Failed to parse Unix address, format is unix://[path];[user];[group];[mode]: %v", addr)
-	}
-
-	ret := &UnixSocket{Path: splitAddr[0]}
-
-	var userVal *user.User
-	var err error
-
-	regex := regexp.MustCompile("[\\d]+")
-	if regex.MatchString(splitAddr[1]) {
-		userVal, err = user.LookupId(splitAddr[1])
-	} else {
-		userVal, err = user.Lookup(splitAddr[1])
-	}
-	if err != nil {
-		return nil, fmt.Errorf("Invalid user given for Unix socket ownership: %v", splitAddr[1])
-	}
-
-	if uid64, err := strconv.ParseInt(userVal.Uid, 10, 32); err != nil {
-		return nil, fmt.Errorf("Failed to parse given user ID of %v into integer", userVal.Uid)
-	} else {
-		ret.Uid = int(uid64)
-	}
-
-	// Go doesn't currently have a way to look up gid from group name,
-	// so require a numeric gid; see
-	// https://codereview.appspot.com/101310044
-	if gid64, err := strconv.ParseInt(splitAddr[2], 10, 32); err != nil {
-		return nil, fmt.Errorf("Socket group must be given as numeric gid. Failed to parse given group ID of %v into integer", splitAddr[2])
-	} else {
-		ret.Gid = int(gid64)
-	}
-
-	if mode, err := strconv.ParseUint(splitAddr[3], 8, 32); err != nil {
-		return nil, fmt.Errorf("Failed to parse given mode of %v into integer", splitAddr[3])
-	} else {
-		if mode > 0777 {
-			return nil, fmt.Errorf("Given mode is invalid; must be an octal number between 0 and 777")
-		} else {
-			ret.Permissions = os.FileMode(mode)
-		}
-	}
-
-	return ret, nil
-}
-
-func adjustUnixSocketPermissions(addr string) error {
-	sock, err := populateUnixSocket(addr)
-	if err != nil {
-		return err
-	}
-
-	if err = os.Chown(sock.Path, sock.Uid, sock.Gid); err != nil {
-		return fmt.Errorf("Error attempting to change socket permissions to userid %v and groupid %v: %v", sock.Uid, sock.Gid, err)
-	}
-
-	if err = os.Chmod(sock.Path, sock.Permissions); err != nil {
-		return fmt.Errorf("Error attempting to change socket permissions to mode %v: %v", sock.Permissions, err)
-	}
-
-	return nil
+	return strings.TrimPrefix(addr, "unix://"), true
 }
 
 type dirEnts []os.FileInfo
@@ -485,31 +406,14 @@ func (c *Config) ClientListener(override string, port int) (net.Addr, error) {
 		addr = c.ClientAddr
 	}
 
-	switch {
-	case strings.HasPrefix(addr, "unix://"):
-		sock, err := populateUnixSocket(addr)
-		if err != nil {
-			return nil, err
-		}
-
-		return &net.UnixAddr{Name: sock.Path, Net: "unix"}, nil
-
-	default:
-		ip := net.ParseIP(addr)
-		if ip == nil {
-			return nil, fmt.Errorf("Failed to parse IP: %v", addr)
-		}
-
-		if ip.IsUnspecified() {
-			ip = net.ParseIP("127.0.0.1")
-		}
-
-		if ip == nil {
-			return nil, fmt.Errorf("Failed to parse IP 127.0.0.1")
-		}
-
-		return &net.TCPAddr{IP: ip, Port: port}, nil
+	if path, ok := unixSocketAddr(addr); ok {
+		return &net.UnixAddr{Name: path, Net: "unix"}, nil
 	}
+	ip := net.ParseIP(addr)
+	if ip == nil {
+		return nil, fmt.Errorf("Failed to parse IP: %v", addr)
+	}
+	return &net.TCPAddr{IP: ip, Port: port}, nil
 }
 
 // DecodeConfig reads the configuration from the given reader in JSON
diff --git a/command/agent/config_test.go b/command/agent/config_test.go
index f10c5b724..fa7bf6f27 100644
--- a/command/agent/config_test.go
+++ b/command/agent/config_test.go
@@ -4,12 +4,9 @@ import (
 	"bytes"
 	"encoding/base64"
 	"io/ioutil"
-	"net"
 	"os"
-	"os/user"
 	"path/filepath"
 	"reflect"
-	"runtime"
 	"strings"
 	"testing"
 	"time"
@@ -1073,107 +1070,13 @@ func TestReadConfigPaths_dir(t *testing.T) {
 }
 
 func TestUnixSockets(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.SkipNow()
+	path1, ok := unixSocketAddr("unix:///path/to/socket")
+	if !ok || path1 != "/path/to/socket" {
+		t.Fatalf("bad: %v %v", ok, path1)
 	}
 
-	usr, err := user.Current()
-	if err != nil {
-		t.Fatal("Could not get current user: ", err)
-	}
-
-	tempdir, err := ioutil.TempDir("", "consul-test-")
-	if err != nil {
-		t.Fatal("Could not create a working directory: ", err)
-	}
-
-	type SocketTestData struct {
-		Path string
-		Uid  string
-		Gid  string
-		Mode string
-	}
-
-	testUnixSocketPopulation := func(s SocketTestData) (*UnixSocket, error) {
-		return populateUnixSocket("unix://" + s.Path + ";" + s.Uid + ";" + s.Gid + ";" + s.Mode)
-	}
-
-	testUnixSocketPermissions := func(s SocketTestData) error {
-		return adjustUnixSocketPermissions("unix://" + s.Path + ";" + s.Uid + ";" + s.Gid + ";" + s.Mode)
-	}
-
-	_, err = populateUnixSocket("tcp://abc123")
-	if err == nil {
-		t.Fatal("Should have rejected invalid scheme")
-	}
-
-	_, err = populateUnixSocket("unix://x;y;z")
-	if err == nil {
-		t.Fatal("Should have rejected invalid number of parameters in Unix socket definition")
-	}
-
-	std := SocketTestData{
-		Path: tempdir + "/unix-config-test.sock",
-		Uid:  usr.Uid,
-		Gid:  usr.Gid,
-		Mode: "640",
-	}
-
-	std.Uid = "orasdfdsnfoinweroiu"
-	_, err = testUnixSocketPopulation(std)
-	if err == nil {
-		t.Fatal("Did not error on invalid username")
-	}
-
-	std.Uid = usr.Username
-	std.Gid = "foinfphawepofhewof"
-	_, err = testUnixSocketPopulation(std)
-	if err == nil {
-		t.Fatal("Did not error on invalid group (a name, must be gid)")
-	}
-
-	std.Gid = usr.Gid
-	std.Mode = "999"
-	_, err = testUnixSocketPopulation(std)
-	if err == nil {
-		t.Fatal("Did not error on invalid socket mode")
-	}
-
-	std.Uid = usr.Username
-	std.Mode = "640"
-	_, err = testUnixSocketPopulation(std)
-	if err != nil {
-		t.Fatal("Unix socket test failed (using username): ", err)
-	}
-
-	std.Uid = usr.Uid
-	sock, err := testUnixSocketPopulation(std)
-	if err != nil {
-		t.Fatal("Unix socket test failed (using uid): ", err)
-	}
-
-	addr := &net.UnixAddr{Name: sock.Path, Net: "unix"}
-	_, err = net.Listen(addr.Network(), addr.String())
-	if err != nil {
-		t.Fatal("Error creating socket for futher tests: ", err)
-	}
-
-	std.Uid = "-999999"
-	err = testUnixSocketPermissions(std)
-	if err == nil {
-		t.Fatal("Did not error on invalid uid")
-	}
-
-	std.Uid = usr.Uid
-	std.Gid = "-999999"
-	err = testUnixSocketPermissions(std)
-	if err == nil {
-		t.Fatal("Did not error on invalid uid")
-	}
-
-	std.Gid = usr.Gid
-	err = testUnixSocketPermissions(std)
-	if err != nil {
-		t.Fatal("Adjusting socket permissions failed: ", err)
+	path2, ok := unixSocketAddr("notunix://blah")
+	if ok || path2 != "" {
+		t.Fatalf("bad: %v %v", ok, path2)
 	}
 }
diff --git a/command/agent/http.go b/command/agent/http.go
index d480de816..2d26e60b1 100644
--- a/command/agent/http.go
+++ b/command/agent/http.go
@@ -59,9 +59,13 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS
 			return nil, err
 		}
 
-		if _, ok := httpAddr.(*net.UnixAddr); ok {
-			// Remove the socket if it exists, or we'll get a bind error
-			_ = os.Remove(httpAddr.String())
+		if path, ok := unixSocketAddr(config.Addresses.HTTPS); ok {
+			// See command/agent/config.go
+			if _, err := os.Stat(path); err == nil {
+				if err := os.Remove(path); err != nil {
+					return nil, err
+				}
+			}
 		}
 
 		ln, err := net.Listen(httpAddr.Network(), httpAddr.String())
@@ -69,18 +73,10 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS
 			return nil, fmt.Errorf("Failed to get Listen on %s: %v", httpAddr.String(), err)
 		}
 
-		switch httpAddr.(type) {
-		case *net.UnixAddr:
-			if err := adjustUnixSocketPermissions(config.Addresses.HTTPS); err != nil {
-				return nil, err
-			}
+		if _, ok := unixSocketAddr(config.Addresses.HTTPS); ok {
 			list = tls.NewListener(ln, tlsConfig)
-
-		case *net.TCPAddr:
+		} else {
 			list = tls.NewListener(tcpKeepAliveListener{ln.(*net.TCPListener)}, tlsConfig)
-
-		default:
-			return nil, fmt.Errorf("Error determining address type when attempting to get Listen on %s: %v", httpAddr.String(), err)
 		}
 
 		// Create the mux
@@ -108,9 +104,13 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS
 			return nil, fmt.Errorf("Failed to get ClientListener address:port: %v", err)
 		}
 
-		if _, ok := httpAddr.(*net.UnixAddr); ok {
-			// Remove the socket if it exists, or we'll get a bind error
-			_ = os.Remove(httpAddr.String())
+		if path, ok := unixSocketAddr(config.Addresses.HTTP); ok {
+			// See command/agent/config.go
+			if _, err := os.Stat(path); err == nil {
+				if err := os.Remove(path); err != nil {
+					return nil, err
+				}
+			}
 		}
 
 		ln, err := net.Listen(httpAddr.Network(), httpAddr.String())
@@ -118,18 +118,10 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS
 			return nil, fmt.Errorf("Failed to get Listen on %s: %v", httpAddr.String(), err)
 		}
 
-		switch httpAddr.(type) {
-		case *net.UnixAddr:
-			if err := adjustUnixSocketPermissions(config.Addresses.HTTP); err != nil {
-				return nil, err
-			}
+		if _, ok := unixSocketAddr(config.Addresses.HTTP); ok {
 			list = ln
-
-		case *net.TCPAddr:
+		} else {
 			list = tcpKeepAliveListener{ln.(*net.TCPListener)}
-
-		default:
-			return nil, fmt.Errorf("Error determining address type when attempting to get Listen on %s: %v", httpAddr.String(), err)
 		}
 
 		// Create the mux
diff --git a/command/agent/rpc_client.go b/command/agent/rpc_client.go
index 1490674f1..cbc9689cf 100644
--- a/command/agent/rpc_client.go
+++ b/command/agent/rpc_client.go
@@ -81,24 +81,19 @@ func (c *RPCClient) send(header *requestHeader, obj interface{}) error {
 // NewRPCClient is used to create a new RPC client given the address.
 // This will properly dial, handshake, and start listening
 func NewRPCClient(addr string) (*RPCClient, error) {
-	sanedAddr := os.Getenv("CONSUL_RPC_ADDR")
-	if len(sanedAddr) == 0 {
-		sanedAddr = addr
-	}
+	var conn net.Conn
+	var err error
 
-	mode := "tcp"
-
-	if strings.HasPrefix(sanedAddr, "unix://") {
-		sanedAddr = strings.TrimPrefix(sanedAddr, "unix://")
-	}
-
-	if strings.HasPrefix(sanedAddr, "/") {
-		mode = "unix"
+	if envAddr := os.Getenv("CONSUL_RPC_ADDR"); envAddr != "" {
+		addr = envAddr
 	}
 
 	// Try to dial to agent
-	conn, err := net.Dial(mode, sanedAddr)
-	if err != nil {
+	mode := "tcp"
+	if strings.HasPrefix(addr, "/") {
+		mode = "unix"
+	}
+	if conn, err = net.Dial(mode, addr); err != nil {
 		return nil, err
 	}
 
diff --git a/command/agent/rpc_client_test.go b/command/agent/rpc_client_test.go
index 2d8dfc9c0..8516943a1 100644
--- a/command/agent/rpc_client_test.go
+++ b/command/agent/rpc_client_test.go
@@ -9,7 +9,6 @@ import (
 	"io/ioutil"
 	"net"
 	"os"
-	"os/user"
 	"runtime"
 	"strings"
 	"testing"
@@ -223,19 +222,13 @@ func TestRPCClientStatsUnix(t *testing.T) {
 
 	tempdir, err := ioutil.TempDir("", "consul-test-")
 	if err != nil {
-		t.Fatal("Could not create a working directory: ", err)
+		t.Fatalf("err: %s", err)
 	}
 
-	user, err := user.Current()
-	if err != nil {
-		t.Fatal("Could not get current user: ", err)
-	}
-
-	cb := func(c *Config) {
-		c.Addresses.RPC = "unix://" + tempdir + "/unix-rpc-test.sock;" + user.Uid + ";" + user.Gid + ";640"
-	}
-
-	p1 := testRPCClientWithConfig(t, cb)
+	p1 := testRPCClientWithConfig(t, func(c *Config) {
+		c.Addresses.RPC = fmt.Sprintf("unix://%s/test.sock", tempdir)
+	})
+	defer p1.Close()
 
 	stats, err := p1.client.Stats()
 	if err != nil {

From 61d2f71d14d8c18ab3cafc1a57b31dfba41ee133 Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Fri, 16 Jan 2015 09:14:52 -0800
Subject: [PATCH 2/9] agent: only ignore errors on IsNotExist()

---
 command/agent/command.go                      |  6 ++----
 command/agent/http.go                         | 12 ++++-------
 .../source/docs/agent/options.html.markdown   | 21 ++++++++++---------
 3 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/command/agent/command.go b/command/agent/command.go
index a677b03b9..3afafca5e 100644
--- a/command/agent/command.go
+++ b/command/agent/command.go
@@ -299,10 +299,8 @@ func (c *Command) setupAgent(config *Config, logOutput io.Writer, logWriter *log
 		// Remove the socket if it exists, or we'll get a bind error. This
 		// is necessary to avoid situations where Consul cannot start if the
 		// socket file exists in case of unexpected termination.
-		if _, err := os.Stat(path); err == nil {
-			if err := os.Remove(path); err != nil {
-				return err
-			}
+		if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+			return err
 		}
 	}
 
diff --git a/command/agent/http.go b/command/agent/http.go
index 2d26e60b1..19a3dc0bf 100644
--- a/command/agent/http.go
+++ b/command/agent/http.go
@@ -61,10 +61,8 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS
 
 		if path, ok := unixSocketAddr(config.Addresses.HTTPS); ok {
 			// See command/agent/config.go
-			if _, err := os.Stat(path); err == nil {
-				if err := os.Remove(path); err != nil {
-					return nil, err
-				}
+			if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+				return nil, err
 			}
 		}
 
@@ -106,10 +104,8 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS
 
 		if path, ok := unixSocketAddr(config.Addresses.HTTP); ok {
 			// See command/agent/config.go
-			if _, err := os.Stat(path); err == nil {
-				if err := os.Remove(path); err != nil {
-					return nil, err
-				}
+			if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
+				return nil, err
 			}
 		}
 
diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown
index 6aa6b61d8..b7423dabe 100644
--- a/website/source/docs/agent/options.html.markdown
+++ b/website/source/docs/agent/options.html.markdown
@@ -240,16 +240,17 @@ definitions support being updated during a reload.
    up to the TTL value.
 
 * `addresses` - This is a nested object that allows setting bind addresses. For `rpc`
-   and `http`, a Unix socket can be specified in the following form:
-   unix://[/path/to/socket];[username|uid];[gid];[mode]. The socket will be created
-   in the specified location with the given username or uid, gid, and mode. The
-   user Consul is running as must have appropriate permissions to change the socket
-   ownership to the given uid or gid. When running Consul agent commands against
-   Unix socket interfaces, use the `-rpc-addr` or `-http-addr` arguments to specify
-   the path to the socket, e.g. "unix://path/to/socket". You can also place the desired
-   values in `CONSUL_RPC_ADDR` and `CONSUL_HTTP_ADDR` environment variables. For TCP
-   addresses, these should be in the form ip:port.
-   The following keys are valid:
+  and `http`, a Unix socket can be specified in the following form
+  `unix:///path/to/socket`. A new domain socket will be created at the given
+  path. Any existing socket file (or any other kind of file) at the specified
+  path will be **overwritten**, so use caution when configuring this argument.
+
+  When running Consul agent commands against Unix socket interfaces, use the
+  `-rpc-addr` or `-http-addr` arguments to specify the path to the socket. You
+  can also place the desired values in `CONSUL_RPC_ADDR` and `CONSUL_HTTP_ADDR`
+  environment variables. For TCP addresses, these should be in the form ip:port.
+
+  The following keys are valid:
     * `dns` - The DNS server. Defaults to `client_addr`
     * `http` - The HTTP API. Defaults to `client_addr`
     * `rpc` - The RPC endpoint. Defaults to `client_addr`

From 705801ccca3659ff36fe8d8a3aa58703eab5b269 Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Fri, 16 Jan 2015 09:58:37 -0800
Subject: [PATCH 3/9] agent: fixing up tests

---
 api/api_test.go                  |  1 -
 command/agent/agent_test.go      |  1 -
 command/agent/http_test.go       | 58 ++++++++++++++++++++++++++++++
 command/agent/rpc_client_test.go | 62 +++++++++++++++++---------------
 4 files changed, 91 insertions(+), 31 deletions(-)

diff --git a/api/api_test.go b/api/api_test.go
index a8c826ed2..722d78b56 100644
--- a/api/api_test.go
+++ b/api/api_test.go
@@ -139,7 +139,6 @@ func makeClientWithConfig(t *testing.T, cb1 configCallback, cb2 serverConfigCall
 	// Make client config
 	conf := DefaultConfig()
 	cb1(conf)
-	fmt.Printf("%#v\n", conf.HttpClient.Transport)
 
 	// Create client
 	client, err := NewClient(conf)
diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go
index d7b7d9153..0add36702 100644
--- a/command/agent/agent_test.go
+++ b/command/agent/agent_test.go
@@ -9,7 +9,6 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
-	"runtime"
 	"sync/atomic"
 	"testing"
 	"time"
diff --git a/command/agent/http_test.go b/command/agent/http_test.go
index eca844f1d..fc5067b46 100644
--- a/command/agent/http_test.go
+++ b/command/agent/http_test.go
@@ -6,10 +6,12 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strconv"
 	"testing"
 	"time"
@@ -19,7 +21,15 @@ import (
 )
 
 func makeHTTPServer(t *testing.T) (string, *HTTPServer) {
+	return makeHTTPServerWithConfig(t, nil)
+}
+
+func makeHTTPServerWithConfig(t *testing.T, cb func(c *Config)) (string, *HTTPServer) {
 	conf := nextConfig()
+	if cb != nil {
+		cb(conf)
+	}
+
 	dir, agent := makeAgent(t, conf)
 	uiDir := filepath.Join(dir, "ui")
 	if err := os.Mkdir(uiDir, 755); err != nil {
@@ -43,6 +53,54 @@ func encodeReq(obj interface{}) io.ReadCloser {
 	return ioutil.NopCloser(buf)
 }
 
+func TestHTTPServer_UnixSocket(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.SkipNow()
+	}
+
+	tempDir, err := ioutil.TempDir("", "consul")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	defer os.RemoveAll(tempDir)
+	socket := filepath.Join(tempDir, "test.sock")
+
+	dir, srv := makeHTTPServerWithConfig(t, func(c *Config) {
+		c.Addresses.HTTP = "unix://" + socket
+	})
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	defer srv.agent.Shutdown()
+
+	// Ensure the socket was created
+	if _, err := os.Stat(socket); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// Ensure we can get a response from the socket.
+	path, _ := unixSocketAddr(srv.agent.config.Addresses.HTTP)
+	client := &http.Client{
+		Transport: &http.Transport{
+			Dial: func(_, _ string) (net.Conn, error) {
+				return net.Dial("unix", path)
+			},
+		},
+	}
+
+	// This URL doesn't look like it makes sense, but the scheme (http://) and
+	// the host (127.0.0.1) are required by the HTTP client library. In reality
+	// this will just use the custom dialer and talk to the socket.
+	resp, err := client.Get("http://127.0.0.1/v1/agent/self")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	defer resp.Body.Close()
+
+	if body, err := ioutil.ReadAll(resp.Body); err != nil || len(body) == 0 {
+		t.Fatalf("bad: %s %v", body, err)
+	}
+}
+
 func TestSetIndex(t *testing.T) {
 	resp := httptest.NewRecorder()
 	setIndex(resp, 1000)
diff --git a/command/agent/rpc_client_test.go b/command/agent/rpc_client_test.go
index 8516943a1..48d833564 100644
--- a/command/agent/rpc_client_test.go
+++ b/command/agent/rpc_client_test.go
@@ -9,6 +9,7 @@ import (
 	"io/ioutil"
 	"net"
 	"os"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"testing"
@@ -68,6 +69,38 @@ func testRPCClientWithConfig(t *testing.T, cb func(c *Config)) *rpcParts {
 	}
 }
 
+func TestRPCClient_UnixSocket(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.SkipNow()
+	}
+
+	tempDir, err := ioutil.TempDir("", "consul")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	defer os.RemoveAll(tempDir)
+	socket := filepath.Join(tempDir, "test.sock")
+
+	p1 := testRPCClientWithConfig(t, func(c *Config) {
+		c.Addresses.RPC = "unix://" + socket
+	})
+	defer p1.Close()
+
+	// Ensure the socket was created
+	if _, err := os.Stat(socket); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// Ensure we can talk with the socket
+	mem, err := p1.client.LANMembers()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if len(mem) != 1 {
+		t.Fatalf("bad: %#v", mem)
+	}
+}
+
 func TestRPCClientForceLeave(t *testing.T) {
 	p1 := testRPCClient(t)
 	p2 := testRPCClient(t)
@@ -215,35 +248,6 @@ func TestRPCClientStats(t *testing.T) {
 	}
 }
 
-func TestRPCClientStatsUnix(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.SkipNow()
-	}
-
-	tempdir, err := ioutil.TempDir("", "consul-test-")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	p1 := testRPCClientWithConfig(t, func(c *Config) {
-		c.Addresses.RPC = fmt.Sprintf("unix://%s/test.sock", tempdir)
-	})
-	defer p1.Close()
-
-	stats, err := p1.client.Stats()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	if _, ok := stats["agent"]; !ok {
-		t.Fatalf("bad: %#v", stats)
-	}
-
-	if _, ok := stats["consul"]; !ok {
-		t.Fatalf("bad: %#v", stats)
-	}
-}
-
 func TestRPCClientLeave(t *testing.T) {
 	p1 := testRPCClient(t)
 	defer p1.Close()

From 208b6c0677eaa9a33f2e26b010bda8e00bb3318b Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Fri, 16 Jan 2015 10:19:04 -0800
Subject: [PATCH 4/9] command: style fixes for env addr reading

---
 command/rpc.go | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/command/rpc.go b/command/rpc.go
index f0c9e5b1f..5c647aed3 100644
--- a/command/rpc.go
+++ b/command/rpc.go
@@ -43,12 +43,10 @@ func HTTPClient(addr string) (*consulapi.Client, error) {
 // HTTPClientDC returns a new Consul HTTP client with the given address and datacenter
 func HTTPClientDC(addr, dc string) (*consulapi.Client, error) {
 	conf := consulapi.DefaultConfig()
-	switch {
-	case len(os.Getenv("CONSUL_HTTP_ADDR")) > 0:
-		conf.Address = os.Getenv("CONSUL_HTTP_ADDR")
-	default:
-		conf.Address = addr
+	if envAddr := os.Getenv("CONSUL_HTTP_ADDR"); addr != "" {
+		addr = envAddr
 	}
+	conf.Address = addr
 	conf.Datacenter = dc
 	return consulapi.NewClient(conf)
 }

From 328204f53504ad3ea26b575ca23a1a631c0e516a Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Fri, 16 Jan 2015 10:37:13 -0800
Subject: [PATCH 5/9] agent: test socket file overwrite

---
 command/agent/http_test.go | 42 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/command/agent/http_test.go b/command/agent/http_test.go
index fc5067b46..93be5b5c4 100644
--- a/command/agent/http_test.go
+++ b/command/agent/http_test.go
@@ -101,6 +101,48 @@ func TestHTTPServer_UnixSocket(t *testing.T) {
 	}
 }
 
+func TestHTTPServer_UnixSocket_OverwriteFile(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.SkipNow()
+	}
+
+	tempDir, err := ioutil.TempDir("", "consul")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	defer os.RemoveAll(tempDir)
+	socket := filepath.Join(tempDir, "test.sock")
+
+	// Create a regular file at the socket path
+	if err := ioutil.WriteFile(socket, []byte("hello world"), 0644); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	fi, err := os.Stat(socket)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if !fi.Mode().IsRegular() {
+		t.Fatalf("not a regular file: %s", socket)
+	}
+
+	// Try to start the server with the same path anyways.
+	dir, srv := makeHTTPServerWithConfig(t, func(c *Config) {
+		c.Addresses.HTTP = "unix://" + socket
+	})
+	defer os.RemoveAll(dir)
+	defer srv.Shutdown()
+	defer srv.agent.Shutdown()
+
+	// Check if the socket overwrote the file
+	fi, err = os.Stat(socket)
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if fi.Mode().IsRegular() {
+		t.Fatalf("should have socket file: %s", socket)
+	}
+}
+
 func TestSetIndex(t *testing.T) {
 	resp := httptest.NewRecorder()
 	setIndex(resp, 1000)

From 7c6455e1ba1db816e2772c56b37fd3557f0f76e4 Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Fri, 16 Jan 2015 10:56:47 -0800
Subject: [PATCH 6/9] api: simple test for unix socket connectivity

---
 api/api_test.go    | 34 ++++++++++++++++++++++++++++++++++
 api/status_test.go | 43 +------------------------------------------
 2 files changed, 35 insertions(+), 42 deletions(-)

diff --git a/api/api_test.go b/api/api_test.go
index 722d78b56..dc25f8b5a 100644
--- a/api/api_test.go
+++ b/api/api_test.go
@@ -8,6 +8,8 @@ import (
 	"net/http"
 	"os"
 	"os/exec"
+	"path/filepath"
+	"runtime"
 	"testing"
 	"time"
 
@@ -287,3 +289,35 @@ func TestParseQueryMeta(t *testing.T) {
 		t.Fatalf("Bad: %v", qm)
 	}
 }
+
+func TestAPI_UnixSocket(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.SkipNow()
+	}
+
+	tempDir, err := ioutil.TempDir("", "consul")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	defer os.RemoveAll(tempDir)
+	socket := filepath.Join(tempDir, "test.sock")
+
+	c, s := makeClientWithConfig(t, func(c *Config) {
+		c.Address = "unix://" + socket
+	}, func(c *testServerConfig) {
+		c.Addresses = &testAddressConfig{
+			HTTP: "unix://" + socket,
+		}
+	})
+	defer s.stop()
+
+	agent := c.Agent()
+
+	info, err := agent.Self()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if info["Config"]["NodeName"] == "" {
+		t.Fatalf("bad: %v", info)
+	}
+}
diff --git a/api/status_test.go b/api/status_test.go
index 61b0a435e..096b13da0 100644
--- a/api/status_test.go
+++ b/api/status_test.go
@@ -1,14 +1,10 @@
 package api
 
 import (
-	"fmt"
-	"io/ioutil"
-	"os"
-	"runtime"
 	"testing"
 )
 
-func TestStatusLeaderTCP(t *testing.T) {
+func TestStatusLeader(t *testing.T) {
 	c, s := makeClient(t)
 	defer s.stop()
 
@@ -23,43 +19,6 @@ func TestStatusLeaderTCP(t *testing.T) {
 	}
 }
 
-func TestStatusLeaderUnix(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.SkipNow()
-	}
-
-	tempdir, err := ioutil.TempDir("", "consul-test-")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.RemoveAll(tempdir)
-	socket := fmt.Sprintf("unix://%s/test.sock", tempdir)
-
-	clientConfig := func(c *Config) {
-		c.Address = socket
-	}
-
-	serverConfig := func(c *testServerConfig) {
-		if c.Addresses == nil {
-			c.Addresses = &testAddressConfig{}
-		}
-		c.Addresses.HTTP = socket
-	}
-
-	c, s := makeClientWithConfig(t, clientConfig, serverConfig)
-	defer s.stop()
-
-	status := c.Status()
-
-	leader, err := status.Leader()
-	if err != nil {
-		t.Fatalf("err: %v", err)
-	}
-	if leader == "" {
-		t.Fatalf("Expected leader")
-	}
-}
-
 func TestStatusPeers(t *testing.T) {
 	c, s := makeClient(t)
 	defer s.stop()

From 263614d596d8be352eae16fa8a5158796271cc21 Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Fri, 16 Jan 2015 12:39:15 -0800
Subject: [PATCH 7/9] agent: error if binding to existing socket file

---
 command/agent/agent.go     |  7 +++++++
 command/agent/command.go   |  9 ++++-----
 command/agent/http.go      | 13 +++----------
 command/agent/http_test.go | 27 ++++++++++++---------------
 4 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/command/agent/agent.go b/command/agent/agent.go
index 778d07135..d5ef05af4 100644
--- a/command/agent/agent.go
+++ b/command/agent/agent.go
@@ -22,6 +22,13 @@ const (
 
 	// Path to save local agent checks
 	checksDir = "checks"
+
+	// errSocketFileExists is the human-friendly error message displayed when
+	// trying to bind a socket to an existing file.
+	errSocketFileExists = "A file exists at the requested socket path %q. " +
+		"If Consul was not shut down properly, the socket file may " +
+		"be left behind. If the path looks correct, remove the file " +
+		"and try again."
 )
 
 /*
diff --git a/command/agent/command.go b/command/agent/command.go
index 3afafca5e..17eb5c577 100644
--- a/command/agent/command.go
+++ b/command/agent/command.go
@@ -295,12 +295,11 @@ func (c *Command) setupAgent(config *Config, logOutput io.Writer, logWriter *log
 		return err
 	}
 
+	// Error if we are trying to bind a domain socket to an existing path
 	if path, ok := unixSocketAddr(config.Addresses.RPC); ok {
-		// Remove the socket if it exists, or we'll get a bind error. This
-		// is necessary to avoid situations where Consul cannot start if the
-		// socket file exists in case of unexpected termination.
-		if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
-			return err
+		if _, err := os.Stat(path); err == nil || !os.IsNotExist(err) {
+			c.Ui.Output(fmt.Sprintf(errSocketFileExists, path))
+			return fmt.Errorf(errSocketFileExists, path)
 		}
 	}
 
diff --git a/command/agent/http.go b/command/agent/http.go
index 19a3dc0bf..708fe5b17 100644
--- a/command/agent/http.go
+++ b/command/agent/http.go
@@ -59,13 +59,6 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS
 			return nil, err
 		}
 
-		if path, ok := unixSocketAddr(config.Addresses.HTTPS); ok {
-			// See command/agent/config.go
-			if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
-				return nil, err
-			}
-		}
-
 		ln, err := net.Listen(httpAddr.Network(), httpAddr.String())
 		if err != nil {
 			return nil, fmt.Errorf("Failed to get Listen on %s: %v", httpAddr.String(), err)
@@ -102,10 +95,10 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS
 			return nil, fmt.Errorf("Failed to get ClientListener address:port: %v", err)
 		}
 
+		// Error if we are trying to bind a domain socket to an existing path
 		if path, ok := unixSocketAddr(config.Addresses.HTTP); ok {
-			// See command/agent/config.go
-			if err := os.Remove(path); err != nil && !os.IsNotExist(err) {
-				return nil, err
+			if _, err := os.Stat(path); err == nil || !os.IsNotExist(err) {
+				return nil, fmt.Errorf(errSocketFileExists, path)
 			}
 		}
 
diff --git a/command/agent/http_test.go b/command/agent/http_test.go
index 93be5b5c4..4677d5ba5 100644
--- a/command/agent/http_test.go
+++ b/command/agent/http_test.go
@@ -101,7 +101,7 @@ func TestHTTPServer_UnixSocket(t *testing.T) {
 	}
 }
 
-func TestHTTPServer_UnixSocket_OverwriteFile(t *testing.T) {
+func TestHTTPServer_UnixSocket_FileExists(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.SkipNow()
 	}
@@ -125,21 +125,18 @@ func TestHTTPServer_UnixSocket_OverwriteFile(t *testing.T) {
 		t.Fatalf("not a regular file: %s", socket)
 	}
 
-	// Try to start the server with the same path anyways.
-	dir, srv := makeHTTPServerWithConfig(t, func(c *Config) {
-		c.Addresses.HTTP = "unix://" + socket
-	})
-	defer os.RemoveAll(dir)
-	defer srv.Shutdown()
-	defer srv.agent.Shutdown()
+	conf := nextConfig()
+	conf.Addresses.HTTP = "unix://" + socket
 
-	// Check if the socket overwrote the file
-	fi, err = os.Stat(socket)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if fi.Mode().IsRegular() {
-		t.Fatalf("should have socket file: %s", socket)
+	dir, agent := makeAgent(t, conf)
+	defer os.RemoveAll(dir)
+
+	// Try to start the server with the same path anyways.
+	if servers, err := NewHTTPServers(agent, conf, agent.logOutput); err == nil {
+		for _, server := range servers {
+			server.Shutdown()
+		}
+		t.Fatalf("expected socket binding error")
 	}
 }
 

From 8ded1063dbee96654a7fb1c58884ae59eccbe93a Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Fri, 16 Jan 2015 13:06:49 -0800
Subject: [PATCH 8/9] website: update docs for agent socket options

---
 .../source/docs/agent/options.html.markdown   | 22 ++++++++++---------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown
index b7423dabe..2ecc5e2db 100644
--- a/website/source/docs/agent/options.html.markdown
+++ b/website/source/docs/agent/options.html.markdown
@@ -239,21 +239,23 @@ definitions support being updated during a reload.
    However, because the caches are not actively invalidated, ACL policy may be stale
    up to the TTL value.
 
-* `addresses` - This is a nested object that allows setting bind addresses. For `rpc`
-  and `http`, a Unix socket can be specified in the following form
-  `unix:///path/to/socket`. A new domain socket will be created at the given
-  path. Any existing socket file (or any other kind of file) at the specified
-  path will be **overwritten**, so use caution when configuring this argument.
-
+* `addresses` - This is a nested object that allows setting bind addresses.
+  <br><br>
+  Both `rpc` and `http` support binding to Unix domain sockets. A socket can be
+  specified in the form `unix:///path/to/socket`. A new domain socket will be
+  created at the given path. If the specified file path already exists, Consul
+  will refuse to start and return an error. For information on how to secure
+  socket file permissions, refer to the manual page for your operating system.
+  <br><br>
   When running Consul agent commands against Unix socket interfaces, use the
   `-rpc-addr` or `-http-addr` arguments to specify the path to the socket. You
   can also place the desired values in `CONSUL_RPC_ADDR` and `CONSUL_HTTP_ADDR`
   environment variables. For TCP addresses, these should be in the form ip:port.
-
+  <br><br>
   The following keys are valid:
-    * `dns` - The DNS server. Defaults to `client_addr`
-    * `http` - The HTTP API. Defaults to `client_addr`
-    * `rpc` - The RPC endpoint. Defaults to `client_addr`
+  * `dns` - The DNS server. Defaults to `client_addr`
+  * `http` - The HTTP API. Defaults to `client_addr`
+  * `rpc` - The RPC endpoint. Defaults to `client_addr`
 
 * `advertise_addr` - Equivalent to the `-advertise` command-line flag.
 

From 952fac6552feac77155e5ddf85ac6ba33dcad9c8 Mon Sep 17 00:00:00 2001
From: Ryan Uber <ru@ryanuber.com>
Date: Fri, 16 Jan 2015 14:01:14 -0800
Subject: [PATCH 9/9] command: fix env var reader for HTTP addr

---
 command/rpc.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/command/rpc.go b/command/rpc.go
index 5c647aed3..84d41e822 100644
--- a/command/rpc.go
+++ b/command/rpc.go
@@ -43,7 +43,7 @@ func HTTPClient(addr string) (*consulapi.Client, error) {
 // HTTPClientDC returns a new Consul HTTP client with the given address and datacenter
 func HTTPClientDC(addr, dc string) (*consulapi.Client, error) {
 	conf := consulapi.DefaultConfig()
-	if envAddr := os.Getenv("CONSUL_HTTP_ADDR"); addr != "" {
+	if envAddr := os.Getenv("CONSUL_HTTP_ADDR"); envAddr != "" {
 		addr = envAddr
 	}
 	conf.Address = addr