From d03759982495f26d7845b912221d7be589292432 Mon Sep 17 00:00:00 2001
From: Tobias Schottdorf <tobias.schottdorf@gmail.com>
Date: Wed, 6 Sep 2017 09:58:57 -0400
Subject: [PATCH] acceptance: deflake decommissioning event log test

The new bits of the test introduced in
https://github.com/cockroachdb/cockroach/pull/18178 were flaky since RPCs may be
sent to the down nodes.
---
 pkg/acceptance/decommission_test.go  | 56 +++++++++++++++++++++-------
 pkg/testutils/sqlutils/sql_runner.go | 22 ++++++++---
 2 files changed, 58 insertions(+), 20 deletions(-)

diff --git a/pkg/acceptance/decommission_test.go b/pkg/acceptance/decommission_test.go
index b6a5d17c353b..573a679517cd 100644
--- a/pkg/acceptance/decommission_test.go
+++ b/pkg/acceptance/decommission_test.go
@@ -15,7 +15,7 @@
 package acceptance
 
 import (
-	"fmt"
+	"reflect"
 	"regexp"
 	"strconv"
 	"testing"
@@ -33,6 +33,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/httputil"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
+	"github.com/kr/pretty"
+	"github.com/pkg/errors"
 )
 
 // TestDecommission starts up an >3 node cluster and decomissions and
@@ -270,18 +272,44 @@ func testDecommissionInner(
 			t.Error(err)
 		}
 	}()
-	sqlutils.MakeSQLRunner(t, db).CheckQueryResults(fmt.Sprintf(`
+
+	var rows *gosql.Rows
+	for {
+		var err error
+		rows, err = db.Query(`
 		SELECT "eventType", "targetID" FROM system.eventlog
-		WHERE "eventType" IN ('%s', '%s') ORDER BY timestamp`,
-		sql.EventLogNodeDecommissioned, sql.EventLogNodeRecommissioned),
-		[][]string{
-			{string(sql.EventLogNodeDecommissioned), idMap[0].String()},
-			{string(sql.EventLogNodeRecommissioned), idMap[0].String()},
-			{string(sql.EventLogNodeDecommissioned), idMap[1].String()},
-			{string(sql.EventLogNodeRecommissioned), idMap[1].String()},
-			{string(sql.EventLogNodeDecommissioned), idMap[2].String()},
-			{string(sql.EventLogNodeRecommissioned), idMap[2].String()},
-			{string(sql.EventLogNodeDecommissioned), idMap[0].String()},
-		},
-	)
+		WHERE "eventType" IN ($1, $2) ORDER BY timestamp`,
+			sql.EventLogNodeDecommissioned, sql.EventLogNodeRecommissioned,
+		)
+		if err != nil {
+			// Spurious errors appear to be possible since we might be trying to
+			// send RPCs to the (relatively recently) down node:
+			//
+			// pq: rpc error: code = Unavailable desc = grpc: the connection is
+			// unavailable
+			//
+			// Seen in https://teamcity.cockroachdb.com/viewLog.html?buildId=344802.
+			log.Warning(ctx, errors.Wrap(err, "retrying after"))
+			continue
+		}
+		break
+	}
+
+	matrix, err := sqlutils.RowsToStrMatrix(rows)
+	if err != nil {
+		t.Fatal(err)
+	}
+	expMatrix := [][]string{
+		{string(sql.EventLogNodeDecommissioned), idMap[0].String()},
+		{string(sql.EventLogNodeRecommissioned), idMap[0].String()},
+		{string(sql.EventLogNodeDecommissioned), idMap[1].String()},
+		{string(sql.EventLogNodeRecommissioned), idMap[1].String()},
+		{string(sql.EventLogNodeDecommissioned), idMap[2].String()},
+		{string(sql.EventLogNodeRecommissioned), idMap[2].String()},
+		{string(sql.EventLogNodeDecommissioned), idMap[0].String()},
+	}
+
+	if !reflect.DeepEqual(matrix, expMatrix) {
+		t.Fatalf("unexpected diff(matrix, expMatrix):\n%s", pretty.Diff(matrix, expMatrix))
+	}
 }
diff --git a/pkg/testutils/sqlutils/sql_runner.go b/pkg/testutils/sqlutils/sql_runner.go
index 13e62933b5a9..be51f417ceef 100644
--- a/pkg/testutils/sqlutils/sql_runner.go
+++ b/pkg/testutils/sqlutils/sql_runner.go
@@ -86,15 +86,25 @@ func (sr *SQLRunner) QueryRow(query string, args ...interface{}) *Row {
 	return &Row{sr.TB, sr.DB.QueryRow(query, args...)}
 }
 
-// QueryStr runs a Query and converts the result to a string matrix; nulls are
-// represented as "NULL". Empty results are represented by an empty (but
-// non-nil) slice. Kills the test on errors.
+// QueryStr runs a Query and converts the result using RowsToStrMatrix. Kills
+// the test on errors.
 func (sr *SQLRunner) QueryStr(query string, args ...interface{}) [][]string {
 	rows := sr.Query(query, args...)
-	cols, err := rows.Columns()
+	r, err := RowsToStrMatrix(rows)
 	if err != nil {
 		sr.Fatal(err)
 	}
+	return r
+}
+
+// RowsToStrMatrix converts the given result rows to a string matrix; nulls are
+// represented as "NULL". Empty results are represented by an empty (but
+// non-nil) slice.
+func RowsToStrMatrix(rows *gosql.Rows) ([][]string, error) {
+	cols, err := rows.Columns()
+	if err != nil {
+		return nil, err
+	}
 	vals := make([]interface{}, len(cols))
 	for i := range vals {
 		vals[i] = new(interface{})
@@ -102,7 +112,7 @@ func (sr *SQLRunner) QueryStr(query string, args ...interface{}) [][]string {
 	res := [][]string{}
 	for rows.Next() {
 		if err := rows.Scan(vals...); err != nil {
-			sr.Fatal(err)
+			return nil, err
 		}
 		row := make([]string, len(vals))
 		for j, v := range vals {
@@ -119,7 +129,7 @@ func (sr *SQLRunner) QueryStr(query string, args ...interface{}) [][]string {
 		}
 		res = append(res, row)
 	}
-	return res
+	return res, nil
 }
 
 // CheckQueryResults checks that the rows returned by a query match the expected