Merge ~raharper/curtin:fix/clear-holders-bcache-partitions-lp1811117 into curtin:master

[Ryan Harper]

clear-holders: handle FileNotFound when probing for bcache device slaves

In some cases, when detecting if a bcache has any slave devices which
need removal the path to the sysfs directory may not be found; this is
non-fatal; it indicates that the existing device has been removed by the
kernel. Allow clear-holders to tolerate this failure.

Updated curtin.block.partition_kname to indicate that bcache partitions
have 'p' in the value. This is needed for the vmtest to recreate bcache
partitions.

LP: #1811117

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

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

vmtest run going here:

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

[Chad Smith (chad.smith)]

review: Needs Information

[Ryan Harper (raharper)]

Thanks, I'll fix most changes. Details inline.

[Chad Smith (chad.smith)]

LGTM +1

review: Approve

339def7... by Ryan Harper on 2019-01-15

Fix typo and drop 'error' string from debug log message.

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

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

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

review: Approve (continuous-integration)

3b33671... by Ryan Harper on 2019-01-16

bcache-partitions: test XenialHWE and accept failure on 4.4 kernels.

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

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

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

review: Approve (continuous-integration)

f451985... by Ryan Harper on 2019-01-16

vmtests: add expected_failure decorator for known broken Xenial test.

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

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Testers have confirmed this branch addresses the issue. Landing.

diff --git a/curtin/block/__init__.py b/curtin/block/__init__.py
index 490c268..eb56674 100644
--- a/curtin/block/__init__.py
+++ b/curtin/block/__init__.py
@@ -103,7 +103,7 @@ def partition_kname(disk_kname, partition_number):
     """
     Add number to disk_kname prepending a 'p' if needed
     """
-    for dev_type in ['nvme', 'mmcblk', 'cciss', 'mpath', 'dm', 'md']:
+    for dev_type in ['bcache', 'nvme', 'mmcblk', 'cciss', 'mpath', 'dm', 'md']:
         if disk_kname.startswith(dev_type):
             partition_number = "p%s" % partition_number
             break
diff --git a/curtin/block/clear_holders.py b/curtin/block/clear_holders.py
index a05c9ca..05d756c 100644
--- a/curtin/block/clear_holders.py
+++ b/curtin/block/clear_holders.py
@@ -130,8 +130,12 @@ def shutdown_bcache(device):
         return
 
     # get slaves [vdb1, vdc], allow for slaves to not have bcache dir
-    slave_paths = [get_bcache_sys_path(k, strict=False) for k in
-                   os.listdir(os.path.join(device, 'slaves'))]
+    try:
+        slave_paths = [get_bcache_sys_path(k, strict=False) for k in
+                       os.listdir(os.path.join(device, 'slaves'))]
+    except util.FileMissingError as e:
+        LOG.debug('Transient race, bcache slave path not found: %s', e)
+        slave_paths = []
 
     # stop cacheset if it exists
     bcache_cache_sysfs = get_bcache_using_dev(device, strict=False)
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index 8decab5..4c3a434 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -692,13 +692,14 @@ def partition_handler(info, storage_config):
     else:
         raise ValueError("parent partition has invalid partition table")
 
+    # ensure partition exists
+    part_path = block.dev_path(block.partition_kname(disk_kname, partnumber))
+    block.rescan_block_devices([disk])
+    udevadm_settle(exists=part_path)
+
     # wipe the created partition if needed, superblocks have already been wiped
     wipe_mode = info.get('wipe', 'superblock')
     if wipe_mode != 'superblock':
-        part_path = block.dev_path(block.partition_kname(disk_kname,
-                                                         partnumber))
-        block.rescan_block_devices([disk])
-        udevadm_settle(exists=part_path)
         LOG.debug('Wiping partition %s mode=%s', part_path, wipe_mode)
         block.wipe_volume(part_path, mode=wipe_mode, exclusive=False)
 
