Merge ~falcojr/curtin:fix-mdraid-name into curtin:master

[James Falcon]

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

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

FAILED: Continuous integration, rev:f538c89eda53e771f168e58b77b5152208c549d7
https://jenkins.ubuntu.com/server/job/curtin-ci/115/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/115/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/115/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/115/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/115/

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

review: Needs Fixing (continuous-integration)

[Ryan Harper (raharper)]

This looks good! I think we can update one or more of our raid vmtests config to use the extended name; and we'll likely want to add a vmtest *unittest* to verify we find the device in /dev/md/XXXX

See examples/tests/raid5boot.yaml and tests/vmtests/test_mdadm_bcache.py:TestRaid5bootAbs

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

FAILED: Continuous integration, rev:aab45a3af8e9a8246974e9c449d8729163d9eeec
https://jenkins.ubuntu.com/server/job/curtin-ci/136/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/136/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/136/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/136/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/136/

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:75935187bf24855022cf0090678a81dc97acc5f6
https://jenkins.ubuntu.com/server/job/curtin-ci/137/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/137/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/137/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/137/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/137/

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:51ccfd450bb035113c5e94afd5402fe46c6c51c7
https://jenkins.ubuntu.com/server/job/curtin-ci/138/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/138/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/138/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/138/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/138/

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:0ecec5423086100acf5368b7c60380752684dd7a
https://jenkins.ubuntu.com/server/job/curtin-ci/139/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/139/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/139/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/139/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/139/

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:eb3b5d2e4da02cffb95545d33b70e9e94db793f2
https://jenkins.ubuntu.com/server/job/curtin-ci/140/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/140/
    ABORTED: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/140/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/140/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/140/

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:4481197790b68cc2105b0c2892c7f8f1b410123e
https://jenkins.ubuntu.com/server/job/curtin-ci/141/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/141/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/141/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/141/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/141/

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

review: Approve (continuous-integration)

[James Falcon (falcojr)]

This is ready for re-review.

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

PASSED: Continuous integration, rev:086ff18785b6fc04c6e599f2bf18a5add1acad70
https://jenkins.ubuntu.com/server/job/curtin-ci/142/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/142/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/142/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/142/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/142/

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:feb769baef03cf6dd8d6953bc971f7e51dd7d9e6
https://jenkins.ubuntu.com/server/job/curtin-ci/143/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/143/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/143/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/143/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/143/

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Thanks! This looks good. I verified the Raid5Boot test passes in vmtest.

review: Approve

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

Commit message lints:
- Line #2 has 44 too many characters. Line starts with: "Allow to pass a raid"...

review: Needs Fixing

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

review: Approve (continuous-integration)

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 ff0f2e9..4bc7da2 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -502,7 +502,7 @@ def get_path_to_storage_volume(volume, storage_config):
     elif vol.get('type') == "raid":
         # For raid partitions, block device is at /dev/mdX
         name = vol.get('name')
-        volume_path = os.path.join("/dev", name)
+        volume_path = block.md_path(name)
 
     elif vol.get('type') == "bcache":
         # For bcache setups, the only reliable way to determine the name of the
@@ -1485,7 +1485,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/examples/tests/raid5boot.yaml b/examples/tests/raid5boot.yaml
index b1df21d..d8afe7f 100644
--- a/examples/tests/raid5boot.yaml
+++ b/examples/tests/raid5boot.yaml
@@ -42,7 +42,7 @@ storage:
        size: 3GB
        device: sdc
      - id: mddevice
-       name: md0
+       name: os-raid1
        type: raid
        raidlevel: 5
        devices:
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index b768cdc..74a14a7 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -1779,6 +1779,7 @@ class TestRaidHandler(CiTestCase):
     def setUp(self):
         super(TestRaidHandler, self).setUp()
 
+        orig_md_path = block_meta.block.md_path
         basepath = 'curtin.commands.block_meta.'
         self.add_patch(basepath + 'get_path_to_storage_volume', 'm_getpath')
         self.add_patch(basepath + 'util', 'm_util')
@@ -1787,6 +1788,10 @@ class TestRaidHandler(CiTestCase):
         self.add_patch(basepath + 'block', 'm_block')
         self.add_patch(basepath + 'udevadm_settle', 'm_uset')
 
+        # The behavior of this function is being directly tested in
+        # these tests, so we can't mock it
+        self.m_block.md_path = orig_md_path
+
         self.target = "my_target"
         self.config = {
             'storage': {
@@ -1850,12 +1855,40 @@ 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 ValueError:
+                if param in ['bad/path']:
+                    continue
+                else:
+                    raise
+
+            actual = self.m_mdadm.mdadm_create.call_args_list[index][0][0]
+            self.assertEqual(
+                expected,
+                actual,
+                "Expected {} to result in mdadm being called with {}. "
+                "mdadm instead called with {}".format(param, expected, actual)
+            )
+
     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 +1901,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 +1914,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 +1929,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 +1946,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
diff --git a/tests/vmtests/__init__.py b/tests/vmtests/__init__.py
index adfcd24..9b24d36 100644
--- a/tests/vmtests/__init__.py
+++ b/tests/vmtests/__init__.py
@@ -1761,6 +1761,19 @@ class VMBaseClass(TestCase):
         self.assertIsNotNone(kname)
         return kname
 
+    def _mdname_to_kname(self, mdname):
+        # extract kname from /dev/md/ on /dev/<kname>
+        # parsing ls -al output on /dev/md/*:
+        # lrwxrwxrwx 1 root root 8 May 28 16:26 /dev/md/os-raid1 -> ../md127
+        ls_dev_md = self.load_collect_file("ls_al_dev_md")
+        knames = [os.path.basename(line.split()[-1])
+                  for line in ls_dev_md.split('\n')
+                  if mdname in line]
+        self.assertEqual(len(knames), 1)
+        kname = knames.pop()
+        self.assertIsNotNone(kname)
+        return kname
+
     def _kname_to_bypath(self, kname):
         # extract path from /dev/disk/by-path on /dev/<kname>
         # parsing ls -al output on /dev/disk/by-path
diff --git a/tests/vmtests/test_mdadm_bcache.py b/tests/vmtests/test_mdadm_bcache.py
index 8e250cc..53eaa5e 100644
--- a/tests/vmtests/test_mdadm_bcache.py
+++ b/tests/vmtests/test_mdadm_bcache.py
@@ -26,6 +26,7 @@ class TestMdadmAbs(VMBaseClass):
         ls -al /dev/bcache* > lsal_dev_bcache_star
         ls -al /dev/bcache/by-uuid/ | cat >ls_al_bcache_byuuid
         ls -al /dev/bcache/by-label/ | cat >ls_al_bcache_bylabel
+        ls -al /dev/md/* | cat >ls_al_dev_md
 
         exit 0
         """)]
@@ -359,11 +360,14 @@ class TestRaid5bootAbs(TestMdadmAbs):
                      ('main_disk', 2),
                      ('second_disk', 1),
                      ('third_disk', 1),
-                     ('md0', 0)]
+                     ('os-raid1', 0)]
 
     def get_fstab_expected(self):
+        kname = self._mdname_to_kname('os-raid1')
         return [
-            (self._kname_to_uuid_devpath('md-uuid', 'md0'), '/', 'defaults'),
+            (self._kname_to_uuid_devpath('md-uuid', kname),
+             '/',
+             'defaults'),
         ]