Warn before running migrations (#1385)

- Refactor MigrationStatus() to return relevant info
- Warn before running migrations

Closes #1368

CHANGELOG.md

@@ -4,6 +4,10 @@
 
 * Kolide will now warn on startup if there are database migrations not yet completed.
 
+* Kolide will prompt for confirmation before running database migrations.
+
+  To disable this, use `kolide prepare db --no-prompt`.
+
 * Kolide now supports emoji, so you can 🔥 to your heart's content.
 
 * When setting the platform for a scheduled query, selecting "All" now clears individually selected platforms.

cli/prepare.go

@@ -1,6 +1,10 @@
 package cli
 
 import (
+	"bufio"
+	"fmt"
+	"os"
+
 	"github.com/WatchBeam/clock"
 	kitlog "github.com/go-kit/kit/log"
 	"github.com/kolide/kolide/server/config"
@@ -27,6 +31,8 @@ To setup kolide infrastructure, use one of the available commands.
 		},
 	}
 
+	noPrompt := false
+
 	var dbCmd = &cobra.Command{
 		Use:   "db",
 		Short: "Given correct database configurations, prepare the databases for use",
@@ -38,6 +44,29 @@ To setup kolide infrastructure, use one of the available commands.
 				initFatal(err, "creating db connection")
 			}
 
+			status, err := ds.MigrationStatus()
+			if err != nil {
+				initFatal(err, "retrieving migration status")
+			}
+
+			switch status {
+			case kolide.AllMigrationsCompleted:
+				fmt.Println("Migrations already completed. Nothing to do.")
+				return
+
+			case kolide.SomeMigrationsCompleted:
+				if !noPrompt {
+					fmt.Printf("################################################################################\n" +
+						"# WARNING:\n" +
+						"#   This will perform Kolide database migrations. Please back up your data before\n" +
+						"#   continuing.\n" +
+						"#\n" +
+						"#   Press Enter to continue, or Control-c to exit.\n" +
+						"################################################################################\n")
+					bufio.NewScanner(os.Stdin).Scan()
+				}
+			}
+
 			if err := ds.MigrateTables(); err != nil {
 				initFatal(err, "migrating db schema")
 			}
@@ -45,9 +74,13 @@ To setup kolide infrastructure, use one of the available commands.
 			if err := ds.MigrateData(); err != nil {
 				initFatal(err, "migrating builtin data")
 			}
+
+			fmt.Println("Migrations completed.")
 		},
 	}
 
+	dbCmd.PersistentFlags().BoolVar(&noPrompt, "no-prompt", false, "disable prompting before migrations (for use in scripts)")
+
 	prepareCmd.AddCommand(dbCmd)
 
 	var testDataCmd = &cobra.Command{

cli/serve.go

@@ -68,7 +68,13 @@ the way that the kolide server works.
 				initFatal(err, "initializing datastore")
 			}
 
-			if ds.MigrationStatus() != nil {
+			migrationStatus, err := ds.MigrationStatus()
+			if err != nil {
+				initFatal(err, "retrieving migration status")
+			}
+
+			switch migrationStatus {
+			case kolide.SomeMigrationsCompleted:
 				fmt.Printf("################################################################################\n"+
 					"# WARNING:\n"+
 					"#   Your Kolide database is missing required migrations. This is likely to cause\n"+
@@ -77,9 +83,16 @@ the way that the kolide server works.
 					"#   Run `%s prepare db` to perform migrations.\n"+
 					"################################################################################\n",
 					os.Args[0])
-				if config.Logging.Debug {
-					fmt.Println("error: ", err.Error())
-				}
+
+			case kolide.NoMigrationsCompleted:
+				fmt.Printf("################################################################################\n"+
+					"# ERROR:\n"+
+					"#   Your Kolide database is not initialized. Kolide cannot start up.\n"+
+					"#\n"+
+					"#   Run `%s prepare db` to initialize the database.\n"+
+					"################################################################################\n",
+					os.Args[0])
+				os.Exit(1)
 			}
 
 			if initializingDS, ok := ds.(initializer); ok {

docs/infrastructure/updating-kolide.md

@@ -78,6 +78,8 @@ Before running the updated server, perform necessary database migrations:
 kolide prepare db
 ```
 
+Note, if you would like to run this in a script, you can use the `--no-prompt` option to disable prompting before the migrations.
+
 The updated Kolide server should now be ready to run:
 
 ```

server/datastore/datastore_migrations_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/kolide/kolide/server/kolide"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -13,12 +14,20 @@ func testMigrationStatus(t *testing.T, ds kolide.Datastore) {
 	}
 
 	require.Nil(t, ds.Drop())
-	require.NotNil(t, ds.MigrationStatus())
+
+	status, err := ds.MigrationStatus()
+	require.Nil(t, err)
+	assert.EqualValues(t, kolide.NoMigrationsCompleted, status)
 
 	require.Nil(t, ds.MigrateTables())
-	require.NotNil(t, ds.MigrationStatus())
 
-	// Should return nil with all migrations completed
+	status, err = ds.MigrationStatus()
+	require.Nil(t, err)
+	assert.EqualValues(t, kolide.SomeMigrationsCompleted, status)
+
 	require.Nil(t, ds.MigrateData())
-	require.Nil(t, ds.MigrationStatus())
+
+	status, err = ds.MigrationStatus()
+	require.Nil(t, err)
+	assert.EqualValues(t, kolide.AllMigrationsCompleted, status)
 }

