Merge ~raharper/curtin:fix/block-discover-no-mpath-name into curtin:master

Proposed by Ryan Harper
Status: Merged
Approved by: Paride Legovini
Approved revision: 84cce55f100b5cf5fb3fb1054b322d351719aa79
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~raharper/curtin:fix/block-discover-no-mpath-name
Merge into: curtin:master
Diff against target: 117 lines (+66/-4)
3 files modified
curtin/storage_config.py (+5/-4)
examples/tests/vmtest_defaults.yaml (+15/-0)
tests/unittests/test_storage_config.py (+46/-0)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Chad Smith Approve
Review via email: mp+379904@code.launchpad.net

Commit message

block-discover: skip 'multipath' key in blockdevice if mpath name is None

Avoid emitting invalid storage-config yaml by not including a 'multipath'
key in block device output without a valid (not None) value for the key.

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) :
Revision history for this message
Chad Smith (chad.smith) wrote :

Quick question on mpath_name values.

I'm wondering if we need to filter out falseish values like '' too.

Revision history for this message
Ryan Harper (raharper) wrote :

Here's the failure path we're fixing:

When curtin is installing multipath scenarios, it will separate the multipath device and select one path on which to create partitions, etc, operating on it as a disk.

When we run block-discover, probert dumps info on the disks and any multipath devices present.

In our case, multipath has been disabled, so our multipath probe data is empty. The disks that were part of multipath have a marker in udev, like so:

"DM_MULTIPATH_DEVICE_PATH": "1",
"ID_FS_TYPE": "mpath_member",

This causes the storage_config generator code to look for a multipath map name like this:

        if blockdev_data.get('DM_MULTIPATH_DEVICE_PATH') == "1":
            mpath_name = self.get_mpath_name(blockdev_data)
            entry['multipath'] = mpath_name

And get_mpath_name does this:

    def get_mpath_name(self, blockdev):
        mpath_data = self.probe_data.get('multipath')
        if not mpath_data:
            return None

And our multipath probe data looks like this:

        "multipath": {},

This is why we get None in the get_mpath_name() return value.

Now, here's what it would look like if multipath were active:

"multipath": {
            "maps": [
                {
                    "multipath": "0QEMU_QEMU_HARDDISK_IPR-0_1234567890",
                    "paths": "2",
                    "sysfs": "dm-0"
                }
            ]
        },

Now, to your question, let's say that for some reason the 'multipath' key is present, but is empty string.

This is possible as probert is not validating the output; it's directly from parsing multipath show maps output.

So, it sounds like, we should not emit multipath key for None or none-ish values.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:79e829295b3eb52451d50bdb2ef69cd8ac05e6da
https://jenkins.ubuntu.com/server/job/curtin-ci/5/
Executed test runs:
    None: https://jenkins.ubuntu.com/server/job/admin-lp-git-vote/2084/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/5//rebuild

review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

+1 Thanks for catching this and the update here to handle/filter other potential problematic cases

review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:3f995fb9822b5e988d540321efde7d5043a75f55
https://jenkins.ubuntu.com/server/job/curtin-ci/6/
Executed test runs:
    None: https://jenkins.ubuntu.com/server/job/admin-lp-git-vote/2085/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/6//rebuild

review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

