Merge ~raharper/curtin:mwhudson-vmtest-reuse-half-a-raid into curtin:master

[Ryan Harper]

Handle partial raid on partitions

Consider disks and partitions when checking for holders that
need removal before they can be used. Additionally when shutting
down a raid, if it is inactive or broken, writes to the /dev/mdX
device will fail which is expected, don't exit instead, log a
non-fatal error and continue with shutting down the raid.

With these fixes TestReuseRAIDMemberPartition vmtests now pass.

LP: #1835091

[Ryan Harper (raharper)]

This needs a full vmtest run to ensure we're not regressing things with the changes.

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

FAILED: Continuous integration, rev:e6c4ef797f7e08f0017f5841f75c92462f832eb6
https://jenkins.ubuntu.com/server/job/curtin-ci/1335/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/1335
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/1335
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/1335/console
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/1335/console

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

review: Needs Fixing (continuous-integration)

[Michael Hudson-Doyle (mwhudson)]

LGTM assuming the vmtests are good.

20033d3... by Ryan Harper on 2019-08-01

Discover how to spell Discovering

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

FAILED: Continuous integration, rev:3cc20609171cc922f5926618674020970ec23bf5
https://jenkins.ubuntu.com/server/job/curtin-ci/1336/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/1336
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/1336
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/1336
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/1336/console

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

review: Needs Fixing (continuous-integration)

05c2aaf... by Ryan Harper on 2019-08-01

Refactor to handle not present devices

In some scenarios, the partitions in the config _may_ be present
but we need to handle if they are not. Additionally, if they were
present when we initially start clearing devices, they wont be present
after the clear so assert_clear needs to adapt to allow that.

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

FAILED: Continuous integration, rev:add83fef6059ddcd41f0f14264225d8f9530f6dd
https://jenkins.ubuntu.com/server/job/curtin-ci/1337/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/1337
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/1337
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/1337
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/1337/console

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

review: Needs Fixing (continuous-integration)

22d9742... by Ryan Harper on 2019-08-01

Don't forget to return the paths

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

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

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

review: Approve (continuous-integration)

[Dan Watkins (oddbloke)]

This looks really good, thanks! I have one inline comment about exception handling that I'd like resolved, but after that I'll be +1.

review: Needs Information

fbc9696... by Ryan Harper on 2019-08-02

Update comment around exception handling for partial raid shutdown

[Dan Watkins (oddbloke)]

LGTM, thanks for the comment!

review: Approve

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

PASSED: Continuous integration, rev:5dc70459632377f8e56d2d1062181560c3c5db9c
https://jenkins.ubuntu.com/server/job/curtin-ci/3666/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/3666/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/3666/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/3666/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/3666/

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

spun up a vmtest, let's see what it says

https://jenkins.ubuntu.com/server/job/curtin-vmtest-devel-debug/184/console

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

PASSED: Continuous integration, rev:6e2836fb4f2564aee63e88d9cfccc55af558015b
https://jenkins.ubuntu.com/server/job/curtin-ci/3677/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/3677/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/3677/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/3677/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=torkoal/3677/

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

This needed a rebase after the change to btrfs-tools package names. After rebase, I'll resubmit vmtest run.

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

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

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

review: Approve (continuous-integration)

diff --git a/curtin/block/clear_holders.py b/curtin/block/clear_holders.py
index 4a099cd..bcc16e6 100644
--- a/curtin/block/clear_holders.py
+++ b/curtin/block/clear_holders.py
@@ -166,14 +166,27 @@ def shutdown_mdadm(device):
 
     blockdev = block.sysfs_to_devpath(device)
 
-    LOG.info('Wiping superblock on raid device: %s', device)
-    _wipe_superblock(blockdev, exclusive=False)
-
+    LOG.info('Discovering raid devices and spares for %s', device)
     md_devs = (
         mdadm.md_get_devices_list(blockdev) +
         mdadm.md_get_spares_list(blockdev))
     mdadm.set_sync_action(blockdev, action="idle")
     mdadm.set_sync_action(blockdev, action="frozen")
