Merge ~blake-rouse/maas:fix-bad-postgres-connection into maas:master

[Blake Rouse]

Ensure a good postgresql connection with the connected utility.

All the twisted helpers including the deferToDatabase use the connected() utility. This utility was not verifying that the postgresql connection that it could use was actually a good connection. This now ensures that it is a usable connection before yielding.

How to test:

sudo systemctl start postgresql
sudo systemctl start maas-regiond
sudo systemctl restart postgresql
sudo systemctl start maas-rackd

No stacktrace will occur with "connection already closed". Before this patch "sudo systemctl restart postgresql" would cause the regiond processes to hold onto a connection to a dead postgresql process.

[Mike Pontillo (mpontillo)]

Looks good.

review: Approve

diff --git a/src/maasserver/utils/orm.py b/src/maasserver/utils/orm.py
index 2ec0590..40f8a3f 100644
--- a/src/maasserver/utils/orm.py
+++ b/src/maasserver/utils/orm.py
@@ -616,14 +616,25 @@ def connected():
 
     If there is not yet a connection to the database, this will connect on
     entry and disconnect on exit. Preexisting connections will be left alone.
+
+    If the preexisting connection is not usable it is closed and a new
+    connection is made.
     """
     if connection.connection is None:
+        connection.close_if_unusable_or_obsolete()
         connection.ensure_connection()
         try:
             yield
         finally:
             connection.close()
+    elif connection.is_usable():
+        yield
     else:
+        # Connection is not usable, so we disconnect and reconnect. Since
+        # the connection was previously connected we do not disconnect this
+        # new connection.
+        connection.close_if_unusable_or_obsolete()
+        connection.ensure_connection()
         yield
 
 
diff --git a/src/maasserver/utils/tests/test_orm.py b/src/maasserver/utils/tests/test_orm.py
index 064f1b0..94ee817 100644
--- a/src/maasserver/utils/tests/test_orm.py
+++ b/src/maasserver/utils/tests/test_orm.py
@@ -1004,6 +1004,22 @@ class TestConnected(MAASTransactionServerTestCase):
             self.assertThat(connection.connection, Is(preexisting_connection))
         self.assertThat(connection.connection, Is(preexisting_connection))
 
+    def test__disconnects_and_reconnects_if_not_usable(self):
+        connection.ensure_connection()
+        preexisting_connection = connection.connection
+
+        connection.errors_occurred = True
+        self.patch(connection, "is_usable").return_value = False
+
+        self.assertThat(connection.connection, Not(Is(None)))
+        with orm.connected():
+            self.assertThat(
+                connection.connection, Not(Is(preexisting_connection)))
+            self.assertThat(connection.connection, Not(Is(None)))
+
+        self.assertThat(connection.connection, Not(Is(preexisting_connection)))
+        self.assertThat(connection.connection, Not(Is(None)))
+
 
 class TestWithConnection(MAASTransactionServerTestCase):
     """Tests for the `orm.with_connection` decorator."""