Let's wait for CI to say yes.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) :
review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) :
review: Approve (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/storage_config.py b/curtin/storage_config.py
2index 545b50c..abc5e4b 100644
3--- a/curtin/storage_config.py
4+++ b/curtin/storage_config.py
5@@ -437,9 +437,9 @@ class ProbertParser(object):
6 if blockdev['DEVTYPE'] == 'partition':
7 bd_name = self.partition_parent_devname(blockdev)
8 bd_name = os.path.basename(bd_name)
9- for path in mpath_data['paths']:
10- if bd_name == path['device']:
11- rv = path['multipath']
12+ for path in mpath_data.get('paths', []):
13+ if bd_name == path.get('device'):
14+ rv = path.get('multipath')
15 return rv
16
17 def find_mpath_member(self, blockdev):
18@@ -744,7 +744,8 @@ class BlockdevParser(ProbertParser):
19 }
20 if blockdev_data.get('DM_MULTIPATH_DEVICE_PATH') == "1":
21 mpath_name = self.get_mpath_name(blockdev_data)
22- entry['multipath'] = mpath_name
23+ if mpath_name:
24+ entry['multipath'] = mpath_name
25
26 # default disks to gpt
27 if entry['type'] == 'disk':
28diff --git a/examples/tests/vmtest_defaults.yaml b/examples/tests/vmtest_defaults.yaml
29index 797fe6c..e562b08 100644
30--- a/examples/tests/vmtest_defaults.yaml
31+++ b/examples/tests/vmtest_defaults.yaml
32@@ -19,6 +19,17 @@ _persist_journal:
33 }
34 exit 0
35
36+_install_probert:
37+ - &install_probert |
38+ command -v apt && {
39+ # bionic probert doesn't work with block-discover
40+ if [ "`lsb_release -sc`" = "bionic" ]; then
41+ exit 0;
42+ fi
43+ apt-get -qy install probert &>/dev/null || { echo "No probert available"; }
44+ }
45+ exit 0
46+
47 # this runs curtin block-discover and stores the result
48 # in the target system root dir
49 _block_discover:
50@@ -51,6 +62,10 @@ _block_discover:
51 exit 0
52
53
54+early_commands:
55+ # use bash to allow capturing all apt errors when probert isn't available
56+ 01_install_probert: ['bash', '-c', *install_probert]
57+
58 late_commands:
59 01_vmtest_pollinate: ['curtin', 'in-target', '--', 'sh', '-c', *pvmtest]
60 02_persist_journal: ['curtin', 'in-target', '--', 'sh', '-c', *persist_journal]
61diff --git a/tests/unittests/test_storage_config.py b/tests/unittests/test_storage_config.py
62index da83fec..8663ba5 100644
63--- a/tests/unittests/test_storage_config.py
64+++ b/tests/unittests/test_storage_config.py
65@@ -419,6 +419,52 @@ class TestBlockdevParser(CiTestCase):
66 'number': 2}
67 self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
68
69+ def test_blockdev_skips_multipath_entry_if_no_multipath_data(self):
70+ self.probe_data = _get_data('probert_storage_multipath.json')
71+ del self.probe_data['multipath']
72+ self.bdevp = BlockdevParser(self.probe_data)
73+ blockdev = self.bdevp.blockdev_data['/dev/sda2']
74+ expected_dict = {
75+ 'flag': 'linux',
76+ 'id': 'partition-sda2',
77+ 'offset': 2097152,
78+ 'size': 10734272512,
79+ 'type': 'partition',
80+ 'device': 'disk-sda',
81+ 'number': 2}
82+ self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
83+
84+ def test_blockdev_skips_multipath_entry_if_bad_multipath_data(self):
85+ self.probe_data = _get_data('probert_storage_multipath.json')
86+ for path in self.probe_data['multipath']['paths']:
87+ path['multipath'] = ''
88+ self.bdevp = BlockdevParser(self.probe_data)
89+ blockdev = self.bdevp.blockdev_data['/dev/sda2']
90+ expected_dict = {
91+ 'flag': 'linux',
92+ 'id': 'partition-sda2',
93+ 'offset': 2097152,
94+ 'size': 10734272512,
95+ 'type': 'partition',
96+ 'device': 'disk-sda',
97+ 'number': 2}
98+ self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
99+
100+ def test_blockdev_skips_multipath_entry_if_no_mp_paths(self):
101+ self.probe_data = _get_data('probert_storage_multipath.json')
102+ del self.probe_data['multipath']['paths']
103+ self.bdevp = BlockdevParser(self.probe_data)
104+ blockdev = self.bdevp.blockdev_data['/dev/sda2']
105+ expected_dict = {
106+ 'flag': 'linux',
107+ 'id': 'partition-sda2',
108+ 'offset': 2097152,
109+ 'size': 10734272512,
110+ 'type': 'partition',
111+ 'device': 'disk-sda',
112+ 'number': 2}
113+ self.assertDictEqual(expected_dict, self.bdevp.asdict(blockdev))
114+
115 def test_blockdev_finds_multipath_id_from_dm_uuid(self):
116 self.probe_data = _get_data('probert_storage_zlp6.json')
117 self.bdevp = BlockdevParser(self.probe_data)

Subscribers

People subscribed via source and target branches