[lxc-devel] [lxd/master] Fix database upgrade logic not inserting interim versions

freeekanayaka on Github lxc-bot at linuxcontainers.org
Mon Aug 28 21:05:46 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 516 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170828/dd254d60/attachment.bin>
-------------- next part --------------
From 88b8b4c8cd4b2823b77848381206ae33b9e27d91 Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <free.ekanayaka at canonical.com>
Date: Mon, 28 Aug 2017 20:55:55 +0000
Subject: [PATCH] Fix database upgrade logic not inserting interim versions

The new schema.Schema class was not matching existing logic in master,
in that it was only inserting the last schema version, deleting
previous ones.

Signed-off-by: Free Ekanayaka <free.ekanayaka at canonical.com>
---
 lxd/db/schema/query.go       |  9 ------
 lxd/db/schema/schema.go      | 69 ++++++++++++++++++++++++++------------------
 lxd/db/schema/schema_test.go | 23 +++++++++------
 3 files changed, 55 insertions(+), 46 deletions(-)

diff --git a/lxd/db/schema/query.go b/lxd/db/schema/query.go
index dd40680dc..beec95ace 100644
--- a/lxd/db/schema/query.go
+++ b/lxd/db/schema/query.go
@@ -68,12 +68,3 @@ INSERT INTO schema (version, updated_at) VALUES (?, strftime("%s"))
 	_, err := tx.Exec(statement, new)
 	return err
 }
