Merge lp:~sil2100/ubuntu-system-image/server-different_devices into lp:ubuntu-system-image/server

[Łukasz Zemczak]

Add an additional optional argument to the system-image generator to sync tarballs from different devices.

Add an additional optional argument to the system-image generator to sync tarballs from different devices.

[Barry Warsaw (barry)]

Comments inlined but otherwise LGTM.

review: Approve

[Łukasz Zemczak (sil2100)]

Thanks for the comments Barry! Let me address those quickly. One of the reasons the self.assertIsNone() method is not used as I tend to try and keep the same code-style as it was originally used. So, a copy-paste since Stephane didn't use it in the original test code. Same for the private method I exported - it was a simple copy-paste in that case.

[lp:~sil2100/ubuntu-system-image/server-different_devices]

288. By Łukasz Zemczak on 2016-03-22

Use assertIsNone instead of what was already in the test-suite.

[Barry Warsaw (barry)]

On Mar 22, 2016, at 11:33 AM, Łukasz Zemczak wrote:

>Thanks for the comments Barry! Let me address those quickly. One of the
>reasons the self.assertIsNone() method is not used as I tend to try and keep
>the same code-style as it was originally used. So, a copy-paste since
>Stephane didn't use it in the original test code. Same for the private method
>I exported - it was a simple copy-paste in that case.

Gotcha. We should probably schedule some tech-debt reduction work. Might
make a good Foundations virtual sprint topic.

=== modified file 'lib/systemimage/generators.py'
--- lib/systemimage/generators.py	2016-02-10 18:03:44 +0000
+++ lib/systemimage/generators.py	2016-03-22 12:07:40 +0000
@@ -1267,24 +1267,33 @@
     channel_name = arguments[0]
     prefix = arguments[1]
 
+    options = {}
+    if len(arguments) > 2:
+        options = unpack_arguments(arguments[2])
+
+    # We also support an optional argument to use a different source device
+    device_name = environment['device_name']
+    if 'device' in options:
+        device_name = options['device']
+
     # Run some checks
     pub = tree.Tree(conf)
     if channel_name not in pub.list_channels():
         logger.debug("Channel not in the published list: %s" % channel_name)
         return None
 
-    if (not environment['device_name'] in
+    if (device_name not in
             pub.list_channels()[channel_name]['devices']):
         logger.debug("Device not in the channel list")
         return None
 
     # Try to find the file
-    device = pub.get_device(channel_name, environment['device_name'])
+    device = pub.get_device(channel_name, device_name)
 
     full_images = sorted([image for image in device.list_images()
                           if image['type'] == "full"],
                          key=lambda image: image['version'])
-    logger.debug("List of full images founds %s" % full_images)
+    logger.debug("List of full images found %s" % full_images)
 
     # No images
     if not full_images:

=== modified file 'lib/systemimage/tests/test_generators.py'
--- lib/systemimage/tests/test_generators.py	2016-03-16 18:31:30 +0000
+++ lib/systemimage/tests/test_generators.py	2016-03-22 12:07:40 +0000
@@ -62,6 +62,20 @@
     def tearDown(self):
         shutil.rmtree(self.temp_directory)
 
+    def _publish_dummy_to_channel(self, device):
+        """Helper function used to publish a dummy image for selected device"""
+        open(os.path.join(self.config.publish_path, "file-1.tar.xz"),
+             "w+").close()
+
+        with open(os.path.join(self.config.publish_path, "file-1.json"),
+                  "w+") as fd:
+            fd.write(json.dumps({'version_detail': "abcd"}))
+
+        gpg.sign_file(self.config, "image-signing",
+                      os.path.join(self.config.publish_path, "file-1.tar.xz"))
+        device.create_image("full", 1234, "abc", ["file-1.tar.xz"],
+                            minversion=1233, bootme=True)
+
     @unittest.skipUnless(HAS_TEST_KEYS, MISSING_KEYS_WARNING)
     def test_unpack_arguments(self):
         self.assertEqual(generators.unpack_arguments("a=1,b=2"),
@@ -844,7 +858,7 @@
         self.assertEqual(
             generators.generate_file_system_image(self.config,
                                                   ['invalid', 'file'],
-                                                  {}),
+                                                  environment),
             None)
 
         # Check for device name
@@ -864,17 +878,7 @@
             None)
 
         # Publish some random stuff
-        open(os.path.join(self.config.publish_path, "file-1.tar.xz"),
-             "w+").close()
-
-        with open(os.path.join(self.config.publish_path, "file-1.json"),
-                  "w+") as fd:
-            fd.write(json.dumps({'version_detail': "abcd"}))
-
-        gpg.sign_file(self.config, "image-signing",
-                      os.path.join(self.config.publish_path, "file-1.tar.xz"))
-        self.device.create_image("full", 1234, "abc", ["file-1.tar.xz"],
-                                 minversion=1233, bootme=True)
+        self._publish_dummy_to_channel(self.device)
 
         # Invalid filename
         self.assertEqual(
@@ -891,6 +895,42 @@
             os.path.join(self.config.publish_path, "file-1.tar.xz"))
 
     @unittest.skipUnless(HAS_TEST_KEYS, MISSING_KEYS_WARNING)
+    def test_generate_file_system_image_different_device(self):
+        """Test the system-image generator for a different source device."""
+        environment = {}
+        environment['channel_name'] = "test"
+        environment['device'] = self.device
+        environment['device_name'] = "test"
+        environment['new_files'] = []
+        environment['version'] = 1234
+        environment['version_detail'] = []
+
+        self.tree.create_device("test", "source")
+        source_device = self.tree.get_device("test", "source")
+
+        # Invalid device
+        self.assertIsNone(
+            generators.generate_file(self.config, "system-image",
+                                     ['test', 'file', 'device=invalid'],
+                                     environment))
+
+        # Empty channel in correct device
+        self.assertIsNone(
+            generators.generate_file(self.config, "system-image",
+                                     ['test', 'file', 'device=source'],
+                                     environment))
+
+        # Publish some random stuff
+        self._publish_dummy_to_channel(source_device)
+
+        # Normal run
+        self.assertEqual(
+            generators.generate_file(self.config, "system-image",
+                                     ['test', 'file', 'device=source'],
+                                     environment),
+            os.path.join(self.config.publish_path, "file-1.tar.xz"))
+
+    @unittest.skipUnless(HAS_TEST_KEYS, MISSING_KEYS_WARNING)
     @mock.patch("systemimage.tools.repack_recovery_keyring")
     @mock.patch("systemimage.generators.urlretrieve")
     @mock.patch("systemimage.generators.urlopen")