Merge lp:~trapnine/maas/rackd-updates-last-image-sync into lp:~maas-committers/maas/trunk

[Jeffrey C Jones]

Racks will ask region to update last_image_sync directly.

Racks will ask region to update last_image_sync directly.

[Lee Trager (ltrager)]

Looks good so far just a couple of comments below. As you previously stated there should be a way to get the connection ID, which is the system_id on the server side but I haven't been able to figure out how to do that.

[Jeffrey C Jones (trapnine)]

Thanks - everything addressed, please take a look.

[Gavin Panella (allenap)]

Something I missed when talking to you yesterday is: this won't work :)
However, not a lot needs changing; see my comments for how I would go
about it.

In general, the problem here could be approached in two ways:

1. Record the image sync time in the database.

2. Ask racks for their last image sync time when needed.

#2 is preferable because then you're asking a primary source. The rack
may not be connected when you ask; however, when it last synced images
is not likely to be interesting information when it's down.

In #1's favour, MAAS relies on LISTEN/NOTIFY for disseminating change
within MAAS, so putting this value into the database makes sense.

However, #1, as in this proposal, can fail to update the sync time and
thus give a false reading. Fortunately racks do periodically import
images on their own, and so the last sync time will eventually converge
on truth.

review: Needs Fixing

[Gavin Panella (allenap)]

Drat, I wrote a long comment in the diff that hasn't been saved; in fact it has been lost. I will write it again.

[Gavin Panella (allenap)]

Ha, *now* it has saved one of my comments. Sadly it's by far the shorter of the two.

[Gavin Panella (allenap)]

Long comment rewritten shorter.

[Gavin Panella (allenap)]

This looks good, but I think you need to resurrect those end-to-end tests and get them passing. I'll help you with that. They were failing for a reason, and I suspect that reason was because things needed to be changed around.

review: Needs Fixing

[Jeffrey C Jones (trapnine)]

Tests added and working.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~trapnine/maas/rackd-updates-last-image-sync into lp:maas failed. Below is the output from the failed tests.

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

[Gavin Panella (allenap)]

You missed one comment from my last review:

> import_images() can return False in three situations, and True in one.
> Each should be tested. There should already be tests for those
> early-returns and run-to-completion so you'll just have to add an
> assert.

Do you mind fixing that in a follow-up?

=== modified file 'src/maasserver/clusterrpc/boot_images.py'
--- src/maasserver/clusterrpc/boot_images.py	2016-04-14 19:54:36 +0000
+++ src/maasserver/clusterrpc/boot_images.py	2016-05-12 17:26:13 +0000
@@ -23,7 +23,6 @@
     BootResource,
     RackController,
 )
-from maasserver.models.timestampedmodel import now
 from maasserver.rpc import (
     getAllClients,
     getClientFor,
@@ -289,11 +288,6 @@
              for system_id in self.system_ids),
             consumeErrors=True)
 
-    @staticmethod
-    def touch_last_image_sync(system_ids):
-        RackController.objects.filter(
-            system_id__in=system_ids).update(last_image_sync=now())
-
     @asynchronous
     def run(self, concurrency=1):
         """Ask the rack controllers to download the region's boot resources.
@@ -314,20 +308,13 @@
                 "Rack controller (%s) did not import boot resources; it is "
                 "not connected to the region at this time."
             )
-            successes = []
             for system_id, (success, result) in zip(self.system_ids, results):
                 if success:
-                    successes.append(system_id)
                     log.msg(message_success % system_id)
                 elif result.check(NoConnectionsAvailable):
                     log.msg(message_disconn % system_id)
                 else:
                     log.err(result, message_failure % system_id)
 
-            return deferToDatabase(
-                RackControllersImporter.touch_last_image_sync,
-                successes).addErrback(
-                log.err, "Failed to touch last image sync timestamps.")
-
         return self(lock).addCallback(report).addErrback(
             log.err, "General failure syncing boot resources.")

=== modified file 'src/maasserver/clusterrpc/tests/test_boot_images.py'
--- src/maasserver/clusterrpc/tests/test_boot_images.py	2016-05-06 11:41:05 +0000
+++ src/maasserver/clusterrpc/tests/test_boot_images.py	2016-05-12 17:26:13 +0000
@@ -5,7 +5,6 @@
 
 __all__ = []
 
-from datetime import timedelta
 import os
 import random
 from unittest.mock import (
@@ -26,7 +25,6 @@
 from maasserver.clusterrpc.testing.boot_images import make_rpc_boot_image
 from maasserver.enum import BOOT_RESOURCE_TYPE
 from maasserver.models.config import Config
-from maasserver.models.timestampedmodel import now
 from maasserver.rpc import getAllClients
 from maasserver.rpc.testing.fixtures import (
     MockLiveRegionToClusterRPCFixture,
@@ -38,7 +36,6 @@
 )
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
-from maasserver.utils.orm import reload_object
 from maastesting.matchers import (
     MockCalledOnceWith,
     MockCallsMatch,
@@ -597,74 +594,3 @@
             connected to the region at this time.
             """ % (rack_1.system_id, rack_2.system_id, rack_3.system_id),
             logger.output)
