Merge lp:~allenap/maas/duplicate-vlan-on-start--bug-1583333 into lp:~maas-committers/maas/trunk

[Gavin Panella]

Register rack controllers in the region serially.

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~allenap/maas/duplicate-vlan-on-start--bug-1583333 into lp:maas failed. Below is the output from the failed tests.

Hit:1 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [94.5 kB]
Hit:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:4 http://security.ubuntu.com/ubuntu xenial-security InRelease [94.5 kB]
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/main amd64 Packages [165 kB]
Get:6 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/universe amd64 Packages [76.9 kB]
Get:7 http://security.ubuntu.com/ubuntu xenial-security/main Sources [20.2 kB]
Get:8 http://security.ubuntu.com/ubuntu xenial-security/main amd64 Packages [76.5 kB]
Get:9 http://security.ubuntu.com/ubuntu xenial-security/main Translation-en [29.6 kB]
Fetched 557 kB in 0s (1,215 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-2ubuntu3).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
build-essential is already the newest version (12.1ubuntu2).
bzr is already the newest version (2.7.0-2ubuntu1).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest ve...

=== modified file 'src/maasserver/locks.py'
--- src/maasserver/locks.py	2015-12-01 18:12:59 +0000
+++ src/maasserver/locks.py	2016-05-31 19:19:02 +0000
@@ -1,12 +1,17 @@
-# Copyright 2014-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Region-wide locks."""
 
 __all__ = [
+    "dns",
     "eventloop",
+    "import_images",
+    "node_acquire",
+    "rack_registration",
     "security",
     "startup",
+    "staticip_acquire",
 ]
 
 from maasserver.utils.dblocks import (
@@ -35,3 +40,8 @@
 
 # Lock to prevent concurrent allocation of StaticIPAddress
 staticip_acquire = DatabaseXactLock(8)
+
+# Lock to prevent concurrent registration of rack controllers. This can be a
+# problem because registration involves populating fabrics, VLANs, and other
+# information that may overlap between rack controller.
+rack_registration = DatabaseLock(9)
 
=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-05-26 17:07:30 +0000
+++ src/maasserver/models/node.py	2016-05-31 19:19:02 +0000
@@ -877,6 +877,13 @@
             ]
 
     @property
