Merge lp:~wesley-wiedenmeier/curtin/1618429 into lp:~curtin-dev/curtin/trunk

[Wesley Wiedenmeier]

Before clear_holders runs, it starts a mdadm --assemble in order to detect mdadm devices on the system and remove them cleanly. However, if part of a mdadm device is present, but not enough to assemble it, then this may fail. Therefore, ignore any errors encountered running mdadm --assemble for device wipe purposes.

[Wesley Wiedenmeier (wesley-wiedenmeier)]

There is a build of this branch here: https://launchpad.net/~wesley-wiedenmeier/+archive/ubuntu/test/+build/10686369

[Wesley Wiedenmeier (wesley-wiedenmeier)]

There is an updated build of this branch at: https://launchpad.net/~wesley-wiedenmeier/+archive/ubuntu/test/+build/10752448

[Ryan Harper (raharper)]

Thanks!

review: Approve

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

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

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

review: Approve (continuous-integration)

=== modified file 'curtin/block/clear_holders.py'
--- curtin/block/clear_holders.py	2016-08-11 20:41:40 +0000
+++ curtin/block/clear_holders.py	2016-09-14 19:52:48 +0000
@@ -360,8 +360,16 @@
     prepare system for clear holders to be able to scan old devices
     """
     # a mdadm scan has to be started in case there is a md device that needs to
-    # be detected
-    block.mdadm.mdadm_assemble(scan=True)
+    # be detected. if the scan fails, it is either because there are no mdadm
+    # devices on the system, or because there is a mdadm device in a damaged
+    # state that could not be started. due to the nature of mdadm tools, it is
+    # difficult to know which is the case. if any errors did occur, then ignore
+    # them, since no action needs to be taken if there were no mdadm devices on
+    # the system, and in the case where there is some mdadm metadata on a disk,
+    # but there was not enough to start the array, the call to wipe_volume on
+    # all disks and partitions should be sufficient to remove the mdadm
+    # metadata
+    block.mdadm.mdadm_assemble(scan=True, ignore_errors=True)
     # the bcache module needs to be present to properly detect bcache devs
     # on some systems (precise without hwe kernel) it may not be possible to
     # lad the bcache module bcause it is not present in the kernel. if this

=== modified file 'curtin/block/mdadm.py'
--- curtin/block/mdadm.py	2016-04-14 01:45:54 +0000
+++ curtin/block/mdadm.py	2016-09-14 19:52:48 +0000
@@ -28,7 +28,7 @@
 from subprocess import CalledProcessError
 
 from curtin.block import (dev_short, dev_path, is_valid_device, sys_block_path)
-from curtin import util
+from curtin import (util, udev)
 from curtin.log import LOG
 
 NOSPARE_RAID_LEVELS = [
@@ -117,21 +117,34 @@
 #
 
 
-def mdadm_assemble(md_devname=None, devices=[], spares=[], scan=False):
+def mdadm_assemble(md_devname=None, devices=[], spares=[], scan=False,
+                   ignore_errors=False):
     # md_devname is a /dev/XXXX
     # devices is non-empty list of /dev/xxx
     # if spares is non-empt list append of /dev/xxx
     cmd = ["mdadm", "--assemble"]
     if scan:
-        cmd += ['--scan']
+        cmd += ['--scan', '-v']
     else:
         valid_mdname(md_devname)
         cmd += [md_devname, "--run"] + devices
         if spares:
             cmd += spares
 
-    util.subp(cmd, capture=True, rcs=[0, 1, 2])
-    util.subp(["udevadm", "settle"])
+    try:
+        # mdadm assemble returns 1 when no arrays are found. this might not be
+        # an error depending on the situation this function was called in, so
+        # accept a return code of 1
+        # mdadm assemble returns 2 when called on an array that is already
+        # assembled. this is not an error, so accept return code of 2
+        # all other return codes can be accepted with ignore_error set to true
+        util.subp(cmd, capture=True, rcs=[0, 1, 2])
+    except util.ProcessExecutionError:
+        LOG.warning("mdadm_assemble had unexpected return code")
+        if not ignore_errors:
+            raise
+
+    udev.udevadm_settle()
 
 
 def mdadm_create(md_devname, raidlevel, devices, spares=None, md_name=""):

=== modified file 'tests/unittests/test_block_mdadm.py'
--- tests/unittests/test_block_mdadm.py	2016-04-14 01:45:54 +0000
+++ tests/unittests/test_block_mdadm.py	2016-09-14 19:52:48 +0000
@@ -2,6 +2,7 @@
 from mock import call, patch
 from curtin.block import dev_short
 from curtin.block import mdadm
+from curtin import util
 import os
 import subprocess
 
@@ -24,34 +25,27 @@
         super(TestBlockMdadmAssemble, self).setUp()
         self.add_patch('curtin.block.mdadm.util', 'mock_util')
         self.add_patch('curtin.block.mdadm.is_valid_device', 'mock_valid')
+        self.add_patch('curtin.block.mdadm.udev', 'mock_udev')
 
         # Common mock settings
         self.mock_valid.return_value = True
         self.mock_util.lsb_release.return_value = {'codename': 'precise'}
-        self.mock_util.subp.side_effect = [
-            ("", ""),  # mdadm assemble
-            ("", ""),  # udevadm settle
-        ]
+        self.mock_util.subp.return_value = ('', '')
 
     def test_mdadm_assemble_scan(self):
         mdadm.mdadm_assemble(scan=True)
-        expected_calls = [
-            call(["mdadm", "--assemble", "--scan"], capture=True,
-                 rcs=[0, 1, 2]),
-            call(["udevadm", "settle"]),
-        ]
-        self.mock_util.subp.assert_has_calls(expected_calls)
+        self.mock_util.subp.assert_called_with(
+            ["mdadm", "--assemble", "--scan", "-v"], capture=True,
+            rcs=[0, 1, 2])
+        self.assertTrue(self.mock_udev.udevadm_settle.called)
 
     def test_mdadm_assemble_md_devname(self):
         md_devname = "/dev/md0"
         mdadm.mdadm_assemble(md_devname=md_devname)
-
-        expected_calls = [
-            call(["mdadm", "--assemble", md_devname, "--run"], capture=True,
-                 rcs=[0, 1, 2]),
-            call(["udevadm", "settle"]),
-        ]
-        self.mock_util.subp.assert_has_calls(expected_calls)
+        self.mock_util.subp.assert_called_with(
+            ["mdadm", "--assemble", md_devname, "--run"], capture=True,
+            rcs=[0, 1, 2])
+        self.assertTrue(self.mock_udev.udevadm_settle.called)
 
     def test_mdadm_assemble_md_devname_short(self):
         with self.assertRaises(ValueError):
@@ -67,12 +61,23 @@
         md_devname = "/dev/md0"
         devices = ["/dev/vdc1", "/dev/vdd1"]
         mdadm.mdadm_assemble(md_devname=md_devname, devices=devices)
-        expected_calls = [
-            call(["mdadm", "--assemble", md_devname, "--run"] + devices,
-                 capture=True, rcs=[0, 1, 2]),
-            call(["udevadm", "settle"]),
-        ]
-        self.mock_util.subp.assert_has_calls(expected_calls)
+        self.mock_util.subp.assert_called_with(
+            ["mdadm", "--assemble", md_devname, "--run"] + devices,
+            capture=True, rcs=[0, 1, 2])
+        self.assertTrue(self.mock_udev.udevadm_settle.called)
+
+    def test_mdadm_assemble_exec_error(self):
+
+        def _raise_pexec_error(*args, **kwargs):
+            raise util.ProcessExecutionError()
+
+        self.mock_util.ProcessExecutionError = util.ProcessExecutionError
+        self.mock_util.subp.side_effect = _raise_pexec_error
+        with self.assertRaises(util.ProcessExecutionError):
+            mdadm.mdadm_assemble(scan=True, ignore_errors=False)
+        self.mock_util.subp.assert_called_with(
+            ['mdadm', '--assemble', '--scan', '-v'], capture=True,
+            rcs=[0, 1, 2])
 
 
 class TestBlockMdadmCreate(MdadmTestBase):

=== modified file 'tests/unittests/test_clear_holders.py'
--- tests/unittests/test_clear_holders.py	2016-08-11 20:30:27 +0000
+++ tests/unittests/test_clear_holders.py	2016-09-14 19:52:48 +0000
@@ -319,3 +319,11 @@
             mock_gen_holders_tree.assert_called_with(device)
         mock_gen_holders_tree.return_value = self.example_holders_trees[1][1]
         clear_holders.assert_clear(device)
+
+    @mock.patch('curtin.block.clear_holders.block.mdadm')
+    @mock.patch('curtin.block.clear_holders.util')
+    def test_start_clear_holders_deps(self, mock_util, mock_mdadm):
+        clear_holders.start_clear_holders_deps()
+        mock_mdadm.mdadm_assemble.assert_called_with(
+            scan=True, ignore_errors=True)
+        mock_util.subp.assert_called_with(['modprobe', 'bcache'], rcs=[0, 1])