-
-    def test__run_touches_synced_racks_last_image_sync(self):
-        # Avoid deferring to the database.
-        self.patch(boot_images_module, "deferToDatabase", maybeDeferred)
-        touch = self.patch_autospec(
-            RackControllersImporter, 'touch_last_image_sync')
-
-        # Some clusters that we'll ask to import resources.
-        rack_1 = factory.make_RackController()
-        rack_2 = factory.make_RackController()
-        rack_3 = factory.make_RackController()
-        # Cluster #1 will work fine.
-        cluster_1 = self.rpc.makeCluster(rack_1, ImportBootImages)
-        cluster_1.ImportBootImages.return_value = succeed({})
-        # Cluster #2 will break.
-        cluster_2 = self.rpc.makeCluster(rack_2, ImportBootImages)
-        cluster_2.ImportBootImages.return_value = fail(ZeroDivisionError())
-        # Cluster #3 is not connected.
-
-        # Do the import.
-        importer = RackControllersImporter.new([
-            rack_1.system_id,
-            rack_2.system_id,
-            rack_3.system_id,
-        ])
-        importer.run().wait(5)
-
-        # Make sure that only rack_1's last_sync_time would get touched
-        self.assertThat(touch, MockCalledOnceWith([rack_1.system_id]))
-
-    def test__broken_touch_last_image_sync_reports_results(self):
-        # Avoid deferring to the database.
-        self.patch(boot_images_module, "deferToDatabase", maybeDeferred)
-        # Make touch_last_image_sync fail.
-        self.patch_autospec(
-            RackControllersImporter,
-            'touch_last_image_sync').side_effect = Exception()
-
-        rack_1 = factory.make_RackController()
-        cluster_1 = self.rpc.makeCluster(rack_1, ImportBootImages)
-        cluster_1.ImportBootImages.return_value = succeed({})
-
-        # Do the import with reporting.
-        importer = RackControllersImporter.new([rack_1.system_id])
-
-        with TwistedLoggerFixture() as logger:
-            importer.run().wait(5)
-
-        self.assertDocTestMatches(
-            """\
-            ...
-            ---
-            Failed to touch last image sync timestamps.
-            ...
-            """, logger.output)
-
-    def test__touch_last_image_sync(self):
-        # Create a few rackd's, two with no sync times.
-        birthday = now() - timedelta(minutes=random.randint(1, 15))
-        rack_1 = factory.make_RackController(last_image_sync=None)
-        rack_2 = factory.make_RackController(last_image_sync=None)
-        rack_3 = factory.make_RackController(last_image_sync=birthday)
-        rack_4 = factory.make_RackController(last_image_sync=birthday)
-
-        # Touch cluster with and without sync_time, confirm properly updated.
-        RackControllersImporter.touch_last_image_sync(
-            [rack_1.system_id, rack_3.system_id])
-        self.assertIsNotNone(reload_object(rack_1).last_image_sync)
-        self.assertIsNone(reload_object(rack_2).last_image_sync)
-        self.assertGreater(reload_object(rack_3).last_image_sync, birthday)
-        self.assertEqual(reload_object(rack_4).last_image_sync, birthday)

