Merge lp:~wesley-wiedenmeier/curtin/1598310 into lp:~curtin-dev/curtin/trunk

[Wesley Wiedenmeier]

Fix (LP: 1598310), in block.get_blockdev_sector_size, do not assume that only 1 result will be returned by _lsblock() as it will return multiple results whenever there are other devices under the device it has been called on (for example, disks with partitions).

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:401
https://server-team-jenkins.canonical.com/job/curtin-ci/295/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/generic-update-mp/292/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/curtin-ci/295/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Good catch. storage branch turning these out now?

review: Approve

[Wesley Wiedenmeier (wesley-wiedenmeier)]

Yeah, this was causing a failure in one of the whole disk format tests.

[lp:~wesley-wiedenmeier/curtin/1598310]

402. By Wesley Wiedenmeier on 2016-07-11

In block.get_blockdev_sector_size, when falling back to first entry in lsblk
return dict, cast dict_keys object to list before indexing

403. By Wesley Wiedenmeier on 2016-07-11

Add test for fallback lsblk entry selection in block.get_blockdev_sector_size

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:403
https://server-team-jenkins.canonical.com/job/curtin-ci/302/
Executed test runs:
    None: https://server-team-jenkins.canonical.com/job/generic-update-mp/299/console

Click here to trigger a rebuild:
https://server-team-jenkins.canonical.com/job/curtin-ci/302/rebuild

review: Approve (continuous-integration)

=== modified file 'curtin/block/__init__.py'
--- curtin/block/__init__.py	2016-06-10 14:49:22 +0000
+++ curtin/block/__init__.py	2016-07-11 19:21:49 +0000
@@ -423,7 +423,18 @@
     """
     info = _lsblock([devpath])
     LOG.debug('get_blockdev_sector_size: info:\n%s' % util.json_dumps(info))
-    [parent] = info
+    # (LP: 1598310) The call to _lsblock() may return multiple results.
+    # If it does, then search for a result with the correct device path.
+    # If no such device is found among the results, then fall back to previous
+    # behavior, which was taking the first of the results
+    assert len(info) > 0
+    for (k, v) in info.items():
+        if v.get('device_path') == devpath:
+            parent = k
+            break
+    else:
+        parent = list(info.keys())[0]
+
     return (int(info[parent]['LOG-SEC']), int(info[parent]['PHY-SEC']))
 
 

=== modified file 'tests/unittests/test_block.py'
--- tests/unittests/test_block.py	2016-04-04 20:12:01 +0000
+++ tests/unittests/test_block.py	2016-07-11 19:21:49 +0000
@@ -4,6 +4,8 @@
 import tempfile
 import shutil
 
+from collections import OrderedDict
+
 from curtin import util
 from curtin import block
 
@@ -39,6 +41,36 @@
         self.assertEqual(sorted(mountpoints),
                          sorted(["/mnt", "/sys"]))
 
+    @mock.patch('curtin.block._lsblock')
+    def test_get_blockdev_sector_size(self, mock_lsblk):
+        mock_lsblk.return_value = {
+            'sda':  {'LOG-SEC': '512', 'PHY-SEC': '4096',
+                     'device_path': '/dev/sda'},
+            'sda1': {'LOG-SEC': '512', 'PHY-SEC': '4096',
+                     'device_path': '/dev/sda1'},
+            'dm-0': {'LOG-SEC': '512', 'PHY-SEC': '512',
+                     'device_path': '/dev/dm-0'},
+        }
+        for (devpath, expected) in [('/dev/sda', (512, 4096)),
+                                    ('/dev/sda1', (512, 4096)),
+                                    ('/dev/dm-0', (512, 512))]:
+            res = block.get_blockdev_sector_size(devpath)
+            mock_lsblk.assert_called_with([devpath])
+            self.assertEqual(res, expected)
+
+        # test that fallback works and gives right return
+        mock_lsblk.return_value = OrderedDict()
+        mock_lsblk.return_value.update({
+            'vda': {'LOG-SEC': '4096', 'PHY-SEC': '4096',
+                    'device_path': '/dev/vda'},
+        })
+        mock_lsblk.return_value.update({
+            'vda1': {'LOG-SEC': '512', 'PHY-SEC': '512',
+                     'device_path': '/dev/vda1'},
+        })
+        res = block.get_blockdev_sector_size('/dev/vda2')
+        self.assertEqual(res, (4096, 4096))
+
     @mock.patch("curtin.block.os.path.realpath")
     @mock.patch("curtin.block.os.path.exists")
     @mock.patch("curtin.block.os.listdir")