Squash logged errors from failed SQL rollbacks

Currently we unconditionally roll back transactions after error,
even if a commit has already been attempted. Commit is guaranteed
to end a transaction, though, whether by successfully committing
or by rolling back if that fails. As such, we attempt a double
rollback if a transaction fails at commit (for example, for a
constraint violation), which doesn't error but does log angry
warning messages. Ensure we don't try rolling back after commit
runs to prevent this.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #327
Approved by: rhatdan

1 files changed, 17 insertions, 5 deletions

diff --git a/libpod/sql_state_internal.go b/libpod/sql_state_internal.go
index 3881305be..9cc4cd861 100644
--- a/libpod/sql_state_internal.go
+++ b/libpod/sql_state_internal.go
@@ -71,12 +71,14 @@ func checkDB(db *sql.DB, r *Runtime) (err error) {
 
 	const checkRuntimeExists = "SELECT name FROM sqlite_master WHERE type='table' AND name='runtime';"
 
+	committed := false
+
 	tx, err := db.Begin()
 	if err != nil {
 		return errors.Wrapf(err, "error beginning database transaction")
 	}
 	defer func() {
-		if err != nil {
+		if err != nil && !committed {
 			if err2 := tx.Rollback(); err2 != nil {
 				logrus.Errorf("Error rolling back transaction to check runtime table: %v", err2)
 			}
@@ -165,6 +167,8 @@ func checkDB(db *sql.DB, r *Runtime) (err error) {
 			graphDriverName, r.config.StorageConfig.GraphDriverName)
 	}
 
+	committed = true
+
 	if err := tx.Commit(); err != nil {
 		return errors.Wrapf(err, "error committing runtime table transaction in database")
 	}
@@ -291,13 +295,15 @@ func prepareDB(db *sql.DB) (err error) {
         );
         `
 
+	committed := false
+
 	// Create the tables
 	tx, err := db.Begin()
 	if err != nil {
 		return errors.Wrapf(err, "error beginning database transaction")
 	}
 	defer func() {
-		if err != nil {
+		if err != nil && !committed {
 			if err2 := tx.Rollback(); err2 != nil {
 				logrus.Errorf("Error rolling back transaction to create tables: %v", err2)
 			}
@@ -318,6 +324,8 @@ func prepareDB(db *sql.DB) (err error) {
 		return errors.Wrapf(err, "error creating pods table in database")
 	}
 
+	committed = true
+
 	if err := tx.Commit(); err != nil {
 		return errors.Wrapf(err, "error committing table creation transaction in database")
 	}
@@ -810,12 +818,14 @@ func (s *SQLState) addContainer(ctr *Container, pod *Pod) (err error) {
 		}
 	}
 
+	committed := false
+
 	tx, err := s.db.Begin()
 	if err != nil {
 		return errors.Wrapf(err, "error beginning database transaction")
 	}
 	defer func() {
-		if err != nil {
+		if err != nil && !committed {
 			if err2 := tx.Rollback(); err2 != nil {
 				logrus.Errorf("Error rolling back transaction to add container %s: %v", ctr.ID(), err2)
 			}
@@ -947,6 +957,8 @@ func (s *SQLState) addContainer(ctr *Container, pod *Pod) (err error) {
 		}()
 	}
 
+	committed = true
+
 	if err := tx.Commit(); err != nil {
 		return errors.Wrapf(err, "error committing transaction to add container %s", ctr.ID())
 	}
@@ -1047,12 +1059,12 @@ func (s *SQLState) removeContainer(ctr *Container, pod *Pod) (err error) {
 		return errors.Wrapf(err, "error removing container %s from name/ID registry", ctr.ID())
 	}
 
+	committed = true
+
 	if err := tx.Commit(); err != nil {
 		return errors.Wrapf(err, "error committing transaction to remove container %s", ctr.ID())
 	}
 
-	committed = true
-
 	// Remove the container's JSON from disk
 	jsonPath := getSpecPath(s.specsDir, ctr.ID())
 	if err := os.Remove(jsonPath); err != nil {