Merge lp:~ltrager/maas/region_refresh into lp:~maas-committers/maas/trunk

[Lee Trager]

Refresh region hardware and networking information on start

[Blake Rouse (blake-rouse)]

review: Needs Information

[Lee Trager (ltrager)]

I accidentally proposed to merge this into one of my branches instead of lp:maas. I've responded to your comments here but please finish the review at https://code.launchpad.net/~ltrager/maas/region_refresh/+merge/297015

[Blake Rouse (blake-rouse)]

Looks good. Not going to block you on anything. Please fix the issues I pointed out and respond to my questions before landing.

review: Approve

[Lee Trager (ltrager)]

Thanks for the review. I've responded to your comments below and corrected to issues you found.

[Gavin Panella (allenap)]

This needs a lot of work! I've put lots of comments inline.

review: Needs Fixing

[Lee Trager (ltrager)]

I've moved the region creation and refresh to happen during start_up and made the corrections you've suggested.

To answer you're question about logging, it was originally done because the user could run a refresh over API/websocket. The only other reason to log it as a request_event is to have a database record of when the details were last refreshed. I could just remove that and leave logging to about refresh to syslog.

[Gavin Panella (allenap)]

It's getting close, but no cigar just yet. The stickiest bits are the newest bits: the stuff in start_up.py. I've put my suggested changes into lp:~allenap/maas/ltrager--region_refresh so just merge that and see what I think needs to change. I didn't update any tests or write new one though (and I noticed that get_running_controller is barely tested anyway).

review: Needs Fixing

[Lee Trager (ltrager)]

Thanks for the review. I'm not sure we want to move creating the region object to Controller.objects.get_running_controller(). get_running_controller() is called in various places throughout MAAS. If a user deletes /var/lib/maas/maas_id, or there is a filesystem read error, while MAAS is running, a call to get_running_controller will create a new object without any hardware or networking details, nor will it have any connections registered. This could cause really weird errors. I think its safer to only create region objects in start_up() and if a user deletes /var/lib/maas/maas_id let things fail. I've fixed everything else.

[Gavin Panella (allenap)]

There are a few more issues that need clearing up, but +1 because
they're small.

> Thanks for the review. I'm not sure we want to move creating the
> region object to Controller.objects.get_running_controller().
> get_running_controller() is called in various places throughout MAAS.
> If a user deletes /var/lib/maas/maas_id, or there is a filesystem read
> error, while MAAS is running, a call to get_running_controller will
> create a new object without any hardware or networking details, nor
> will it have any connections registered. This could cause really weird
> errors. I think its safer to only create region objects in start_up()
> and if a user deletes /var/lib/maas/maas_id let things fail. I've
> fixed everything else.

There are only three call-sites for get_running_controller, but I kind
of agree with your point. I think create_region_obj belongs elsewhere,
but leave it for now: I have a branch that moves it elsewhere which I'll
propose after this lands.

[...]
> The caller isn't necessarily wrong. Calling RackController.refresh()
> without this will work its just that it will go over RPC. If the
> maas_url for the rack controller is configured with a VIP the node
> result data may then go over the network and be processed by a foreign
> region controller. I want to prevent that and use the shortest and
> quickest path for refresh available.

I think you're missing my point (and also trying to over-optimise).

The caller may be explicitly getting a RackController:

  rack = typecast_node(node, RackController)
  return rack.refresh()

but the work is then being silently redirected to the region. There's no
way to say "do this on the *rack* dammit".

One problem is that refreshing a rack-not-region controller returns
rapidly with the basic system info, whereas refreshing a region waits
for all the refresh scripts to run too. The caller doesn't know this for
sure. All in, I think refreshing is poorly factored: getting basic
system info should be one task (done synchronously during start-up of a
region, and done during the first handshake with a rack), and running
the commissioning scripts a separate task, for both region and rack.
Don't change that here though; enough has changed already!

An underlying problem is that there's no RegionRackController model and
corresponding manager. If there was, it would make sense for its refresh
method to choose the region if that's where it's running, else ask the
rack. Getting RackController's refresh to alter the method's dispatch is
black-market polymorphism.

> The reason I put that logic in RackController.refresh() and not rely
> on the caller is its something that could be easily overlooked. For
> example if we decide to bring back the rack refresh API and websocket
> the developer will have to remember to put this logic in both places.

There's a lot that can be overlooked; that's one reason for tests :)

In addition to the missing RegionRackController model, there's a problem
with the use of typecast_node and node_type:

- typecast_node can be used to cast a Node to any node class, whether it
  makes sense or not.

- node_type can be modified so that the model does not match.

This is a hard...

review: Approve

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-06-10 23:22:35 +0000
+++ src/maasserver/models/node.py	2016-06-16 01:30:55 +0000
@@ -160,6 +160,10 @@
 )
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.power import QUERY_POWER_TYPES
+from provisioningserver.refresh import (
+    get_sys_info,
+    refresh,
+)
 from provisioningserver.rpc.cluster import (
     AddChassis,
     DisableAndShutoffRackd,
@@ -179,6 +183,7 @@
 )
 from provisioningserver.utils.enum import map_enum_reverse
 from provisioningserver.utils.env import get_maas_id
+from provisioningserver.utils.fs import NamedLock
 from provisioningserver.utils.twisted import (
     asynchronous,
     callOut,
@@ -189,6 +194,7 @@
     inlineCallbacks,
     succeed,
 )
+from twisted.internet.threads import deferToThread
 
 
 maaslog = get_maas_logger("node")
@@ -546,7 +552,7 @@
             ]}
 
     def get_running_controller(self):
-        return self.get(system_id=get_maas_id())
+        return self.get(system_id=get_maas_id(), owner_id__isnull=False)
 
 
 class RackControllerManager(ControllerManager):
@@ -3629,30 +3635,28 @@
     @transactional
     def _signal_start_of_refresh(self):
         self._register_request_event(
-            self.owner, EVENT_TYPES.REQUEST_RACK_CONTROLLER_REFRESH,
+            self.owner, EVENT_TYPES.REQUEST_CONTROLLER_REFRESH,
             action='starting refresh')
 
     @transactional
-    def _process_refresh(self, response, previous_result_ids):
-        # Avoid circular imports.
-        from metadataserver.models import NodeResult
-
-        # Delete existing results. This is a bit shonk because it assumes that
-        # the commissioning scripts will run to completion on the controller.
-        # After deletion there will be a period when the controller has no
-        # results in the database while waiting to recieve the new data. If
-        # uploading fails this will leave the controller with no NodeResults.
-        NodeResult.objects.filter(id__in=previous_result_ids).delete()
-
+    def _process_sys_info(self, response):
+        update_fields = []
+        if response['hostname'] != '':
+            self.hostname = response['hostname']
+            update_fields.append('hostname')
         if response['architecture'] != '':
             self.architecture = response['architecture']
+            update_fields.append('architecture')
         if response['osystem'] != '':
             self.osystem = response['osystem']
+            update_fields.append('osystem')
         if response['distro_series'] != '':
             self.distro_series = response['distro_series']
+            update_fields.append('distro_series')
         if response['interfaces'] != {}:
             self.update_interfaces(response['interfaces'])
-        self.save()
+        if len(update_fields) > 0:
+            self.save(update_fields=update_fields)
 
 
 class RackController(Controller):
@@ -3667,6 +3671,19 @@
         super(RackController, self).__init__(
             node_type=NODE_TYPE.RACK_CONTROLLER, *args, **kwargs)
 
+    @transactional
+    def _delete_node_result(self, ids):
+        # Avoid circular imports.
+        from metadataserver.models import NodeResult
+
+        # Delete existing node results. This is a bit shonky for rack
+        # controllers because it assumes that the commissioning scripts will
+        # run to completion on rack controller. After deletion there will be a
+        # period when the rack controller has no results in the database while
+        # waiting to recieve the new data. If uploading fails this will leave
+        # the rack controller with no NodeResults.
+        NodeResult.objects.filter(id__in=ids).delete()
+
     @inlineCallbacks
     def refresh(self):
         """Refresh the hardware and networking columns of the rack controller.
@@ -3674,6 +3691,13 @@
         :raises NoConnectionsAvailable: If no connections to the cluster
             are available.
         """
+        if self.system_id == get_maas_id():
+            # If the refresh is occuring on the running region execute it using
+            # the region process. This avoids using RPC and sends the node
+            # results back to this host when in HA.
+            yield typecast_node(self, RegionController).refresh()
+            return
+
         client = yield getClientFor(self.system_id, timeout=1)
 
         token = yield deferToDatabase(self._get_token_for_controller)
@@ -3690,10 +3714,11 @@
                 token_key=token.key, token_secret=token.secret)
         except RefreshAlreadyInProgress:
             # If another refresh is in progress let the other process
-            # handle it and don't update the database
+            # handle it and don't update the database.
             return
         else:
