Merge ~bjornt/maas:bug-1845459-2.6 into maas:2.6

[Björn Tillenius]

LP #1845459: Failure creating KVM pod in CI

Re-raise DatabaseErrors if form_pod.save() raises an exception.

The old code tried to mark the node as failed deployment, which failed
since the transaction was broken. Further more, the transaction wouldn't
be retried, which would have make the deploy succeed.

This is a stop-gap fix that improves the situation slightly. It's made
small so tha it can be backported to 2.6. The real fix is to refactor
the processing of the messages, so that marking the node as failed or
succeeded happens in a separate transaction.

(cherry picked from commit 6df0418010f49265549dac05233bc6cdfbc177e0)

[Björn Tillenius (bjornt)]

Self-approve backport

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b bug-1845459-2.6 lp:~bjornt/maas/+git/maas into -b 2.6 lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/6544/consoleText

[MAAS Lander (maas-lander)]

UNIT TESTS
-b bug-1845459-2.6 lp:~bjornt/maas/+git/maas into -b 2.6 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: a512bc1b0c461d121457864568406f9082aa36b8

review: Approve

diff --git a/src/metadataserver/api_twisted.py b/src/metadataserver/api_twisted.py
index e1d5ded..429e8e3 100644
--- a/src/metadataserver/api_twisted.py
+++ b/src/metadataserver/api_twisted.py
@@ -9,6 +9,7 @@ from collections import defaultdict
 from datetime import datetime
 import json
 
+from django.db.utils import DatabaseError
 from maasserver.api.utils import extract_oauth_key_from_auth_header
 from maasserver.enum import (
     NODE_STATUS,
@@ -211,6 +212,15 @@ def _create_pod_for_deployment(node):
         if pod_form.is_valid():
             try:
                 pod_form.save()
+            except DatabaseError:
+                # Re-raise database errors, since we want it to be
+                # retried if possible. If it's not retriable, we
+                # couldn't mark the node as failed anyway, since the
+                # transaction will be broken.
+                # XXX: We should refactor the processing of messages so
+                # that the node is marked failed/deployed in a seperate
+                # transaction than the one doing the processing.
+                raise
             except Exception:
                 node.mark_failed(comment=POD_CREATION_ERROR, commit=False)
                 log.err(None, "Exception while saving pod form.")
diff --git a/src/metadataserver/tests/test_api_twisted.py b/src/metadataserver/tests/test_api_twisted.py
index 4c2add2..dce81a3 100644
--- a/src/metadataserver/tests/test_api_twisted.py
+++ b/src/metadataserver/tests/test_api_twisted.py
@@ -21,6 +21,7 @@ from unittest.mock import (
 )
 
 from crochet import wait_for
+from django.db.utils import DatabaseError
 from maasserver.enum import NODE_STATUS
 from maasserver.models import (
     Event,
@@ -1125,3 +1126,19 @@ class TestCreatePodForDeployment(MAASServerTestCase):
         self.assertThat(node.status, Equals(NODE_STATUS.FAILED_DEPLOYMENT))
         self.assertThat(node.error_description, DocTestMatches(
             POD_CREATION_ERROR))
+
+    def test__raises_if_save_raises_database_error(self):
+        mock_pod_form = Mock()
+        self.mock_PodForm.return_value = mock_pod_form
+        mock_pod_form.errors = {}
+        mock_pod_form.is_valid = Mock()
+        mock_pod_form.is_valid.return_value = True
+        mock_pod_form.save = Mock()
+        mock_pod_form.save.side_effect = DatabaseError('broken transaction')
+        node = factory.make_Node_with_Interface_on_Subnet(
+            status=NODE_STATUS.DEPLOYING, agent_name="maas-kvm-pod",
+            install_kvm=True)
+        factory.make_StaticIPAddress(interface=node.boot_interface)
+        NodeMetadata.objects.create(
+            node=node, key="virsh_password", value="xyz123")
+        self.assertRaises(DatabaseError, _create_pod_for_deployment, node)