Merge ~raharper/curtin:fix/no-by-id-grub-debconf into curtin:master

Proposed by Ryan Harper
Status: Merged
Approved by: Chad Smith
Approved revision: 56361669c20a0002b68390f2b2b8a11e1dd563ad
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~raharper/curtin:fix/no-by-id-grub-debconf
Merge into: curtin:master
Diff against target: 36 lines (+14/-0)
2 files modified
curtin/block/__init__.py (+2/-0)
tests/unittests/test_block.py (+12/-0)
Reviewer Review Type Date Requested Status
Chad Smith Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+389670@code.launchpad.net

Commit message

block: disk_to_byid_path handle missing /dev/disk/by-id directory

If a system has disks without any persistent indentifier (like serial)
then udev won't create any /dev/disk/by-id symlinks. This can happen
in VMs which do not provide serial numbers by default (e.g. virtio).

If /dev/disk/by-id did not exist then os.listdir on that path would
raise a FileNotFoundException and prevented disk_to_byid_path from
returning None (or the correct path).

This is fixed by checking if the target prefix exists and if not
return an empty dict.

LP: #1876258

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

+1! LGTM!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py
2index a422264..10b8b9e 100644
3--- a/curtin/block/__init__.py
4+++ b/curtin/block/__init__.py
5@@ -853,6 +853,8 @@ def _get_dev_disk_by_prefix(prefix):
6 '/dev/sda1': '/dev/disk/<prefix>/virtio-aaaa-part1',
7 }
8 """
9+ if not os.path.exists(prefix):
10+ return {}
11 return {
12 os.path.realpath(bypfx): bypfx
13 for bypfx in [os.path.join(prefix, path)
14diff --git a/tests/unittests/test_block.py b/tests/unittests/test_block.py
15index c62c153..78e331d 100644
16--- a/tests/unittests/test_block.py
17+++ b/tests/unittests/test_block.py
18@@ -179,6 +179,18 @@ class TestBlock(CiTestCase):
19 byid_path = block.disk_to_byid_path('/dev/sdb')
20 self.assertEqual(mapping.get('/dev/sdb'), byid_path)
21
22+ @mock.patch("curtin.block.os.path.exists")
23+ def test__get_dev_disk_by_prefix_returns_empty_dict(self, m_exists):
24+ """ _get_disk_by_prefix returns empty dict prefix dir does not exit """
25+ m_exists.return_value = False
26+ self.assertEqual({}, block._get_dev_disk_by_prefix("/dev/disk/by-id"))
27+
28+ @mock.patch("curtin.block.os.path.exists")
29+ def test_disk_to_byid_returns_none_if_disk_byid_missing(self, m_exists):
30+ """ disk_to_byid path returns None if /dev/disk/by-id is missing """
31+ m_exists.return_value = False
32+ self.assertEqual(None, block.disk_to_byid_path('/dev/sdb'))
33+
34
35 class TestSysBlockPath(CiTestCase):
36 @mock.patch("curtin.block.get_blockdev_for_partition")

Subscribers

People subscribed via source and target branches