=== modified file 'src/maasserver/rpc/rackcontrollers.py'
--- src/maasserver/rpc/rackcontrollers.py	2016-05-06 07:49:56 +0000
+++ src/maasserver/rpc/rackcontrollers.py	2016-05-12 17:26:13 +0000
@@ -5,9 +5,9 @@
 
 __all__ = [
     "register_rackcontroller",
+    "update_last_image_sync",
 ]
 
-
 from django.db import (
     IntegrityError,
     transaction,
@@ -22,6 +22,7 @@
     StaticIPAddress,
 )
 from maasserver.models.node import typecast_node
+from maasserver.models.timestampedmodel import now
 from maasserver.utils.orm import transactional
 from provisioningserver.logger import get_maas_logger
 from provisioningserver.utils.twisted import synchronous
@@ -198,3 +199,14 @@
     """Update the interface definition on the rack controller."""
     rack_controller = RackController.objects.get(system_id=system_id)
     rack_controller.update_interfaces(interfaces)
+
+
+@synchronous
+@transactional
+def update_last_image_sync(system_id):
+    """Update rack controller's last_image_sync.
+
+    for :py:class:`~provisioningserver.rpc.region.UpdateLastImageSync.
+    """
+    RackController.objects.filter(
+        system_id=system_id).update(last_image_sync=now())

=== modified file 'src/maasserver/rpc/regionservice.py'
--- src/maasserver/rpc/regionservice.py	2016-04-29 22:46:46 +0000
+++ src/maasserver/rpc/regionservice.py	2016-05-12 17:26:13 +0000
@@ -291,6 +291,18 @@
         d.addCallback(lambda nodes: {"nodes": nodes})
         return d
 