+
+    LOG.info('Wiping superblock on raid device: %s', device)
+    try:
+        _wipe_superblock(blockdev, exclusive=False)
+    except ValueError as e:
+        # if the array is not functional, writes to the device may fail
+        # and _wipe_superblock will raise ValueError for short writes
+        # which happens on inactive raid volumes.  In that case we
+        # shouldn't give up yet as we still want to disassemble
+        # array and wipe members.  Other errors such as IOError or OSError
+        # are unwelcome and will stop deployment.
+        LOG.debug('Non-fatal error writing to array device %s, '
+                  'proceeding with shutdown: %s', blockdev, e)
+
+    LOG.info('Removing raid array members: %s', md_devs)
     for mddev in md_devs:
         try:
             mdadm.fail_device(blockdev, mddev)
@@ -534,8 +547,10 @@ def assert_clear(base_paths):
     valid = ('disk', 'partition')
     if not isinstance(base_paths, (list, tuple)):
         base_paths = [base_paths]
-    base_paths = [block.sys_block_path(path) for path in base_paths]
-    for holders_tree in [gen_holders_tree(p) for p in base_paths]:
+    base_paths = [block.sys_block_path(path, strict=False)
+                  for path in base_paths]
+    for holders_tree in [gen_holders_tree(p)
+                         for p in base_paths if os.path.exists(p)]:
         if any(holder_type not in valid and path not in base_paths
                for (holder_type, path) in get_holder_types(holders_tree)):
             raise OSError('Storage not clear, remaining:\n{}'
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index a9110a9..37b93dd 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -67,7 +67,7 @@ def block_meta(args):
     if devices is None:
         devices = []
         if 'storage' in cfg:
-            devices = get_disk_paths_from_storage_config(
+            devices = get_device_paths_from_storage_config(
                 extract_storage_ordered_dict(cfg))
         if len(devices) == 0:
             devices = cfg.get('block-meta', {}).get('devices', [])
@@ -1535,20 +1535,24 @@ def zfs_handler(info, storage_config):
             util.write_file(state['fstab'], fstab_entry, omode='a')
 
 
-def get_disk_paths_from_storage_config(storage_config):
-    """Returns a list of disk paths in a storage config filtering out
-       preserved or disks which do not have wipe configuration.
+def get_device_paths_from_storage_config(storage_config):
+    """Returns a list of device paths in a storage config filtering out
+       preserved or devices which do not have wipe configuration.
 
     :param: storage_config: Ordered dict of storage configation
     """
-    def get_path_if_present(disk, config):
-        return get_path_to_storage_volume(disk, config)
-
-    return [get_path_if_present(k, storage_config)
-            for (k, v) in storage_config.items()
-            if v.get('type') == 'disk' and
-            config.value_as_boolean(v.get('wipe')) and
-            not config.value_as_boolean(v.get('preserve'))]
+    dpaths = []
+    for (k, v) in storage_config.items():
+        if v.get('type') in ['disk', 'partition']:
+            if config.value_as_boolean(v.get('wipe')):
+                if config.value_as_boolean(v.get('preserve')):
+                    continue
+                try:
+                    dpaths.append(
+                        get_path_to_storage_volume(k, storage_config))
+                except Exception:
+                    pass
+    return dpaths
 
 
 def zfsroot_update_storage_config(storage_config):
diff --git a/examples/tests/reuse-raid-member-partition.yaml b/examples/tests/reuse-raid-member-partition.yaml
new file mode 100644
index 0000000..3fe2d83
--- /dev/null
+++ b/examples/tests/reuse-raid-member-partition.yaml
@@ -0,0 +1,73 @@
+showtrace: true
+
+# The point of this test is to test installing to a partition that used to
+# be a RAID member where the other disks that used to be part of the
+# RAID are not present (the scenario that the disk was just grabbed
+# out of a pile of previously used disks and shoved into a server).
+
+# So what it does is to create a RAID0 out of two partition from two
+# disks, stop the RAID, wipe one of the disks and then install to the
+# other, reusing the partition that was part of the RAID.
+
+bucket:
+  - &setup |
+    parted /dev/disk/by-id/virtio-disk-a --script -- \
+        mklabel gpt              \
+        mkpart primary 1GiB 2GiB \
+        mkpart primary 2GiB 9GiB
+    parted /dev/disk/by-id/virtio-disk-b --script -- \
+        mklabel gpt              \
+        mkpart primary 2GiB 9GiB
+    udevadm settle
+    mdadm --create --metadata 1.2 --level 0 -n 2 /dev/md1 --assume-clean \
+        /dev/disk/by-id/virtio-disk-a-part2 /dev/disk/by-id/virtio-disk-b-part1
+    udevadm settle
+    mdadm --stop /dev/md1
+    udevadm settle
+    mdadm --zero-superblock /dev/disk/by-id/virtio-disk-b-part1
+    wipefs -a /dev/disk/by-id/virtio-disk-b
+    udevadm settle
+
+early_commands:
+  00-setup-raid: [sh, -exuc, *setup]
+
+storage:
+  config:
+  - type: disk
+    id: id_disk0
+    serial: disk-a
+    ptable: gpt
+    preserve: true
+  - type: disk
+    id: id_disk1
+    serial: disk-b
+  - type: partition
+    id: id_disk0_part1
+    preserve: true
+    device: id_disk0
+    flag: boot
+    number: 1
+    size: 1G
+  - type: partition
+    id: id_disk0_part2
+    preserve: true
+    device: id_disk0
+    number: 2
+    size: 7G
+  - type: format
+    id: id_efi_format
+    volume: id_disk0_part1
+    fstype: fat32
+  - type: format
+    id: id_root_format
+    volume: id_disk0_part2
+    fstype: ext4
+  - type: mount
+    device: id_root_format
+    id: id_root_mount
+    path: /
+  - type: mount
+    id: id_efi_mount
+    device: id_efi_format
+    path: /boot/efi
+  version: 1
diff --git a/examples/tests/reuse-raid-member-wipe.yaml b/examples/tests/reuse-raid-member-wipe.yaml
new file mode 100644
index 0000000..84a2686
--- /dev/null
+++ b/examples/tests/reuse-raid-member-wipe.yaml
@@ -0,0 +1,71 @@
+showtrace: true
+
+# The point of this test is to test installing to a disk that contains
+# a partition that used to be a RAID member where the other parts of
+# the RAID are not present (the scenario is that the disk was just
+# grabbed out of a pile of previously used disks and shoved into a
+# server).
+
+# So what it does is to create a RAID0 out of partitions on two disks,
+# stop the RAID, wipe the superblock on one of them and then install
+# to the other using a standard partitioning scheme.
+
+bucket:
+  - &setup |
+    parted /dev/disk/by-id/virtio-disk-a --script -- \
+        mklabel gpt              \
+        mkpart primary 1GiB 9GiB
+    parted /dev/disk/by-id/virtio-disk-b --script -- \
+        mklabel gpt              \
+        mkpart primary 1GiB 9GiB
+    udevadm settle
+    mdadm --create --metadata 1.2 --level 0 -n 2 /dev/md1 --assume-clean \
+        /dev/disk/by-id/virtio-disk-a-part1 /dev/disk/by-id/virtio-disk-b-part1
+    udevadm settle
+    mdadm --stop /dev/md1
+    udevadm settle
+    mdadm --zero-superblock /dev/disk/by-id/virtio-disk-b-part1
+    wipefs -a /dev/disk/by-id/virtio-disk-b
+    udevadm settle
+
+early_commands:
+  00-setup-raid: [sh, -exuc, *setup]
+
+storage:
+  config:
+  - type: disk
+    id: id_disk0
+    serial: disk-a
+    ptable: gpt
+    wipe: superblock-recursive
+  - type: disk
+    id: id_disk1
+    serial: disk-b
+  - type: partition
+    id: id_disk0_part1
+    device: id_disk0
+    flag: boot
+    number: 1
+    size: 512M
+  - type: partition
+    id: id_disk0_part2
+    device: id_disk0
+    number: 2
+    size: 8G
+  - type: format
+    id: id_efi_format
+    volume: id_disk0_part1
+    fstype: fat32
+  - type: format
+    id: id_root_format
+    volume: id_disk0_part2
+    fstype: ext4
+  - type: mount
+    device: id_root_format
+    id: id_root_mount
+    path: /
+  - type: mount
+    id: id_efi_mount
+    device: id_efi_format
+    path: /boot/efi
+  version: 1
diff --git a/tests/unittests/test_clear_holders.py b/tests/unittests/test_clear_holders.py
index e4d8f8d..906ae06 100644
--- a/tests/unittests/test_clear_holders.py
+++ b/tests/unittests/test_clear_holders.py
@@ -566,11 +566,17 @@ class TestClearHolders(CiTestCase):
         for tree, result in test_trees_and_results:
             self.assertEqual(clear_holders.get_holder_types(tree), result)
 
+    @mock.patch('curtin.block.clear_holders.os.path.exists')
     @mock.patch('curtin.block.clear_holders.block.sys_block_path')
     @mock.patch('curtin.block.clear_holders.gen_holders_tree')
-    def test_assert_clear(self, mock_gen_holders_tree, mock_syspath):
+    def test_assert_clear(self, mock_gen_holders_tree, mock_syspath, m_ospe):
+        def my_sysblock(p, strict=False):
+            return '/sys/class/block/%s' % os.path.basename(p)
+
         mock_gen_holders_tree.return_value = self.example_holders_trees[0][0]
-        mock_syspath.side_effect = lambda x: x
+        mock_syspath.side_effect = my_sysblock
+        # os.path.exists set to True to include all devices in the call list
+        m_ospe.return_value = True
         device = self.test_blockdev
         with self.assertRaises(OSError):
             clear_holders.assert_clear(device)
diff --git a/tests/vmtests/test_reuse_raid_member.py b/tests/vmtests/test_reuse_raid_member.py
new file mode 100644
index 0000000..3052c9c
--- /dev/null
+++ b/tests/vmtests/test_reuse_raid_member.py
@@ -0,0 +1,62 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+from . import VMBaseClass
+from .releases import base_vm_classes as relbase
+
+
+class TestReuseRAIDMember(VMBaseClass):
+    """ Curtin can install to a RAID member if other members are missing. """
+    conf_file = "examples/tests/reuse-raid-member-wipe.yaml"
+    extra_disks = ['10G', '10G']
+    uefi = True
+
+    def test_simple(self):
+        pass
+
+
+class TestReuseRAIDMemberPartition(VMBaseClass):
+    """ Curtin can install to a RAID member if other members are missing. """
+    conf_file = "examples/tests/reuse-raid-member-wipe.yaml"
+    extra_disks = ['10G', '10G']
+    uefi = True
+
+    def test_simple(self):
+        pass
+
+
+class BionicTestReuseRAIDMember(relbase.bionic, TestReuseRAIDMember):
+    __test__ = True
+
+
+class CosmicTestReuseRAIDMember(relbase.cosmic, TestReuseRAIDMember):
+    __test__ = True
+
+
+class DiscoTestReuseRAIDMember(relbase.disco, TestReuseRAIDMember):
+    __test__ = True
+
+
+class EoanTestReuseRAIDMember(relbase.eoan, TestReuseRAIDMember):
+    __test__ = True
+
+
+class BionicTestReuseRAIDMemberPartition(
+        relbase.bionic, TestReuseRAIDMemberPartition):
+    __test__ = True
+
+
+class CosmicTestReuseRAIDMemberPartition(
+        relbase.cosmic, TestReuseRAIDMemberPartition):
+    __test__ = True
+
+
+class DiscoTestReuseRAIDMemberPartition(
+        relbase.disco, TestReuseRAIDMemberPartition):
+    __test__ = True
+
+
+class EoanTestReuseRAIDMemberPartition(
+        relbase.eoan, TestReuseRAIDMemberPartition):
+    __test__ = True
+
+# vi: ts=4 expandtab syntax=python