diff --git a/curtin/commands/unmount.py b/curtin/commands/unmount.py
index defaae4..602fcb2 100644
--- a/curtin/commands/unmount.py
+++ b/curtin/commands/unmount.py
@@ -6,11 +6,6 @@ from . import populate_one_subcmd
 
 import os
 
-try:
-    FileMissingError = FileNotFoundError
-except NameError:
-    FileMissingError = IOError
-
 
 def unmount_main(args):
     """
@@ -22,7 +17,7 @@ def unmount_main(args):
 
     if not os.path.exists(args.target):
         msg = "Cannot unmount target path %s: it does not exist" % args.target
-        raise FileMissingError(msg)
+        raise util.FileMissingError(msg)
 
     LOG.info("Unmounting devices from target path: %s", args.target)
     recursive_mode = not args.disable_recursive_mounts
diff --git a/curtin/util.py b/curtin/util.py
index 238d7c5..35e004f 100644
--- a/curtin/util.py
+++ b/curtin/util.py
@@ -37,6 +37,11 @@ except NameError:
     # python3 does not have a long type.
     numeric_types = (int, float)
 
+try:
+    FileMissingError = FileNotFoundError
+except NameError:
+    FileMissingError = IOError
+
 from . import paths
 from .log import LOG, log_call
 
diff --git a/examples/tests/bcache-partitions.yaml b/examples/tests/bcache-partitions.yaml
new file mode 100644
index 0000000..20ccddc
--- /dev/null
+++ b/examples/tests/bcache-partitions.yaml
@@ -0,0 +1,97 @@
+showtrace: true
+
+storage:
+  config:
+  - id: id_rotary0
+    type: disk
+    name: rotary0
+    serial: disk-a
+    ptable: msdos
+    wipe: superblock
+    grub_device: true
+  - id: id_ssd0
+    type: disk
+    name: ssd0
+    serial: disk-b
+    wipe: superblock
+  - id: id_rotary1
+    type: disk
+    name: rotary1
+    serial: disk-c
+    ptable: gpt
+    wipe: superblock
+  - id: id_rotary0_part1
+    type: partition
+    name: rotary0-part1
+    device: id_rotary0
+    number: 1
+    offset: 1M
+    size: 999M
+    wipe: superblock
+  - id: id_rotary0_part2
+    type: partition
+    name: rotary0-part2
+    device: id_rotary0
+    number: 2
+    size: 9G
+    wipe: superblock
+  - id: id_bcache0
+    type: bcache
+    name: bcache0
+    backing_device: id_rotary0_part2
+    cache_device: id_ssd0
+    cache_mode: writeback
+  - id: id_bcache1
+    type: bcache
+    name: bcache1
+    backing_device: id_rotary1
+    cache_device: id_ssd0
+    cache_mode: writeback
+  - id: id_bcache1_disk
+    type: disk
+    path: /dev/bcache1
+    ptable: gpt
+  - id: id_bcache1_part1
+    type: partition
+    device: id_bcache1_disk
+    number: 1
+    offset: 1M
+    size: 1G
+    wipe: superblock
+  - id: id_bcache1_part2
+    type: partition
+    device: id_bcache1_disk
+    number: 2
+    size: 1G
+    wipe: superblock
+  - id: id_bcache1_part3
+    type: partition
+    device: id_bcache1_disk
+    number: 3
+    size: 1G
+    wipe: superblock
+  - id: id_bcache1_part4
+    type: partition
+    device: id_bcache1_disk
+    number: 4
+    size: 1G
+    wipe: superblock
+  - id: bootfs
+    type: format
+    label: boot-fs
+    volume: id_rotary0_part1
+    fstype: ext4
+  - id: rootfs
+    type: format
+    label: root-fs
+    volume: id_bcache0
+    fstype: ext4
+  - id: rootfs_mount
+    type: mount
+    path: /
+    device: rootfs
+  - id: bootfs_mount
+    type: mount
+    path: /boot
+    device: bootfs
+  version: 1
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 45b9906..acb0d29 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -146,6 +146,8 @@ class TestBlockMeta(CiTestCase):
                        'mock_block_get_volume_uuid')
         self.add_patch('curtin.block.zero_file_at_offsets',
                        'mock_block_zero_file')
+        self.add_patch('curtin.block.rescan_block_devices',
+                       'mock_block_rescan')
 
         self.target = "my_target"
         self.config = {
@@ -225,9 +227,9 @@ class TestBlockMeta(CiTestCase):
         part_offset = 2048 * 512
         self.mock_block_zero_file.assert_called_with(disk_kname, [part_offset],
                                                      exclusive=False)
-        self.mock_subp.assert_called_with(['parted', disk_kname, '--script',
-                                           'mkpart', 'primary', '2048s',
-                                           '1001471s'], capture=True)
+        self.mock_subp.assert_has_calls(
+            [call(['parted', disk_kname, '--script',
+                   'mkpart', 'primary', '2048s', '1001471s'], capture=True)])
 
     @patch('curtin.util.write_file')
     def test_mount_handler_defaults(self, mock_write_file):
diff --git a/tests/vmtests/test_bcache_basic.py b/tests/vmtests/test_bcache_basic.py
index a62fb17..b6044d4 100644
--- a/tests/vmtests/test_bcache_basic.py
+++ b/tests/vmtests/test_bcache_basic.py
@@ -1,6 +1,6 @@
 # This file is part of curtin. See LICENSE file for copyright and license info.
 
-from . import VMBaseClass
+from . import VMBaseClass, skip_if_flag
 from .releases import base_vm_classes as relbase
 
 import textwrap
@@ -24,10 +24,12 @@ class TestBcacheBasic(VMBaseClass):
         exit 0
         """)]
 