+    @region.UpdateLastImageSync.responder
+    def update_last_image_sync(self, system_id):
+        """update_last_image_sync()
+
+        Implementation of
+        :py:class:`~provisioningserver.rpc.region.UpdateLastImageSync`.
+        """
+        d = deferToDatabase(
+            rackcontrollers.update_last_image_sync, system_id)
+        d.addCallback(lambda args: {})
+        return d
+
     @region.UpdateNodePowerState.responder
     def update_node_power_state(self, system_id, power_state):
         """update_node_power_state()

=== modified file 'src/maasserver/rpc/tests/test_rackcontrollers.py'
--- src/maasserver/rpc/tests/test_rackcontrollers.py	2016-05-09 23:54:19 +0000
+++ src/maasserver/rpc/tests/test_rackcontrollers.py	2016-05-12 17:26:13 +0000
@@ -21,12 +21,14 @@
     NodeGroupToRackController,
     RackController,
 )
+from maasserver.models.timestampedmodel import now
 from maasserver.rpc.rackcontrollers import (
     handle_upgrade,
     register_new_rackcontroller,
     register_rackcontroller,
     update_foreign_dhcp,
     update_interfaces,
+    update_last_image_sync,
 )
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
@@ -322,3 +324,13 @@
         self.assertThat(
             patched_update_interfaces,
             MockCalledOnceWith(sentinel.interfaces))
+
+
+class TestUpdateLastImageSync(MAASServerTestCase):
+
+    def test__updates_last_image_sync(self):
+        rack = factory.make_RackController()
+
+        update_last_image_sync(rack.system_id)
+
+        self.assertEqual(now(), reload_object(rack).last_image_sync)

=== modified file 'src/provisioningserver/import_images/boot_resources.py'
--- src/provisioningserver/import_images/boot_resources.py	2016-04-27 18:25:56 +0000
+++ src/provisioningserver/import_images/boot_resources.py	2016-05-12 17:26:13 +0000
@@ -32,6 +32,7 @@
 from provisioningserver.import_images.helpers import maaslog
 from provisioningserver.import_images.keyrings import write_all_keyrings
 from provisioningserver.import_images.product_mapping import map_products
+from provisioningserver.rpc import getRegionClient
 from provisioningserver.service_monitor import service_monitor
 from provisioningserver.utils.fs import (
     atomic_symlink,
@@ -40,6 +41,7 @@
     tempdir,
 )
 from provisioningserver.utils.shell import call_and_check
+from twisted.internet.defer import inlineCallbacks
 from twisted.python.filepath import FilePath
 
 
@@ -236,7 +238,7 @@
     maaslog.info("Started importing boot images.")
     if len(sources) == 0:
         maaslog.warning("Can't import: region did not provide a source.")
-        return
+        return False
 
     with tempdir('keyrings') as keyrings_path:
         # XXX: Band-aid to ensure that the keyring_data is bytes. Future task:
@@ -256,7 +258,7 @@
             maaslog.warning(
                 "Finished importing boot images, the region does not have "
                 "any boot images available.")
-            return
+            return False
 
         with ClusterConfiguration.open() as config:
             storage = FilePath(config.tftp_root).parent().path
@@ -265,7 +267,7 @@
             maaslog.info(
                 "Finished importing boot images, the region does not "
                 "have any new images.")
-            return
+            return False
 
         product_mapping = map_products(image_descriptions)
 
@@ -290,6 +292,7 @@
 
     # Import is now finished.
     maaslog.info("Finished importing boot images.")
+    return True
 
 
 def main(args):
@@ -318,12 +321,10 @@
     import traceback
 
     from provisioningserver import services
-    from provisioningserver.rpc import getRegionClient
     from provisioningserver.rpc.clusterservice import ClusterClientService
     from provisioningserver.rpc.exceptions import NoConnectionsAvailable
     from provisioningserver.utils.twisted import retries, pause
     from twisted.internet import reactor
-    from twisted.internet.defer import inlineCallbacks
     from twisted.internet.threads import deferToThread
 
     @inlineCallbacks

=== modified file 'src/provisioningserver/import_images/tests/test_boot_resources.py'
--- src/provisioningserver/import_images/tests/test_boot_resources.py	2016-05-06 11:41:05 +0000
+++ src/provisioningserver/import_images/tests/test_boot_resources.py	2016-05-12 17:26:13 +0000
@@ -28,7 +28,10 @@
     MockCalledWith,
     MockCallsMatch,
 )
-from maastesting.testcase import MAASTestCase
+from maastesting.testcase import (
+    MAASTestCase,
+    MAASTwistedRunTest,
+)
 from maastesting.utils import age_file
 from provisioningserver.boot import BootMethodRegistry
 from provisioningserver.config import (
@@ -563,6 +566,8 @@
 class TestImportImages(MAASTestCase):
     """Tests for the `import_images`() function."""
 
+    run_tests_with = MAASTwistedRunTest.make_factory(timeout=5)
+
     def test_writes_source_keyrings(self):
         # Stop import_images() from actually doing anything.
         self.patch(boot_resources, 'maaslog')

=== modified file 'src/provisioningserver/rpc/boot_images.py'
--- src/provisioningserver/rpc/boot_images.py	2016-04-14 19:54:36 +0000
+++ src/provisioningserver/rpc/boot_images.py	2016-05-12 17:26:13 +0000
@@ -16,9 +16,19 @@
 from provisioningserver.boot import tftppath
 from provisioningserver.config import ClusterConfiguration
 from provisioningserver.import_images import boot_resources
-from provisioningserver.utils.env import environment_variables
+from provisioningserver.rpc import getRegionClient
+from provisioningserver.rpc.region import UpdateLastImageSync
+from provisioningserver.utils.env import (
+    environment_variables,
+    get_maas_id,
+)
 from provisioningserver.utils.twisted import synchronous
+from twisted.internet.defer import (
+    fail,
+    inlineCallbacks,
+)
 from twisted.internet.threads import deferToThread
+from twisted.python import log
 
 
 CACHED_BOOT_IMAGES = None
@@ -103,22 +113,52 @@
     no_proxy_hosts += list(get_hosts_from_sources(sources))
     variables['no_proxy'] = ','.join(no_proxy_hosts)
     with environment_variables(variables):
-        boot_resources.import_images(sources)
+        imported = boot_resources.import_images(sources)
 
     # Update the boot images cache so `list_boot_images` returns the
     # correct information.
     reload_boot_images()
 
+    # Tell callers if anything happened.
+    return imported
+
 
 def import_boot_images(sources, http_proxy=None, https_proxy=None):
     """Imports the boot images from the given sources."""
     lock = concurrency.boot_images
     if not lock.locked:
         return lock.run(
-            deferToThread, _run_import, sources,
-            http_proxy=http_proxy, https_proxy=https_proxy)
+            _import_boot_images, sources, http_proxy=http_proxy,
+            https_proxy=https_proxy)
+
+
+@inlineCallbacks
+def _import_boot_images(sources, http_proxy=None, https_proxy=None):
+    """Import boot images then inform the region.
+
+    Helper for `import_boot_images`.
+    """
+    proxies = dict(http_proxy=http_proxy, https_proxy=https_proxy)
+    imported = yield deferToThread(_run_import, sources, **proxies)
+    if imported:
+        yield touch_last_image_sync_timestamp().addErrback(
+            log.err, "Failure touching last image sync timestamp.")
 
 
 def is_import_boot_images_running():
     """Return True if the import process is currently running."""
     return concurrency.boot_images.locked
+
+
+def touch_last_image_sync_timestamp():
+    """Inform the region that images have just been synchronised.
+
+    :return: :class:`Deferred` that can fail with `NoConnectionsAvailable` or
+        any exception arising from an `UpdateLastImageSync` RPC.
+    """
+    try:
+        client = getRegionClient()
+    except:
+        return fail()
+    else:
+        return client(UpdateLastImageSync, system_id=get_maas_id())

=== modified file 'src/provisioningserver/rpc/region.py'
--- src/provisioningserver/rpc/region.py	2016-04-11 16:23:26 +0000
+++ src/provisioningserver/rpc/region.py	2016-05-12 17:26:13 +0000
@@ -26,6 +26,7 @@
     "SendEvent",
     "SendEventMACAddress",
     "UpdateInterfaces",
+    "UpdateLastImageSync",
     "UpdateNodePowerState",
 ]
 
@@ -262,6 +263,20 @@
     }
 
 
+class UpdateLastImageSync(amp.Command):
+    """Update Rack Controller's Last Image Sync.
+
+    :since: 2.0
+    """
+
+    arguments = [
+        # A rack controller's system_id.
+        (b'system_id', amp.Unicode()),
+    ]
+    response = []
+    errors = []
+
+
 class UpdateNodePowerState(amp.Command):
     """Update Node Power State.
 

