Merge lp:~allenap/maas/no-such-cluster-for-all-the-things into lp:~maas-committers/maas/trunk

[Raphaël Badin]

When update_leases() does not find the given cluster, raise NoSuchCluster.

Previously it was raising a Django HTTP 404 error.

[Gavin Panella (allenap)]

I was seeing logs like the following in maas.log when starting from a
clean-slate (i.e. no auto-accepted master cluster yet exists in the
database):

  maas.lease_upload_service: [ERROR] Failed to upload leases:
  Code<UNKNOWN>: Unknown Error

The RPC connection was dropped after this because that's what AMP does:
it drops connections when unrecognised exceptions are transmitted. The
cluster would eventually be registered, but there can be more failures
before it does.

With this branch I get:

  maas.lease_upload_service: [ERROR] Failed to upload leases: The
  cluster (a.k.a. node-group) with UUID
  d791c8c0-244e-4144-8cfc-b4dbc8cba188 could not be found.

and the RPC connection does not drop. This means that when
start-cluster-controller (upstart job: maas-cluster-register) wakes up
(every 60 seconds) there's a good chance that the cluster will be
connected to the region, and the registration will be successful.

[Newell Jensen (newell-jensen)]

Straight forward change. Approved.

[Newell Jensen (newell-jensen)]

review: Approve

[Graham Binns (gmb)]

Whilst you're playing in this paddling pool, would it be worth
updating the (API)RPCErrorMiddleware to handle NoSuchCluster nicely?
(I don't know if this would ever arise in the UI, but if it arises in
the API we could certainly catch the error and do something
meaningful.

=== modified file 'src/maasserver/rpc/leases.py'
--- src/maasserver/rpc/leases.py	2014-09-10 16:20:31 +0000
+++ src/maasserver/rpc/leases.py	2014-10-01 23:28:16 +0000
@@ -16,7 +16,6 @@
     "update_leases",
 ]
 
-from django.shortcuts import get_object_or_404
 from maasserver.models.dhcplease import DHCPLease
 from maasserver.models.macaddress import update_mac_cluster_interfaces
 from maasserver.models.nodegroup import NodeGroup
@@ -24,6 +23,7 @@
 from provisioningserver.pserv_services.lease_upload_service import (
     convert_mappings_to_leases,
     )
+from provisioningserver.rpc.exceptions import NoSuchCluster
 from provisioningserver.utils.twisted import synchronous
 
 
@@ -39,9 +39,16 @@
 
     Converts the mappings format into a dict that
     DHCPLease.objects.update_leases needs and then calls it.
+
+    :raises NoSuchCluster: If the cluster identified by `uuid` does not
+        exist.
     """
-    nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
-    leases = convert_mappings_to_leases(mappings)
-    DHCPLease.objects.update_leases(nodegroup, leases)
-    update_mac_cluster_interfaces(leases, nodegroup)
-    return {}
+    try:
+        nodegroup = NodeGroup.objects.get_by_natural_key(uuid)
+    except NodeGroup.DoesNotExist:
+        raise NoSuchCluster.from_uuid(uuid)
+    else:
+        leases = convert_mappings_to_leases(mappings)
+        DHCPLease.objects.update_leases(nodegroup, leases)
+        update_mac_cluster_interfaces(leases, nodegroup)
+        return {}

=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
--- src/maasserver/rpc/tests/test_regionservice.py	2014-10-01 18:41:08 +0000
+++ src/maasserver/rpc/tests/test_regionservice.py	2014-10-01 23:28:16 +0000
@@ -333,6 +333,13 @@
         observed = yield deferToThread(get_cluster_interface)
         self.assertThat(observed, Equals(cluster_interface))
 
+    @wait_for_reactor
+    def test__raises_NoSuchCluster_if_cluster_not_found(self):
+        uuid = factory.make_name("uuid")
+        d = call_responder(
+            Region(), UpdateLeases, {b"uuid": uuid, b"mappings": []})
+        return assert_fails_with(d, NoSuchCluster)
+
 
 class TestRegionProtocol_GetBootSources(TransactionTestCase):
 

=== modified file 'src/provisioningserver/rpc/region.py'
--- src/provisioningserver/rpc/region.py	2014-09-24 22:20:11 +0000
+++ src/provisioningserver/rpc/region.py	2014-10-01 23:28:16 +0000
@@ -121,7 +121,9 @@
              (b"mac", amp.Unicode())]))
     ]
     response = []
-    errors = []
+    errors = {
+        NoSuchCluster: b"NoSuchCluster",
+    }
 
 
 class GetArchiveMirrors(amp.Command):