+    @skip_if_flag('expected_failure')
     def test_bcache_output_files_exist(self):
         self.output_files_exist(["bcache_super_vda2", "bcache_ls",
                                  "bcache_cache_mode"])
 
+    @skip_if_flag('expected_failure')
     def test_bcache_status(self):
         bcache_cset_uuid = None
         for line in self.load_collect_file("bcache_super_vda2").splitlines():
@@ -37,9 +39,11 @@ class TestBcacheBasic(VMBaseClass):
         self.assertTrue(bcache_cset_uuid in
                         self.load_collect_file("bcache_ls").splitlines())
 
+    @skip_if_flag('expected_failure')
     def test_bcache_cachemode(self):
         self.check_file_regex("bcache_cache_mode", r"\[writeback\]")
 
+    @skip_if_flag('expected_failure')
     def test_proc_cmdline_root_by_uuid(self):
         self.check_file_regex("proc_cmdline", r"root=UUID=")
 
diff --git a/tests/vmtests/test_bcache_partitions.py b/tests/vmtests/test_bcache_partitions.py
new file mode 100644
index 0000000..ff86b51
--- /dev/null
+++ b/tests/vmtests/test_bcache_partitions.py
@@ -0,0 +1,35 @@
+# This file is part of curtin. See LICENSE file for copyright and license info.
+
+from .releases import base_vm_classes as relbase
+from .test_bcache_basic import TestBcacheBasic
+
+
+class TestBcachePartitions(TestBcacheBasic):
+    conf_file = "examples/tests/bcache-partitions.yaml"
+    dirty_disks = True
+    nr_cpus = 2
+    extra_disks = ['10G', '10G']
+
+
+class XenialTestBcachePartitions(relbase.xenial, TestBcachePartitions):
+    # Xenial 4.4 kernel does not support bcache partitions
+    expected_failure = True
+    __test__ = True
+
+
+class XenialHWETestBcachePartitions(relbase.xenial_hwe, TestBcachePartitions):
+    __test__ = True
+
+
+class BionicTestBcachePartitions(relbase.bionic, TestBcachePartitions):
+    __test__ = True
+
+
+class CosmicTestBcachePartitions(relbase.cosmic, TestBcachePartitions):
+    __test__ = True
+
+
+class DiscoTestBcachePartitions(relbase.disco, TestBcachePartitions):
+    __test__ = True
+
+# vi: ts=4 expandtab syntax=python