curtin-dev team mailing list archive

[James Falcon <james.falcon@xxxxxxxxxxxxx>]

James Falcon has proposed merging ~falcojr/curtin:fix-mdraid-name into curtin:master.

Commit message:
Fix mdraid name creates broken configuration
    
Allow to pass a raid device name that works for either /dev/mdN format or /dev/md/NAME format, with or without prefix.
    
LP: #1803933


Requested reviews:
  Ryan Harper (raharper)
Related bugs:
  Bug #1803933 in curtin: "mdraid name creates broken configuration"
  https://bugs.launchpad.net/curtin/+bug/1803933

For more details, see:
https://code.launchpad.net/~falcojr/curtin/+git/curtin/+merge/384209
-- 
Your team curtin developers is subscribed to branch curtin:master.

diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py
index 35e3a64..a422264 100644
--- a/curtin/block/__init__.py
+++ b/curtin/block/__init__.py
@@ -1,5 +1,5 @@
 # This file is part of curtin. See LICENSE file for copyright and license info.
-
+import re
 from contextlib import contextmanager
 import errno
 import itertools
@@ -67,6 +67,19 @@ def dev_path(devname):
         return '/dev/' + devname
 
 
+def md_path(mdname):
+    """ Convert device name to path in /dev/md """
+    full_mdname = dev_path(mdname)
+    if full_mdname.startswith('/dev/md/'):
+        return full_mdname
+    elif re.match(r'/dev/md\d+$', full_mdname):
+        return full_mdname
+    elif '/' in mdname:
+        raise ValueError("Invalid RAID device name: {}".format(mdname))
+    else:
+        return '/dev/md/{}'.format(mdname)
+
+
 def path_to_kname(path):
     """
     converts a path in /dev or a path in /sys/block to the device kname,
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index f2bb8da..6665d5f 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -1483,7 +1483,7 @@ def raid_handler(info, storage_config):
     devices = info.get('devices')
     raidlevel = info.get('raidlevel')
     spare_devices = info.get('spare_devices')
-    md_devname = block.dev_path(info.get('name'))
+    md_devname = block.md_path(info.get('name'))
     preserve = config.value_as_boolean(info.get('preserve'))
     if not devices:
         raise ValueError("devices for raid must be specified")
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 4cc9299..8789b82 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -1784,7 +1784,6 @@ class TestRaidHandler(CiTestCase):
         self.add_patch(basepath + 'util', 'm_util')
         self.add_patch(basepath + 'make_dname', 'm_dname')
         self.add_patch(basepath + 'mdadm', 'm_mdadm')
-        self.add_patch(basepath + 'block', 'm_block')
         self.add_patch(basepath + 'udevadm_settle', 'm_uset')
 
         self.target = "my_target"
@@ -1850,12 +1849,35 @@ class TestRaidHandler(CiTestCase):
             block_meta.extract_storage_ordered_dict(self.config))
         self.m_util.load_command_environment.return_value = {'fstab': None}
 
+    def test_md_name(self):
+        input_to_result = [
+            ('md1', '/dev/md1'),
+            ('os-raid1', '/dev/md/os-raid1'),
+            ('md/os-raid1', '/dev/md/os-raid1'),
+            ('/dev/md1', '/dev/md1'),
+            ('/dev/md/os-raid1', '/dev/md/os-raid1'),
+            ('bad/path', ValueError)
+        ]
+        for index, test in enumerate(input_to_result):
+            param, expected = test
+            self.storage_config['mddevice']['name'] = param
+            try:
+                block_meta.raid_handler(self.storage_config['mddevice'],
+                                        self.storage_config)
+            except expected:
+                continue
+
+            self.assertEqual(
+                expected,
+                self.m_mdadm.mdadm_create.call_args_list[index].args[0]
+            )
+
+
     def test_raid_handler(self):
         """ raid_handler creates raid device. """
         devices = [self.random_string(), self.random_string(),
                    self.random_string()]
         md_devname = '/dev/' + self.storage_config['mddevice']['name']
-        self.m_block.dev_path.return_value = '/dev/md0'
         self.m_getpath.side_effect = iter(devices)
         block_meta.raid_handler(self.storage_config['mddevice'],
                                 self.storage_config)
@@ -1868,7 +1890,6 @@ class TestRaidHandler(CiTestCase):
 
         devices = [self.random_string(), self.random_string(),
                    self.random_string()]
-        self.m_block.dev_path.return_value = '/dev/md0'
         self.m_getpath.side_effect = iter(devices)
         m_verify.return_value = True
         self.storage_config['mddevice']['preserve'] = True
@@ -1882,7 +1903,6 @@ class TestRaidHandler(CiTestCase):
         devices = [self.random_string(), self.random_string(),
                    self.random_string()]
         md_devname = '/dev/' + self.storage_config['mddevice']['name']
-        self.m_block.dev_path.return_value = '/dev/md0'
         self.m_getpath.side_effect = iter(devices)
         self.m_mdadm.md_check.return_value = True
         self.storage_config['mddevice']['preserve'] = True
@@ -1898,7 +1918,6 @@ class TestRaidHandler(CiTestCase):
         devices = [self.random_string(), self.random_string(),
                    self.random_string()]
         md_devname = '/dev/' + self.storage_config['mddevice']['name']
-        self.m_block.dev_path.return_value = '/dev/md0'
         self.m_getpath.side_effect = iter(devices)
         self.m_mdadm.md_check.side_effect = iter([False, True])
         self.storage_config['mddevice']['preserve'] = True
@@ -1916,7 +1935,6 @@ class TestRaidHandler(CiTestCase):
         devices = [self.random_string(), self.random_string(),
                    self.random_string()]
         md_devname = '/dev/' + self.storage_config['mddevice']['name']
-        self.m_block.dev_path.return_value = '/dev/md0'
         self.m_getpath.side_effect = iter(devices)
         self.m_mdadm.md_check.side_effect = iter([False, False])
         self.storage_config['mddevice']['preserve'] = True