+    def is_region_controller(self):
+        return self.node_type in [
+            NODE_TYPE.REGION_AND_RACK_CONTROLLER,
+            NODE_TYPE.REGION_CONTROLLER,
+            ]
+
+    @property
     def is_controller(self):
         return self.node_type in [
             NODE_TYPE.REGION_CONTROLLER,
 
=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2016-05-26 17:07:30 +0000
+++ src/maasserver/models/tests/test_node.py	2016-05-31 19:19:02 +0000
@@ -517,6 +517,26 @@
         rack = factory.make_RackController()
         self.assertTrue(rack.is_rack_controller)
 
+    def test_is_region_controller_machine(self):
+        machine = factory.make_Node()
+        self.assertFalse(machine.is_region_controller)
+
+    def test_is_region_controller_device(self):
+        device = factory.make_Device()
+        self.assertFalse(device.is_region_controller)
+
+    def test_is_region_controller_region_controller(self):
+        region = factory.make_RegionController()
+        self.assertTrue(region.is_region_controller)
+
+    def test_is_region_controller_region_rack_controller(self):
+        region_rack = factory.make_RegionRackController()
+        self.assertTrue(region_rack.is_region_controller)
+
+    def test_is_region_controller_rack_controller(self):
+        rack = factory.make_RackController()
+        self.assertFalse(rack.is_region_controller)
+
     def test_is_controller_machine(self):
         machine = factory.make_Node()
         self.assertFalse(machine.is_controller)
 
=== modified file 'src/maasserver/rpc/rackcontrollers.py'
--- src/maasserver/rpc/rackcontrollers.py	2016-05-24 07:30:16 +0000
+++ src/maasserver/rpc/rackcontrollers.py	2016-05-31 19:19:02 +0000
@@ -4,16 +4,19 @@
 """RPC helpers relating to rack controllers."""
 
 __all__ = [
-    "register_rackcontroller",
+    "handle_upgrade",
+    "register",
+    "update_interfaces",
     "update_last_image_sync",
 ]
 
-from django.db import (
-    IntegrityError,
-    transaction,
-)
+from typing import Optional
+
 from django.db.models import Q
-from maasserver import worker_user
+from maasserver import (
+    locks,
+    worker_user,
+)
 from maasserver.enum import NODE_TYPE
 from maasserver.models import (
     Node,
@@ -23,8 +26,13 @@
 )
 from maasserver.models.node import typecast_node
 from maasserver.models.timestampedmodel import now
-from maasserver.utils.orm import transactional
+from maasserver.utils import synchronised
+from maasserver.utils.orm import (
+    transactional,
+    with_connection,
+)
 from provisioningserver.logger import get_maas_logger
+from provisioningserver.utils import typed
 from provisioningserver.utils.twisted import synchronous
 
 
@@ -59,22 +67,37 @@
 
 
 @synchronous
+@with_connection
+@synchronised(locks.rack_registration)
 @transactional
-def register_rackcontroller(
-        system_id=None, hostname='', interfaces={}, url=None,
-        nodegroup_uuid=None):
+def register(system_id=None, hostname='', interfaces=None, url=None):
     """Register a new rack controller if not already registered.
 
     Attempt to see if the rack controller was already registered as a node.
     This can be looked up either by system_id, hostname, or mac address. If
-    found convert the existing node into a rack controller. If not found create
-    a new rack controller. After the rack controller has been registered and
-    successfully connected we will refresh all commissioning data."""
-    rackcontroller = find_and_register_existing(
-        system_id, hostname, interfaces)
-    if rackcontroller is None:
-        rackcontroller = register_new_rackcontroller(system_id, hostname)
-
+    found convert the existing node into a rack controller. If not found
+    create a new rack controller. After the rack controller has been
+    registered and successfully connected we will refresh all commissioning
+    data.
+
+    :return: A ``(rack-controller, refresh-hint)`` tuple.
+    """
+    if interfaces is None:
+        interfaces = {}
+
+    node = find(system_id, hostname, interfaces)
+    if node is None:
+        node = RackController.objects.create(hostname=hostname)
+        maaslog.info("Created new rack controller %s.", node.hostname)
+    elif node.is_rack_controller:
+        maaslog.info("Registering existing rack controller %s.", node.hostname)
+    else:
+        maaslog.info(
+            "Found existing node %s as candidate for rack controller.",
+            node.hostname)
+
+    # This may be a no-op, but it does provide us with a refresh hint.
+    rackcontroller, needs_refresh = upgrade(node)
     # Update `rackcontroller.url` from the given URL, but only when the
     # hostname is not 'localhost' (i.e. the default value used when the master
     # cluster connects).
@@ -87,31 +110,46 @@
         rackcontroller.owner = worker_user.get_worker_user()
         update_fields.append("owner")
     rackcontroller.save(update_fields=update_fields)
-    return rackcontroller
-
-
-def find_and_register_existing(system_id, hostname, interfaces):
-    mac_addresses = set(
-        interface["mac_address"]
-        for _, interface in interfaces.items()
+    # Update networking information every time we see a rack.
+    rackcontroller.update_interfaces(interfaces)
+    # Hint to callers whether or not this rack needs to be refreshed.
+    return rackcontroller, needs_refresh
+
+
+@typed
+def find(system_id: Optional[str], hostname: str, interfaces: dict):
+    """Find an existing node by `system_id`, `hostname`, and `interfaces`.
+
+    :type system_id: str or None
+    :type hostname: str
+    :type interfaces: dict
+    :return: An instance of :class:`Node` or `None`
+    """
+    mac_addresses = {
+        interface["mac_address"] for interface in interfaces.values()
         if "mac_address" in interface
+    }
+    query = (
+        Q(system_id=system_id) | Q(hostname=hostname) |
+        Q(interface__mac_address__in=mac_addresses)
     )
-    node = Node.objects.filter(
-        Q(system_id=system_id) |
-        Q(hostname=hostname) |
-        Q(interface__mac_address__in=mac_addresses)).first()
-    if node is None:
-        return None
+    return Node.objects.filter(query).first()
+
+
+def upgrade(node):
+    """Upgrade `node` to a rack controller if it isn't already.
+
+    Return a hint as to whether a refresh is necessary.
+
+    :return: A ``(rack-controller, refresh-hint)`` tuple.
+    """
     # Refresh whenever an existing node is converted for use as a rack
     # controller. This is needed for two reasons. First, when the region starts
     # it creates a node for itself but only gathers networking information.
     # Second, information about the node may have changed since its last use.
     needs_refresh = True
-    if node.node_type in (
-            NODE_TYPE.RACK_CONTROLLER,
-            NODE_TYPE.REGION_AND_RACK_CONTROLLER):
-        maaslog.info(
-            "Registering existing rack controller %s." % node.hostname)
+
+    if node.is_rack_controller:
         # We don't want to refresh existing rack controllers as each time a
         # rack controller connects to a region it creates four connections.
         # This means for every region we connect to we would refresh
@@ -119,47 +157,17 @@
         # and memory is non-zero our information at this point should be
         # current and the user can always manually refresh.
         needs_refresh = (node.cpu_count == 0 or node.memory == 0)
-    elif node.node_type == NODE_TYPE.REGION_CONTROLLER:
+    elif node.is_region_controller:
         maaslog.info(
-            "Converting %s into a region and rack controller." % node.hostname)
+            "Converting %s into a region and rack controller.", node.hostname)
         node.node_type = NODE_TYPE.REGION_AND_RACK_CONTROLLER
         node.save()
     else:
-        maaslog.info("Converting %s into a rack controller." % node.hostname)
+        maaslog.info("Converting %s into a rack controller.", node.hostname)
         node.node_type = NODE_TYPE.RACK_CONTROLLER
         node.save()
 
-    rackcontroller = typecast_node(node, RackController)
-    # Tell register RPC call a refresh isn't needed
-    rackcontroller.needs_refresh = needs_refresh
-    return rackcontroller
-
-
-def register_new_rackcontroller(system_id, hostname):
-    try:
-        with transaction.atomic():
-            rackcontroller = RackController.objects.create(hostname=hostname)
-            # Tell register RPC call a refresh is needed
-            rackcontroller.needs_refresh = True
-    except IntegrityError as e:
-        # regiond runs on each server with four threads. When a new rack
-        # controller connects it connects to all threads on all servers.  We
-        # use the fact that hostnames must be unique to prevent us from
-        # creating multiple node objects for a single node.
-        maaslog.info(
-            "Rack controller(%s) currently being registered, retrying..." %
-            hostname)
-        rackcontroller = find_and_register_existing(system_id, hostname, {})
-        if rackcontroller is not None:
-            return rackcontroller
-        else:
-            # If we still can't find it something has gone wrong so throw the
-            # exception
-            raise e from None
-    maaslog.info(
-        "%s has been created as a new rack controller" %
-        rackcontroller.hostname)
-    return rackcontroller
+    return typecast_node(node, RackController), needs_refresh
 
 
 @transactional
 
=== modified file 'src/maasserver/rpc/regionservice.py'
--- src/maasserver/rpc/regionservice.py	2016-05-13 21:51:16 +0000
+++ src/maasserver/rpc/regionservice.py	2016-05-31 19:19:02 +0000
@@ -533,14 +533,11 @@
             self, system_id, hostname, interfaces, url, nodegroup_uuid=None):
 
         try:
-            rack_controller = yield deferToDatabase(
-                rackcontrollers.register_rackcontroller, system_id=system_id,
+            # Register, which includes updating interfaces.
+            rack_controller, needs_refresh = yield deferToDatabase(
+                rackcontrollers.register, system_id=system_id,
                 hostname=hostname, interfaces=interfaces, url=url)
 
-            # Update the interfaces.
-            yield deferToDatabase(
-                transactional(rack_controller.update_interfaces), interfaces)
-
             # Check for upgrade.
             if nodegroup_uuid is not None:
                 yield deferToDatabase(
@@ -571,7 +568,7 @@
             # the information about the rack controller if needed.
             log.msg(
                 "Rack controller '%s' has been registered." % self.ident)
-            if self.hostIsRemote and rack_controller.needs_refresh:
+            if self.hostIsRemote and needs_refresh:
                 # Needs to be refresh. Perform this operation in a thread but
                 # we ignore when it is done.
                 log.msg(
 
=== modified file 'src/maasserver/rpc/testing/fixtures.py'
--- src/maasserver/rpc/testing/fixtures.py	2016-03-28 13:54:47 +0000
+++ src/maasserver/rpc/testing/fixtures.py	2016-05-31 19:19:02 +0000
@@ -319,16 +319,16 @@
         protocol = yield endpoints.connectProtocol(endpoint, protocol)
 
         # Mock the registration into the database, as the rack controller is
-        # already created. We reset this once registration is complete so not
-        # to interfer with other tests.
-        reg_original_func = rackcontrollers.register_rackcontroller
-        update_original_func = RackController.update_interfaces
-        rackcontrollers.register_rackcontroller = (
-            lambda *args, **kwargs: rack_controller)
-        RackController.update_interfaces = (
-            lambda *args, **kwargs: None)
+        # already created. We reset this once registration is complete so as
+        # to not interfere with other tests.
+        registered = rack_controller, False  # Hint that no refresh needed.
+        patcher = MonkeyPatcher()
+        patcher.add_patch(
+            rackcontrollers, "register",
+            (lambda *args, **kwargs: registered))
 
         # Register the rack controller with the region.
+        patcher.patch()
         try:
             yield protocol.callRemote(
                 region.RegisterRackController,
@@ -336,9 +336,7 @@
                 hostname=rack_controller.hostname, interfaces={},
                 url=urlparse(""))
         finally:
-            # Restore the original functions.
-            rackcontrollers.register_rackcontroller = reg_original_func
-            RackController.update_interfaces = update_original_func
+            patcher.restore()
 
         defer.returnValue(protocol)
 
 
=== modified file 'src/maasserver/rpc/tests/test_rackcontrollers.py'
--- src/maasserver/rpc/tests/test_rackcontrollers.py	2016-05-24 07:30:16 +0000
+++ src/maasserver/rpc/tests/test_rackcontrollers.py	2016-05-31 19:19:02 +0000
@@ -8,9 +8,11 @@
 import random
 from unittest.mock import sentinel
 
-from django.db import IntegrityError
 from fixtures import FakeLogger
-from maasserver import worker_user
+from maasserver import (
+    locks,
+    worker_user,
+)
 from maasserver.enum import (
     INTERFACE_TYPE,
     IPADDRESS_TYPE,
@@ -21,11 +23,12 @@
     NodeGroupToRackController,
     RackController,
 )
+from maasserver.models.interface import PhysicalInterface
 from maasserver.models.timestampedmodel import now
+from maasserver.rpc import rackcontrollers
 from maasserver.rpc.rackcontrollers import (
     handle_upgrade,
-    register_new_rackcontroller,
-    register_rackcontroller,
+    register,
     update_foreign_dhcp,
     update_interfaces,
     update_last_image_sync,
@@ -34,6 +37,12 @@
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils.orm import reload_object
 from maastesting.matchers import MockCalledOnceWith
+from testtools.matchers import (
+    IsInstance,
+    MatchesAll,
+    MatchesSetwise,
+    MatchesStructure,
+)
 
 
 class TestHandleUpgrade(MAASServerTestCase):
@@ -99,23 +108,23 @@
 
     def test_sets_owner_to_worker_when_none(self):
         node = factory.make_Node()
-        rack_registered = register_rackcontroller(system_id=node.system_id)
+        rack_registered, needs_refresh = register(system_id=node.system_id)
         self.assertEqual(worker_user.get_worker_user(), rack_registered.owner)
 
     def test_leaves_owner_when_owned(self):
         user = factory.make_User()
         node = factory.make_Machine(owner=user)
-        rack_registered = register_rackcontroller(system_id=node.system_id)
+        rack_registered, needs_refresh = register(system_id=node.system_id)
         self.assertEqual(user, rack_registered.owner)
 
     def test_finds_existing_node_by_system_id(self):
         node = factory.make_Node()
-        rack_registered = register_rackcontroller(system_id=node.system_id)
+        rack_registered, needs_refresh = register(system_id=node.system_id)
         self.assertEqual(node.system_id, rack_registered.system_id)
 
     def test_finds_existing_node_by_hostname(self):
         node = factory.make_Node()
-        rack_registered = register_rackcontroller(hostname=node.hostname)
+        rack_registered, needs_refresh = register(hostname=node.hostname)
         self.assertEqual(node.system_id, rack_registered.system_id)
 
     def test_finds_existing_node_by_mac(self):
@@ -131,7 +140,7 @@
                 "enabled": True,
             }
         }
-        rack_registered = register_rackcontroller(interfaces=interfaces)
+        rack_registered, needs_refresh = register(interfaces=interfaces)
         self.assertEqual(node.system_id, rack_registered.system_id)
 
     def test_finds_existing_controller_needs_refresh_with_bad_info(self):
@@ -140,8 +149,8 @@
             NODE_TYPE.REGION_AND_RACK_CONTROLLER,
         ])
         node = factory.make_Node(node_type=node_type)
-        rack_registered = register_rackcontroller(system_id=node.system_id)
-        self.assertTrue(rack_registered.needs_refresh)
+        rack_registered, needs_refresh = register(system_id=node.system_id)
+        self.assertTrue(needs_refresh)
 
     def test_finds_existing_controller_doesnt_need_refresh_good_info(self):
         node_type = random.choice([
@@ -151,8 +160,8 @@
         node = factory.make_Node(
             node_type=node_type, cpu_count=random.randint(1, 32),
             memory=random.randint(1024, 8096))
-        rack_registered = register_rackcontroller(system_id=node.system_id)
-        self.assertFalse(rack_registered.needs_refresh)
+        rack_registered, needs_refresh = register(system_id=node.system_id)
+        self.assertFalse(needs_refresh)
 
     def test_converts_existing_node_sets_needs_refresh_to_true(self):
         node_type = random.choice([
@@ -161,50 +170,54 @@
             NODE_TYPE.REGION_CONTROLLER,
         ])
         node = factory.make_Node(node_type=node_type)
-        rack_registered = register_rackcontroller(system_id=node.system_id)
-        self.assertTrue(rack_registered.needs_refresh)
+        rack_registered, needs_refresh = register(system_id=node.system_id)
+        self.assertTrue(needs_refresh)
 
     def test_find_existing_keeps_type(self):
         node_type = random.choice(
             (NODE_TYPE.RACK_CONTROLLER, NODE_TYPE.REGION_AND_RACK_CONTROLLER))
         node = factory.make_Node(node_type=node_type)
-        register_rackcontroller(system_id=node.system_id)
+        register(system_id=node.system_id)
         self.assertEqual(node_type, node.node_type)
 
     def test_logs_finding_existing_node(self):
         logger = self.useFixture(FakeLogger("maas"))
         node = factory.make_Node(node_type=NODE_TYPE.RACK_CONTROLLER)
-        register_rackcontroller(system_id=node.system_id)
+        register(system_id=node.system_id)
         self.assertEqual(
             "Registering existing rack controller %s." % node.hostname,
             logger.output.strip())
 
     def test_converts_region_controller(self):
         node = factory.make_Node(node_type=NODE_TYPE.REGION_CONTROLLER)
-        rack_registered = register_rackcontroller(system_id=node.system_id)
+        rack_registered, needs_refresh = register(system_id=node.system_id)
         self.assertEqual(
             rack_registered.node_type, NODE_TYPE.REGION_AND_RACK_CONTROLLER)
 
     def test_logs_converting_region_controller(self):
         logger = self.useFixture(FakeLogger("maas"))
         node = factory.make_Node(node_type=NODE_TYPE.REGION_CONTROLLER)
-        register_rackcontroller(system_id=node.system_id)
+        register(system_id=node.system_id)
         self.assertEqual(
-            "Converting %s into a region and rack controller." % node.hostname,
-            logger.output.strip())
+            "Found existing node %s as candidate for rack controller.\n"
+            "Converting %s into a region and rack controller.\n"
+            % (node.hostname, node.hostname),
+            logger.output)
 
     def test_converts_existing_node(self):
         node = factory.make_Node(node_type=NODE_TYPE.MACHINE)
-        rack_registered = register_rackcontroller(system_id=node.system_id)
+        rack_registered, needs_refresh = register(system_id=node.system_id)
         self.assertEqual(rack_registered.node_type, NODE_TYPE.RACK_CONTROLLER)
 
     def test_logs_converting_existing_node(self):
         logger = self.useFixture(FakeLogger("maas"))
         node = factory.make_Node(node_type=NODE_TYPE.MACHINE)
-        register_rackcontroller(system_id=node.system_id)
+        register(system_id=node.system_id)
         self.assertEqual(
-            "Converting %s into a rack controller." % node.hostname,
-            logger.output.strip())
+            "Found existing node %s as candidate for rack controller.\n"
+            "Converting %s into a rack controller.\n"
+            % (node.hostname, node.hostname),
+            logger.output)
 
     def test_creates_new_rackcontroller(self):
         factory.make_Node()
@@ -218,7 +231,7 @@
                 "enabled": True,
             }
         }
-        register_rackcontroller(interfaces=interfaces)
+        register(interfaces=interfaces)
         self.assertEqual(node_count + 1, len(Node.objects.all()))
 
     def test_creates_new_rackcontroller_sets_needs_refresh_to_true(self):
@@ -231,45 +244,84 @@
                 "enabled": True,
             }
         }
-        rack_registered = register_rackcontroller(
-            interfaces=interfaces)
-        self.assertTrue(rack_registered.needs_refresh)
+        rack_registered, needs_refresh = register(interfaces=interfaces)
+        self.assertTrue(needs_refresh)
 
     def test_logs_creating_new_rackcontroller(self):
         logger = self.useFixture(FakeLogger("maas"))
         hostname = factory.make_name("hostname")
-        register_rackcontroller(hostname=hostname)
+        register(hostname=hostname)
         self.assertEqual(
-            "%s has been created as a new rack controller" % hostname,
+            "Created new rack controller %s." % hostname,
             logger.output.strip())
 
-    def test_retries_existing_on_new_integrity_error(self):
-        hostname = factory.make_name("hostname")
-        node = factory.make_Node(hostname=hostname)
-        patched_create = self.patch(RackController.objects, 'create')
-        patched_create.side_effect = IntegrityError()
-        rack_registered = register_new_rackcontroller(None, hostname)
-        self.assertEqual(rack_registered.system_id, node.system_id)
-
-    def test_raises_exception_on_new_and_existing_failure(self):
-        patched_create = self.patch(RackController.objects, 'create')
-        patched_create.side_effect = IntegrityError()
-        self.assertRaises(
-            IntegrityError, register_new_rackcontroller,
-            None, factory.make_name("hostname"))
-
-    def test_logs_retrying_existing_on_new_integrity_error(self):
-        logger = self.useFixture(FakeLogger("maas"))
-        hostname = factory.make_name("hostname")
-        patched_create = self.patch(RackController.objects, 'create')
-        patched_create.side_effect = IntegrityError()
-        try:
-            register_new_rackcontroller(None, hostname)
-        except IntegrityError:
-            pass
-        self.assertEqual(
-            "Rack controller(%s) currently being registered, retrying..." %
-            hostname, logger.output.strip())
+    def test_sets_interfaces(self):
+        # Interfaces are set on new rack controllers.
+        interfaces = {
+            factory.make_name("eth0"): {
+                "type": "physical",
+                "mac_address": factory.make_mac_address(),
+                "parents": [],
+                "links": [],
+                "enabled": True,
+            }
+        }
+        rack_registered, needs_refresh = register(interfaces=interfaces)
+        self.assertThat(
+            rack_registered.interface_set.all(),
+            MatchesSetwise(*(
+                MatchesAll(
+                    IsInstance(PhysicalInterface),
+                    MatchesStructure.byEquality(
+                        name=name, mac_address=interface["mac_address"],
+                        enabled=interface["enabled"],
+                    ),
+                    first_only=True,
+                )
+                for name, interface in interfaces.items()
+            )))
+
+    def test_updates_interfaces(self):
+        # Interfaces are set on existing rack controllers.
+        rack_controller = factory.make_RackController()
+        interfaces = {
+            factory.make_name("eth0"): {
+                "type": "physical",
+                "mac_address": factory.make_mac_address(),
+                "parents": [],
+                "links": [],
+                "enabled": True,
+            }
+        }
+        rack_registered, needs_refresh = register(
+            rack_controller.system_id, interfaces=interfaces)
+        self.assertThat(
+            rack_registered.interface_set.all(),
+            MatchesSetwise(*(
+                MatchesAll(
+                    IsInstance(PhysicalInterface),
+                    MatchesStructure.byEquality(
+                        name=name, mac_address=interface["mac_address"],
+                        enabled=interface["enabled"],
+                    ),
+                    first_only=True,
+                )
+                for name, interface in interfaces.items()
+            )))
+
+    def test_registers_with_rack_registration_lock_held(self):
+        lock_status = []
+
+        def record_lock_status(*args):
+            lock_status.append(locks.rack_registration.is_locked())
+            return None  # Simulate that no rack found.
+
+        find = self.patch(rackcontrollers, "find")
+        find.side_effect = record_lock_status
+
+        register()
+
+        self.assertEqual([True], lock_status)
 
 
 class TestUpdateForeignDHCP(MAASServerTestCase):
 
=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
--- src/maasserver/rpc/tests/test_regionservice.py	2016-05-18 09:00:43 +0000
+++ src/maasserver/rpc/tests/test_regionservice.py	2016-05-31 19:19:02 +0000
@@ -522,6 +522,10 @@
             port=random.randint(1, 400))
         protocol.transport.getHost.return_value = host
         mock_deferToDatabase = self.patch(regionservice, "deferToDatabase")
+        mock_deferToDatabase.side_effect = [
+            succeed((rack_controller, False)),
+            succeed(None),
+        ]
         yield call_responder(
             protocol, RegisterRackController, {
                 "system_id": rack_controller.system_id,