Merge lp:~allenap/maas/repo-dumper-do-not-suppress-io-errors--2.1 into lp:maas/2.1

[Gavin Panella]

Backport r5635 and r5636 from trunk: Don't suppress IOError in RepoDumper.sync.

This was allowing garbage to reach cache_boot_sources.update_cache. This most often resulted in BootSourceCache being emptied in full or in part.

MP for trunk: https://code.launchpad.net/~allenap/maas/repo-dumper-do-not-suppress-io-errors/+merge/314524

[Mike Pontillo (mpontillo)]

LGTM.

review: Approve

=== modified file 'src/provisioningserver/import_images/download_descriptions.py'
--- src/provisioningserver/import_images/download_descriptions.py	2016-09-30 13:56:04 +0000
+++ src/provisioningserver/import_images/download_descriptions.py	2017-01-26 09:43:15 +0000
@@ -117,6 +117,13 @@
             maaslog.warning(
                 "I/O error while syncing boot images. If this problem "
                 "persists, verify network connectivity and disk usage.")
+            # This MUST NOT suppress the I/O error because callers use
+            # self.boot_images_dict as the "return" value. Suppressing
+            # exceptions here gives the caller no reason to doubt that
+            # boot_images_dict is not utter garbage and so pass it up the
+            # stack where it is then acted upon, to empty out BootSourceCache
+            # for example. True story.
+            raise
 
 
 def value_passes_filter_list(filter_list, property_value):

=== modified file 'src/provisioningserver/import_images/tests/test_download_descriptions.py'
--- src/provisioningserver/import_images/tests/test_download_descriptions.py	2016-09-30 19:08:51 +0000
+++ src/provisioningserver/import_images/tests/test_download_descriptions.py	2017-01-26 09:43:15 +0000
@@ -393,20 +393,18 @@
         ]
         self.assertItemsEqual(image_specs, list(boot_images_dict.mapping))
 
-    def test_sync_does_not_propagate_ioerror(self):
+    def test_sync_does_propagate_ioerror(self):
+        io_error = factory.make_exception_type(bases=(IOError,))
+
         mock_sync = self.patch(download_descriptions.BasicMirrorWriter, "sync")
-        mock_sync.side_effect = IOError()
+        mock_sync.side_effect = io_error()
 
         boot_images_dict = BootImageMapping()
         dumper = RepoDumper(boot_images_dict)
 
         with FakeLogger("maas.import-images", level=logging.INFO) as maaslog:
-            # What we're testing here is that sync() doesn't raise IOError...
-            dumper.sync(sentinel.reader, sentinel.path)
-            # ... but we'll validate that we properly called the [mock]
-            # superclass method, and logged something, as well.
-            self.assertThat(
-                mock_sync, MockCalledOnceWith(sentinel.reader, sentinel.path))
+            self.assertRaises(
+                io_error, dumper.sync, sentinel.reader, sentinel.path)
             self.assertDocTestMatches(
                 "...error...syncing boot images...", maaslog.output)