From f0fc0d52ae0fdbee7be95a63be86a24c44d39b9b Mon Sep 17 00:00:00 2001
From: Raphael 'kena' Poss <knz@cockroachlabs.com>
Date: Tue, 29 May 2018 13:07:32 +0200
Subject: [PATCH] cli: fix `cockroach quit`

This patch fixes the following:

- the logic in `doShutdown()` aims to ignore errors caused by attempts
  connect to a server which is closing its gRPC channels, but was
  missing one case of such errors: during the initial check whether
  the node was running. This patch causes gRPC "closed connection"
  errors to become also ignored in that case.

- previously if there was a transient gRPC error during a hard
  shutdown whereby the shutdown could still succeed, then `cockroach
  quit` would fail no matter what. This patch makes `cockroach quit`
  retry a hard shutdown.

- the warning messages are now emitted on stderr (via `log.Warningf`)
  instead of stdout.

Release note (bug fix): fix a bug where `cockroach quit` would
erroneously fail even though the node already successfully shut down.

Release note (cli change): `cockroach quit` now emits warning message
on its standard error stream, not standard output.
---
 pkg/cli/start.go | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/pkg/cli/start.go b/pkg/cli/start.go
index 32e660eb13d8..960f971e4aa7 100644
--- a/pkg/cli/start.go
+++ b/pkg/cli/start.go
@@ -1042,6 +1042,9 @@ func doShutdown(ctx context.Context, c serverpb.AdminClient, onModes []int32) er
 	// out, or perhaps drops the connection while waiting). To that end, we first
 	// run a noop DrainRequest. If that fails, we give up.
 	if err := checkNodeRunning(ctx, c); err != nil {
+		if grpcutil.IsClosedConnection(err) {
+			return nil
+		}
 		return err
 	}
 	// Send a drain request and continue reading until the connection drops (which
@@ -1113,16 +1116,21 @@ func runQuit(cmd *cobra.Command, args []string) (err error) {
 	case err := <-errChan:
 		if err != nil {
 			if _, ok := err.(errTryHardShutdown); ok {
-				fmt.Printf("graceful shutdown failed: %s; proceeding with hard shutdown\n", err)
+				log.Warningf(ctx, "graceful shutdown failed: %s; proceeding with hard shutdown\n", err)
 				break
 			}
 			return err
 		}
 		return nil
 	case <-time.After(time.Minute):
-		fmt.Println("timed out; proceeding with hard shutdown")
+		log.Warningf(ctx, "timed out; proceeding with hard shutdown")
 	}
 	// Not passing drain modes tells the server to not bother and go
-	// straight to shutdown.
+	// straight to shutdown. We try two times just in case there is a transient error.
+	err = doShutdown(ctx, c, nil)
+	if err != nil {
+		log.Warningf(ctx, "hard shutdown attempt failed, retrying: %v", err)
+		err = doShutdown(ctx, c, nil)
+	}
 	return errors.Wrap(doShutdown(ctx, c, nil), "hard shutdown failed")
 }