-            yield deferToDatabase(self._process_refresh, response, result_ids)
+            yield deferToDatabase(self._delete_node_result, result_ids)
+            yield deferToDatabase(self._process_sys_info, response)
 
     def add_chassis(
             self, user, chassis_type, hostname, username=None, password=None,
@@ -3915,6 +3940,38 @@
         else:
             super().delete()
 
+    @transactional
+    def _delete_node_results(self):
+        # Avoid circular imports.
+        from metadataserver.models import NodeResult
+
+        NodeResult.objects.filter(node=self).delete()
+
+    @inlineCallbacks
+    def refresh(self):
+        """Refresh the region controller."""
+        # XXX ltrager 2016-05-25 - MAAS doesn't have an RPC method between
+        # region controllers. If this method refreshes a foreign region
+        # controller the foreign region controller will contain the running
+        # region's hardware and networking information.
+        if self.system_id != get_maas_id():
+            raise NotImplementedError(
+                'Can only refresh the running region controller')
+
+        try:
+            with NamedLock('refresh'):
+                token = yield deferToDatabase(self._get_token_for_controller)
+                yield deferToDatabase(self._signal_start_of_refresh)
+                yield deferToDatabase(self._delete_node_results)
+                sys_info = yield deferToThread(get_sys_info)
+                yield deferToDatabase(self._process_sys_info, sys_info)
+                yield deferToThread(
+                    refresh, self.system_id, token.consumer.key, token.key,
+                    token.secret, 'http://127.0.0.1:5240/MAAS')
+        except NamedLock.NotAvailable:
+            # Refresh already running.
+            pass
+
 
 class Device(Node):
     """A non-installable node."""

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2016-06-10 01:48:36 +0000
+++ src/maasserver/models/tests/test_node.py	2016-06-16 01:30:55 +0000
@@ -156,6 +156,7 @@
     map_enum,
     map_enum_reverse,
 )
