Merge lp:~julian-edwards/maas/pserv-services-dir into lp:~maas-committers/maas/trunk

[Julian Edwards]

Create a provisioningserver/pserv_services/ directory and put (nearly) all the twistd services in it.

This moves a load of scattered pserv services into a single directory called pserv_services. It would have just been called services, however provisioningserver.__init__.py defines a module property called "services" which clashes. I hate code in __init__.py!

There is one service left to migrate, ClusterClientService but I lost the will to live after slogging through the rest of the code, which was not just a simple bzr rename, but I had to split files because services had been mixed in with unrelated RPC code.

[Julian Edwards (julian-edwards)]

I promise I have not hidden any sneaky code changes inside the moved code ;)

[Jeroen T. Vermeulen (jtv)]

Your revulsion at the __init__.py services situation is noted. Now please drop that README file and just live with the new name!

(For the record, what bugs me most about the new name is probably that the "serv" and the "services" repeat the same root.)

.

For the XXX in node_power_monitor_service.py, could you file a bug and include its number in the comment?

.

I see you preserved the "one_week_ago = er, 15 minutes ago" sickness. That makes my review easier (no need to look for changes in the moved code) but I hope we'll get rid of that soon as well.

review: Approve

[Julian Edwards (julian-edwards)]

Thank you!

On Wednesday 27 Aug 2014 04:02:05 you wrote:
> Review: Approve
>
> Your revulsion at the __init__.py services situation is noted. Now please
> drop that README file and just live with the new name!

I wrote that in a fit of pique. I'll get rid of it.

> (For the record, what bugs me most about the new name is probably that the
> "serv" and the "services" repeat the same root.)

Yes. All of the yes.

> For the XXX in node_power_monitor_service.py, could you file a bug and
> include its number in the comment?

Yes. I forgot to do that.

> I see you preserved the "one_week_ago = er, 15 minutes ago" sickness. That
> makes my review easier (no need to look for changes in the moved code) but
> I hope we'll get rid of that soon as well.

Yeah deliberately so, I didn't want to touch the code at all. The next change
can fix it, pending my questions about it in Blake's last merge.

[MAAS Lander (maas-lander)]

