Robustness to existing socket files (#35)

Also cleanup in response to `go vet`, `golint`, and `gofmt`.

Author: pboothe

ipinfo/ipinfo_test.go

@@ -33,8 +33,8 @@ func TestParse(t *testing.T) {
 			want: ASNames{},
 		},
 		{
-			name: "Not a CSV",
-			data: []byte("two,records\nonerecord\n"),
+			name:    "Not a CSV",
+			data:    []byte("two,records\nonerecord\n"),
 			wantErr: true,
 		},
 	}

ipservice/ipservice_test.go

@@ -96,7 +96,7 @@ func TestServerAndClientE2E(t *testing.T) {
 			name: "Localhost-v4",
 			ips:  []string{"127.0.0.1"},
 			want: map[string]*annotator.ClientAnnotations{
-				"127.0.0.1": &annotator.ClientAnnotations{
+				"127.0.0.1": {
 					Network: &annotator.Network{
 						Missing: true,
 					},
@@ -110,7 +110,7 @@ func TestServerAndClientE2E(t *testing.T) {
 			name: "Localhost-v6",
 			ips:  []string{"::1"},
 			want: map[string]*annotator.ClientAnnotations{
-				"::1": &annotator.ClientAnnotations{
+				"::1": {
 					Network: &annotator.Network{
 						Missing: true,
 					},
@@ -124,7 +124,7 @@ func TestServerAndClientE2E(t *testing.T) {
 			name: "IP that has everything",
 			ips:  []string{"2.125.160.216"},
 			want: map[string]*annotator.ClientAnnotations{
-				"2.125.160.216": &annotator.ClientAnnotations{
+				"2.125.160.216": {
 					Network: &annotator.Network{
 						CIDR:     "2.120.0.0/13",
 						ASNumber: 5607,
@@ -154,7 +154,7 @@ func TestServerAndClientE2E(t *testing.T) {
 			name: "Multiple IPs",
 			ips:  []string{"2.125.160.216", "127.0.0.1"},
 			want: map[string]*annotator.ClientAnnotations{
-				"2.125.160.216": &annotator.ClientAnnotations{
+				"2.125.160.216": {
 					Network: &annotator.Network{
 						CIDR:     "2.120.0.0/13",
 						ASNumber: 5607,
@@ -178,7 +178,7 @@ func TestServerAndClientE2E(t *testing.T) {
 						AccuracyRadiusKm:    100,
 					},
 				},
-				"127.0.0.1": &annotator.ClientAnnotations{
+				"127.0.0.1": {
 					Network: &annotator.Network{
 						Missing: true,
 					},
@@ -211,17 +211,15 @@ func TestServerAndClientE2E(t *testing.T) {
 	wg.Wait()
 }
 
-func TestNewServerError(t *testing.T) {
-	// Server creation fails when the socket file already exists. So make a file
-	// and use its name to generate an error.
+func TestNewServerWithExistingFile(t *testing.T) {
+	// Server creation should succeed even when the socket file already exists.
+	// So make a file and use its name to start the server, hopefully without error.
 	f, err := ioutil.TempFile("", "TextNewServerError")
 	rtx.Must(err, "Could not create tempfile")
 	defer os.Remove(f.Name())
 
 	_, err = NewServer(f.Name(), asn, geo)
-	if err == nil {
-		t.Error("We should have had an error, but did not")
-	}
+	rtx.Must(err, "Should not have had an error when starting NewServer wth an existing file")
 }
 
 func TestNewClient(t *testing.T) {

ipservice/server.go

@@ -5,6 +5,7 @@ import (
 	"log"
 	"net"
 	"net/http"
+	"os"
 
 	"github.com/m-lab/go/errorx"
 	"github.com/m-lab/go/rtx"
@@ -110,6 +111,11 @@ func NewServer(sockfilename string, asn asnannotator.ASNAnnotator, geo geoannota
 	if sockfilename != *SocketFilename {
 		log.Printf("WARNING: socket filename of %q differs from command-line flag value of %q\n", sockfilename, *SocketFilename)
 	}
+	// Unconditionally attempt to remove the file before you make a new one with
+	// that name. It is possible for race conditions in container starting to
+	// mean that prior start attempts have left an old bad socket file in the
+	// way.
+	os.Remove(sockfilename)
 	listener, err := net.Listen("unix", sockfilename)
 	if err != nil {
 		return nil, err