From 6bd72d21a82c8ac4ee3aa95ca274a83eaea0e0e1 Mon Sep 17 00:00:00 2001
From: Matt Hook <hookenz@gmail.com>
Date: Fri, 1 Oct 2021 20:35:43 +1300
Subject: [PATCH] fix(migration) datastore always marked new and migrations
 skipped EE-1775 (#5788)

* fix issue with broken store init

* minor logic improvement

* Remove fileexists logic as its redundant and handled implicitely by bolt.Open

* Added re-open test on IsNew flag.  Essential for migrations to be able to run
---
 api/bolt/datastore.go         | 18 +++++-------------
 api/bolt/migrate_data_test.go | 13 ++++++++++++-
 api/bolt/teststore.go         |  6 +-----
 api/cmd/portainer/main.go     |  8 ++------
 4 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/api/bolt/datastore.go b/api/bolt/datastore.go
index f729e84b0..448a454e9 100644
--- a/api/bolt/datastore.go
+++ b/api/bolt/datastore.go
@@ -90,20 +90,13 @@ func (store *Store) edition() portainer.SoftwareEdition {
 }
 
 // NewStore initializes a new Store and the associated services
-func NewStore(storePath string, fileService portainer.FileService) (*Store, error) {
-	store := &Store{
+func NewStore(storePath string, fileService portainer.FileService) *Store {
+	return &Store{
 		path:        storePath,
 		fileService: fileService,
 		isNew:       true,
 		connection:  &internal.DbConnection{},
 	}
-
-	databasePath := path.Join(storePath, databaseFileName)
-	if _, err := fileService.FileExists(databasePath); err != nil {
-		return nil, err
-	}
-
-	return store, nil
 }
 
 // Open opens and initializes the BoltDB database.
@@ -120,10 +113,9 @@ func (store *Store) Open() error {
 		return err
 	}
 
-	//if failed to retrieve DBVersion from database
-	//treat it as a new store
-	if _, err := store.VersionService.DBVersion(); err != nil {
-		store.isNew = true
+	// if we have DBVersion in the database then ensure we flag this as NOT a new store
+	if _, err := store.VersionService.DBVersion(); err == nil {
+		store.isNew = false
 	}
 
 	return nil
diff --git a/api/bolt/migrate_data_test.go b/api/bolt/migrate_data_test.go
index 895d8695e..552f2f211 100644
--- a/api/bolt/migrate_data_test.go
+++ b/api/bolt/migrate_data_test.go
@@ -17,12 +17,23 @@ func testVersion(store *Store, versionWant int, t *testing.T) {
 }
 
 func TestMigrateData(t *testing.T) {
-	t.Run("MigrateData for New Store", func(t *testing.T) {
+	t.Run("MigrateData for New Store & Re-Open Check", func(t *testing.T) {
 		store, teardown := MustNewTestStore(false)
 		defer teardown()
+
+		if !store.IsNew() {
+			t.Error("Expect a new DB")
+		}
+
 		store.MigrateData(false)
+
 		testVersion(store, portainer.DBVersion, t)
 		store.Close()
+
+		store.Open()
+		if store.IsNew() {
+			t.Error("Expect store to NOT be new DB")
+		}
 	})
 
 	tests := []struct {
diff --git a/api/bolt/teststore.go b/api/bolt/teststore.go
index bfec62acb..5e9d5899e 100644
--- a/api/bolt/teststore.go
+++ b/api/bolt/teststore.go
@@ -35,11 +35,7 @@ func NewTestStore(init bool) (*Store, func(), error) {
 		return nil, nil, err
 	}
 
-	store, err := NewStore(dataStorePath, fileService)
-	if err != nil {
-		return nil, nil, err
-	}
-
+	store := NewStore(dataStorePath, fileService)
 	err = store.Open()
 	if err != nil {
 		return nil, nil, err
diff --git a/api/cmd/portainer/main.go b/api/cmd/portainer/main.go
index 5a9139dae..2d646d80a 100644
--- a/api/cmd/portainer/main.go
+++ b/api/cmd/portainer/main.go
@@ -57,12 +57,8 @@ func initFileService(dataStorePath string) portainer.FileService {
 }
 
 func initDataStore(dataStorePath string, rollback bool, fileService portainer.FileService, shutdownCtx context.Context) portainer.DataStore {
-	store, err := bolt.NewStore(dataStorePath, fileService)
-	if err != nil {
-		log.Fatalf("failed creating data store: %v", err)
-	}
-
-	err = store.Open()
+	store := bolt.NewStore(dataStorePath, fileService)
+	err := store.Open()
 	if err != nil {
 		log.Fatalf("failed opening store: %v", err)
 	}