-
-// Update a version in the schema table.
-func updateSchemaVersion(tx *sql.Tx, old, new int) error {
-	statement := `
-UPDATE schema SET version=?, updated_at=strftime("%s") WHERE version=?
-`
-	_, err := tx.Exec(statement, new, old)
-	return err
-}
diff --git a/lxd/db/schema/schema.go b/lxd/db/schema/schema.go
index b21592706..ccdf73aeb 100644
--- a/lxd/db/schema/schema.go
+++ b/lxd/db/schema/schema.go
@@ -159,37 +159,23 @@ func ensureSchemaTableExists(tx *sql.Tx) error {
 
 // Apply any pending update that was not yet applied.
 func ensureUpdatesAreApplied(tx *sql.Tx, updates []Update, hook Hook) error {
-	current := 0 // Current update level in the database
-
 	versions, err := selectSchemaVersions(tx)
 	if err != nil {
 		return fmt.Errorf("failed to fetch update versions: %v", err)
 	}
-	if len(versions) > 1 {
-		return fmt.Errorf(
-			"schema table contains %d rows, expected at most one", len(versions))
-	}
 
-	// If this is a fresh database insert a row with this schema's update
-	// level, otherwise update the existing row (it's okay to do this
-	// before actually running the updates since the transaction will be
-	// rolled back in case of errors).
-	if len(versions) == 0 {
-		err := insertSchemaVersion(tx, len(updates))
-		if err != nil {
-			return fmt.Errorf("failed to insert version %d: %v", len(updates), err)
-		}
-	} else {
-		current = versions[0]
-		if current > len(updates) {
-			return fmt.Errorf(
-				"schema version '%d' is more recent than expected '%d'",
-				current, len(updates))
-		}
-		err := updateSchemaVersion(tx, current, len(updates))
+	current := 0
+	if len(versions) > 0 {
+		err = checkSchemaVersionsHaveNoHoles(versions)
 		if err != nil {
-			return fmt.Errorf("failed to update version %d: %v", current, err)
+			return err
 		}
+		current = versions[len(versions)-1] // Highest recorded version
+	}
+	if current > len(updates) {
+		return fmt.Errorf(
+			"schema version '%d' is more recent than expected '%d'",
+			current, len(updates))
 	}
 
 	// If there are no updates, there's nothing to do.
@@ -210,22 +196,49 @@ func ensureUpdatesAreApplied(tx *sql.Tx, updates []Update, hook Hook) error {
 			return fmt.Errorf("failed to apply update %d: %v", current, err)
 		}
 		current++
+
+		err = insertSchemaVersion(tx, current)
+		if err != nil {
+			return fmt.Errorf("failed to insert version %d", current)
+		}
 	}
 
 	return nil
 }
 
+// Check that the given list of update version numbers doesn't have "holes",
+// that is each version equal the preceeding version plus 1.
+func checkSchemaVersionsHaveNoHoles(versions []int) error {
+	// Sanity check that there are no "holes" in the recorded
+	// versions.
+	for i := range versions[:len(versions)-1] {
+		if versions[i+1] != versions[i]+1 {
+			return fmt.Errorf(
+				"missing updates: %d -> %d", versions[i], versions[i+1])
+		}
+	}
+	return nil
+}
+
 // Check that all the given updates are applied.
 func checkAllUpdatesAreApplied(tx *sql.Tx, updates []Update) error {
 	versions, err := selectSchemaVersions(tx)
 	if err != nil {
 		return fmt.Errorf("failed to fetch update versions: %v", err)
 	}
-	if len(versions) != 1 {
-		return fmt.Errorf("schema table contains %d rows, expected 1", len(versions))
+
+	if len(versions) == 0 {
+		return fmt.Errorf("schema table to contain at least one row")
+	}
+
+	err = checkSchemaVersionsHaveNoHoles(versions)
+	if err != nil {
+		return err
 	}
-	if versions[0] != len(updates) {
-		return fmt.Errorf("update level is %d, expected %d", versions[0], len(updates))
+
+	current := versions[len(versions)-1]
+	if current != len(updates) {
+		return fmt.Errorf("update level is %d, expected %d", current, len(updates))
 	}
 	return nil
 }
diff --git a/lxd/db/schema/schema_test.go b/lxd/db/schema/schema_test.go
index e251573b6..5bac5b863 100644
--- a/lxd/db/schema/schema_test.go
+++ b/lxd/db/schema/schema_test.go
@@ -46,19 +46,24 @@ func TestSchemaEnsure_VersionMoreRecentThanExpected(t *testing.T) {
 	assert.EqualError(t, err, "schema version '1' is more recent than expected '0'")
 }
 
-// If there's more than one row in the schema table, an error is returned.
-func TestSchemaEnsure_ExtraVersions(t *testing.T) {
+// If the database schema contains "holes" in the applied versions, an error is
+// returned.
+func TestSchemaEnsure_MissingVersion(t *testing.T) {
 	schema, db := newSchemaAndDB(t)
+	schema.Add(updateNoop)
 	assert.NoError(t, schema.Ensure(db))
 
-	_, err := db.Exec(`INSERT INTO schema (version, updated_at) VALUES (2, strftime("%s"))`)
-	assert.NoError(t, err)
+	_, err := db.Exec(`INSERT INTO schema (version, updated_at) VALUES (3, strftime("%s"))`)
+
+	schema.Add(updateNoop)
+	schema.Add(updateNoop)
 
 	err = schema.Ensure(db)
-	assert.EqualError(t, err, "schema table contains 2 rows, expected at most one")
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "missing updates: 1 -> 3")
 }
 
-// If the schema has no update, the schema table gets created and has version 0.
+// If the schema has no update, the schema table gets created and has no version.
 func TestSchemaEnsure_ZeroUpdates(t *testing.T) {
 	schema, db := newSchemaAndDB(t)
 
@@ -70,7 +75,7 @@ func TestSchemaEnsure_ZeroUpdates(t *testing.T) {
 
 	versions, err := query.SelectIntegers(tx, "SELECT version FROM SCHEMA")
 	assert.NoError(t, err)
-	assert.Equal(t, []int{0}, versions)
+	assert.Equal(t, []int{}, versions)
 }
 
 // If the schema has updates and no one was applied yet, all of them get
@@ -89,7 +94,7 @@ func TestSchemaEnsure_ApplyAllUpdates(t *testing.T) {
 	// THe update version is recorded.
 	versions, err := query.SelectIntegers(tx, "SELECT version FROM SCHEMA")
 	assert.NoError(t, err)
-	assert.Equal(t, []int{2}, versions)
+	assert.Equal(t, []int{1, 2}, versions)
 
 	// The two updates have been applied in order.
 	ids, err := query.SelectIntegers(tx, "SELECT id FROM test")
@@ -113,7 +118,7 @@ func TestSchemaEnsure_OnlyApplyMissing(t *testing.T) {
 	// All update versions are recorded.
 	versions, err := query.SelectIntegers(tx, "SELECT version FROM SCHEMA")
 	assert.NoError(t, err)
-	assert.Equal(t, []int{2}, versions)
+	assert.Equal(t, []int{1, 2}, versions)
 
 	// The two updates have been applied in order.
 	ids, err := query.SelectIntegers(tx, "SELECT id FROM test")


More information about the lxc-devel mailing list