server/datastore/inmem/inmem.go

@@ -128,8 +128,8 @@ func (d *Datastore) MigrateData() error {
 	return nil
 }
 
-func (m *Datastore) MigrationStatus() error {
-	return nil
+func (m *Datastore) MigrationStatus() (kolide.MigrationStatus, error) {
+	return 0, nil
 }
 
 func (d *Datastore) Drop() error {

server/datastore/mysql/mysql.go

@@ -103,40 +103,42 @@ func (d *Datastore) MigrateData() error {
 	return nil
 }
 
-func (d *Datastore) MigrationStatus() error {
+func (d *Datastore) MigrationStatus() (kolide.MigrationStatus, error) {
 	if tables.MigrationClient.Migrations == nil || data.MigrationClient.Migrations == nil {
-		return errors.New("unexpected nil migrations list")
+		return 0, errors.New("unexpected nil migrations list")
 	}
 
 	lastTablesMigration, err := tables.MigrationClient.Migrations.Last()
 	if err != nil {
-		return errors.New("missing tables migrations")
+		return 0, errors.New("missing tables migrations")
 	}
 
 	currentTablesVersion, err := tables.MigrationClient.GetDBVersion(d.db.DB)
 	if err != nil {
-		return errors.New("cannot get table migration status")
-	}
-
-	if currentTablesVersion != lastTablesMigration.Version {
-		return errors.New("table migrations must be run")
+		return 0, errors.New("cannot get table migration status")
 	}
 
 	lastDataMigration, err := data.MigrationClient.Migrations.Last()
 	if err != nil {
-		return errors.New("missing data migrations")
+		return 0, errors.New("missing data migrations")
 	}
 
 	currentDataVersion, err := data.MigrationClient.GetDBVersion(d.db.DB)
 	if err != nil {
-		return errors.New("cannot get table migration status")
+		return 0, errors.New("cannot get table migration status")
 	}
 
-	if currentDataVersion != lastDataMigration.Version {
-		return errors.New("data migrations must be run")
-	}
+	switch {
+	case currentDataVersion == 0 && currentTablesVersion == 0:
+		return kolide.NoMigrationsCompleted, nil
 
-	return nil
+	case currentTablesVersion != lastTablesMigration.Version ||
+		currentDataVersion != lastDataMigration.Version:
+		return kolide.SomeMigrationsCompleted, nil
+
+	default:
+		return kolide.AllMigrationsCompleted, nil
+	}
 }
 
 // Drop removes database

server/kolide/datastore.go

@@ -26,9 +26,17 @@ type Datastore interface {
 	MigrateData() error
 	// MigrationStatus returns nil if migrations are complete, and an error
 	// if migrations need to be run.
-	MigrationStatus() error
+	MigrationStatus() (MigrationStatus, error)
 }
 
+type MigrationStatus int
+
+const (
+	NoMigrationsCompleted = iota
+	SomeMigrationsCompleted
+	AllMigrationsCompleted
+)
+
 // NotFoundError is returned when the datastore resource cannot be found.
 type NotFoundError interface {
 	error

server/mock/datastore.go

@@ -38,8 +38,8 @@ func (m *Store) MigrateTables() error {
 func (m *Store) MigrateData() error {
 	return nil
 }
-func (m *Store) MigrationStatus() error {
-	return nil
+func (m *Store) MigrationStatus() (kolide.MigrationStatus, error) {
+	return 0, nil
 }
 func (m *Store) Name() string {
 	return "mock"

tools/ci/deploy-k8s-testing.sh

@@ -49,7 +49,7 @@ copy_db() {
 migrate_kolide_db() {
     dbname=$1
 
-    ./build/kolide prepare db \
+    ./build/kolide prepare db --no-prompt \
         --mysql_address=127.0.0.1:3310 \
         --mysql_database=${dbname} \
         --mysql_username=${CLOUDSQL_USER} \
@@ -63,7 +63,7 @@ deploy_pr() {
     $exec_template -json="$jsn" -template=./tools/ci/k8s-templates/pr-deployment.template > /tmp/deployment.yml
 
     # TODO(@groob):
-    # we have to deploy a new copy of redis for each PR. In the future, 
+    # we have to deploy a new copy of redis for each PR. In the future,
     # it would be nice to deploy a single redis instance and allow multiple DBs to connect.
     $exec_template -json="$jsn" -template=./tools/ci/k8s-templates/redis-pr-service.template > /tmp/redis-service.yml
     $exec_template -json="$jsn" -template=./tools/ci/k8s-templates/redis-pr-deployment.template > /tmp/redis-deployment.yml
@@ -117,7 +117,7 @@ main() {
         migrate_kolide_db "${dbname}"
         deploy_pr
     fi
-    
+
     docker stop $(docker ps -a -q)
 }