=== modified file 'src/provisioningserver/rpc/tests/test_boot_images.py'
--- src/provisioningserver/rpc/tests/test_boot_images.py	2016-05-06 11:41:05 +0000
+++ src/provisioningserver/rpc/tests/test_boot_images.py	2016-05-12 17:26:13 +0000
@@ -21,7 +21,10 @@
 from provisioningserver import concurrency
 from provisioningserver.boot import tftppath
 from provisioningserver.import_images import boot_resources
-from provisioningserver.rpc import boot_images
+from provisioningserver.rpc import (
+    boot_images,
+    region,
+)
 from provisioningserver.rpc.boot_images import (
     _run_import,
     fix_sources_for_cluster,
@@ -31,14 +34,23 @@
     list_boot_images,
     reload_boot_images,
 )
+from provisioningserver.rpc.region import UpdateLastImageSync
+from provisioningserver.rpc.testing import MockLiveClusterToRegionRPCFixture
 from provisioningserver.testing.config import (
     BootSourcesFixture,
     ClusterConfigurationFixture,
 )
 from provisioningserver.testing.testcase import PservTestCase
 from provisioningserver.utils.twisted import pause
-from testtools.matchers import Equals
+from testtools.matchers import (
+    Equals,
+    Is,
+)
 from twisted.internet import defer