The attempt to merge lp:~julian-edwards/maas/pserv-services-dir into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Get:2 http://security.ubuntu.com trusty-security Release [59.7 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [59.7 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:5 http://security.ubuntu.com trusty-security/main Sources [41.3 kB]
Get:6 http://security.ubuntu.com trusty-security/universe Sources [11.3 kB]
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [132 kB]
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [46.7 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [111 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [78.1 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [302 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [188 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 1,032 kB in 0s (1,628 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsgi python...

[Julian Edwards (julian-edwards)]

Seems like you now have to wait for the branch scanner to catch up with the approved revno before you can mark it approved. :(

[MAAS Lander (maas-lander)]

The attempt to merge lp:~julian-edwards/maas/pserv-services-dir into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Hit http://security.ubuntu.com trusty-security Release
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsgi python-openssl python-paramiko python-pexpect python-pip python-pocket-lint python-psycopg2 python-pyinotify python-seamicroclient python-simplejson python-simplestreams python-sphi...

[Julian Edwards (julian-edwards)]

Sigh.

[Julian Edwards (julian-edwards)]

Forgot to move the tftp test, re-landing.

=== modified file 'src/provisioningserver/boot/tests/test_powernv.py'
--- src/provisioningserver/boot/tests/test_powernv.py	2014-08-13 21:49:35 +0000
+++ src/provisioningserver/boot/tests/test_powernv.py	2014-08-27 06:01:25 +0000
@@ -31,9 +31,9 @@
     )
 from provisioningserver.boot.tests.test_pxe import parse_pxe_config
 from provisioningserver.boot.tftppath import compose_image_path
+from provisioningserver.pserv_services.tftp import TFTPBackend
 from provisioningserver.testing.config import set_tftp_root
 from provisioningserver.tests.test_kernel_opts import make_kernel_parameters
-from provisioningserver.tftp import TFTPBackend
 from testtools.matchers import (
     IsInstance,
     MatchesAll,

=== modified file 'src/provisioningserver/plugin.py'
--- src/provisioningserver/plugin.py	2014-08-26 23:55:04 +0000
+++ src/provisioningserver/plugin.py	2014-08-27 06:01:25 +0000
@@ -31,10 +31,14 @@
 import provisioningserver
 from provisioningserver.cluster_config import get_cluster_uuid
 from provisioningserver.config import Config
-from provisioningserver.rpc.boot_images import PeriodicImageDownloadService
+from provisioningserver.pserv_services.image_download_service import (
+    PeriodicImageDownloadService,
+    )
+from provisioningserver.pserv_services.node_power_monitor_service import (
+    NodePowerMonitorService,
+    )
+from provisioningserver.pserv_services.tftp import TFTPService
 from provisioningserver.rpc.clusterservice import ClusterClientService
-from provisioningserver.rpc.power import NodePowerMonitorService
-from provisioningserver.tftp import TFTPService
 from twisted.application.internet import TCPServer
 from twisted.application.service import (
     IServiceMaker,

=== added directory 'src/provisioningserver/pserv_services'
=== added file 'src/provisioningserver/pserv_services/__init__.py'
=== added file 'src/provisioningserver/pserv_services/image_download_service.py'
--- src/provisioningserver/pserv_services/image_download_service.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/pserv_services/image_download_service.py	2014-08-27 06:01:25 +0000
@@ -0,0 +1,120 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Service to periodically refresh the boot images."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    "PeriodicImageDownloadService",
+    ]
+
+
+from datetime import timedelta
+
+from provisioningserver.boot import tftppath
+from provisioningserver.logger import get_maas_logger
+from provisioningserver.rpc.boot_images import import_boot_images
+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
+from provisioningserver.rpc.region import (
+    GetBootSources,
+    GetBootSourcesV2,
+    )
+from provisioningserver.utils.twisted import pause
+from twisted.application.internet import TimerService
+from twisted.internet.defer import (
+    DeferredLock,
+    inlineCallbacks,
+    returnValue,
+    )
+from twisted.spread.pb import NoSuchMethod
+
+
+maaslog = get_maas_logger("boot_image_download_service")
+service_lock = DeferredLock()
+
+
+class PeriodicImageDownloadService(TimerService, object):
+    """Twisted service to periodically refresh ephemeral images.
+
+    :param client_service: A `ClusterClientService` instance for talking
+        to the region controller.
+    :param reactor: An `IReactor` instance.
+    """
+
+    check_interval = timedelta(minutes=5).total_seconds()
+
+    def __init__(self, client_service, reactor, cluster_uuid):
+        # Call self.check() every self.check_interval.
+        super(PeriodicImageDownloadService, self).__init__(
+            self.check_interval, self.maybe_start_download)
+        self.clock = reactor
+        self.client_service = client_service
+        self.uuid = cluster_uuid
+
+    @inlineCallbacks
+    def _get_boot_sources(self, client):
+        """Gets the boot sources from the region."""
+        try:
+            sources = yield client(GetBootSourcesV2, uuid=self.uuid)
+        except NoSuchMethod:
+            # Region has not been upgraded to support the new call, use the
+            # old call. The old call did not provide the new os selection
+            # parameter. Region does not support boot source selection by os,
+            # so its set too allow all operating systems.
+            sources = yield client(GetBootSources, uuid=self.uuid)
+            for source in sources['sources']:
+                for selection in source['selections']:
+                    selection['os'] = '*'
+        returnValue(sources)
+
+    @inlineCallbacks
+    def _start_download(self):
+        client = None
+        # Retry a few times, since this service usually comes up before
+        # the RPC service.
+        for _ in range(3):
+            try:
+                client = self.client_service.getClient()
+                break
+            except NoConnectionsAvailable:
+                yield pause(5)
+        if client is None:
+            maaslog.error(
+                "Can't initiate image download, no RPC connection to region.")
+            return
+
+        # Get sources from region
+        sources = yield self._get_boot_sources(client)
+        yield import_boot_images(sources.get("sources"))
+
+    @inlineCallbacks
+    def maybe_start_download(self):
+        """Check the time the last image refresh happened and initiate a new
+        one if older than 15 minutes.
+        """
+        # Use a DeferredLock to prevent simultaneous downloads.
+        if service_lock.locked:
+            # Don't want to block on lock release.
+            return
+        yield service_lock.acquire()
+        try:
+            last_modified = tftppath.maas_meta_last_modified()
+            if last_modified is None:
+                # Don't auto-refresh if the user has never manually initiated
+                # a download.
+                return
+
+            age_in_seconds = self.clock.seconds() - last_modified
+            if age_in_seconds >= timedelta(minutes=15).total_seconds():
+                yield self._start_download()
+
+        finally:
+            service_lock.release()

=== added file 'src/provisioningserver/pserv_services/node_power_monitor_service.py'
--- src/provisioningserver/pserv_services/node_power_monitor_service.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/pserv_services/node_power_monitor_service.py	2014-08-27 06:01:25 +0000
@@ -0,0 +1,75 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Service to periodically query the power state on this cluster's nodes."""
+
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    "NodePowerMonitorService"
+]
+
+
+from provisioningserver.logger.log import get_maas_logger
+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
+from provisioningserver.rpc.power import query_all_nodes
+from provisioningserver.rpc.region import ListNodePowerParameters
+from provisioningserver.utils.twisted import pause
+from twisted.application.internet import TimerService
+from twisted.internet.defer import inlineCallbacks
+
+
+maaslog = get_maas_logger("power_monitor_service")
+
+
+class NodePowerMonitorService(TimerService, object):
+    """Twisted service to monitor the status of all nodes
+    controlled by this cluster.
+
+    :param client_service: A `ClusterClientService` instance for talking
+        to the region controller.
+    :param reactor: An `IReactor` instance.
+    """
+
+    # XXX 2014-08-27 bug=1361967
+    # This service is COMPLETELY UNTESTED.
+
+    check_interval = 600  # 5 minutes.
+
+    def __init__(self, client_service, reactor, cluster_uuid):
+        # Call self.check() every self.check_interval.
+        super(NodePowerMonitorService, self).__init__(
+            self.check_interval, self.query_nodes)
+        self.clock = reactor
+        self.client_service = client_service
+        self.uuid = cluster_uuid
+
+    @inlineCallbacks
+    def query_nodes(self):
+        client = None
+        # Retry a few times, since this service usually comes up before
+        # the RPC service.
+        for _ in range(3):
+            try:
+                client = self.client_service.getClient()
+                break
+            except NoConnectionsAvailable:
+                yield pause(5)
+        if client is None:
+            maaslog.error(
+                "Can't query nodes's BMC for power state, no RPC connection "
+                "to region.")
+            return
+
+        # Get the nodes from the Region
+        response = yield client(ListNodePowerParameters, uuid=self.uuid)
+        nodes = response['nodes']
+        yield query_all_nodes(nodes)

=== added directory 'src/provisioningserver/pserv_services/tests'
=== added file 'src/provisioningserver/pserv_services/tests/__init__.py'
=== added file 'src/provisioningserver/pserv_services/tests/test_image_download_service.py'
--- src/provisioningserver/pserv_services/tests/test_image_download_service.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/pserv_services/tests/test_image_download_service.py	2014-08-27 06:01:25 +0000
@@ -0,0 +1,279 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for provisioningserver.pserv_services.image_download_service"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = []
+
+
+from datetime import timedelta
+
+from maastesting.factory import factory
+from maastesting.matchers import (
+    get_mock_calls,
+    MockCalledOnceWith,
+    MockCallsMatch,
+    MockNotCalled,
+    )
+from mock import (
+    call,
+    Mock,
+    sentinel,
+    )
+from provisioningserver.boot import tftppath
+from provisioningserver.pserv_services.image_download_service import (
+    PeriodicImageDownloadService,
+    service_lock,
+    )
+from provisioningserver.rpc import boot_images
+from provisioningserver.rpc.boot_images import _run_import
+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
+from provisioningserver.rpc.region import (
+    GetBootSources,
+    GetBootSourcesV2,
+    )
+from provisioningserver.testing.testcase import PservTestCase
+from provisioningserver.utils.twisted import pause
+from testtools.deferredruntest import AsynchronousDeferredRunTest
+from twisted.application.internet import TimerService
+from twisted.internet import defer
+from twisted.internet.task import Clock
+from twisted.spread.pb import NoSuchMethod
+
+
+class TestPeriodicImageDownloadService(PservTestCase):
+
+    run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
+
+    def test_init(self):
+        service = PeriodicImageDownloadService(
+            sentinel.service, sentinel.clock, sentinel.uuid)
+        self.assertIsInstance(service, TimerService)
+        self.assertIs(service.clock, sentinel.clock)
+        self.assertIs(service.uuid, sentinel.uuid)
+        self.assertIs(service.client_service, sentinel.service)
+
+    def patch_download(self, service, return_value):
+        patched = self.patch(service, '_start_download')
+        patched.return_value = defer.succeed(return_value)
+        return patched
+
+    def test_is_called_every_interval(self):
+        clock = Clock()
+        service = PeriodicImageDownloadService(
+            sentinel.service, clock, sentinel.uuid)
+        # Avoid actual downloads:
+        self.patch_download(service, None)
+        maas_meta_last_modified = self.patch(
+            tftppath, 'maas_meta_last_modified')
+        maas_meta_last_modified.return_value = None
+        service.startService()
+
+        # The first call is issued at startup.
+        self.assertEqual(1, len(get_mock_calls(maas_meta_last_modified)))
+
+        # Wind clock forward one second less than the desired interval.
+        clock.advance(service.check_interval - 1)
+        # No more periodic calls made.
+        self.assertEqual(1, len(get_mock_calls(maas_meta_last_modified)))
+
+        # Wind clock forward one second, past the interval.
+        clock.advance(1)
+
+        # Now there were two calls.
+        self.assertEqual(2, len(get_mock_calls(maas_meta_last_modified)))
+
+        # Forward another interval, should be three calls.
+        clock.advance(service.check_interval)
+        self.assertEqual(3, len(get_mock_calls(maas_meta_last_modified)))
+
+    def test_no_download_if_no_meta_file(self):
+        clock = Clock()
+        service = PeriodicImageDownloadService(
+            sentinel.service, clock, sentinel.uuid)
+        _start_download = self.patch_download(service, None)
+        self.patch(
+            tftppath,
+            'maas_meta_last_modified').return_value = None
+        service.startService()
+        self.assertThat(_start_download, MockNotCalled())
+
+    def test_initiates_download_if_15_minutes_has_passed(self):
+        clock = Clock()
+        service = PeriodicImageDownloadService(
+            sentinel.service, clock, sentinel.uuid)
+        _start_download = self.patch_download(service, None)
+        one_week_ago = clock.seconds() - timedelta(minutes=15).total_seconds()
+        self.patch(
+            tftppath,
+            'maas_meta_last_modified').return_value = one_week_ago
+        service.startService()
+        self.assertThat(_start_download, MockCalledOnceWith())
+
+    def test_no_download_if_15_minutes_has_not_passed(self):
+        clock = Clock()
+        service = PeriodicImageDownloadService(
+            sentinel.service, clock, sentinel.uuid)
+        _start_download = self.patch_download(service, None)
+        one_week = timedelta(minutes=15).total_seconds()
+        self.patch(
+            tftppath,
+            'maas_meta_last_modified').return_value = clock.seconds()
+        clock.advance(one_week - 1)
+        service.startService()
+        self.assertThat(_start_download, MockNotCalled())
+
+    def test_download_is_initiated_in_new_thread(self):
+        clock = Clock()
+        maas_meta_last_modified = self.patch(
+            tftppath, 'maas_meta_last_modified')
+        one_week = timedelta(minutes=15).total_seconds()
+        maas_meta_last_modified.return_value = clock.seconds() - one_week
+        rpc_client = Mock()
+        client_call = Mock()
+        client_call.side_effect = [
+            defer.succeed(dict(sources=sentinel.sources)),
+            ]
+        rpc_client.getClient.return_value = client_call
+
+        # We could patch out 'import_boot_images' instead here but I
+        # don't do that for 2 reasons:
+        # 1. It requires spinning the reactor again before being able to
+        # test the result.
+        # 2. It means there's no thread to clean up after the test.
+        deferToThread = self.patch(boot_images, 'deferToThread')
+        deferToThread.return_value = defer.succeed(None)
+        service = PeriodicImageDownloadService(
+            rpc_client, clock, sentinel.uuid)
+        service.startService()
+        self.assertThat(
+            deferToThread, MockCalledOnceWith(
+                _run_import, sentinel.sources))
+
+    def test_no_download_if_no_rpc_connections(self):
+        rpc_client = Mock()
+        failure = NoConnectionsAvailable()
+        rpc_client.getClient.return_value.side_effect = failure
+
+        deferToThread = self.patch(boot_images, 'deferToThread')
+        service = PeriodicImageDownloadService(
+            rpc_client, Clock(), sentinel.uuid)
+        service.startService()
+        self.assertThat(deferToThread, MockNotCalled())
+
+    @defer.inlineCallbacks
+    def test_does_not_run_if_lock_taken(self):
+        maas_meta_last_modified = self.patch(
+            tftppath, 'maas_meta_last_modified')
+        yield service_lock.acquire()
+        self.addCleanup(service_lock.release)
+        service = PeriodicImageDownloadService(
+            sentinel.rpc, Clock(), sentinel.uuid)
+        service.startService()
+        self.assertThat(maas_meta_last_modified, MockNotCalled())
+
+    def test_takes_lock_when_running(self):
+        clock = Clock()
+        service = PeriodicImageDownloadService(
+            sentinel.rpc, clock, sentinel.uuid)
+
+        # Patch the download func so it's just a Deferred that waits for
+        # one second.
+        _start_download = self.patch(service, '_start_download')
+        _start_download.return_value = pause(1, clock)
+
+        # Set conditions for a required download:
+        one_week_ago = clock.seconds() - timedelta(minutes=15).total_seconds()
+        self.patch(
+            tftppath,
+            'maas_meta_last_modified').return_value = one_week_ago
+
+        # Lock is acquired for the first download after startup.
+        service.startService()
+        self.assertTrue(service_lock.locked)
+
+        # Lock is released once the download is done.
+        clock.advance(1)
+        self.assertFalse(service_lock.locked)
+
+    @defer.inlineCallbacks
+    def test__get_boot_sources_calls_get_boot_sources_v2_before_v1(self):
+        clock = Clock()
+        client_call = Mock()
+        client_call.side_effect = [
+            defer.succeed(dict(sources=sentinel.sources)),
+            ]
+
+        service = PeriodicImageDownloadService(
+            sentinel.rpc, clock, sentinel.uuid)
+        sources = yield service._get_boot_sources(client_call)
+        self.assertEqual(sources.get('sources'), sentinel.sources)
+        self.assertThat(
+            client_call,
+            MockCalledOnceWith(GetBootSourcesV2, uuid=sentinel.uuid))
+
+    @defer.inlineCallbacks
+    def test__get_boot_sources_calls_get_boot_sources_v1_on_v2_missing(self):
+        clock = Clock()
+        client_call = Mock()
+        client_call.side_effect = [
+            defer.fail(NoSuchMethod()),
+            defer.succeed(dict(sources=[])),
+            ]
+
+        service = PeriodicImageDownloadService(
+            sentinel.rpc, clock, sentinel.uuid)
+        yield service._get_boot_sources(client_call)
+        self.assertThat(
+            client_call,
+            MockCallsMatch(
+                call(GetBootSourcesV2, uuid=sentinel.uuid),
+                call(GetBootSources, uuid=sentinel.uuid)))
+
+    @defer.inlineCallbacks
+    def test__get_boot_sources_v1_sets_os_to_wildcard(self):
+        sources = [
+            {
+                'path': factory.make_url(),
+                'selections': [
+                    {
+                        'release': "trusty",
+                        'arches': ["amd64"],
+                        'subarches': ["generic"],
+                        'labels': ["release"],
+                    },
+                    {
+                        'release': "precise",
+                        'arches': ["amd64"],
+                        'subarches': ["generic"],
+                        'labels': ["release"],
+                    },
+                ],
+            },
+        ]
+
+        clock = Clock()
+        client_call = Mock()
+        client_call.side_effect = [
+            defer.fail(NoSuchMethod()),
+            defer.succeed(dict(sources=sources)),
+            ]
+
+        service = PeriodicImageDownloadService(
+            sentinel.rpc, clock, sentinel.uuid)
+        sources = yield service._get_boot_sources(client_call)
+        os_selections = [
+            selection.get('os')
+            for source in sources['sources']
+            for selection in source['selections']
+            ]
+        self.assertEqual(['*', '*'], os_selections)

=== renamed file 'src/provisioningserver/tftp.py' => 'src/provisioningserver/pserv_services/tftp.py'
=== modified file 'src/provisioningserver/rpc/boot_images.py'
--- src/provisioningserver/rpc/boot_images.py	2014-08-26 17:23:16 +0000
+++ src/provisioningserver/rpc/boot_images.py	2014-08-27 06:01:25 +0000
@@ -15,39 +15,17 @@
 __all__ = [
     "import_boot_images",
     "list_boot_images",
-    "PeriodicImageDownloadService",
     ]
 
 
-from datetime import timedelta
-
 from provisioningserver.auth import MAAS_USER_GPGHOME
 from provisioningserver.boot import tftppath
 from provisioningserver.config import Config
 from provisioningserver.import_images import boot_resources
-from provisioningserver.logger import get_maas_logger
-from provisioningserver.rpc.exceptions import NoConnectionsAvailable
-from provisioningserver.rpc.region import (
-    GetBootSources,
-    GetBootSourcesV2,
-    )
 from provisioningserver.utils.env import environment_variables
-from provisioningserver.utils.twisted import (
-    pause,
-    synchronous,
-    )
-from twisted.application.internet import TimerService
-from twisted.internet.defer import (
-    DeferredLock,
-    inlineCallbacks,
-    returnValue,
-    )
+from provisioningserver.utils.twisted import synchronous
+from twisted.internet.defer import inlineCallbacks
 from twisted.internet.threads import deferToThread
-from twisted.spread.pb import NoSuchMethod
-
-
-maaslog = get_maas_logger("boot_images")
-service_lock = DeferredLock()
 
 
 def list_boot_images():
@@ -73,82 +51,3 @@
 def import_boot_images(sources):
     """Imports the boot images from the given sources."""
     yield deferToThread(_run_import, sources)
-
-
-class PeriodicImageDownloadService(TimerService, object):
-    """Twisted service to periodically refresh ephemeral images.
-
-    :param client_service: A `ClusterClientService` instance for talking
-        to the region controller.
-    :param reactor: An `IReactor` instance.
-    """
-
-    check_interval = timedelta(minutes=5).total_seconds()
-
-    def __init__(self, client_service, reactor, cluster_uuid):
-        # Call self.check() every self.check_interval.
-        super(PeriodicImageDownloadService, self).__init__(
-            self.check_interval, self.maybe_start_download)
-        self.clock = reactor
-        self.client_service = client_service
-        self.uuid = cluster_uuid
-
-    @inlineCallbacks
-    def _get_boot_sources(self, client):
-        """Gets the boot sources from the region."""
-        try:
-            sources = yield client(GetBootSourcesV2, uuid=self.uuid)
-        except NoSuchMethod:
-            # Region has not been upgraded to support the new call, use the
-            # old call. The old call did not provide the new os selection
-            # parameter. Region does not support boot source selection by os,
-            # so its set too allow all operating systems.
-            sources = yield client(GetBootSources, uuid=self.uuid)
-            for source in sources['sources']:
-                for selection in source['selections']:
-                    selection['os'] = '*'
-        returnValue(sources)
-
-    @inlineCallbacks
-    def _start_download(self):
-        client = None
-        # Retry a few times, since this service usually comes up before
-        # the RPC service.
-        for _ in range(3):
-            try:
-                client = self.client_service.getClient()
-                break
-            except NoConnectionsAvailable:
-                yield pause(5)
-        if client is None:
-            maaslog.error(
-                "Can't initiate image download, no RPC connection to region.")
-            return
-
-        # Get sources from region
-        sources = yield self._get_boot_sources(client)
-        yield import_boot_images(sources.get("sources"))
-
-    @inlineCallbacks
-    def maybe_start_download(self):
-        """Check the time the last image refresh happened and initiate a new
-        one if older than 15 minutes.
-        """
-        # Use a DeferredLock to prevent simultaneous downloads.
-        if service_lock.locked:
-            # Don't want to block on lock release.
-            return
-        yield service_lock.acquire()
-        try:
-            last_modified = tftppath.maas_meta_last_modified()
-            if last_modified is None:
-                # Don't auto-refresh if the user has never manually initiated
-                # a download.
-                return
-
-            age_in_seconds = self.clock.seconds() - last_modified
-            if age_in_seconds >= timedelta(minutes=15).total_seconds():
-                yield self._start_download()
-
-        finally:
-            service_lock.release()

=== modified file 'src/provisioningserver/rpc/power.py'
--- src/provisioningserver/rpc/power.py	2014-08-22 16:49:23 +0000
+++ src/provisioningserver/rpc/power.py	2014-08-27 06:01:25 +0000
@@ -13,7 +13,8 @@
 
 __metaclass__ = type
 __all__ = [
-    "get_power_state"
+    "get_power_state",
+    "query_all_nodes",
 ]
 
 
@@ -27,17 +28,12 @@
     PowerActionFail,
     )
 from provisioningserver.rpc import getRegionClient
-from provisioningserver.rpc.exceptions import (
-    NoConnectionsAvailable,
-    NoSuchNode,
-    )
+from provisioningserver.rpc.exceptions import NoSuchNode
 from provisioningserver.rpc.region import (
-    ListNodePowerParameters,
     MarkNodeBroken,
     UpdateNodePowerState,
     )
 from provisioningserver.utils.twisted import pause
-from twisted.application.internet import TimerService
 from twisted.internet import reactor
 from twisted.internet.defer import (
     inlineCallbacks,
@@ -256,45 +252,3 @@
                 maaslog.debug(
                     "%s: Could not update power status; "
                     "no such node.", hostname)
-
-
-class NodePowerMonitorService(TimerService, object):
-    """Twisted service to monitor the status of all nodes
-    controlled by this cluster.
-
-    :param client_service: A `ClusterClientService` instance for talking
-        to the region controller.
-    :param reactor: An `IReactor` instance.
-    """
-
-    check_interval = 600  # 5 minutes.
-
-    def __init__(self, client_service, reactor, cluster_uuid):
-        # Call self.check() every self.check_interval.
-        super(NodePowerMonitorService, self).__init__(
-            self.check_interval, self.query_nodes)
-        self.clock = reactor
-        self.client_service = client_service
-        self.uuid = cluster_uuid
-
-    @inlineCallbacks
-    def query_nodes(self):
-        client = None
-        # Retry a few times, since this service usually comes up before
-        # the RPC service.
-        for _ in range(3):
-            try:
-                client = self.client_service.getClient()
-                break
-            except NoConnectionsAvailable:
-                yield pause(5)
-        if client is None:
-            maaslog.error(
-                "Can't query nodes's BMC for power state, no RPC connection "
-                "to region.")
-            return
-
-        # Get the nodes from the Region
-        response = yield client(ListNodePowerParameters, uuid=self.uuid)
-        nodes = response['nodes']
-        yield query_all_nodes(nodes)

=== modified file 'src/provisioningserver/rpc/tests/test_boot_images.py'
--- src/provisioningserver/rpc/tests/test_boot_images.py	2014-08-26 17:23:16 +0000
+++ src/provisioningserver/rpc/tests/test_boot_images.py	2014-08-27 06:01:25 +0000
@@ -14,21 +14,11 @@
 __metaclass__ = type
 __all__ = []
 
-from datetime import timedelta
 import os
 
 from maastesting.factory import factory
-from maastesting.matchers import (
-    get_mock_calls,
-    MockCalledOnceWith,
-    MockCallsMatch,
-    MockNotCalled,
-    )
-from mock import (
-    call,
-    Mock,
-    sentinel,
-    )
+from maastesting.matchers import MockCalledOnceWith
+from mock import sentinel
 from provisioningserver.boot import tftppath
 from provisioningserver.config import Config
 from provisioningserver.import_images import boot_resources
@@ -37,22 +27,11 @@
     _run_import,
     import_boot_images,
     list_boot_images,
-    PeriodicImageDownloadService,
-    service_lock,
-    )
-from provisioningserver.rpc.exceptions import NoConnectionsAvailable
-from provisioningserver.rpc.region import (
-    GetBootSources,
-    GetBootSourcesV2,
     )
 from provisioningserver.testing.config import BootSourcesFixture
 from provisioningserver.testing.testcase import PservTestCase
-from provisioningserver.utils.twisted import pause
 from testtools.deferredruntest import AsynchronousDeferredRunTest
-from twisted.application.internet import TimerService
 from twisted.internet import defer
-from twisted.internet.task import Clock
-from twisted.spread.pb import NoSuchMethod
 
 
 class TestListBootImages(PservTestCase):
@@ -145,232 +124,3 @@
         self.assertThat(
             deferToThread, MockCalledOnceWith(
                 _run_import, sentinel.sources))
-
-
-class TestPeriodicImageDownloadService(PservTestCase):
-
-    run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
-
-    def test_init(self):
-        service = PeriodicImageDownloadService(
-            sentinel.service, sentinel.clock, sentinel.uuid)
-        self.assertIsInstance(service, TimerService)
-        self.assertIs(service.clock, sentinel.clock)
-        self.assertIs(service.uuid, sentinel.uuid)
-        self.assertIs(service.client_service, sentinel.service)
-
-    def patch_download(self, service, return_value):
-        patched = self.patch(service, '_start_download')
-        patched.return_value = defer.succeed(return_value)
-        return patched
-
-    def test_is_called_every_interval(self):
-        clock = Clock()
-        service = PeriodicImageDownloadService(
-            sentinel.service, clock, sentinel.uuid)
-        # Avoid actual downloads:
-        self.patch_download(service, None)
-        maas_meta_last_modified = self.patch(
-            tftppath, 'maas_meta_last_modified')
-        maas_meta_last_modified.return_value = None
-        service.startService()
-
-        # The first call is issued at startup.
-        self.assertEqual(1, len(get_mock_calls(maas_meta_last_modified)))
-
-        # Wind clock forward one second less than the desired interval.
-        clock.advance(service.check_interval - 1)
-        # No more periodic calls made.
-        self.assertEqual(1, len(get_mock_calls(maas_meta_last_modified)))
-
-        # Wind clock forward one second, past the interval.
-        clock.advance(1)
-
-        # Now there were two calls.
-        self.assertEqual(2, len(get_mock_calls(maas_meta_last_modified)))
-
-        # Forward another interval, should be three calls.
-        clock.advance(service.check_interval)
-        self.assertEqual(3, len(get_mock_calls(maas_meta_last_modified)))
-
-    def test_no_download_if_no_meta_file(self):
-        clock = Clock()
-        service = PeriodicImageDownloadService(
-            sentinel.service, clock, sentinel.uuid)
-        _start_download = self.patch_download(service, None)
-        self.patch(
-            tftppath,
-            'maas_meta_last_modified').return_value = None
-        service.startService()
-        self.assertThat(_start_download, MockNotCalled())
-
-    def test_initiates_download_if_15_minutes_has_passed(self):
-        clock = Clock()
-        service = PeriodicImageDownloadService(
-            sentinel.service, clock, sentinel.uuid)
-        _start_download = self.patch_download(service, None)
-        one_week_ago = clock.seconds() - timedelta(minutes=15).total_seconds()
-        self.patch(
-            tftppath,
-            'maas_meta_last_modified').return_value = one_week_ago
-        service.startService()
-        self.assertThat(_start_download, MockCalledOnceWith())
-
-    def test_no_download_if_15_minutes_has_not_passed(self):
-        clock = Clock()
-        service = PeriodicImageDownloadService(
-            sentinel.service, clock, sentinel.uuid)
-        _start_download = self.patch_download(service, None)
-        one_week = timedelta(minutes=15).total_seconds()
-        self.patch(
-            tftppath,
-            'maas_meta_last_modified').return_value = clock.seconds()
-        clock.advance(one_week - 1)
-        service.startService()
-        self.assertThat(_start_download, MockNotCalled())
-
-    def test_download_is_initiated_in_new_thread(self):
-        clock = Clock()
-        maas_meta_last_modified = self.patch(
-            tftppath, 'maas_meta_last_modified')
-        one_week = timedelta(minutes=15).total_seconds()
-        maas_meta_last_modified.return_value = clock.seconds() - one_week
-        rpc_client = Mock()
-        client_call = Mock()
-        client_call.side_effect = [
-            defer.succeed(dict(sources=sentinel.sources)),
-            ]
-        rpc_client.getClient.return_value = client_call
-
-        # We could patch out 'import_boot_images' instead here but I
-        # don't do that for 2 reasons:
-        # 1. It requires spinning the reactor again before being able to
-        # test the result.
-        # 2. It means there's no thread to clean up after the test.
-        deferToThread = self.patch(boot_images, 'deferToThread')
-        deferToThread.return_value = defer.succeed(None)
-        service = PeriodicImageDownloadService(
-            rpc_client, clock, sentinel.uuid)
-        service.startService()
-        self.assertThat(
-            deferToThread, MockCalledOnceWith(
-                _run_import, sentinel.sources))
-
-    def test_no_download_if_no_rpc_connections(self):
-        rpc_client = Mock()
-        failure = NoConnectionsAvailable()
-        rpc_client.getClient.return_value.side_effect = failure
-
-        deferToThread = self.patch(boot_images, 'deferToThread')
-        service = PeriodicImageDownloadService(
-            rpc_client, Clock(), sentinel.uuid)
-        service.startService()
-        self.assertThat(deferToThread, MockNotCalled())
-
-    @defer.inlineCallbacks
-    def test_does_not_run_if_lock_taken(self):
-        maas_meta_last_modified = self.patch(
-            tftppath, 'maas_meta_last_modified')
-        yield service_lock.acquire()
-        self.addCleanup(service_lock.release)
-        service = PeriodicImageDownloadService(
-            sentinel.rpc, Clock(), sentinel.uuid)
-        service.startService()
-        self.assertThat(maas_meta_last_modified, MockNotCalled())
-
-    def test_takes_lock_when_running(self):
-        clock = Clock()
-        service = PeriodicImageDownloadService(
-            sentinel.rpc, clock, sentinel.uuid)
-
-        # Patch the download func so it's just a Deferred that waits for
-        # one second.
-        _start_download = self.patch(service, '_start_download')
-        _start_download.return_value = pause(1, clock)
-
-        # Set conditions for a required download:
-        one_week_ago = clock.seconds() - timedelta(minutes=15).total_seconds()
-        self.patch(
-            tftppath,
-            'maas_meta_last_modified').return_value = one_week_ago
-
-        # Lock is acquired for the first download after startup.
-        service.startService()
-        self.assertTrue(service_lock.locked)
-
-        # Lock is released once the download is done.
-        clock.advance(1)
-        self.assertFalse(service_lock.locked)
-
-    @defer.inlineCallbacks
-    def test__get_boot_sources_calls_get_boot_sources_v2_before_v1(self):
-        clock = Clock()
-        client_call = Mock()
-        client_call.side_effect = [
-            defer.succeed(dict(sources=sentinel.sources)),
-            ]
-
-        service = PeriodicImageDownloadService(
-            sentinel.rpc, clock, sentinel.uuid)
-        sources = yield service._get_boot_sources(client_call)
-        self.assertEqual(sources.get('sources'), sentinel.sources)
-        self.assertThat(
-            client_call,
-            MockCalledOnceWith(GetBootSourcesV2, uuid=sentinel.uuid))
-
-    @defer.inlineCallbacks
-    def test__get_boot_sources_calls_get_boot_sources_v1_on_v2_missing(self):
-        clock = Clock()
-        client_call = Mock()
-        client_call.side_effect = [
-            defer.fail(NoSuchMethod()),
-            defer.succeed(dict(sources=[])),
-            ]
-
-        service = PeriodicImageDownloadService(
-            sentinel.rpc, clock, sentinel.uuid)
-        yield service._get_boot_sources(client_call)
-        self.assertThat(
-            client_call,
-            MockCallsMatch(
-                call(GetBootSourcesV2, uuid=sentinel.uuid),
-                call(GetBootSources, uuid=sentinel.uuid)))
-
-    @defer.inlineCallbacks
-    def test__get_boot_sources_v1_sets_os_to_wildcard(self):
-        sources = [
-            {
-                'path': factory.make_url(),
-                'selections': [
-                    {
-                        'release': "trusty",
-                        'arches': ["amd64"],
-                        'subarches': ["generic"],
-                        'labels': ["release"],
-                    },
-                    {
-                        'release': "precise",
-                        'arches': ["amd64"],
-                        'subarches': ["generic"],
-                        'labels': ["release"],
-                    },
-                ],
-            },
-        ]
-
-        clock = Clock()
-        client_call = Mock()
-        client_call.side_effect = [
-            defer.fail(NoSuchMethod()),
-            defer.succeed(dict(sources=sources)),
-            ]
-
-        service = PeriodicImageDownloadService(
-            sentinel.rpc, clock, sentinel.uuid)
-        sources = yield service._get_boot_sources(client_call)
-        os_selections = [
-            selection.get('os')
-            for source in sources['sources']
-            for selection in source['selections']
-            ]
-        self.assertEqual(['*', '*'], os_selections)

=== modified file 'src/provisioningserver/tests/test_plugin.py'
--- src/provisioningserver/tests/test_plugin.py	2014-08-26 23:55:04 +0000
+++ src/provisioningserver/tests/test_plugin.py	2014-08-27 06:01:25 +0000
@@ -27,9 +27,13 @@
     ProvisioningServiceMaker,
     SingleUsernamePasswordChecker,
     )
-from provisioningserver.rpc.boot_images import PeriodicImageDownloadService
-from provisioningserver.rpc.power import NodePowerMonitorService
-from provisioningserver.tftp import (
+from provisioningserver.pserv_services.image_download_service import (
+    PeriodicImageDownloadService,
+    )
+from provisioningserver.pserv_services.node_power_monitor_service import (
+    NodePowerMonitorService,
+    )
+from provisioningserver.pserv_services.tftp import (
     TFTPBackend,
     TFTPService,
     )

=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py	2014-08-13 21:49:35 +0000
+++ src/provisioningserver/tests/test_tftp.py	2014-08-27 06:01:25 +0000
@@ -37,17 +37,17 @@
     IPV4_LINK_LOCAL,
     IPV6_LINK_LOCAL,
     )
-from provisioningserver import tftp as tftp_module
 from provisioningserver.boot import BytesReader
 from provisioningserver.boot.pxe import PXEBootMethod
 from provisioningserver.boot.tests.test_pxe import compose_config_path
-from provisioningserver.tests.test_kernel_opts import make_kernel_parameters
-from provisioningserver.tftp import (
+from provisioningserver.pserv_services import tftp as tftp_module
+from provisioningserver.pserv_services.tftp import (
     Port,
     TFTPBackend,
     TFTPService,
     UDPServer,
     )
+from provisioningserver.tests.test_kernel_opts import make_kernel_parameters
 from testtools.deferredruntest import AsynchronousDeferredRunTest
 from testtools.matchers import (
     AfterPreprocessing,