+from provisioningserver.utils.fs import NamedLock
 from testtools import ExpectedException
 from testtools.matchers import (
     AfterPreprocessing,
@@ -6918,10 +6919,18 @@
         token.consumer.key  # Fetch this now while we're in the database.
         return token
 
+    def test_refresh_calls_region_refresh_when_on_node(self):
+        rack = factory.make_RackController()
+        self.patch(node_module, 'get_maas_id').return_value = rack.system_id
+        mock_refresh = self.patch(node_module.RegionController, 'refresh')
+        rack.refresh()
+        self.assertThat(mock_refresh, MockCalledOnce())
+
     @wait_for_reactor
     @defer.inlineCallbacks
     def test_refresh_issues_rpc_call(self):
         self.protocol.RefreshRackControllerInfo.return_value = defer.succeed({
+            'hostname': self.rackcontroller.hostname,
             'architecture': self.rackcontroller.architecture,
             'osystem': '',
             'distro_series': '',
@@ -6942,6 +6951,7 @@
     @defer.inlineCallbacks
     def test_refresh_logs_user_request(self):
         self.protocol.RefreshRackControllerInfo.return_value = defer.succeed({
+            'hostname': self.rackcontroller.hostname,
             'architecture': self.rackcontroller.architecture,
             'osystem': '',
             'distro_series': '',
@@ -6954,13 +6964,14 @@
         yield self.rackcontroller.refresh()
         self.assertThat(register_event, MockCalledOnceWith(
             self.rackcontroller.owner,
-            EVENT_TYPES.REQUEST_RACK_CONTROLLER_REFRESH,
+            EVENT_TYPES.REQUEST_CONTROLLER_REFRESH,
             action='starting refresh'))
 
     @wait_for_reactor
     @defer.inlineCallbacks
     def test_refresh_clears_node_results(self):
         self.protocol.RefreshRackControllerInfo.return_value = defer.succeed({
+            'hostname': self.rackcontroller.hostname,
             'architecture': self.rackcontroller.architecture,
             'osystem': '',
             'distro_series': '',
@@ -6988,6 +6999,7 @@
     @wait_for_reactor
     @defer.inlineCallbacks
     def test_refresh_sets_extra_values(self):
+        hostname = factory.make_hostname()
         osystem = factory.make_name('osystem')
         distro_series = factory.make_name('distro_series')
         interfaces = {
@@ -7001,6 +7013,7 @@
         }
 
         self.protocol.RefreshRackControllerInfo.return_value = defer.succeed({
+            'hostname': hostname,
             'architecture': self.rackcontroller.architecture,
             'osystem': osystem,
             'distro_series': distro_series,
@@ -7010,6 +7023,7 @@
         yield self.rackcontroller.refresh()
         rackcontroller = yield deferToDatabase(
             reload_object, self.rackcontroller)
+        self.assertEquals(hostname, rackcontroller.hostname)
         self.assertEquals(osystem, rackcontroller.osystem)
         self.assertEquals(distro_series, rackcontroller.distro_series)
 
@@ -7377,3 +7391,171 @@
         region = factory.make_RegionController()
         region.delete()
         self.assertIsNone(reload_object(region))
+
+
+class TestRegionControllerRefresh(MAASTransactionServerTestCase):
+
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test__only_runs_on_running_region(self):
+        region = yield deferToDatabase(factory.make_RegionController)
+
+        with ExpectedException(NotImplementedError):
+            yield region.refresh()
+
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test__acquires_and_releases_lock(self):
+        def mock_refresh(*args, **kwargs):
+            lock = NamedLock('refresh')
+            self.assertTrue(lock.is_locked())
+        self.patch(node_module, 'refresh', mock_refresh)
+        region = yield deferToDatabase(factory.make_RegionController)
+        self.patch(node_module, 'get_maas_id').return_value = region.system_id
+        self.patch(node_module, 'get_sys_info').return_value = {
+            'hostname': region.hostname,
+            'architecture': region.architecture,
+            'osystem': '',
+            'distro_series': '',
+            'interfaces': {},
+        }
+        yield region.refresh()
+        lock = NamedLock('refresh')
+        self.assertFalse(lock.is_locked())
+
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test__lock_released_on_error(self):
+        exception = factory.make_exception()
+        self.patch(node_module, 'refresh').side_effect = exception
+        region = yield deferToDatabase(factory.make_RegionController)
+        self.patch(node_module, 'get_maas_id').return_value = region.system_id
+        self.patch(node_module, 'get_sys_info').return_value = {
+            'hostname': region.hostname,
+            'architecture': region.architecture,
+            'osystem': '',
+            'distro_series': '',
+            'interfaces': {},
+        }
+        with ExpectedException(type(exception)):
+            yield region.refresh()
+        lock = NamedLock('refresh')
+        self.assertFalse(lock.is_locked())
+
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test__does_nothing_when_locked(self):
+        region = yield deferToDatabase(factory.make_RegionController)
+        self.patch(node_module, 'get_maas_id').return_value = region.system_id
+        mock_deferToDatabase = self.patch(node_module, 'deferToDatabase')
+        with NamedLock('refresh'):
+            yield region.refresh()
+        self.assertThat(mock_deferToDatabase, MockNotCalled())
+
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test__logs_user_request(self):
+        region = yield deferToDatabase(factory.make_RegionController)
+        self.patch(node_module, 'get_maas_id').return_value = region.system_id
+        self.patch(node_module, 'refresh')
+        self.patch(node_module, 'get_sys_info').return_value = {
+            'hostname': region.hostname,
+            'architecture': region.architecture,
+            'osystem': '',
+            'distro_series': '',
+            'interfaces': {},
+        }
+        register_event = self.patch(region, '_register_request_event')
+
+        yield region.refresh()
+        self.assertThat(register_event, MockCalledOnceWith(
+            region.owner,
+            EVENT_TYPES.REQUEST_CONTROLLER_REFRESH,
+            action='starting refresh'))
+
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test__runs_refresh(self):
+        def get_token_for_controller(region):
+            token = NodeKey.objects.get_token_for_node(region)
+            token.consumer.key  # Fetch this now while we're in the database.
+            return token
+
+        region = yield deferToDatabase(factory.make_RegionController)
+        self.patch(node_module, 'get_maas_id').return_value = region.system_id
+        mock_refresh = self.patch(node_module, 'refresh')
+        self.patch(node_module, 'get_sys_info').return_value = {
+            'hostname': region.hostname,
+            'architecture': region.architecture,
+            'osystem': '',
+            'distro_series': '',
+            'interfaces': {},
+        }
+        yield region.refresh()
+        token = yield deferToDatabase(get_token_for_controller, region)
+
+        self.expectThat(
+            mock_refresh,
+            MockCalledOnceWith(
+                region.system_id, token.consumer.key, token.key,
+                token.secret, 'http://127.0.0.1:5240/MAAS'))
+
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test__clears_node_results(self):
+        region = yield deferToDatabase(factory.make_RegionController)
+        node_result = yield deferToDatabase(
+            factory.make_NodeResult_for_installation, node=region)
+        self.patch(node_module, 'get_maas_id').return_value = region.system_id
+        self.patch(node_module, 'refresh')
+        self.patch(node_module, 'get_sys_info').return_value = {
+            'hostname': region.hostname,
+            'architecture': region.architecture,
+            'osystem': '',
+            'distro_series': '',
+            'interfaces': {},
+        }
+        yield region.refresh()
+
+        def has_results():
+            return NodeResult.objects.filter(id=node_result.id).exists()
+
+        self.assertFalse((yield deferToDatabase(has_results)))
+
+    @wait_for_reactor
+    @defer.inlineCallbacks
+    def test__sets_extra_values(self):
+        region = yield deferToDatabase(factory.make_RegionController)
+        self.patch(node_module, 'get_maas_id').return_value = region.system_id
+        self.patch(node_module, 'refresh')
+        hostname = factory.make_hostname()
+        osystem = factory.make_name('osystem')
+        distro_series = factory.make_name('distro_series')
+        interfaces = {
+            "eth0": {
+                "type": "physical",
+                "mac_address": factory.make_mac_address(),
+                "parents": [],
+                "links": [],
+                "enabled": True,
+            }
+        }
+        self.patch(node_module, 'get_sys_info').return_value = {
+            'hostname': hostname,
+            'architecture': region.architecture,
+            'osystem': osystem,
+            'distro_series': distro_series,
+            'interfaces': interfaces,
+        }
+        yield region.refresh()
+        region = yield deferToDatabase(reload_object, region)
+        self.assertEquals(hostname, region.hostname)
+        self.assertEquals(osystem, region.osystem)
+        self.assertEquals(distro_series, region.distro_series)
+
+        def has_nic(region):
+            mac_address = interfaces["eth0"]["mac_address"]
+            return Interface.objects.filter(
+                node=region, mac_address=mac_address).exists()
+
+        self.assertTrue((yield deferToDatabase(has_nic, region)))

=== modified file 'src/maasserver/regiondservices/tests/test_networks_monitoring.py'
--- src/maasserver/regiondservices/tests/test_networks_monitoring.py	2016-06-08 16:52:07 +0000
+++ src/maasserver/regiondservices/tests/test_networks_monitoring.py	2016-06-16 01:30:55 +0000
@@ -36,6 +36,8 @@
     @inlineCallbacks
     def test_updates_interfaces_in_database(self):
         region = yield deferToDatabase(factory.make_RegionController)
+        region.owner = yield deferToDatabase(factory.make_admin)
+        yield deferToDatabase(region.save)
         # Declare this region controller as the one running here.
         self.useFixture(MAASIDFixture(region.system_id))
 

=== modified file 'src/maasserver/rpc/regionservice.py'
--- src/maasserver/rpc/regionservice.py	2016-06-08 16:11:14 +0000
+++ src/maasserver/rpc/regionservice.py	2016-06-16 01:30:55 +0000
@@ -22,16 +22,12 @@
 )
 import threading
 
-from django.db.models import Q
 from maasserver import (
     eventloop,
     locks,
 )
 from maasserver.bootresources import get_simplestream_endpoint
-from maasserver.enum import (
-    NODE_TYPE,
-    SERVICE_STATUS,
-)
+from maasserver.enum import SERVICE_STATUS
 from maasserver.models.node import (
     Node,
     RackController,
@@ -61,7 +57,6 @@
 from maasserver.security import get_shared_secret
 from maasserver.utils import synchronised
 from maasserver.utils.orm import (
-    get_one,
     transactional,
     with_connection,
 )
@@ -77,12 +72,7 @@
 from provisioningserver.rpc.exceptions import NoSuchCluster
 from provisioningserver.rpc.interfaces import IConnection
 from provisioningserver.security import calculate_digest
-from provisioningserver.utils.env import (
-    get_maas_id,
-    set_maas_id,
-)
 from provisioningserver.utils.events import EventGroup
-from provisioningserver.utils.ipaddr import get_mac_addresses
 from provisioningserver.utils.network import get_all_interface_addresses
 from provisioningserver.utils.twisted import (
     asynchronous,
@@ -116,7 +106,6 @@
 from twisted.internet.error import ConnectionClosed
 from twisted.internet.protocol import Factory
 from twisted.internet.task import LoopingCall
-from twisted.internet.threads import deferToThread
 from twisted.protocols import amp
 from twisted.python import log
 from zope.interface import implementer
@@ -576,8 +565,7 @@
                 yield deferToDatabase(
                     registerConnection, region_id, rack_controller, self.host)
 
-            # Rack controller is now registered. Log this status and refresh
-            # the information about the rack controller if needed.
+            # Rack controller is now registered. Log this status.
             log.msg(
                 "Rack controller '%s' has been registered." % self.ident)
 
@@ -1014,10 +1002,10 @@
         """
         while self.running:
             try:
-                advertising_info = yield deferToThread(
-                    self._getAdvertisingInfo)
-                advertising = yield deferToDatabase(
-                    RegionAdvertising.promote, *advertising_info)
+                advertising = yield deferToDatabase(RegionAdvertising.promote)
+            except RegionController.DoesNotExist:
+                # Wait for the region controller object to be created
+                yield pause(1)
             except:
                 log.err(None, (
                     "Promotion of %s failed; will try again in "
@@ -1030,14 +1018,6 @@
             # so raise CancelledError to prevent callbacks from being called.
             raise defer.CancelledError()
 
-    @staticmethod
-    def _getAdvertisingInfo():
-        """Return the info required to promote this regiond.
-
-        :return: A ``(region-id, hostname, mac-addresses)`` tuple.
-        """
-        return get_maas_id(), gethostname(), get_mac_addresses()
-
     def _promotionOkay(self, advertising):
         """Store the region advertising object.
 
@@ -1047,7 +1027,6 @@
 
         :param advertising: An instance of `RegionAdvertising`.
         """
-        set_maas_id(advertising.region_id)  # Create `maas_id` file.
         self.advertising.set(advertising)
 
     def _promotionFailed(self, failure):
@@ -1184,7 +1163,7 @@
     @with_connection  # Needed by the following lock.
     @synchronised(locks.eventloop)
     @transactional
-    def promote(cls, region_id, hostname, mac_addresses):
+    def promote(cls):
         """Promote this regiond to active duty.
 
         Ensure that `RegionController` and `RegionControllerProcess` records
@@ -1192,17 +1171,7 @@
 
         :return: An instance of `RegionAdvertising`.
         """
-        region_filter = Q() if region_id is None else Q(system_id=region_id)
-        region_filter |= Q(hostname=hostname)
-        region_filter |= Q(interface__mac_address__in=mac_addresses)
-        region_obj = get_one(Node.objects.filter(region_filter).distinct())
-        if region_obj is None:
-            # This is the first time MAAS has run on this node.
-            region_obj = RegionController.objects.create(hostname=hostname)
-        else:
-            # Already exists. Make sure it's configured as a region.
-            region_obj = fix_node_for_region(region_obj, hostname)
-
+        region_obj = RegionController.objects.get_running_controller()
         # Create the process for this region. This process object will be
         # updated by calls to `update`, which is the responsibility of the
         # rpc-advertise service. Calls to `update` also remove old process
@@ -1329,26 +1298,3 @@
         for region_obj in Node.objects.filter(system_id=self.region_id):
             RegionControllerProcess.objects.get(
                 region=region_obj, pid=os.getpid()).delete()
-
-
-@transactional
-def fix_node_for_region(region_obj, hostname):
-    """Fix the `node_type` and `hostname` fields on `region_obj`.
-
-    This method only updates the database if it has changed.
-    """
-    update_fields = []
-    if region_obj.node_type not in [
-            NODE_TYPE.REGION_CONTROLLER,
-            NODE_TYPE.REGION_AND_RACK_CONTROLLER]:
-        if region_obj.node_type == NODE_TYPE.RACK_CONTROLLER:
-            region_obj.node_type = NODE_TYPE.REGION_AND_RACK_CONTROLLER
-        else:
-            region_obj.node_type = NODE_TYPE.REGION_CONTROLLER
-        update_fields.append("node_type")
-    if region_obj.hostname != hostname:
-        region_obj.hostname = hostname
-        update_fields.append("hostname")
-    if len(update_fields) > 0:
-        region_obj.save(update_fields=update_fields)
-    return region_obj

=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
--- src/maasserver/rpc/tests/test_regionservice.py	2016-06-08 16:11:14 +0000
+++ src/maasserver/rpc/tests/test_regionservice.py	2016-06-16 01:30:55 +0000
@@ -24,10 +24,7 @@
 
 from crochet import wait_for
 from django.db import IntegrityError
-from maasserver import (
-    eventloop,
-    locks,
-)
+from maasserver import eventloop
 from maasserver.enum import (
     NODE_TYPE,
     SERVICE_STATUS,
@@ -67,7 +64,6 @@
     IsFiredDeferred,
     IsUnfiredDeferred,
     MockAnyCall,
-    MockCalledOnce,
     MockCalledOnceWith,
     MockCallsMatch,
     Provides,
@@ -91,6 +87,7 @@
 from provisioningserver.rpc.testing import call_responder
 from provisioningserver.rpc.testing.doubles import DummyConnection
 from provisioningserver.utils import events
+from provisioningserver.utils.testing import MAASIDFixture
 from provisioningserver.utils.twisted import (
     callInReactorWithTimeout,
     DeferredValue,
@@ -553,27 +550,6 @@
         yield deferToDatabase(
             RackController.objects.get, hostname=hostname)
 
-    @skip("XXX: GavinPanella 2016-03-09 bug=1555236: Fails spuriously.")
-    @wait_for_reactor
-    @inlineCallbacks
-    def test_register_calls_refresh_when_needed(self):
-        protocol = self.make_Region()
-        protocol.transport = MagicMock()
-        mock_gethost = self.patch(protocol.transport, 'getHost')
-        mock_gethost.return_value = IPv4Address(
-            type='TCP', host=factory.make_ipv4_address(),
-            port=random.randint(1, 65535))
-        mock_refresh = self.patch(RackController, 'refresh')
-        self.patch(regionservice, 'registerConnection')
-        hostname = factory.make_hostname()
-        yield call_responder(
-            protocol, RegisterRackController, {
-                "system_id": None,
-                "hostname": hostname,
-                "interfaces": {},
-            })
-        self.assertThat(mock_refresh, MockCalledOnce())
-
     @wait_for_reactor
     @inlineCallbacks
     def test_register_raises_CannotRegisterRackController_when_it_cant(self):
@@ -1087,20 +1063,12 @@
     run_tests_with = MAASCrochetRunTest
 
     def setUp(self):
-        super(TestRegionAdvertisingService, self).setUp()
-        self.maas_id = None
-
-        def set_maas_id(maas_id):
-            self.maas_id = maas_id
-
-        self.set_maas_id = self.patch(regionservice, "set_maas_id")
-        self.set_maas_id.side_effect = set_maas_id
-
-        def get_maas_id():
-            return self.maas_id
-
-        self.get_maas_id = self.patch(regionservice, "get_maas_id")
-        self.get_maas_id.side_effect = get_maas_id
+        super().setUp()
+        # Create the running region controller object
+        region = factory.make_RegionController()
+        region.owner = factory.make_admin()
+        region.save()
+        self.useFixture(MAASIDFixture(region.system_id))
 
     def test_init(self):
         ras = RegionAdvertisingService()
@@ -1164,10 +1132,8 @@
             region_ids = yield deferToDatabase(lambda: {
                 region.system_id for region in RegionController.objects.all()})
             self.assertThat(region_ids, HasLength(1))
-            # The maas_id file has been created too.
+            # Finally, the advertising value has been set.
             region_id = region_ids.pop()
-            self.assertThat(self.set_maas_id, MockCalledOnceWith(region_id))
-            # Finally, the advertising value has been set.
             advertising = yield service.advertising.get(0.0)
             self.assertThat(
                 advertising, MatchesAll(
@@ -1191,18 +1157,18 @@
         service = RegionAdvertisingService()
         # Prevent real pauses.
         self.patch_autospec(regionservice, "pause").return_value = None
-        # Make service._getAdvertisingInfo fail the first time it's called.
+        # Make service.promote fail the first time it's called.
         exceptions = [ValueError("You don't vote for kings!")]
-        original = service._getAdvertisingInfo
+        original = regionservice.RegionAdvertising.promote
 
-        def _getAdvertisingInfo():
+        def promote():
             if len(exceptions) == 0:
                 return original()
             else:
                 raise exceptions.pop(0)
 
-        gao = self.patch(service, "_getAdvertisingInfo")
-        gao.side_effect = _getAdvertisingInfo
+        fake_promote = self.patch(regionservice.RegionAdvertising, "promote")
+        fake_promote.side_effect = promote
         # Capture all Twisted logs.
         logger = self.useFixture(TwistedLoggerFixture())
 
@@ -1218,6 +1184,31 @@
         finally:
             yield service.stopService()
 
+    @wait_for_reactor
+    @inlineCallbacks
+    def test_start_up_waits_for_region_obj(self):
+        service = RegionAdvertisingService()
+        # Prevent real pauses.
+        self.patch_autospec(regionservice, "pause").return_value = None
+        # Make service.promote fail the first time it's called.
+        exceptions = [RegionController.DoesNotExist()]
+        original = regionservice.RegionAdvertising.promote
+
+        def promote():
+            if len(exceptions) == 0:
+                return original()
+            else:
+                raise exceptions.pop(0)
+
+        fake_promote = self.patch(regionservice.RegionAdvertising, "promote")
+        fake_promote.side_effect = promote
+
+        yield service.startService()
+        try:
+            self.assertThat(fake_promote, MockCallsMatch(call(), call()))
+        finally:
+            yield service.stopService()
+
     def test_stopping_waits_for_startup(self):
         service = RegionAdvertisingService()
         synchronise = threading.Condition()
@@ -1226,14 +1217,15 @@
         service._startAdvertising = lambda: None
         service._stopAdvertising = lambda: None
 
-        # Prevent the service's _getAdvertisingInfo method - which is deferred
-        # to a thread - from completing while we hold the lock.
-        def _getAdvertisingInfo(original=service._getAdvertisingInfo):
+        # Prevent the service's promote method - which is deferred to a thread,
+        # from completing while we hold the lock.
+        def promote(original=regionservice.RegionAdvertising.promote):
             with synchronise:
                 synchronise.notify()
                 synchronise.wait(2.0)
             return original()
-        service._getAdvertisingInfo = _getAdvertisingInfo
+        fake_promote = self.patch(regionservice.RegionAdvertising, "promote")
+        fake_promote.side_effect = promote
 
         with synchronise:
             # Start the service, but stop it again before promote is able to
@@ -1374,11 +1366,13 @@
 
     hostname = gethostname()
 
-    def promote(self, region_id=None, hostname=hostname, mac_addresses=None):
-        """Convenient wrapper around `RegionAdvertising.promote`."""
-        return RegionAdvertising.promote(
-            factory.make_name("region-id") if region_id is None else region_id,
-            hostname, [] if mac_addresses is None else mac_addresses)
+    def setUp(self):
+        super().setUp()
+        # Create the running region controller object
+        region = factory.make_RegionController()
+        region.owner = factory.make_admin()
+        region.save()
+        self.useFixture(MAASIDFixture(region.system_id))
 
     def make_addresses(self):
         """Return a set of a couple of ``(addr, port)`` tuples."""
@@ -1396,107 +1390,8 @@
             for endpoint in process.endpoints.all()
         }
 
-    def test_promote_new_region(self):
-        # Before promotion there are no RegionControllers.
-        self.assertEquals(
-            0, RegionController.objects.count(),
-            "No RegionControllers should exist.")
-
-        advertising = self.promote()
-
-        # Now a RegionController exists for the given hostname.
-        region = RegionController.objects.get(hostname=gethostname())
-        self.assertThat(advertising.region_id, Equals(region.system_id))
-
-    def test_promote_converts_from_node(self):
-        node = factory.make_Node(interface=True)
-        interfaces = [
-            factory.make_Interface(node=node),
-            factory.make_Interface(node=node),
-        ]
-        mac_addresses = [
-            str(interface.mac_address)
-            for interface in interfaces
-        ]
-
-        self.promote(node.system_id, self.hostname, mac_addresses)
-
-        # Node should have been converted to a RegionController.
-        node = reload_object(node)
-        self.assertEquals(NODE_TYPE.REGION_CONTROLLER, node.node_type)
-        # The hostname has also been set.
-        self.assertEquals(self.hostname, node.hostname)
-
-    def test_promote_converts_from_rack(self):
-        node = factory.make_Node(
-            interface=True, node_type=NODE_TYPE.RACK_CONTROLLER)
-        interface = node.get_boot_interface()
-        mac_address = str(interface.mac_address)
-
-        self.promote(node.system_id, self.hostname, [mac_address])
-
-        # Node should have been converted to a RegionRackController.
-        node = reload_object(node)
-        self.assertEquals(NODE_TYPE.REGION_AND_RACK_CONTROLLER, node.node_type)
-        # The hostname has also been set.
-        self.assertEquals(self.hostname, node.hostname)
-
-    def test_promote_sets_region_hostname(self):
-        node = factory.make_Node(node_type=NODE_TYPE.REGION_CONTROLLER)
-
-        self.promote(node.system_id, self.hostname)
-
-        # The hostname has been set.
-        self.assertEquals(self.hostname, reload_object(node).hostname)
-
-    def test_promote_holds_startup_lock(self):
-        # Creating tables in PostgreSQL is a transactional operation like any
-        # other. If the isolation level is not sufficient it is susceptible to
-        # races. Using a higher isolation level may lead to serialisation
-        # failures, for example. However, PostgreSQL provides advisory locking
-        # functions, and that's what RegionAdvertising.promote takes advantage
-        # of to prevent concurrent creation of the region controllers.
-
-        # A record of the lock's status, populated when a custom
-        # patched-in _do_create() is called.
-        locked = []
-
-        # Capture the state of `locks.eventloop` while `promote` is running.
-        original_fix_node_for_region = regionservice.fix_node_for_region
-
-        def fix_node_for_region(*args, **kwargs):
-            locked.append(locks.eventloop.is_locked())
-            return original_fix_node_for_region(*args, **kwargs)
-
-        fnfr = self.patch(regionservice, "fix_node_for_region")
-        fnfr.side_effect = fix_node_for_region
-
-        # `fix_node_for_region` is only called for preexisting nodes.
-        node = factory.make_Node(node_type=NODE_TYPE.REGION_CONTROLLER)
-
-        # The lock is not held before and after `promote` is called.
-        self.assertFalse(locks.eventloop.is_locked())
-        self.promote(node.system_id)
-        self.assertFalse(locks.eventloop.is_locked())
-
-        # The lock was held when `fix_node_for_region` was called.
-        self.assertEqual([True], locked)
-
-    def test_update_updates_region_hostname(self):
-        advertising = self.promote()
-
-        region = RegionController.objects.get(system_id=advertising.region_id)
-        region.hostname = factory.make_name("host")
-        region.save()
-
-        advertising.update(self.make_addresses())
-
-        # RegionController should have hostname updated.
-        region = reload_object(region)
-        self.assertEquals(self.hostname, region.hostname)
-
     def test_update_creates_process_when_removed(self):
-        advertising = self.promote()
+        advertising = RegionAdvertising.promote()
 
         region = RegionController.objects.get(system_id=advertising.region_id)
         [process] = region.processes.all()
@@ -1511,7 +1406,7 @@
         self.assertEquals(process.pid, os.getpid())
 
     def test_update_removes_old_processes(self):
-        advertising = self.promote()
+        advertising = RegionAdvertising.promote()
 
         old_time = now() - timedelta(seconds=90)
         region = RegionController.objects.get(system_id=advertising.region_id)
@@ -1532,7 +1427,7 @@
         current_time = now()
         self.patch(timestampedmodel, "now").return_value = current_time
 
-        advertising = self.promote()
+        advertising = RegionAdvertising.promote()
 
         old_time = current_time - timedelta(seconds=90)
         region = RegionController.objects.get(system_id=advertising.region_id)
@@ -1554,14 +1449,14 @@
     def test_update_creates_endpoints_on_process(self):
         addresses = self.make_addresses()
 
-        advertising = self.promote()
+        advertising = RegionAdvertising.promote()
         advertising.update(addresses)
 
         saved_endpoints = self.get_endpoints(advertising.region_id)
         self.assertEqual(addresses, saved_endpoints)
 
     def test_update_does_not_insert_endpoints_when_nothings_listening(self):
-        advertising = self.promote()
+        advertising = RegionAdvertising.promote()
         advertising.update(set())  # No addresses.
 
         saved_endpoints = self.get_endpoints(advertising.region_id)
@@ -1572,7 +1467,7 @@
         addresses_one = self.make_addresses().union(addresses_common)
         addresses_two = self.make_addresses().union(addresses_common)
 
-        advertising = self.promote()
+        advertising = RegionAdvertising.promote()
 
         advertising.update(addresses_one)
         self.assertEqual(
@@ -1583,7 +1478,7 @@
             addresses_two, self.get_endpoints(advertising.region_id))
 
     def test_update_sets_regiond_degraded_with_less_than_4_processes(self):
-        advertising = self.promote()
+        advertising = RegionAdvertising.promote()
         advertising.update(self.make_addresses())
 
         region = RegionController.objects.get(system_id=advertising.region_id)
@@ -1594,7 +1489,7 @@
             status_info="1 process running but 4 were expected."))
 
     def test_update_sets_regiond_running_with_4_processes(self):
-        advertising = self.promote()
+        advertising = RegionAdvertising.promote()
 
         region = RegionController.objects.get(system_id=advertising.region_id)
         [process] = region.processes.all()
@@ -1610,7 +1505,7 @@
             status=SERVICE_STATUS.RUNNING, status_info=""))
 
     def test_update_calls_mark_dead_on_regions_without_processes(self):
-        advertising = self.promote()
+        advertising = RegionAdvertising.promote()
 
         other_region = factory.make_RegionController()
         mock_mark_dead = self.patch(ServiceModel.objects, "mark_dead")
@@ -1622,29 +1517,25 @@
             MockCalledOnceWith(other_region, dead_region=True))
 
     def test_demote(self):
-        region_id = factory.make_name("region-id")
-        hostname = gethostname()
         addresses = {
             (factory.make_ipv4_address(), factory.pick_port()),
             (factory.make_ipv4_address(), factory.pick_port()),
         }
-        advertising = RegionAdvertising.promote(region_id, hostname, [])
+        advertising = RegionAdvertising.promote()
         advertising.update(addresses)
         advertising.demote()
         self.assertItemsEqual([], advertising.dump())
 
     def test_dump(self):
-        region_id = factory.make_name("region-id")
-        hostname = gethostname()
         addresses = {
             (factory.make_ipv4_address(), factory.pick_port()),
             (factory.make_ipv4_address(), factory.pick_port()),
         }
-        advertising = RegionAdvertising.promote(region_id, hostname, [])
+        advertising = RegionAdvertising.promote()
         advertising.update(addresses)
 
         expected = [
-            ("%s:pid=%d" % (hostname, os.getpid()), addr, port)
+            ("%s:pid=%d" % (gethostname(), os.getpid()), addr, port)
             for (addr, port) in addresses
         ]
         self.assertItemsEqual(expected, advertising.dump())

=== modified file 'src/maasserver/start_up.py'
--- src/maasserver/start_up.py	2016-06-04 01:35:39 +0000
+++ src/maasserver/start_up.py	2016-06-16 01:30:55 +0000
@@ -8,30 +8,48 @@
 ]
 
 import logging
+from socket import gethostname
 
 from django.db import connection
+from django.db.models import Q
 from django.db.utils import DatabaseError
 from maasserver import (
     is_master_process,
     locks,
     security,
 )
+from maasserver.enum import NODE_TYPE
 from maasserver.fields import register_mac_type
 from maasserver.models.domain import dns_kms_setting_changed
+from maasserver.models.node import (
+    Node,
+    RegionController,
+    typecast_node,
+)
 from maasserver.utils import synchronised
 from maasserver.utils.orm import (
+    get_one,
     get_psycopg2_exception,
+    post_commit_do,
     transactional,
     with_connection,
 )
 from maasserver.utils.threads import deferToDatabase
+from maasserver.worker_user import get_worker_user
 from provisioningserver.logger import get_maas_logger
+from provisioningserver.utils.env import (
+    get_maas_id,
+    set_maas_id,
+)
+from provisioningserver.utils.ipaddr import get_mac_addresses
 from provisioningserver.utils.twisted import (
     asynchronous,
     FOREVER,
     pause,
 )
+from twisted.internet import reactor
 from twisted.internet.defer import inlineCallbacks
+from twisted.python import log
 
 
 maaslog = get_maas_logger("start-up")
@@ -98,8 +116,55 @@
     # Register our MAC data type with psycopg.
     register_mac_type(connection.cursor())
 
+    def refresh_region(region_obj):
+        # The RegionAdvertisingService uses
+        # RegionController.objects.get_running_controller() which calls
+        # get_maas_id() to find the system_id of the running region. If this
+        # isn't done here RegionAdvertisingService won't be able to figure out
+        # the region object for the running machine.
+        set_maas_id(region_obj.system_id)
+        d = region_obj.refresh()
+        d.addErrback(log.err, "Failure when refreshing region")
+        return d
+
     # Only perform the following if the master process for the
     # region controller.
     if is_master_process():
         # Freshen the kms SRV records.
         dns_kms_setting_changed()
+
+        region_obj = create_region_obj()
+        post_commit_do(reactor.callLater, 0, refresh_region, region_obj)
+        # Trigger post commit
+        region_obj.save()
+
+
+def create_region_obj():
+    region_id = get_maas_id()
+    hostname = gethostname()
+    update_fields = []
+    region_filter = Q() if region_id is None else Q(system_id=region_id)
+    region_filter |= Q(hostname=hostname)
+    region_filter |= Q(interface__mac_address__in=get_mac_addresses())
+    region_obj = get_one(Node.objects.filter(region_filter).distinct())
+    if region_obj is None:
+        # This is the first time MAAS has run on this node.
+        region_obj = RegionController.objects.create(hostname=hostname)
+    else:
+        # Already exists. Make sure it's configured as a region.
+        if region_obj.node_type == NODE_TYPE.RACK_CONTROLLER:
+            region_obj.node_type = NODE_TYPE.REGION_AND_RACK_CONTROLLER
+            update_fields.append("node_type")
+        elif not region_obj.is_region_controller:
+            region_obj.node_type = NODE_TYPE.REGION_CONTROLLER
+            update_fields.append("node_type")
+        region_obj = typecast_node(region_obj, RegionController)
+
+    if region_obj.owner is None:
+        region_obj.owner = get_worker_user()
+        update_fields.append("owner")
+
+    if len(update_fields) > 0:
+        region_obj.save(update_fields=update_fields)
+
+    return region_obj

=== modified file 'src/maasserver/tests/test_start_up.py'
--- src/maasserver/tests/test_start_up.py	2016-06-04 01:35:39 +0000
+++ src/maasserver/tests/test_start_up.py	2016-06-16 01:30:55 +0000
@@ -12,15 +12,24 @@
     locks,
     start_up,
 )
+from maasserver.enum import (
+    NODE_TYPE,
+    NODE_TYPE_CHOICES,
+)
+from maasserver.models.node import RegionController
 from maasserver.models.signals import bootsources
 from maasserver.testing.eventloop import RegionEventLoopFixture
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
+from maasserver.utils.orm import post_commit_hooks
+from maasserver.worker_user import get_worker_user
 from maastesting.matchers import (
+    MockCalledOnce,
     MockCalledOnceWith,
     MockCallsMatch,
     MockNotCalled,
 )
+from provisioningserver.utils.testing import MAASIDFixture
 
 
 class LockChecker:
@@ -64,6 +73,17 @@
         self.assertEqual(1, lock_checker.call_count)
         self.assertEqual(True, lock_checker.lock_was_held)
 
+    def test_refresh_on_master(self):
+        # Disable boot source cache signals.
+        self.addCleanup(bootsources.signals.enable)
+        bootsources.signals.disable()
+
+        self.patch(start_up, 'is_master_process').return_value = True
+        self.patch(start_up, 'register_mac_type')
+        mock_refresh = self.patch_autospec(RegionController, 'refresh')
+        start_up.start_up()
+        self.assertThat(mock_refresh, MockCalledOnce())
+
     def test_start_up_retries_with_wait_on_exception(self):
         inner_start_up = self.patch(start_up, 'inner_start_up')
         inner_start_up.side_effect = [
@@ -94,10 +114,114 @@
 
     def test__calls_dns_kms_setting_changed_if_master(self):
         self.patch(start_up, "is_master_process").return_value = True
-        start_up.inner_start_up()
+        self.patch(start_up, "post_commit_do")
+        with post_commit_hooks:
+            start_up.inner_start_up()
         self.assertThat(start_up.dns_kms_setting_changed, MockCalledOnceWith())
 
     def test__doesnt_call_dns_kms_setting_changed_if_not_master(self):
         self.patch(start_up, "is_master_process").return_value = False
-        start_up.inner_start_up()
+        with post_commit_hooks:
+            start_up.inner_start_up()
         self.assertThat(start_up.dns_kms_setting_changed, MockNotCalled())
+
+    def test__creates_region_obj_if_master(self):
+        self.patch(start_up, "is_master_process").return_value = True
+        self.patch(start_up, "set_maas_id")
+        mock_create_region_obj = self.patch_autospec(
+            start_up, "create_region_obj")
+        with post_commit_hooks:
+            start_up.inner_start_up()
+        self.assertThat(mock_create_region_obj, MockCalledOnce())
+
+    def test__doesnt_create_region_obj_if_not_master(self):
+        self.patch(start_up, "is_master_process").return_value = False
+        mock_create_region_obj = self.patch_autospec(
+            start_up, "create_region_obj")
+        with post_commit_hooks:
+            start_up.inner_start_up()
+        self.assertThat(mock_create_region_obj, MockNotCalled())
+
+    def test__creates_maas_id_file(self):
+        self.patch(start_up, "is_master_process").return_value = True
+        mock_set_maas_id = self.patch_autospec(start_up, "set_maas_id")
+        self.patch(start_up.RegionController, 'refresh')
+        with post_commit_hooks:
+            start_up.inner_start_up()
+        self.assertThat(mock_set_maas_id, MockCalledOnce())
+
+    def test__doesnt_create_maas_id_file_if_not_master(self):
+        self.patch(start_up, "is_master_process").return_value = False
+        mock_set_maas_id = self.patch_autospec(start_up, "set_maas_id")
+        with post_commit_hooks:
+            start_up.inner_start_up()
+        self.assertThat(mock_set_maas_id, MockNotCalled())
+
+
+class TestCreateRegionObj(MAASServerTestCase):
+
+    """Tests for the actual work done in `create_region_obj`."""
+
+    def test__creates_obj(self):
+        region = start_up.create_region_obj()
+        self.assertIsNotNone(region)
+        self.assertIsNotNone(
+            RegionController.objects.get(system_id=region.system_id))
+
+    def test__finds_region_by_maas_id(self):
+        region = factory.make_RegionController()
+        self.useFixture(MAASIDFixture(region.system_id))
+        self.assertEquals(region, start_up.create_region_obj())
+
+    def test__finds_region_by_hostname(self):
+        region = factory.make_RegionController()
+        mock_gethostname = self.patch_autospec(start_up, "gethostname")
+        mock_gethostname.return_value = region.hostname
+        self.assertEquals(region, start_up.create_region_obj())
+
+    def test__finds_region_by_mac(self):
+        region = factory.make_RegionController()
+        factory.make_Interface(node=region)
+        mock_get_mac_addresses = self.patch_autospec(
+            start_up, "get_mac_addresses")
+        mock_get_mac_addresses.return_value = [
+            nic.mac_address.raw
+            for nic in region.interface_set.all()
+        ]
+        self.assertEquals(region, start_up.create_region_obj())
+
+    def test__converts_rack_to_region_rack(self):
+        rack = factory.make_RackController()
+        self.useFixture(MAASIDFixture(rack.system_id))
+        region_rack = start_up.create_region_obj()
+        self.assertEquals(rack, region_rack)
+        self.assertEquals(
+            region_rack.node_type, NODE_TYPE.REGION_AND_RACK_CONTROLLER)
+
+    def test__converts_node_to_region_rack(self):
+        node = factory.make_Node(
+            node_type=factory.pick_choice(
+                NODE_TYPE_CHOICES,
+                but_not=[
+                    NODE_TYPE.REGION_CONTROLLER,
+                    NODE_TYPE.RACK_CONTROLLER,
+                    NODE_TYPE.REGION_AND_RACK_CONTROLLER,
+                ]))
+        self.useFixture(MAASIDFixture(node.system_id))
+        region = start_up.create_region_obj()
+        self.assertEquals(node, region)
+        self.assertEquals(region.node_type, NODE_TYPE.REGION_CONTROLLER)
+
+    def test__sets_owner_if_none(self):
+        region = factory.make_RegionController()
+        self.useFixture(MAASIDFixture(region.system_id))
+        self.assertEquals(
+            get_worker_user(), start_up.create_region_obj().owner)
+
+    def test__leaves_owner_if_set(self):
+        region = factory.make_RegionController()
+        self.useFixture(MAASIDFixture(region.system_id))
+        user = factory.make_User()
+        region.owner = user
+        region.save()
+        self.assertEquals(user, start_up.create_region_obj().owner)

=== modified file 'src/maasserver/views/tests/test_rpc.py'
--- src/maasserver/views/tests/test_rpc.py	2016-05-11 19:01:48 +0000
+++ src/maasserver/views/tests/test_rpc.py	2016-06-16 01:30:55 +0000
@@ -12,10 +12,11 @@
 from maasserver import eventloop
 from maasserver.rpc import regionservice
 from maasserver.testing.eventloop import RegionEventLoopFixture
+from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASTransactionServerTestCase
-from maastesting.factory import factory
 from netaddr import IPAddress
 from provisioningserver.utils.network import get_all_interface_addresses
+from provisioningserver.utils.testing import MAASIDFixture
 from testtools.matchers import (
     Equals,
     GreaterThan,
@@ -108,6 +109,10 @@
         self.assertEqual({"eventloops": None}, info)
 
     def test_rpc_info_when_rpc_advertise_running(self):
+        region = factory.make_RegionController()
+        self.useFixture(MAASIDFixture(region.system_id))
+        region.owner = factory.make_admin()
+        region.save()
         self.useFixture(RegionEventLoopFixture("rpc", "rpc-advertise"))
 
         eventloop.start().wait(5)

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2016-05-12 19:07:37 +0000
+++ src/metadataserver/api.py	2016-06-16 01:30:55 +0000
@@ -166,7 +166,7 @@
     elif node.node_type in [
             NODE_TYPE.RACK_CONTROLLER,
             NODE_TYPE.REGION_AND_RACK_CONTROLLER]:
-        type_name = EVENT_TYPES.REQUEST_RACK_CONTROLLER_REFRESH
+        type_name = EVENT_TYPES.REQUEST_CONTROLLER_REFRESH
     else:
         type_name = EVENT_TYPES.NODE_STATUS_EVENT
 

=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py	2016-06-11 22:29:53 +0000
+++ src/metadataserver/tests/test_api.py	2016-06-16 01:30:55 +0000
@@ -210,7 +210,7 @@
         self.assertIn(origin, event.description)
         self.assertIn(description, event.description)
         self.assertEqual(
-            EVENT_TYPES.REQUEST_RACK_CONTROLLER_REFRESH, event.type.name)
+            EVENT_TYPES.REQUEST_CONTROLLER_REFRESH, event.type.name)
 
 
 def make_node_client(node=None):

=== modified file 'src/provisioningserver/events.py'
--- src/provisioningserver/events.py	2016-04-26 19:52:34 +0000
+++ src/provisioningserver/events.py	2016-06-16 01:30:55 +0000
@@ -81,7 +81,7 @@
     REQUEST_NODE_START = "REQUEST_NODE_START"
     REQUEST_NODE_STOP = "REQUEST_NODE_STOP"
     # Rack controller request events
-    REQUEST_RACK_CONTROLLER_REFRESH = "REQUEST_RACK_CONTROLLER_REFRESH"
+    REQUEST_CONTROLLER_REFRESH = "REQUEST_CONTROLLER_REFRESH"
     REQUEST_RACK_CONTROLLER_ADD_CHASSIS = "REQUEST_RACK_CONTROLLER_ADD_CHASSIS"
 
 
@@ -202,9 +202,9 @@
         description="User powering down node",
         level=INFO,
     ),
-    EVENT_TYPES.REQUEST_RACK_CONTROLLER_REFRESH: EventDetail(
-        description=("Starting refresh of rack controller hardware and "
-                     "networking information"),
+    EVENT_TYPES.REQUEST_CONTROLLER_REFRESH: EventDetail(
+        description=("Starting refresh of controller hardware and networking "
+                     "information"),
         level=INFO,
     ),
     EVENT_TYPES.REQUEST_RACK_CONTROLLER_ADD_CHASSIS: EventDetail(

=== modified file 'src/provisioningserver/refresh/__init__.py'
--- src/provisioningserver/refresh/__init__.py	2016-06-06 23:56:40 +0000
+++ src/provisioningserver/refresh/__init__.py	2016-06-16 01:30:55 +0000
@@ -9,13 +9,13 @@
 import tempfile
 import urllib
 
-from provisioningserver.config import ClusterConfiguration
 from provisioningserver.logger.log import get_maas_logger
 from provisioningserver.refresh.maas_api_helper import (
     encode_multipart_data,
     geturl,
 )
 from provisioningserver.refresh.node_info_scripts import NODE_INFO_SCRIPTS
+from provisioningserver.utils.network import get_all_interfaces_definition
 from provisioningserver.utils.shell import (
     call_and_check,
     ExternalProcessError,
@@ -52,6 +52,31 @@
     return os_release
 
 
+@synchronous
+def get_sys_info():
+    """Return basic system information in a dictionary."""
+    os_release = get_os_release()
+    if 'ID' in os_release:
+        osystem = os_release['ID']
+    elif 'NAME' in os_release:
+        osystem = os_release['NAME']
+    else:
+        osystem = ''
+    if 'UBUNTU_CODENAME' in os_release:
+        distro_series = os_release['UBUNTU_CODENAME']
+    elif 'VERSION_ID' in os_release:
+        distro_series = os_release['VERSION_ID']
+    else:
+        distro_series = ''
+    return {
+        'hostname': socket.gethostname().split('.')[0],
+        'architecture': get_architecture(),
+        'osystem': osystem,
+        'distro_series': distro_series,
+        'interfaces': get_all_interfaces_definition(),
+    }
+
+
 def signal(
         url, creds, status, message, files={}, script_result=None,
         extra_headers=None):
@@ -91,13 +116,12 @@
 
 
 @synchronous
-def refresh(system_id, consumer_key, token_key, token_secret):
+def refresh(system_id, consumer_key, token_key, token_secret, maas_url=None):
     """Run all builtin commissioning scripts and report results to region."""
     maaslog.info(
         "Refreshing rack controller hardware information.")
 
-    with ClusterConfiguration.open() as config:
-        url = "%s/metadata/status/%s/latest" % (config.maas_url, system_id)
+    url = "%s/metadata/status/%s/latest" % (maas_url, system_id)
 
     creds = {
         'consumer_key': consumer_key,

=== modified file 'src/provisioningserver/refresh/tests/test_refresh.py'
--- src/provisioningserver/refresh/tests/test_refresh.py	2016-06-06 23:56:40 +0000
+++ src/provisioningserver/refresh/tests/test_refresh.py	2016-06-16 01:30:55 +0000
@@ -15,6 +15,7 @@
 
 from maastesting.factory import factory
 from maastesting.matchers import (
+    Equals,
     MockAnyCall,
     MockCalledWith,
 )
@@ -31,7 +32,7 @@
     def test_get_architecture_returns_arch_with_generic(self):
         arch = random.choice(['i386', 'amd64', 'arm64', 'ppc64el'])
         subarch = factory.make_name('subarch')
-        self.patch(refresh, 'call_and_check').return_value = (
+        self.patch_autospec(refresh, 'call_and_check').return_value = (
             "%s/%s" % (arch, subarch)).encode('utf-8')
         ret_arch = refresh.get_architecture()
         self.assertEquals("%s/generic" % arch, ret_arch)
@@ -40,7 +41,7 @@
         arch = factory.make_name('arch')
         subarch = factory.make_name('subarch')
         architecture = "%s/%s" % (arch, subarch)
-        self.patch(refresh, 'call_and_check').return_value = (
+        self.patch_autospec(refresh, 'call_and_check').return_value = (
             architecture.encode('utf-8'))
         ret_arch = refresh.get_architecture()
         self.assertEquals(architecture, ret_arch)
@@ -60,6 +61,73 @@
         self.assertIn('UBUNTU_CODENAME', os_release)
         self.assertIn('VERSION_ID', os_release)
 
+    def test_get_sys_info(self):
+        hostname = factory.make_hostname()
+        osystem = factory.make_name('id')
+        distro_series = factory.make_name('ubuntu_codename')
+        architecture = factory.make_name('architecture')
+        interfaces = factory.make_name('interfaces')
+        self.patch(refresh.socket, 'gethostname').return_value = hostname
+        self.patch_autospec(refresh, 'get_os_release').return_value = {
+            'ID': osystem,
+            'UBUNTU_CODENAME': distro_series,
+        }
+        self.patch_autospec(
+            refresh, 'get_architecture').return_value = architecture
+        self.patch_autospec(
+            refresh, 'get_all_interfaces_definition').return_value = interfaces
+        self.assertThat({
+            'hostname': hostname,
+            'architecture': architecture,
+            'osystem': osystem,
+            'distro_series': distro_series,
+            'interfaces': interfaces,
+            }, Equals(refresh.get_sys_info()))
+
+    def test_get_sys_info_on_host(self):
+        self.assertNotIn(None, refresh.get_sys_info())
+
+    def test_get_sys_info_alt(self):
+        hostname = factory.make_hostname()
+        osystem = factory.make_name('name')
+        distro_series = factory.make_name('version_id')
+        architecture = factory.make_name('architecture')
+        interfaces = factory.make_name('interfaces')
+        self.patch(refresh.socket, 'gethostname').return_value = hostname
+        self.patch_autospec(refresh, 'get_os_release').return_value = {
+            'NAME': osystem,
+            'VERSION_ID': distro_series,
+        }
+        self.patch_autospec(
+            refresh, 'get_architecture').return_value = architecture
+        self.patch_autospec(
+            refresh, 'get_all_interfaces_definition').return_value = interfaces
+        self.assertThat({
+            'hostname': hostname,
+            'architecture': architecture,
+            'osystem': osystem,
+            'distro_series': distro_series,
+            'interfaces': interfaces,
+            }, Equals(refresh.get_sys_info()))
+
+    def test_get_sys_info_empty(self):
+        hostname = factory.make_hostname()
+        architecture = factory.make_name('architecture')
+        interfaces = factory.make_name('interfaces')
+        self.patch(refresh.socket, 'gethostname').return_value = hostname
+        self.patch_autospec(refresh, 'get_os_release').return_value = {}
+        self.patch_autospec(
+            refresh, 'get_architecture').return_value = architecture
+        self.patch_autospec(
+            refresh, 'get_all_interfaces_definition').return_value = interfaces
+        self.assertThat({
+            'hostname': hostname,
+            'architecture': architecture,
+            'osystem': '',
+            'distro_series': '',
+            'interfaces': interfaces,
+            }, Equals(refresh.get_sys_info()))
+
 
 class TestSignal(MAASTestCase):
     def test_signal_formats_params(self):
@@ -181,6 +249,29 @@
             })
         ])
 
+    def test_refresh_accepts_defined_url(self):
+        signal = self.patch(refresh, 'signal')
+        self.patch_scripts_success()
+
+        system_id = factory.make_name('system_id')
+        consumer_key = factory.make_name('consumer_key')
+        token_key = factory.make_name('token_key')
+        token_secret = factory.make_name('token_secret')
+        url = factory.make_url()
+
+        refresh.refresh(system_id, consumer_key, token_key, token_secret, url)
+        self.assertThat(signal, MockAnyCall(
+            "%s/metadata/status/%s/latest" % (url, system_id),
+            {
+                'consumer_secret': '',
+                'consumer_key': consumer_key,
+                'token_key': token_key,
+                'token_secret': token_secret,
+            },
+            'WORKING',
+            'Starting test_script [1/1]',
+        ))
+
     def test_refresh_signals_starting(self):
         signal = self.patch(refresh, 'signal')
         self.patch_scripts_success()
@@ -189,10 +280,11 @@
         consumer_key = factory.make_name('consumer_key')
         token_key = factory.make_name('token_key')
         token_secret = factory.make_name('token_secret')
+        url = factory.make_url()
 
-        refresh.refresh(system_id, consumer_key, token_key, token_secret)
-        self.assertItemsEqual([
-            "http://localhost:5240/MAAS/metadata/status/%s/latest" % system_id,
+        refresh.refresh(system_id, consumer_key, token_key, token_secret, url)
+        self.assertThat(signal, MockAnyCall(
+            "%s/metadata/status/%s/latest" % (url, system_id),
             {
                 'consumer_secret': '',
                 'consumer_key': consumer_key,
@@ -200,8 +292,8 @@
                 'token_secret': token_secret,
             },
             'WORKING',
-            'Starting test_script [1/1]'],
-            signal.call_args_list[0][0])
+            'Starting test_script [1/1]',
+        ))
 
     def test_refresh_signals_results(self):
         signal = self.patch(refresh, 'signal')
@@ -211,10 +303,11 @@
         consumer_key = factory.make_name('consumer_key')
         token_key = factory.make_name('token_key')
         token_secret = factory.make_name('token_secret')
+        url = factory.make_url()
 
-        refresh.refresh(system_id, consumer_key, token_key, token_secret)
-        self.assertItemsEqual([
-            "http://localhost:5240/MAAS/metadata/status/%s/latest" % system_id,
+        refresh.refresh(system_id, consumer_key, token_key, token_secret, url)
+        self.assertThat(signal, MockAnyCall(
+            "%s/metadata/status/%s/latest" % (url, system_id),
             {
                 'consumer_secret': '',
                 'consumer_key': consumer_key,
@@ -227,8 +320,8 @@
                 'test_script.out': b'test script\n',
                 'test_script.err': b'',
             },
-            0],
-            signal.call_args_list[1][0])
+            0,
+        ))
 
     def test_refresh_signals_finished(self):
         signal = self.patch(refresh, 'signal')
@@ -238,10 +331,11 @@
         consumer_key = factory.make_name('consumer_key')
         token_key = factory.make_name('token_key')
         token_secret = factory.make_name('token_secret')
+        url = factory.make_url()
 
-        refresh.refresh(system_id, consumer_key, token_key, token_secret)
-        self.assertItemsEqual([
-            "http://localhost:5240/MAAS/metadata/status/%s/latest" % system_id,
+        refresh.refresh(system_id, consumer_key, token_key, token_secret, url)
+        self.assertThat(signal, MockAnyCall(
+            "%s/metadata/status/%s/latest" % (url, system_id),
             {
                 'consumer_secret': '',
                 'consumer_key': consumer_key,
@@ -249,8 +343,8 @@
                 'token_secret': token_secret,
             },
             'OK',
-            "Finished refreshing %s" % system_id],
-            signal.call_args_list[2][0])
+            "Finished refreshing %s" % system_id
+        ))
 
     def test_refresh_signals_failure(self):
         signal = self.patch(refresh, 'signal')
@@ -260,10 +354,11 @@
         consumer_key = factory.make_name('consumer_key')
         token_key = factory.make_name('token_key')
         token_secret = factory.make_name('token_secret')
+        url = factory.make_url()
 
-        refresh.refresh(system_id, consumer_key, token_key, token_secret)
-        self.assertItemsEqual([
-            "http://localhost:5240/MAAS/metadata/status/%s/latest" % system_id,
+        refresh.refresh(system_id, consumer_key, token_key, token_secret, url)
+        self.assertThat(signal, MockAnyCall(
+            "%s/metadata/status/%s/latest" % (url, system_id),
             {
                 'consumer_secret': '',
                 'consumer_key': consumer_key,
@@ -271,8 +366,8 @@
                 'token_secret': token_secret,
             },
             'FAILED',
-            "Failed refreshing %s" % system_id],
-            signal.call_args_list[2][0])
+            "Failed refreshing %s" % system_id,
+        ))
 
     def test_refresh_clears_up_temporary_directory(self):
 

=== modified file 'src/provisioningserver/rpc/cluster.py'
--- src/provisioningserver/rpc/cluster.py	2016-06-06 22:19:59 +0000
+++ src/provisioningserver/rpc/cluster.py	2016-06-16 01:30:55 +0000
@@ -450,6 +450,7 @@
         (b"token_secret", amp.Unicode()),
     ]
     response = [
+        (b"hostname", amp.Unicode()),
         (b"architecture", amp.Unicode()),
         (b"osystem", amp.Unicode()),
         (b"distro_series", amp.Unicode()),

=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2016-06-09 02:13:37 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2016-06-16 01:30:55 +0000
@@ -36,8 +36,7 @@
 from provisioningserver.power.change import maybe_change_power_state
 from provisioningserver.power.query import get_power_state
 from provisioningserver.refresh import (
-    get_architecture,
-    get_os_release,
+    get_sys_info,
     refresh,
 )
 from provisioningserver.rpc import (
@@ -79,7 +78,6 @@
 from provisioningserver.utils.twisted import (
     callOut,
     DeferredValue,
-    synchronous,
 )
 from twisted import web
 from twisted.application.internet import TimerService
@@ -385,43 +383,16 @@
             # Refresh is already running, don't do anything
             raise exceptions.RefreshAlreadyInProgress()
         else:
+            with ClusterConfiguration.open() as config:
+                maas_url = config.maas_url
             # Start gathering node results(lshw, lsblk, etc) but don't wait.
             d = deferToThread(
-                refresh, system_id, consumer_key, token_key, token_secret)
+                refresh, system_id, consumer_key, token_key, token_secret,
+                maas_url)
             d.addErrback(log.err, 'Failed to refresh the rack controller.')
             d.addBoth(callOut, lock.release)
 
-        @synchronous
-        def perform_refresh():
-            architecture = get_architecture()
-            os_release = get_os_release()
-            interfaces = get_all_interfaces_definition()
-            return architecture, os_release, interfaces
-
-        def cb_result(result):
-            architecture, os_release, interfaces = result
-            if 'ID' in os_release:
-                osystem = os_release['ID']
-            elif 'NAME' in os_release:
-                osystem = os_release['NAME']
-            else:
-                osystem = ''
-            if 'UBUNTU_CODENAME' in os_release:
-                distro_series = os_release['UBUNTU_CODENAME']
-            elif 'VERSION_ID' in os_release:
-                distro_series = os_release['VERSION_ID']
-            else:
-                distro_series = ''
-            return {
-                'architecture': architecture,
-                'osystem': osystem,
-                'distro_series': distro_series,
-                'interfaces': interfaces
-            }
-
-        d = deferToThread(perform_refresh)
-        d.addCallback(cb_result)
-        return d
+        return deferToThread(get_sys_info)
 
     @cluster.AddChassis.responder
     def add_chassis(

=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2016-06-09 19:30:17 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2016-06-16 01:30:55 +0000
@@ -2029,7 +2029,13 @@
             clusterservice, 'deferToThread')
         mock_deferToThread.side_effect = [
             succeed(None),
-            succeed(('', {}, {})),
+            succeed({
+                'hostname': '',
+                'architecture': '',
+                'osystem': '',
+                'distro_series': '',
+                'interfaces': {}
+            }),
         ]
 
         system_id = factory.make_name('system_id')
@@ -2038,16 +2044,16 @@
         token_secret = factory.make_name('token_secret')
 
         yield call_responder(Cluster(), cluster.RefreshRackControllerInfo, {
-            "system_id": system_id,
-            "consumer_key": consumer_key,
-            "token_key": token_key,
-            "token_secret": token_secret,
+            'system_id': system_id,
+            'consumer_key': consumer_key,
+            'token_key': token_key,
+            'token_secret': token_secret,
         })
 
         self.assertThat(
             mock_deferToThread, MockAnyCall(
                 clusterservice.refresh, system_id, consumer_key, token_key,
-                token_secret))
+                token_secret, ANY))
 
     @inlineCallbacks
     def test_returns_extra_info(self):
@@ -2057,18 +2063,18 @@
         consumer_key = factory.make_name('consumer_key')
         token_key = factory.make_name('token_key')
         token_secret = factory.make_name('token_secret')
-        architecture = factory.make_name("architecture")
-        osystem = factory.make_name("osystem")
-        distro_series = factory.make_name("distro_series")
-        os_release = {
-            'ID': osystem,
-            'UBUNTU_CODENAME': distro_series,
+        hostname = factory.make_hostname()
+        architecture = factory.make_name('architecture')
+        osystem = factory.make_name('osystem')
+        distro_series = factory.make_name('distro_series')
+        self.patch_autospec(clusterservice, 'get_sys_info').return_value = {
+            'hostname': hostname,
+            'osystem': osystem,
+            'distro_series': distro_series,
+            'architecture': architecture,
+            'interfaces': {},
         }
 
-        self.patch(clusterservice, 'get_architecture').return_value = (
-            architecture)
-        self.patch(clusterservice, 'get_os_release').return_value = os_release
-
         response = yield call_responder(
             Cluster(), cluster.RefreshRackControllerInfo, {
                 "system_id": system_id,
@@ -2079,10 +2085,11 @@
 
         self.assertItemsEqual(
             {
+                'hostname': hostname,
                 'osystem': osystem,
                 'distro_series': distro_series,
                 'architecture': architecture,
-                'interfaces': get_all_interfaces_definition(),
+                'interfaces': {},
             }, response)