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, 22 insertions, 6 deletions

diff --git a/libpod/sql_state.go b/libpod/sql_state.go
index a1830c468..9efa12111 100644
--- a/libpod/sql_state.go
+++ b/libpod/sql_state.go
@@ -108,12 +108,14 @@ func (s *SQLState) Refresh() (err error) {
 		return ErrDBClosed
 	}
 
+	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 refresh state: %v", err2)
 			}
@@ -135,6 +137,8 @@ func (s *SQLState) Refresh() (err error) {
 		return errors.Wrapf(err, "error refreshing database state")
 	}
 
+	committed = true
+
 	if err := tx.Commit(); err != nil {
 		return errors.Wrapf(err, "error committing transaction to refresh database")
 	}
@@ -407,12 +411,14 @@ func (s *SQLState) SaveContainer(ctr *Container) (err error) {
 		return ErrDBClosed
 	}
 
+	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)
 			}
@@ -447,6 +453,8 @@ func (s *SQLState) SaveContainer(ctr *Container) (err error) {
 		return ErrNoSuchCtr
 	}
 
+	committed = true
+
 	if err := tx.Commit(); err != nil {
 		return errors.Wrapf(err, "error committing transaction to update container %s", ctr.ID())
 	}
@@ -769,12 +777,14 @@ func (s *SQLState) AddPod(pod *Pod) (err error) {
 		return errors.Wrapf(err, "error marshaling pod %s labels to JSON", pod.ID())
 	}
 
+	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 pod %s: %v", pod.ID(), err2)
 			}
@@ -789,6 +799,8 @@ func (s *SQLState) AddPod(pod *Pod) (err error) {
 		return errors.Wrapf(err, "error adding pod %s to database", pod.ID())
 	}
 
+	committed = true
+
 	if err := tx.Commit(); err != nil {
 		return errors.Wrapf(err, "error committing transaction to add pod %s", pod.ID())
 	}
@@ -808,12 +820,14 @@ func (s *SQLState) RemovePod(pod *Pod) (err error) {
 		return ErrDBClosed
 	}
 
+	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 remove pod %s: %v", pod.ID(), err2)
 			}
@@ -838,6 +852,8 @@ func (s *SQLState) RemovePod(pod *Pod) (err error) {
 		return errors.Wrapf(err, "error removing pod %s from name/ID registry", pod.ID())
 	}
 
+	committed = true
+
 	if err := tx.Commit(); err != nil {
 		return errors.Wrapf(err, "error committing transaction to remove pod %s", pod.ID())
 	}
@@ -925,12 +941,12 @@ func (s *SQLState) RemovePodContainers(pod *Pod) (err error) {
 		return errors.Wrapf(err, "error removing pod %s containers from containers table", pod.ID())
 	}
 
+	committed = true
+
 	if err := tx.Commit(); err != nil {
 		return errors.Wrapf(err, "error committing transaction remove pod %s containers", pod.ID())
 	}
 
-	committed = true
-
 	// Remove JSON files from the containers in question
 	hasError := false
 	for _, ctr := range containers {