From 494948821702691549e9e44e5e778147b614856f Mon Sep 17 00:00:00 2001
From: Michael Schurter <mschurter@hashicorp.com>
Date: Thu, 16 Dec 2021 11:41:01 -0800
Subject: [PATCH] cli: return error from raft commands if db is open

Before this change trying to run `nomad operator raft {info,logs}` on an
inuse raft.db would cause the command to block until the agent using
raft.db is closed.

After this change the command will block for 1s before returning a
(hopefully) helpful error message.

This change also sets the ReadOnly mode on the underlying BoltDb to
ensure diagnostics make no changes to the underlying store. We have no
evidence this has ever occurred, but it seems like a useful safety
measure.

No changelog added since this is a minor tweak in a "new" feature (it
was hidden in previous relases).
---
 helper/raftutil/state.go      | 20 +++++++++++++-
 helper/raftutil/state_test.go | 52 +++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 helper/raftutil/state_test.go

diff --git a/helper/raftutil/state.go b/helper/raftutil/state.go
index f902a32ef..68c26b5c0 100644
--- a/helper/raftutil/state.go
+++ b/helper/raftutil/state.go
@@ -2,20 +2,38 @@ package raftutil
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
+	"time"
 
+	"github.com/boltdb/bolt"
 	"github.com/hashicorp/go-msgpack/codec"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/raft"
 	raftboltdb "github.com/hashicorp/raft-boltdb"
 )
 
+var (
+	errAlreadyOpen = errors.New("unable to open raft logs that are in use")
+)
+
 // RaftStateInfo returns info about the nomad state, as found in the passed data-dir directory
 func RaftStateInfo(p string) (store *raftboltdb.BoltStore, firstIdx uint64, lastIdx uint64, err error) {
-	s, err := raftboltdb.NewBoltStore(p)
+	opts := raftboltdb.Options{
+		Path: p,
+		BoltOptions: &bolt.Options{
+			ReadOnly: true,
+			Timeout:  1 * time.Second,
+		},
+	}
+	s, err := raftboltdb.New(opts)
 	if err != nil {
+		if strings.HasSuffix(err.Error(), "timeout") {
+			return nil, 0, 0, errAlreadyOpen
+		}
 		return nil, 0, 0, fmt.Errorf("failed to open raft logs: %v", err)
 	}
 
diff --git a/helper/raftutil/state_test.go b/helper/raftutil/state_test.go
new file mode 100644
index 000000000..ea8075ca3
--- /dev/null
+++ b/helper/raftutil/state_test.go
@@ -0,0 +1,52 @@
+package raftutil
+
+import (
+	"path/filepath"
+	"testing"
+
+	raftboltdb "github.com/hashicorp/raft-boltdb"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestRaftStateInfo_InUse asserts that commands that inspect raft
+// state such as "nomad operator raft info" and "nomad operator raft
+// logs" fail with a helpful error message when called on an inuse
+// database.
+func TestRaftStateInfo_InUse(t *testing.T) {
+	t.Parallel() // since there's a 1s timeout.
+
+	// First create an empty raft db
+	dir := filepath.Join(t.TempDir(), "raft.db")
+
+	fakedb, err := raftboltdb.NewBoltStore(dir)
+	require.NoError(t, err)
+
+	// Next try to read the db without closing it
+	s, _, _, err := RaftStateInfo(dir)
+	assert.Nil(t, s)
+	require.EqualError(t, err, errAlreadyOpen.Error())
+
+	// LogEntries should produce the same error
+	_, _, err = LogEntries(dir)
+	require.EqualError(t, err, "failed to open raft logs: "+errAlreadyOpen.Error())
+
+	// Commands should work once the db is closed
+	require.NoError(t, fakedb.Close())
+
+	s, _, _, err = RaftStateInfo(dir)
+	assert.NotNil(t, s)
+	require.NoError(t, err)
+	require.NoError(t, s.Close())
+
+	logCh, errCh, err := LogEntries(dir)
+	require.NoError(t, err)
+
+	// Consume entries to cleanly close db
+	for closed := false; closed; {
+		select {
+		case _, closed = <-logCh:
+		case <-errCh:
+		}
+	}
+}