From a97e48e08d9d3b9fc0ffe9d524278cc60af72e8d Mon Sep 17 00:00:00 2001
From: Marek Siarkowicz <siarkowicz@google.com>
Date: Mon, 5 Jul 2021 16:06:37 +0200
Subject: [PATCH] Cleanup references to bucket module

---
 server/auth/store.go                          |  4 +-
 server/storage/mvcc/kvstore.go                |  5 +-
 .../storage/mvcc/kvstore_compaction_test.go   |  2 +-
 server/storage/mvcc/kvstore_test.go           |  2 +-
 server/storage/schema/auth_roles.go           |  4 ++
 server/storage/schema/auth_users.go           |  4 ++
 server/storage/schema/schema.go               |  4 +-
 server/storage/schema/schema_test.go          | 64 ++++++++++---------
 8 files changed, 50 insertions(+), 39 deletions(-)

diff --git a/server/auth/store.go b/server/auth/store.go
index ceab53cdb..3baff1016 100644
--- a/server/auth/store.go
+++ b/server/auth/store.go
@@ -931,8 +931,8 @@ func NewAuthStore(lg *zap.Logger, be backend.Backend, tp TokenProvider, bcryptCo
 	tx.Lock()
 
 	schema.UnsafeCreateAuthBucket(tx)
-	tx.UnsafeCreateBucket(schema.AuthUsers)
-	tx.UnsafeCreateBucket(schema.AuthRoles)
+	schema.UnsafeCreateAuthUsersBucket(tx)
+	schema.UnsafeCreateAuthRolesBucket(tx)
 
 	enabled := schema.UnsafeReadAuthEnabled(tx)
 
diff --git a/server/storage/mvcc/kvstore.go b/server/storage/mvcc/kvstore.go
index 135509bcf..3c0a60bef 100644
--- a/server/storage/mvcc/kvstore.go
+++ b/server/storage/mvcc/kvstore.go
@@ -123,7 +123,7 @@ func NewStore(lg *zap.Logger, b backend.Backend, le lease.Lessor, cfg StoreConfi
 	tx := s.b.BatchTx()
 	tx.Lock()
 	tx.UnsafeCreateBucket(schema.Key)
-	tx.UnsafeCreateBucket(schema.Meta)
+	schema.UnsafeCreateMetaBucket(tx)
 	tx.Unlock()
 	s.b.ForceCommit()
 
@@ -340,7 +340,6 @@ func (s *store) restore() error {
 
 		s.lg.Info(
 			"restored last compact revision",
-			zap.Stringer("meta-bucket-name", schema.Meta),
 			zap.String("meta-bucket-name-key", string(schema.FinishedCompactKeyName)),
 			zap.Int64("restored-compact-revision", s.compactMainRev),
 		)
@@ -412,8 +411,6 @@ func (s *store) restore() error {
 
 		s.lg.Info(
 			"resume scheduled compaction",
-			zap.Stringer("meta-bucket-name", schema.Meta),
-			zap.String("meta-bucket-name-key", string(schema.ScheduledCompactKeyName)),
 			zap.Int64("scheduled-compact-revision", scheduledCompact),
 		)
 	}
diff --git a/server/storage/mvcc/kvstore_compaction_test.go b/server/storage/mvcc/kvstore_compaction_test.go
index 53209e3db..ccf6df9b9 100644
--- a/server/storage/mvcc/kvstore_compaction_test.go
+++ b/server/storage/mvcc/kvstore_compaction_test.go
@@ -91,7 +91,7 @@ func TestScheduleCompaction(t *testing.T) {
 		}
 		vals, _ := UnsafeReadFinishedCompact(tx)
 		if !reflect.DeepEqual(vals, tt.rev) {
-			t.Errorf("#%d: vals on %v = %+v, want %+v", i, schema.FinishedCompactKeyName, vals, tt.rev)
+			t.Errorf("#%d: finished compact equal %+v, want %+v", i, vals, tt.rev)
 		}
 		tx.Unlock()
 
diff --git a/server/storage/mvcc/kvstore_test.go b/server/storage/mvcc/kvstore_test.go
index 87708e796..027430814 100644
--- a/server/storage/mvcc/kvstore_test.go
+++ b/server/storage/mvcc/kvstore_test.go
@@ -485,7 +485,7 @@ func TestRestoreContinueUnfinishedCompaction(t *testing.T) {
 		revToBytes(revision{main: 2}, rbytes)
 		tx := s0.b.BatchTx()
 		tx.Lock()
-		tx.UnsafePut(schema.Meta, schema.ScheduledCompactKeyName, rbytes)
+		UnsafeSetScheduledCompact(tx, 2)
 		tx.Unlock()
 
 		s0.Close()
diff --git a/server/storage/schema/auth_roles.go b/server/storage/schema/auth_roles.go
index c568f3a1b..4c395710c 100644
--- a/server/storage/schema/auth_roles.go
+++ b/server/storage/schema/auth_roles.go
@@ -20,6 +20,10 @@ import (
 	"go.uber.org/zap"
 )
 
+func UnsafeCreateAuthRolesBucket(tx backend.BatchTx) {
+	tx.UnsafeCreateBucket(AuthRoles)
+}
+
 func UnsafeGetRole(lg *zap.Logger, tx backend.BatchTx, roleName string) *authpb.Role {
 	_, vs := tx.UnsafeRange(AuthRoles, []byte(roleName), nil, 0)
 	if len(vs) == 0 {
diff --git a/server/storage/schema/auth_users.go b/server/storage/schema/auth_users.go
index 334017457..2e65600ec 100644
--- a/server/storage/schema/auth_users.go
+++ b/server/storage/schema/auth_users.go
@@ -20,6 +20,10 @@ import (
 	"go.uber.org/zap"
 )
 
+func UnsafeCreateAuthUsersBucket(tx backend.BatchTx) {
+	tx.UnsafeCreateBucket(AuthUsers)
+}
+
 func UnsafeGetUser(lg *zap.Logger, tx backend.BatchTx, username string) *authpb.User {
 	_, vs := tx.UnsafeRange(AuthUsers, []byte(username), nil, 0)
 	if len(vs) == 0 {
diff --git a/server/storage/schema/schema.go b/server/storage/schema/schema.go
index 04acc66d5..12f8c3c21 100644
--- a/server/storage/schema/schema.go
+++ b/server/storage/schema/schema.go
@@ -55,11 +55,11 @@ func detectStorageVersion(lg *zap.Logger, tx backend.ReadTx) (*semver.Version, e
 	}
 	confstate := UnsafeConfStateFromBackend(lg, tx)
 	if confstate == nil {
-		return nil, fmt.Errorf("missing %q key", MetaConfStateName)
+		return nil, fmt.Errorf("missing confstate information")
 	}
 	_, term := UnsafeReadConsistentIndex(tx)
 	if term == 0 {
-		return nil, fmt.Errorf("missing %q key", MetaTermKeyName)
+		return nil, fmt.Errorf("missing term information")
 	}
 	copied := V3_5
 	return &copied, nil
diff --git a/server/storage/schema/schema_test.go b/server/storage/schema/schema_test.go
index 42563b2ac..f34abe0aa 100644
--- a/server/storage/schema/schema_test.go
+++ b/server/storage/schema/schema_test.go
@@ -31,48 +31,63 @@ func TestUpdateStorageVersion(t *testing.T) {
 	tcs := []struct {
 		name             string
 		version          string
-		metaKeys         [][]byte
+		setupKeys        func(tx backend.BatchTx)
 		expectVersion    *semver.Version
 		expectError      bool
 		expectedErrorMsg string
 	}{
 		{
-			name:             `Backend before 3.6 without "confState" should be rejected`,
+			name:             `Backend before 3.6 without confstate should be rejected`,
 			version:          "",
 			expectVersion:    nil,
+			setupKeys:        func(tx backend.BatchTx) {},
 			expectError:      true,
-			expectedErrorMsg: `cannot determine storage version: missing "confState" key`,
+			expectedErrorMsg: `cannot determine storage version: missing confstate information`,
 		},
 		{
-			name:             `Backend before 3.6 without "term" should be rejected`,
-			version:          "",
-			metaKeys:         [][]byte{MetaConfStateName},
+			name:    `Backend before 3.6 without term should be rejected`,
+			version: "",
+			setupKeys: func(tx backend.BatchTx) {
+				MustUnsafeSaveConfStateToBackend(zap.NewNop(), tx, &raftpb.ConfState{})
+			},
 			expectVersion:    nil,
 			expectError:      true,
-			expectedErrorMsg: `cannot determine storage version: missing "term" key`,
+			expectedErrorMsg: `cannot determine storage version: missing term information`,
 		},
 		{
-			name:          "Backend with 3.5 with all metadata keys should be upgraded to v3.6",
-			version:       "",
-			metaKeys:      [][]byte{MetaTermKeyName, MetaConfStateName},
+			name:    "Backend with 3.5 with all metadata keys should be upgraded to v3.6",
+			version: "",
+			setupKeys: func(tx backend.BatchTx) {
+				MustUnsafeSaveConfStateToBackend(zap.NewNop(), tx, &raftpb.ConfState{})
+				UnsafeUpdateConsistentIndex(tx, 1, 1, false)
+			},
 			expectVersion: &semver.Version{Major: 3, Minor: 6},
 		},
 		{
-			name:          "Backend in 3.6.0 should be skipped",
-			version:       "3.6.0",
-			metaKeys:      [][]byte{MetaTermKeyName, MetaConfStateName, MetaStorageVersionName},
+			name:    "Backend in 3.6.0 should be skipped",
+			version: "3.6.0",
+			setupKeys: func(tx backend.BatchTx) {
+				MustUnsafeSaveConfStateToBackend(zap.NewNop(), tx, &raftpb.ConfState{})
+				UnsafeUpdateConsistentIndex(tx, 1, 1, false)
+			},
 			expectVersion: &semver.Version{Major: 3, Minor: 6},
 		},
 		{
-			name:          "Backend with current version should be skipped",
-			version:       version.Version,
-			metaKeys:      [][]byte{MetaTermKeyName, MetaConfStateName, MetaStorageVersionName},
+			name:    "Backend with current version should be skipped",
+			version: version.Version,
+			setupKeys: func(tx backend.BatchTx) {
+				MustUnsafeSaveConfStateToBackend(zap.NewNop(), tx, &raftpb.ConfState{})
+				UnsafeUpdateConsistentIndex(tx, 1, 1, false)
+			},
 			expectVersion: &semver.Version{Major: 3, Minor: 6},
 		},
 		{
-			name:          "Backend in 3.7.0 should be skipped",
-			version:       "3.7.0",
-			metaKeys:      [][]byte{MetaTermKeyName, MetaConfStateName, MetaStorageVersionName, []byte("future-key")},
+			name:    "Backend in 3.7.0 should be skipped",
+			version: "3.7.0",
+			setupKeys: func(tx backend.BatchTx) {
+				MustUnsafeSaveConfStateToBackend(zap.NewNop(), tx, &raftpb.ConfState{})
+				UnsafeUpdateConsistentIndex(tx, 1, 1, false)
+			},
 			expectVersion: &semver.Version{Major: 3, Minor: 7},
 		},
 	}
@@ -86,16 +101,7 @@ func TestUpdateStorageVersion(t *testing.T) {
 			}
 			tx.Lock()
 			UnsafeCreateMetaBucket(tx)
-			for _, k := range tc.metaKeys {
-				switch string(k) {
-				case string(MetaConfStateName):
-					MustUnsafeSaveConfStateToBackend(lg, tx, &raftpb.ConfState{})
-				case string(MetaTermKeyName):
-					UnsafeUpdateConsistentIndex(tx, 1, 1, false)
-				default:
-					tx.UnsafePut(Meta, k, []byte{})
-				}
-			}
+			tc.setupKeys(tx)
 			if tc.version != "" {
 				UnsafeSetStorageVersion(tx, semver.New(tc.version))
 			}