Merge lp:~raharper/curtin/trunk.fix-lvm-over-raid into lp:~curtin-dev/curtin/trunk

[Ryan Harper]

mdadm: handle write failures to sysfs entries when stopping mdadm

mdadm_stop command tries a number of methods to stop an existing
array. Writes to sysfs fail differently, this patch catches both
IOErrors and OSErrors when writing to sysfs entries, logs and continues
with the stop operation.

Updating the LVM over RAID vmtest to run in dirty_disk mode reproduces
a failure reported and is resolved with this branch.

LP: #1708052

[Ryan Harper (raharper)]

vmtest on this branch running over the LVM over RAID test-case is here:

https://jenkins.ubuntu.com/server/view/curtin/job/curtin-vmtest-devel-amd64/25/

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

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

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

minor comments.

[lp:~raharper/curtin/trunk.fix-lvm-over-raid]

524. By Ryan Harper on 2017-08-31

Drop 'pass' in exception handling, expand logging message

[Ryan Harper (raharper)]

I updated per comments, however, I didn't log the exception stacktrace itself; this triggers exception detection in curtin's vmtest install.log parsing; it's not terribly useful since we're ignoring the error (it's an expected possibility while shutting down a block device).

Ready for review again.

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

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

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

review: Approve (continuous-integration)

[lp:~raharper/curtin/trunk.fix-lvm-over-raid]

525. By Ryan Harper on 2017-09-01

merge from trunk

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

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

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

review: Approve (continuous-integration)

[lp:~raharper/curtin/trunk.fix-lvm-over-raid]

526. By Ryan Harper on 2017-09-05

merge from trunk

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

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

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

I agree with Chad's request for the actual errors.

The only other thing that I would suggest, is that if we can determine in the exception block that this failure can be ignored, then we could/should not log it at WARN. Ie, if we tried and everything still ends up OK, then the warning is just loud noise that looks scary later.

[Ryan Harper (raharper)]

Thanks, I'll move the LOG message.

[Ryan Harper (raharper)]

> I agree with Chad's request for the actual errors.
>
> The only other thing that I would suggest, is that if we can determine in the
> exception block that this failure can be ignored, then we could/should not log
> it at WARN. Ie, if we tried and everything still ends up OK, then the warning
> is just loud noise that looks scary later.

I'll replace the WARN with debug.

[lp:~raharper/curtin/trunk.fix-lvm-over-raid]

527. By Ryan Harper on 2017-09-05

LOG outside of try/except, reduce log messages to debug (expected errors)

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

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

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

review: Approve (continuous-integration)

[lp:~raharper/curtin/trunk.fix-lvm-over-raid]

528. By Ryan Harper on 2017-09-07

Include exception as string, indicate error is non-fatal

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

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

The log messages of sysfs write failures look like this:

[ 64.954808] cloud-init[1435]: mdadm: (non-fatal) write to /sys/class/block/md2/md/sync_max failed [Errno 16] Device or resource busy
[ 65.302878] cloud-init[1435]: mdadm: (non-fatal) write to /sys/class/block/md3/md/sync_action failed [Errno 13] Permission denied: '/sys/class/block/md3/md/sync_action'
[ 65.305867] cloud-init[1435]: mdadm: (non-fatal) write to /sys/class/block/md3/md/sync_max failed [Errno 13] Permission denied: '/sys/class/block/md3/md/sync_max'

[Chad Smith (chad.smith)]

Thank you for the change here +1

review: Approve

[Scott Moser (smoser)]

This is generally what i was asking for.
It means that at some point if we ever saw "[Errno 2] No such file or directory" it might hint to something else.

review: Approve

=== modified file 'curtin/block/mdadm.py'
--- curtin/block/mdadm.py	2017-05-12 21:06:28 +0000
+++ curtin/block/mdadm.py	2017-09-07 16:10:35 +0000
@@ -273,7 +273,11 @@
             LOG.debug('%s/sync_max = %s', sync_action, val)
             if val != "idle":
                 LOG.debug("mdadm: setting array sync_action=idle")
-                util.write_file(sync_action, content="idle")
+                try:
+                    util.write_file(sync_action, content="idle")
+                except (IOError, OSError) as e:
+                    LOG.debug("mdadm: (non-fatal) write to %s failed %s",
+                              sync_action, e)
 
             # Setting the sync_{max,min} may can help prevent the array from
             # changing back to 'resync' which may prevent the array from being
@@ -283,11 +287,11 @@
             if val != "0":
                 LOG.debug("mdadm: setting array sync_{min,max}=0")
                 try:
-                    util.write_file(sync_max, content="0")
-                    util.write_file(sync_min, content="0")
-                except IOError:
-                    LOG.warning('mdadm: failed to set sync_{max,min} values')
-                    pass
+                    for sync_file in [sync_max, sync_min]:
+                        util.write_file(sync_file, content="0")
+                except (IOError, OSError) as e:
+                    LOG.debug('mdadm: (non-fatal) write to %s failed %s',
+                              sync_file, e)
 
             # one wonders why this command doesn't do any of the above itself?
             out, err = util.subp(["mdadm", "--manage", "--stop", devpath],

=== modified file 'tests/vmtests/test_mdadm_bcache.py'
--- tests/vmtests/test_mdadm_bcache.py	2017-08-02 15:46:35 +0000
+++ tests/vmtests/test_mdadm_bcache.py	2017-09-07 16:10:35 +0000
@@ -413,6 +413,9 @@
     # we have to avoid a systemd hang due to the way it handles dmcrypt
     extra_kern_args = "--- luks=no"
     active_mdadm = "4"
+    # running in dirty mode catches some race/errors with mdadm_stop
+    nr_cpus = 2
+    dirty_disks = True
     # initialize secondary disk
     extra_disks = ['5G', '5G', '5G']
     disk_to_check = [('main_disk', 1),