Merge Consul OSS branch 'master' at commit 95dbb7f2f1

.circleci/config.yml

@@ -23,6 +23,14 @@ references:
     S3_ARTIFACT_BUCKET:  consul-dev-artifacts
 
 jobs:
+  # lint consul tests
+  lint-consul-retry:
+    docker:
+      - image: *GOLANG_IMAGE
+    steps:
+      - checkout
+      - run: go get -u github.com/hashicorp/lint-consul-retry && lint-consul-retry
+
   # Runs go fmt and go vet
   go-fmt-and-vet:
     docker:
@@ -562,7 +570,10 @@ workflows:
                 - release/1-6
   build-distros:
     jobs:
-      - go-fmt-and-vet
+      - lint-consul-retry
+      - go-fmt-and-vet:
+          requires:
+            - lint-consul-retry
       - build-386: &require-go-fmt-vet
           requires:
             - go-fmt-and-vet

agent/consul/catalog_endpoint_test.go

@@ -1049,7 +1049,7 @@ func TestCatalog_ListNodes_StaleRead(t *testing.T) {
 
 	retry.Run(t, func(r *retry.R) {
 		if err := msgpackrpc.CallWithCodec(codec, "Catalog.ListNodes", &args, &out); err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 
 		found := false

agent/coordinate_endpoint_test.go

@@ -84,17 +84,17 @@ func TestCoordinate_Nodes(t *testing.T) {
 	retry.Run(t, func(r *retry.R) {
 		obj, err := a.srv.CoordinateNodes(resp, req)
 		if err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 
 		// Check that coordinates are empty before registering a node
 		coordinates, ok := obj.(structs.Coordinates)
 		if !ok {
-			t.Fatalf("expected: structs.Coordinates, received: %+v", obj)
+			r.Fatalf("expected: structs.Coordinates, received: %+v", obj)
 		}
 
 		if len(coordinates) != 0 {
-			t.Fatalf("coordinates should be empty, received: %v", coordinates)
+			r.Fatalf("coordinates should be empty, received: %v", coordinates)
 		}
 	})
 
@@ -141,17 +141,17 @@ func TestCoordinate_Nodes(t *testing.T) {
 	retry.Run(t, func(r *retry.R) {
 		obj, err := a.srv.CoordinateNodes(resp, req)
 		if err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 
 		coordinates, ok := obj.(structs.Coordinates)
 		if !ok {
-			t.Fatalf("expected: structs.Coordinates, received: %+v", obj)
+			r.Fatalf("expected: structs.Coordinates, received: %+v", obj)
 		}
 		if len(coordinates) != 2 ||
 			coordinates[0].Node != "bar" ||
 			coordinates[1].Node != "foo" {
-			t.Fatalf("expected: bar, foo recieved: %v", coordinates)
+			r.Fatalf("expected: bar, foo recieved: %v", coordinates)
 		}
 	})
 	// Filter on a nonexistent node segment
@@ -160,15 +160,15 @@ func TestCoordinate_Nodes(t *testing.T) {
 	retry.Run(t, func(r *retry.R) {
 		obj, err := a.srv.CoordinateNodes(resp, req)
 		if err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 
 		coordinates, ok := obj.(structs.Coordinates)
 		if !ok {
-			t.Fatalf("expected: structs.Coordinates, received: %+v", obj)
+			r.Fatalf("expected: structs.Coordinates, received: %+v", obj)
 		}
 		if len(coordinates) != 0 {
-			t.Fatalf("coordinates should be empty, received: %v", coordinates)
+			r.Fatalf("coordinates should be empty, received: %v", coordinates)
 		}
 	})
 	// Filter on a real node segment
@@ -177,15 +177,15 @@ func TestCoordinate_Nodes(t *testing.T) {
 	retry.Run(t, func(r *retry.R) {
 		obj, err := a.srv.CoordinateNodes(resp, req)
 		if err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 
 		coordinates, ok := obj.(structs.Coordinates)
 		if !ok {
-			t.Fatalf("expected: structs.Coordinates, received: %+v", obj)
+			r.Fatalf("expected: structs.Coordinates, received: %+v", obj)
 		}
 		if len(coordinates) != 1 || coordinates[0].Node != "foo" {
-			t.Fatalf("expected: foo received: %v", coordinates)
+			r.Fatalf("expected: foo received: %v", coordinates)
 		}
 	})
 	// Make sure the empty filter works
@@ -194,15 +194,15 @@ func TestCoordinate_Nodes(t *testing.T) {
 	retry.Run(t, func(r *retry.R) {
 		obj, err := a.srv.CoordinateNodes(resp, req)
 		if err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 
 		coordinates, ok := obj.(structs.Coordinates)
 		if !ok {
-			t.Fatalf("expected: structs.Coordinates, received: %+v", obj)
+			r.Fatalf("expected: structs.Coordinates, received: %+v", obj)
 		}
 		if len(coordinates) != 1 || coordinates[0].Node != "bar" {
-			t.Fatalf("expected: bar received: %v", coordinates)
+			r.Fatalf("expected: bar received: %v", coordinates)
 		}
 	})
 }

agent/remote_exec_test.go

@@ -157,7 +157,9 @@ func testRemoteExecGetSpec(t *testing.T, hcl string, token string, shouldSucceed
 		t.Fatalf("err: %v", err)
 	}
 	key := "_rexec/" + event.Session + "/job"
-	setKV(t, a.Agent, key, buf, token)
+	if err := setKV(a.Agent, key, buf, token); err != nil {
+		t.Fatalf("err: %v", err)
+	}
 
 	var out remoteExecSpec
 	if shouldSucceed != a.remoteExecGetSpec(event, &out) {
@@ -243,26 +245,26 @@ func testRemoteExecWrites(t *testing.T, hcl string, token string, shouldSucceed
 	}
 
 	key := "_rexec/" + event.Session + "/" + a.Config.NodeName + "/ack"
-	d := getKV(t, a.Agent, key, token)
+	d, err := getKV(a.Agent, key, token)
-	if d == nil || d.Session != event.Session {
+	if d == nil || d.Session != event.Session || err != nil {
 		t.Fatalf("bad ack: %#v", d)
 	}
 
 	key = "_rexec/" + event.Session + "/" + a.Config.NodeName + "/out/00000"
-	d = getKV(t, a.Agent, key, token)
+	d, err = getKV(a.Agent, key, token)
-	if d == nil || d.Session != event.Session || !bytes.Equal(d.Value, output) {
+	if d == nil || d.Session != event.Session || !bytes.Equal(d.Value, output) || err != nil {
 		t.Fatalf("bad output: %#v", d)
 	}
 
 	key = "_rexec/" + event.Session + "/" + a.Config.NodeName + "/out/0000a"
-	d = getKV(t, a.Agent, key, token)
+	d, err = getKV(a.Agent, key, token)
-	if d == nil || d.Session != event.Session || !bytes.Equal(d.Value, output) {
+	if d == nil || d.Session != event.Session || !bytes.Equal(d.Value, output) || err != nil {
 		t.Fatalf("bad output: %#v", d)
 	}
 
 	key = "_rexec/" + event.Session + "/" + a.Config.NodeName + "/exit"
-	d = getKV(t, a.Agent, key, token)
+	d, err = getKV(a.Agent, key, token)
-	if d == nil || d.Session != event.Session || string(d.Value) != "1" {
+	if d == nil || d.Session != event.Session || string(d.Value) != "1" || err != nil {
 		t.Fatalf("bad output: %#v", d)
 	}
 }
@@ -284,14 +286,16 @@ func testHandleRemoteExec(t *testing.T, command string, expectedSubstring string
 		}
 		buf, err := json.Marshal(spec)
 		if err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 		key := "_rexec/" + event.Session + "/job"
-		setKV(t, a.Agent, key, buf, "")
+		if err := setKV(a.Agent, key, buf, ""); err != nil {
+			r.Fatalf("err: %v", err)
+		}
 
 		buf, err = json.Marshal(event)
 		if err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 		msg := &UserEvent{
 			ID:      generateUUID(),
@@ -303,24 +307,24 @@ func testHandleRemoteExec(t *testing.T, command string, expectedSubstring string
 
 		// Verify we have an ack
 		key = "_rexec/" + event.Session + "/" + a.Config.NodeName + "/ack"
-		d := getKV(t, a.Agent, key, "")
+		d, err := getKV(a.Agent, key, "")
-		if d == nil || d.Session != event.Session {
+		if d == nil || d.Session != event.Session || err != nil {
-			t.Fatalf("bad ack: %#v", d)
+			r.Fatalf("bad ack: %#v", d)
 		}
 
 		// Verify we have output
 		key = "_rexec/" + event.Session + "/" + a.Config.NodeName + "/out/00000"
-		d = getKV(t, a.Agent, key, "")
+		d, err = getKV(a.Agent, key, "")
 		if d == nil || d.Session != event.Session ||
-			!bytes.Contains(d.Value, []byte(expectedSubstring)) {
+			!bytes.Contains(d.Value, []byte(expectedSubstring)) || err != nil {
-			t.Fatalf("bad output: %#v", d)
+			r.Fatalf("bad output: %#v", d)
 		}
 
 		// Verify we have an exit code
 		key = "_rexec/" + event.Session + "/" + a.Config.NodeName + "/exit"
-		d = getKV(t, a.Agent, key, "")
+		d, err = getKV(a.Agent, key, "")
-		if d == nil || d.Session != event.Session || string(d.Value) != expectedReturnCode {
+		if d == nil || d.Session != event.Session || string(d.Value) != expectedReturnCode || err != nil {
-			t.Fatalf("bad output: %#v", d)
+			r.Fatalf("bad output: %#v", d)
 		}
 	})
 }
@@ -371,7 +375,7 @@ func destroySession(t *testing.T, a *Agent, session string, token string) {
 	}
 }
 
-func setKV(t *testing.T, a *Agent, key string, val []byte, token string) {
+func setKV(a *Agent, key string, val []byte, token string) error {
 	write := structs.KVSRequest{
 		Datacenter: a.config.Datacenter,
 		Op:         api.KVSet,
@@ -385,11 +389,12 @@ func setKV(t *testing.T, a *Agent, key string, val []byte, token string) {
 	}
 	var success bool
 	if err := a.RPC("KVS.Apply", &write, &success); err != nil {
-		t.Fatalf("err: %v", err)
+		return err
 	}
+	return nil
 }
 
-func getKV(t *testing.T, a *Agent, key string, token string) *structs.DirEntry {
+func getKV(a *Agent, key string, token string) (*structs.DirEntry, error) {
 	req := structs.KeyRequest{
 		Datacenter: a.config.Datacenter,
 		Key:        key,
@@ -399,10 +404,10 @@ func getKV(t *testing.T, a *Agent, key string, token string) *structs.DirEntry {
 	}
 	var out structs.IndexedDirEntries
 	if err := a.RPC("KVS.Get", &req, &out); err != nil {
-		t.Fatalf("err: %v", err)
+		return nil, err
 	}
 	if len(out.Entries) > 0 {
-		return out.Entries[0]
+		return out.Entries[0], nil
 	}
-	return nil
+	return nil, nil
 }

api/acl_test.go

@@ -643,10 +643,10 @@ func TestAPI_RulesTranslate_FromToken(t *testing.T) {
 
 	// This relies on the token upgrade loop running in the background
 	// to assign an accessor
-	retry.Run(t, func(t *retry.R) {
+	retry.Run(t, func(r *retry.R) {
 		token, _, err := acl.TokenReadSelf(nil)
-		require.NoError(t, err)
+		require.NoError(r, err)
-		require.NotEqual(t, "", token.AccessorID)
+		require.NotEqual(r, "", token.AccessorID)
 		accessor = token.AccessorID
 	})
 	acl.c.config.Token = "root"

api/catalog_test.go

@@ -342,10 +342,10 @@ func TestAPI_CatalogService_SingleTag(t *testing.T) {
 
 	retry.Run(t, func(r *retry.R) {
 		services, meta, err := catalog.Service("foo", "bar", nil)
-		require.NoError(t, err)
+		require.NoError(r, err)
-		require.NotEqual(t, meta.LastIndex, 0)
+		require.NotEqual(r, meta.LastIndex, 0)
-		require.Len(t, services, 1)
+		require.Len(r, services, 1)
-		require.Equal(t, services[0].ServiceID, "foo1")
+		require.Equal(r, services[0].ServiceID, "foo1")
 	})
 }
 
@@ -380,23 +380,23 @@ func TestAPI_CatalogService_MultipleTags(t *testing.T) {
 	retry.Run(t, func(r *retry.R) {
 		services, meta, err := catalog.ServiceMultipleTags("foo", []string{"bar"}, nil)
 
-		require.NoError(t, err)
+		require.NoError(r, err)
-		require.NotEqual(t, meta.LastIndex, 0)
+		require.NotEqual(r, meta.LastIndex, 0)
 
 		// Should be 2 services with the `bar` tag
-		require.Len(t, services, 2)
+		require.Len(r, services, 2)
 	})
 
 	// Test searching with two tags (one result)
 	retry.Run(t, func(r *retry.R) {
 		services, meta, err := catalog.ServiceMultipleTags("foo", []string{"bar", "v2"}, nil)
 
-		require.NoError(t, err)
+		require.NoError(r, err)
-		require.NotEqual(t, meta.LastIndex, 0)
+		require.NotEqual(r, meta.LastIndex, 0)
 
 		// Should be exactly 1 service, named "foo2"
-		require.Len(t, services, 1)
+		require.Len(r, services, 1)
-		require.Equal(t, services[0].ServiceID, "foo2")
+		require.Equal(r, services[0].ServiceID, "foo2")
 	})
 }
 

api/lock_test.go

@@ -109,10 +109,10 @@ func TestAPI_LockForceInvalidate(t *testing.T) {
 		// Should work
 		leaderCh, err := lock.Lock(nil)
 		if err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 		if leaderCh == nil {
-			t.Fatalf("not leader")
+			r.Fatalf("not leader")
 		}
 		defer lock.Unlock()
 
@@ -127,7 +127,7 @@ func TestAPI_LockForceInvalidate(t *testing.T) {
 		select {
 		case <-leaderCh:
 		case <-time.After(time.Second):
-			t.Fatalf("should not be leader")
+			r.Fatalf("should not be leader")
 		}
 	})
 }

api/operator_autopilot_test.go

@@ -46,10 +46,10 @@ func TestAPI_OperatorAutopilotCASConfiguration(t *testing.T) {
 		operator := c.Operator()
 		config, err := operator.AutopilotGetConfiguration(nil)
 		if err != nil {
-			t.Fatalf("err: %v", err)
+			r.Fatalf("err: %v", err)
 		}
 		if !config.CleanupDeadServers {
-			t.Fatalf("bad: %v", config)
+			r.Fatalf("bad: %v", config)
 		}
 
 		// Pass an invalid ModifyIndex
@@ -60,10 +60,10 @@ func TestAPI_OperatorAutopilotCASConfiguration(t *testing.T) {
 			}
 			resp, err := operator.AutopilotCASConfiguration(newConf, nil)
 			if err != nil {
-				t.Fatalf("err: %v", err)
+				r.Fatalf("err: %v", err)
 			}
 			if resp {
-				t.Fatalf("bad: %v", resp)
+				r.Fatalf("bad: %v", resp)
 			}
 		}
 
@@ -75,10 +75,10 @@ func TestAPI_OperatorAutopilotCASConfiguration(t *testing.T) {
 			}
 			resp, err := operator.AutopilotCASConfiguration(newConf, nil)
 			if err != nil {
-				t.Fatalf("err: %v", err)
+				r.Fatalf("err: %v", err)
 			}
 			if !resp {
-				t.Fatalf("bad: %v", resp)
+				r.Fatalf("bad: %v", resp)
 			}
 		}
 	})

command/connect/proxy/register_test.go

@@ -37,7 +37,6 @@ func TestRegisterMonitor_good(t *testing.T) {
 
 func TestRegisterMonitor_heartbeat(t *testing.T) {
 	t.Parallel()
-	require := require.New(t)
 
 	a := agent.NewTestAgent(t, t.Name(), ``)
 	defer a.Shutdown()
@@ -49,11 +48,11 @@ func TestRegisterMonitor_heartbeat(t *testing.T) {
 	retry.Run(t, func(r *retry.R) {
 		// Get the check and verify that it is passing
 		checks, err := client.Agent().Checks()
-		require.NoError(err)
+		require.NoError(r, err)
-		require.Contains(checks, m.checkID())
+		require.Contains(r, checks, m.checkID())
-		require.Equal("passing", checks[m.checkID()].Status)
+		require.Equal(r, "passing", checks[m.checkID()].Status)
 		// Purposely fail the TTL check, verify it becomes healthy again
-		require.NoError(client.Agent().FailTTL(m.checkID(), ""))
+		require.NoError(r, client.Agent().FailTTL(m.checkID(), ""))
 	})
 
 	retry.Run(t, func(r *retry.R) {