+from twisted.internet.defer import (
+    inlineCallbacks,
+    succeed,
+)
 from twisted.internet.task import Clock
 
 
@@ -205,7 +217,7 @@
         self.patch(boot_resources, 'logger')
         self.patch(boot_resources, 'locate_config').return_value = (
             fixture.filename)
-        self.assertIsNone(_run_import(sources=[]))
+        self.assertThat(_run_import(sources=[]), Is(False))
 
     def test__run_import_sets_GPGHOME(self):
         home = factory.make_name('home')
@@ -288,6 +300,76 @@
         clock.advance(1)
         self.assertFalse(concurrency.boot_images.locked)
 
+    @inlineCallbacks
+    def test_update_last_image_sync(self):
+        get_maas_id = self.patch(boot_images, "get_maas_id")
+        get_maas_id.return_value = factory.make_string()
+        getRegionClient = self.patch(boot_images, "getRegionClient")
+        _run_import = self.patch_autospec(boot_images, '_run_import')
+        _run_import.return_value = True
+        yield boot_images._import_boot_images(sentinel.sources)
+        self.assertThat(
+            _run_import, MockCalledOnceWith(sentinel.sources, None, None))
+        self.assertThat(getRegionClient, MockCalledOnceWith())
+        self.assertThat(get_maas_id, MockCalledOnceWith())
+        client = getRegionClient.return_value
+        self.assertThat(
+            client, MockCalledOnceWith(
+                UpdateLastImageSync, system_id=get_maas_id()))
+
+    @inlineCallbacks
+    def test_update_last_image_sync_not_performed(self):
+        get_maas_id = self.patch(boot_images, "get_maas_id")
+        get_maas_id.return_value = factory.make_string()
+        getRegionClient = self.patch(boot_images, "getRegionClient")
+        _run_import = self.patch_autospec(boot_images, '_run_import')
+        _run_import.return_value = False
+        yield boot_images._import_boot_images(sentinel.sources)
+        self.assertThat(
+            _run_import, MockCalledOnceWith(sentinel.sources, None, None))
+        self.assertThat(getRegionClient, MockNotCalled())
+        self.assertThat(get_maas_id, MockNotCalled())
+
+    @inlineCallbacks
+    def test_update_last_image_sync_end_to_end(self):
+        get_maas_id = self.patch(boot_images, "get_maas_id")
+        get_maas_id.return_value = factory.make_string()
+        self.useFixture(ClusterConfigurationFixture())
+        fixture = self.useFixture(MockLiveClusterToRegionRPCFixture())
+        protocol, connecting = fixture.makeEventLoop(
+            region.UpdateLastImageSync)
+        protocol.UpdateLastImageSync.return_value = succeed({})
+        self.addCleanup((yield connecting))
+        self.patch_autospec(boot_resources, 'import_images')
+        boot_resources.import_images.return_value = True
+        sources, hosts = make_sources()
+        yield boot_images.import_boot_images(sources)
+        self.assertThat(
+            boot_resources.import_images,
+            MockCalledOnceWith(sources))
+        self.assertThat(
+            protocol.UpdateLastImageSync,
+            MockCalledOnceWith(protocol, system_id=get_maas_id()))
+
+    @inlineCallbacks
+    def test_update_last_image_sync_end_to_end_import_not_performed(self):
+        self.useFixture(ClusterConfigurationFixture())
+        fixture = self.useFixture(MockLiveClusterToRegionRPCFixture())
+        protocol, connecting = fixture.makeEventLoop(
+            region.UpdateLastImageSync)
+        protocol.UpdateLastImageSync.return_value = succeed({})
+        self.addCleanup((yield connecting))
+        self.patch_autospec(boot_resources, 'import_images')
+        boot_resources.import_images.return_value = False
+        sources, hosts = make_sources()
+        yield boot_images.import_boot_images(sources)
+        self.assertThat(
+            boot_resources.import_images,
+            MockCalledOnceWith(sources))
+        self.assertThat(
+            protocol.UpdateLastImageSync,
+            MockNotCalled())
+
 
 class TestIsImportBootImagesRunning(PservTestCase):