Merge lp:~raharper/curtin/trunk.iscsi-stacked into lp:~curtin-dev/curtin/trunk

[Ryan Harper]

Fix stacked storage configurations with iSCSI

Currently, we fail to correctly handle more complex storage configurations where one or more members of a LVM or MD configuration are on iSCSI. We only check for disks and partitions if they are on iSCSI -- instead, in the mount_handler, check if the to-be-mounted volume (i.e., in the case of a disk or partition) is itself on iSCSI, or if any of the "slaves" of the kname for the to-be-mounted volume (i.e, in the case of LVM or MD) are on iSCSI.

LP: #1683910

[Ryan Harper (raharper)]

I've pulled Nish's branch, refactored how we determine slave knames, in particular adding support for arbitrary level of nested block devices (vs. just one level deep) which allows us to find an iscsi device under a bcache used in a raid device with lvm's on top, etc...

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

PASSED: Continuous integration, rev:498
https://jenkins.ubuntu.com/server/job/curtin-ci/502/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/502
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/502
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/502
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/502

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

review: Approve (continuous-integration)

[lp:~raharper/curtin/trunk.iscsi-stacked]

499. By Ryan Harper on 2017-05-15

merge from trunk

500. By Ryan Harper on 2017-05-15

Add some verbose debugging for kname slaves, restructure stacked iscsi tests

501. By Ryan Harper on 2017-05-15

Unmount the devices before unplugging iscsi devices

502. By Ryan Harper on 2017-05-15

Check for _netdev within each of the fstab lines

503. By Ryan Harper on 2017-05-15

block: slaves_kname: Handle present but empty slaves dir

504. By Ryan Harper on 2017-05-15

vmtests: update stacked iscsi testfile configuration, add tunable

505. By Ryan Harper on 2017-05-15

Fix lint

506. By Ryan Harper on 2017-05-15

Try appending collect_scripts instead of clobbering

507. By Ryan Harper on 2017-05-15

Must manually build scripts from parent classes

508. By Ryan Harper on 2017-05-15

Rework syntax

509. By Ryan Harper on 2017-05-15

Fix up collect scripts for iscsi subclassed tests

510. By Ryan Harper on 2017-05-15

Refactor lvm_iscsi, use fs uuids so we can match fstab, skiptest for precise,trusty dname

511. By Ryan Harper on 2017-05-15

Fix vg name in lvm_iscsi test

512. By Ryan Harper on 2017-05-15

Drop verbose slave kname debugging

[Ryan Harper (raharper)]

After failing VMTest run, fix up a few issues and refactor the vmtests.

1) Make sure we run iscsi disconnect *after* install unmounts the target

This is the other critical fix for using iscsi devices.

2) Fix an issue with obtaining slave knames when a device has a 'slaves' sysfs entry but it is empty.

3) Update the iscsi vmtests to check for '_netdev' in fstab

4) Derive the lvm and mdadm iscsi tests from respective parent classes so we confirm that those devices are built as expected as well as ensuring the use of iscsi block devices at the bottom layer works.

Started new vmtest run of iscsi tests:

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

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

PASSED: Continuous integration, rev:512
https://jenkins.ubuntu.com/server/job/curtin-ci/507/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/507
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/507
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/507
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/507

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

All tests look good, the one failure is ArtfulIscsi which won't pass till jenkins does it's nightly sync-images.

[Scott Moser (smoser)]

This looks really good. I'll approve, you are welcome to fix what you want and not what you dont.
I do think the unit tests are more implementation dependent than they really should be.

review: Approve

[Ryan Harper (raharper)]

Thanks for the review, will push an update and retest on jenkins.

[lp:~raharper/curtin/trunk.iscsi-stacked]

513. By Ryan Harper on 2017-05-18

Switch to append

514. By Ryan Harper on 2017-05-18

Change Exception message on invalid input

515. By Ryan Harper on 2017-05-19

vmtests: modify output_files_{exist,dont_exist} to check all of the input

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

PASSED: Continuous integration, rev:515
https://jenkins.ubuntu.com/server/job/curtin-ci/513/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/513
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/513
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/513
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=vm-i386/513

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

review: Approve (continuous-integration)

=== modified file 'curtin/block/__init__.py'
--- curtin/block/__init__.py	2017-05-04 17:23:42 +0000
+++ curtin/block/__init__.py	2017-05-19 00:57:33 +0000
@@ -176,6 +176,34 @@
     return holders
 
 
+def get_device_slave_knames(device):
+    """
+    Find the underlying knames of a given device by walking sysfs
+    recursively.
+
+    Returns a list of knames
+    """
+    slave_knames = []
+    slaves_dir_path = os.path.join(sys_block_path(device), 'slaves')
+
+    # if we find a 'slaves' dir, recurse and check
+    # the underlying devices
+    if os.path.exists(slaves_dir_path):
+        slaves = os.listdir(slaves_dir_path)
+        if len(slaves) > 0:
+            for slave_kname in slaves:
+                slave_knames.extend(get_device_slave_knames(slave_kname))
+        else:
+            slave_knames.append(path_to_kname(device))
+
+        return slave_knames
+    else:
+        # if a device has no 'slaves' attribute then
+        # we've found the underlying device, return
+        # the kname of the device
+        return [path_to_kname(device)]
+
+
 def _shlex_split(str_in):
     # shlex.split takes a string
     # but in python2 if input here is a unicode, encode it to a string.

=== modified file 'curtin/block/iscsi.py'
--- curtin/block/iscsi.py	2017-04-04 17:58:52 +0000
+++ curtin/block/iscsi.py	2017-05-19 00:57:33 +0000
@@ -27,6 +27,9 @@
 import shutil
 
 from curtin import (util, udev)
+from curtin.block import (get_device_slave_knames,
+                          path_to_kname)
+
 from curtin.log import LOG
 
 _ISCSI_DISKS = {}
@@ -272,6 +275,23 @@
     return False
 
 
+def volpath_is_iscsi(volume_path):
+    """ Determine if the volume_path's kname is backed by iSCSI.
+        Recursively check volume_path's slave devices as well in
+        case volume_path is a stacked block device (like LVM/MD)
+
+        returns a boolean
+    """
+    if not volume_path:
+        raise ValueError("Invalid input for volume_path: '%s'", volume_path)
+
+    volume_path_slaves = get_device_slave_knames(volume_path)
+    LOG.debug('volume_path=%s found slaves: %s', volume_path,
+              volume_path_slaves)
+    knames = [path_to_kname(volume_path)] + volume_path_slaves
+    return any([kname_is_iscsi(kname) for kname in knames])
+
+
 class IscsiDisk(object):
     # Per Debian bug 804162, the iscsi specifier looks like
     # TARGETSPEC=host:proto:port:lun:targetname

=== modified file 'curtin/commands/block_meta.py'
--- curtin/commands/block_meta.py	2017-04-12 01:38:04 +0000
+++ curtin/commands/block_meta.py	2017-05-19 00:57:33 +0000
@@ -640,6 +640,19 @@
         # Mount volume
         util.subp(['mount', volume_path, mount_point])
 
+        path = "/%s" % path
+
+        options = ["defaults"]
+        # If the volume_path's kname is backed by iSCSI or (in the case of
+        # LVM/DM) if any of its slaves are backed by iSCSI, then we need to
+        # append _netdev to the fstab line
+        if iscsi.volpath_is_iscsi(volume_path):
+            LOG.debug("Marking volume_path:%s as '_netdev'", volume_path)
+            options.append("_netdev")
+    else:
+        path = "none"
+        options = ["sw"]
+
     # Add volume to fstab
     if state['fstab']:
         with open(state['fstab'], "a") as fp:
@@ -649,34 +662,13 @@
             if len(uuid) > 0:
                 location = "UUID=%s" % uuid
 
-            if filesystem.get('fstype') == "swap":
-                path = "none"
-                options = "sw"
-            else:
-                path = "/%s" % path
-                if volume.get('type') == "partition":
-                    disk_block_path = get_path_to_storage_volume(
-                        volume.get('device'), storage_config)
-                    disk_kname = block.path_to_kname(disk_block_path)
-                    if iscsi.kname_is_iscsi(disk_kname):
-                        options = "_netdev"
-                    else:
-                        options = "defaults"
-                elif volume.get('type') == "disk":
-                    disk_kname = block.path_to_kname(location)
-                    if iscsi.kname_is_iscsi(disk_kname):
-                        options = "_netdev"
-                    else:
-                        options = "defaults"
-                else:
-                    options = "defaults"
-
             if filesystem.get('fstype') in ["fat", "fat12", "fat16", "fat32",
                                             "fat64"]:
                 fstype = "vfat"
             else:
                 fstype = filesystem.get('fstype')
-            fp.write("%s %s %s %s 0 0\n" % (location, path, fstype, options))
+            fp.write("%s %s %s %s 0 0\n" % (location, path, fstype,
+                                            ",".join(options)))
     else:
         LOG.info("fstab not in environment, so not writing")
 

=== modified file 'curtin/commands/install.py'
--- curtin/commands/install.py	2017-04-20 15:57:21 +0000
+++ curtin/commands/install.py	2017-05-19 00:57:33 +0000
@@ -452,11 +452,11 @@
     finally:
         log_target_path = instcfg.get('save_install_log',
                                       '/root/curtin-install.log')
-        # need to do some processing on iscsi disks to disconnect?
-        iscsi.disconnect_target_disks(workingd.target)
         if log_target_path:
             copy_install_log(logfile, workingd.target, log_target_path)
         util.do_umount(workingd.target, recursive=True)
+        # need to do some processing on iscsi disks to disconnect?
+        iscsi.disconnect_target_disks(workingd.target)
         shutil.rmtree(workingd.top)
 
     apply_power_state(cfg.get('power_state'))

=== added file 'examples/tests/lvm_iscsi.yaml'
--- examples/tests/lvm_iscsi.yaml	1970-01-01 00:00:00 +0000
+++ examples/tests/lvm_iscsi.yaml	2017-05-19 00:57:33 +0000
@@ -0,0 +1,175 @@
+storage:
+   version: 1
+   config:
+      - id: vdb
+        type: disk
+        ptable: msdos
+        model: QEMU HARDDISK
+        path: /dev/vdb
+        name: main_disk
+        wipe: superblock
+        grub_device: true
+      - id: vdb1
+        type: partition
+        number: 1
+        size: 3GB
+        device: vdb
+        flag: boot
+      - id: vdb2
+        type: partition
+        number: 2
+        size: 1GB
+        device: vdb
+      - id: vdb1_root
+        type: format
+        fstype: ext4
+        volume: vdb1
+      - id: vdb2_home
+        type: format
+        fstype: ext4
+        volume: vdb2
+      - id: vdb1_mount
+        type: mount
+        path: /
+        device: vdb1_root
+      - id: vdb2_mount
+        type: mount
+        path: /home
+        device: vdb2_home
+      - id: sda
+        type: disk
+        path: iscsi:__RFC4173__
+        name: iscsi_disk1
+        ptable: msdos
+        wipe: superblock
+      - id: sda_extended
+        type: partition
+        size: 5G
+        flag: extended
+        device: sda
+      - id: sda1
+        type: partition
+        size: 2G
+        flag: logical
+        device: sda
+      - id: sda2
+        type: partition
+        size: 3G
+        flag: logical
+        device: sda
+      - id: volgroup1
+        name: vg1
+        type: lvm_volgroup
+        devices:
+            - sda1
+            - sda2
+      - id: lvmpart1
+        name: lv1
+        size: 1G
+        type: lvm_partition
+        volgroup: volgroup1
+      - id: lvmpart2
+        name: lv2
+        type: lvm_partition
+        volgroup: volgroup1
+      - id: lv1_fs
+        name: storage
+        type: format
+        fstype: ext4
+        volume: lvmpart1
+        uuid: 6de56115-9500-424b-8151-221b270ec708
+      - id: lv2_fs
+        name: storage
+        type: format
+        fstype: ext3
+        volume: lvmpart2
+        uuid: 9604e4c4-e5ae-40dd-ab1f-940de6b59047
+      - id: lv1_mount
+        type: mount
+        path: /mnt/iscsi1
+        device: lv1_fs
+      - id: lv2_mount
+        type: mount
+        path: /mnt/iscsi2
+        device: lv2_fs
+      - id: sdb
+        type: disk
+        path: iscsi:__RFC4173__
+        name: iscsi_disk2
+        ptable: msdos
+        wipe: superblock
+      - id: sdb_extended
+        type: partition
+        size: 4G
+        flag: extended
+        device: sdb
+      - id: sdb1
+        type: partition
+        size: 2G
+        flag: logical
+        device: sdb
+      - id: sdb2
+        type: partition
+        size: 2G
+        flag: logical
+        device: sdb
+      - id: volgroup2
+        name: vg2
+        type: lvm_volgroup
+        devices:
+            - sdb1
+            - sdb2
+      - id: lvmpart3
+        name: lv3
+        size: 1G
+        type: lvm_partition
+        volgroup: volgroup2
+      - id: lvmpart4
+        name: lv4
+        type: lvm_partition
+        volgroup: volgroup2
+      - id: lv3_fs
+        name: storage
+        type: format
+        fstype: ext4
+        volume: lvmpart3
+        uuid: 18bec31c-09a8-4a02-91c6-e9bf6efb6fad
+      - id: lv4_fs
+        name: storage
+        type: format
+        fstype: ext3
+        volume: lvmpart4
+        uuid: a98f706b-b064-4682-8eb2-6c2c1284060c
+      - id: lv3_mount
+        type: mount
+        path: /mnt/iscsi3
+        device: lv3_fs
+      - id: lv4_mount
+        type: mount
+        path: /mnt/iscsi4
+        device: lv4_fs
+network:
+   version: 1
+   config:
+      - type: physical
+        name: interface0
+        mac_address: "52:54:00:12:34:00"
+        subnets:
+           - type: dhcp
+write_files:
+   f1:
+      path: /mnt/iscsi1/testfile
+      content: "test1"
+      permissions: 0777
+   f2:
+      path: /mnt/iscsi2/testfile
+      content: "test2"
+      permissions: 0777
+   f3:
+      path: /mnt/iscsi3/testfile
+      content: "test3"
+      permissions: 0777
+   f4:
+      path: /mnt/iscsi4/testfile
+      content: "test4"
+      permissions: 0777

=== added file 'examples/tests/mdadm_iscsi.yaml'
--- examples/tests/mdadm_iscsi.yaml	1970-01-01 00:00:00 +0000
+++ examples/tests/mdadm_iscsi.yaml	2017-05-19 00:57:33 +0000
@@ -0,0 +1,87 @@
+storage:
+   version: 1
+   config:
+      - id: vdb
+        type: disk
+        ptable: msdos
+        model: QEMU HARDDISK
+        path: /dev/vdb
+        name: main_disk
+        wipe: superblock
+        grub_device: true
+      - id: vdb1
+        type: partition
+        number: 1
+        size: 3GB
+        device: vdb
+        flag: boot
+      - id: vdb2
+        type: partition
+        number: 2
+        size: 1GB
+        device: vdb
+      - id: vdb1_root
+        type: format
+        fstype: ext4
+        volume: vdb1
+      - id: vdb2_home
+        type: format
+        fstype: ext4
+        volume: vdb2
+      - id: vdb1_mount
+        type: mount
+        path: /
+        device: vdb1_root
+      - id: vdb2_mount
+        type: mount
+        path: /home
+        device: vdb2_home
+      - id: sda
+        type: disk
+        path: iscsi:__RFC4173__
+        name: iscsi_disk1
+        ptable: msdos
+        wipe: superblock
+      - id: sdb
+        type: disk
+        path: iscsi:__RFC4173__
+        name: iscsi_disk2
+        ptable: msdos
+        wipe: superblock
+      - id: sdc
+        type: disk
+        path: iscsi:__RFC4173__
+        name: iscsi_disk3
+        ptable: msdos
+        wipe: superblock
+      - devices:
+        - sda
+        - sdb
+        - sdc
+        id: md0
+        name: md0
+        raidlevel: 5
+        spare_devices: []
+        type: raid
+      - fstype: ext4
+        id: md0_format
+        label: ''
+        type: format
+        volume: md0
+      - device: md0_format
+        id: md0_mount
+        path: /mnt/iscsi1
+        type: mount
+network:
+   version: 1
+   config:
+      - type: physical
+        name: interface0
+        mac_address: "52:54:00:12:34:00"
+        subnets:
+           - type: dhcp
+write_files:
+   f1:
+      path: /mnt/iscsi1/testfile
+      content: "test1"
+      permissions: 0777

=== modified file 'tests/unittests/test_block.py'
--- tests/unittests/test_block.py	2017-02-08 15:19:42 +0000
+++ tests/unittests/test_block.py	2017-05-19 00:57:33 +0000
@@ -515,4 +515,122 @@
         mock_subp.return_value = (out, err)
         block.blkid()
 
+
+class TestSlaveKnames(TestCase):
+    def add_patch(self, target, attr, autospec=True):
+        """Patches specified target object and sets it as attr on test
+        instance also schedules cleanup"""
+        m = mock.patch(target, autospec=autospec)
+        p = m.start()
+        self.addCleanup(m.stop)
+        setattr(self, attr, p)
+
+    def setUp(self):
+        super(TestSlaveKnames, self).setUp()
+        self.add_patch('curtin.block.get_blockdev_for_partition',
+                       'm_blockdev_for_partition')
+        self.add_patch('curtin.block.os.path.exists',
+                       'm_os_path_exists')
+        # trusty-p3 does not like autospec=True for os.listdir
+        self.add_patch('curtin.block.os.listdir',
+                       'm_os_listdir', autospec=False)
+
+    def _prepare_mocks(self, device, cfg):
+        """
+        Construct the correct sequence of mocks
+        give a mapping of device and slaves
+
+        cfg = {
+            'wark': ['foo', 'bar'],
+            'foo': [],
+            'bar': [],
+        }
+        device = 'wark', slaves = ['foo, 'bar']
+
+        cfg = {
+            'wark': ['foo', 'bar'],
+            'foo': ['zip'],
+            'bar': [],
+            'zip': []
+        }
+        device = 'wark', slaves = ['zip', 'bar']
+        """
+        # kname side-effect mapping
+        parts = [(k, None) for k in cfg.keys()]
+        self.m_blockdev_for_partition.side_effect = iter(parts)
+
+        # construct side effects to os.path.exists
+        # and os.listdir based on mapping.
+        dirs = []
+        exists = []
+        for (dev, slvs) in cfg.items():
+            # sys_block_dev checks if dev exists
+            exists.append(True)
+            if slvs:
+                # os.path.exists on slaves dir
+                exists.append(True)
+                # result of os.listdir
+                dirs.append(slvs)
+            else:
+                # os.path.exists on slaves dir
+                exists.append(False)
+
+        self.m_os_path_exists.side_effect = iter(exists)
+        self.m_os_listdir.side_effect = iter(dirs)
+
+    def test_get_device_slave_knames(self):
+        #
+        # /sys/class/block/wark/slaves/foo -> ../../foo
+        # /sys/class/block/foo #
+        # should return 'bar'
+        cfg = OrderedDict([
+            ('wark', ['foo']),
+            ('foo', []),
+        ])
+        device = "/dev/wark"
+        slaves = ["foo"]
+        self._prepare_mocks(device, cfg)
+        knames = block.get_device_slave_knames(device)
+        self.assertEqual(slaves, knames)
+
+    def test_get_device_slave_knames_stacked(self):
+        #
+        # /sys/class/block/wark/slaves/foo -> ../../foo
+        # /sys/class/block/wark/slaves/bar -> ../../bar
+        # /sys/class/block/foo
+        # /sys/class/block/bar
+        #
+        # should return ['foo', 'bar']
+        cfg = OrderedDict([
+            ('wark', ['foo', 'bar']),
+            ('foo', []),
+            ('bar', []),
+        ])
+        device = 'wark'
+        slaves = ['foo', 'bar']
+        self._prepare_mocks(device, cfg)
+        knames = block.get_device_slave_knames(device)
+        self.assertEqual(slaves, knames)
+
+    def test_get_device_slave_knames_double_stacked(self):
+        # /sys/class/block/wark/slaves/foo -> ../../foo
+        # /sys/class/block/wark/slaves/bar -> ../../bar
+        # /sys/class/block/foo
+        # /sys/class/block/bar/slaves/zip -> ../../zip
+        # /sys/class/block/zip
+        #
+        # mapping of device:
+        cfg = OrderedDict([
+            ('wark', ['foo', 'bar']),
+            ('foo', []),
+            ('bar', ['zip']),
+            ('zip', []),
+        ])
+        device = 'wark'
+        slaves = ['foo', 'zip']
+        self._prepare_mocks(device, cfg)
+        knames = block.get_device_slave_knames(device)
+        self.assertEqual(slaves, knames)
+
+
 # vi: ts=4 expandtab syntax=python

=== modified file 'tests/unittests/test_block_iscsi.py'
--- tests/unittests/test_block_iscsi.py	2017-04-04 17:58:52 +0000
+++ tests/unittests/test_block_iscsi.py	2017-05-19 00:57:33 +0000
@@ -1,8 +1,23 @@
+import mock
+
 from unittest import TestCase
 from curtin.block import iscsi
 
 
-class TestBlockIscsiPortalParsing(TestCase):
+class IscsiTestBase(TestCase):
+    def setUp(self):
+        super(IscsiTestBase, self).setUp()
+
+    def add_patch(self, target, attr):
+        """Patches specified target object and sets it as attr on test
+        instance also schedules cleanup"""
+        m = mock.patch(target, autospec=True)
+        p = m.start()
+        self.addCleanup(m.stop)
+        setattr(self, attr, p)
+
+
+class TestBlockIscsiPortalParsing(IscsiTestBase):
     def test_iscsi_portal_parsing_string(self):
         with self.assertRaisesRegexp(ValueError, 'not a string'):
             iscsi.assert_valid_iscsi_portal(1234)
@@ -474,4 +489,84 @@
         self.assertEquals(i.lun, 0)
         self.assertEquals(i.target, 'iqn.2017-04.com.example.test:target-name')
 
+
+class TestBlockIscsiVolPath(IscsiTestBase):
+    # non-iscsi backed disk returns false
+    # regular iscsi-backed disk returns true
+    # layered setup without an iscsi member returns false
+    # layered setup with an iscsi member returns true
+
+    def setUp(self):
+        super(TestBlockIscsiVolPath, self).setUp()
+        self.add_patch('curtin.block.iscsi.get_device_slave_knames',
+                       'mock_get_device_slave_knames')
+        self.add_patch('curtin.block.iscsi.path_to_kname',
+                       'mock_path_to_kname')
+        self.add_patch('curtin.block.iscsi.kname_is_iscsi',
+                       'mock_kname_is_iscsi')
+
+    def test_volpath_is_iscsi_false(self):
+        volume_path = '/dev/wark'
+        kname = 'wark'
+        slaves = []
+        self.mock_get_device_slave_knames.return_value = slaves
+        self.mock_path_to_kname.return_value = kname
+        self.mock_kname_is_iscsi.return_value = 'iscsi' in kname
+
+        is_iscsi = iscsi.volpath_is_iscsi(volume_path)
+
+        self.assertFalse(is_iscsi)
+        self.mock_get_device_slave_knames.assert_called_with(volume_path)
+        self.mock_path_to_kname.assert_called_with(volume_path)
+        self.mock_kname_is_iscsi.assert_called_with(kname)
+
+    def test_volpath_is_iscsi_true(self):
+        volume_path = '/dev/wark'
+        kname = 'wark-iscsi-lun-2'
+        slaves = []
+        self.mock_get_device_slave_knames.return_value = slaves
+        self.mock_path_to_kname.return_value = kname
+        self.mock_kname_is_iscsi.return_value = 'iscsi' in kname
+
+        is_iscsi = iscsi.volpath_is_iscsi(volume_path)
+
+        self.assertTrue(is_iscsi)
+        self.mock_get_device_slave_knames.assert_called_with(volume_path)
+        self.mock_path_to_kname.assert_called_with(volume_path)
+        self.mock_kname_is_iscsi.assert_called_with(kname)
+
+    def test_volpath_is_iscsi_layered_true(self):
+        volume_path = '/dev/wark'
+        slaves = ['wark', 'bzr', 'super-iscsi-lun-27']
+        self.mock_get_device_slave_knames.return_value = slaves
+        self.mock_path_to_kname.side_effect = lambda x: x
+        self.mock_kname_is_iscsi.side_effect = lambda x: 'iscsi' in x
+
+        is_iscsi = iscsi.volpath_is_iscsi(volume_path)
+
+        self.assertTrue(is_iscsi)
+        self.mock_get_device_slave_knames.assert_called_with(volume_path)
+        self.mock_path_to_kname.assert_called_with(volume_path)
+        self.mock_kname_is_iscsi.assert_has_calls([
+            mock.call(x) for x in slaves])
+
+    def test_volpath_is_iscsi_layered_false(self):
+        volume_path = '/dev/wark'
+        slaves = ['wark', 'bzr', 'nvmen27p47']
+        self.mock_get_device_slave_knames.return_value = slaves
+        self.mock_path_to_kname.side_effect = lambda x: x
+        self.mock_kname_is_iscsi.side_effect = lambda x: 'iscsi' in x
+
+        is_iscsi = iscsi.volpath_is_iscsi(volume_path)
+
+        self.assertFalse(is_iscsi)
+        self.mock_get_device_slave_knames.assert_called_with(volume_path)
+        self.mock_path_to_kname.assert_called_with(volume_path)
+        self.mock_kname_is_iscsi.assert_has_calls([
+            mock.call(x) for x in slaves])
+
+    def test_volpath_is_iscsi_missing_param(self):
+        with self.assertRaises(ValueError):
+            iscsi.volpath_is_iscsi(None)
+
 # vi: ts=4 expandtab syntax=python

=== modified file 'tests/vmtests/__init__.py'
--- tests/vmtests/__init__.py	2017-04-25 20:30:31 +0000
+++ tests/vmtests/__init__.py	2017-05-19 00:57:33 +0000
@@ -973,14 +973,18 @@
 
     # Misc functions that are useful for many tests
     def output_files_exist(self, files):
-        for f in files:
-            logger.debug('checking file %s', f)
-            self.assertTrue(os.path.exists(os.path.join(self.td.collect, f)))
+        logger.debug('checking files: %s', files)
+        results = {f: os.path.exists(os.path.join(self.td.collect, f))
+                   for f in files}
+        logger.debug('results: %s', results)
+        self.assertTrue(False not in results.values())
 
     def output_files_dont_exist(self, files):
-        for f in files:
-            logger.debug('checking file %s', f)
-            self.assertFalse(os.path.exists(os.path.join(self.td.collect, f)))
+        logger.debug('checking files: %s', files)
+        results = {f: os.path.exists(os.path.join(self.td.collect, f))
+                   for f in files}
+        logger.debug('results: %s', results)
+        self.assertTrue(True not in results.values())
 
     def load_collect_file(self, filename, mode="r"):
         with open(os.path.join(self.td.collect, filename), mode) as fp:

=== modified file 'tests/vmtests/test_iscsi.py'
--- tests/vmtests/test_iscsi.py	2017-04-26 16:14:04 +0000
+++ tests/vmtests/test_iscsi.py	2017-05-19 00:57:33 +0000
@@ -12,6 +12,7 @@
         {'size': '5G', 'auth': 'user:passw0rd', 'iauth': 'iuser:ipassw0rd'},
         {'size': '6G', 'iauth': 'iuser:ipassw0rd'}]
     conf_file = "examples/tests/basic_iscsi.yaml"
+    nr_testfiles = 4
 
     collect_scripts = [textwrap.dedent(
         """
@@ -19,16 +20,31 @@
         cat /etc/fstab > fstab
         ls /dev/disk/by-dname/ > ls_dname
         find /etc/network/interfaces.d > find_interfacesd
-        cat /mnt/iscsi1/testfile > testfile1
-        cat /mnt/iscsi2/testfile > testfile2
-        cat /mnt/iscsi3/testfile > testfile3
-        cat /mnt/iscsi4/testfile > testfile4
+        bash -c \
+        'for f in /mnt/iscsi*; do cat $f/testfile > testfile${f: -1}; done'
         """)]
 
-    def test_output_files_exist(self):
+    def test_fstab_has_netdev_option(self):
+        self.output_files_exist(["fstab"])
+        fstab = self.load_collect_file("fstab").strip()
+        self.assertTrue(any(["_netdev" in line
+                             for line in fstab.splitlines()]))
+
+    def test_iscsi_testfiles(self):
         # add check by SN or UUID that the iSCSI disks are attached?
-        self.output_files_exist(["fstab", "testfile1", "testfile2",
-                                 "testfile3", "testfile4"])
+        testfiles = ["testfile%s" % t for t in range(1, self.nr_testfiles + 1)]
+
+        # make sure all required files are present:
+        print('Expecting testfiles: %s' % testfiles)
+        self.output_files_exist(testfiles)
+
+        for testfile in testfiles:
+            print('checking file content: %s' % testfile)
+            expected_content = "test%s" % testfile[-1]
+            content = self.load_collect_file(testfile).strip()
+            self.assertEqual(expected_content, content,
+                             "Checking %s, expected:\n%s\nfound:\n%s" %
+                             (testfile, expected_content, content))
 
 
 class PreciseTestIscsiBasic(relbase.precise, TestBasicIscsiAbs):

=== modified file 'tests/vmtests/test_lvm.py'
--- tests/vmtests/test_lvm.py	2017-04-26 16:14:04 +0000
+++ tests/vmtests/test_lvm.py	2017-05-19 00:57:33 +0000
@@ -1,5 +1,6 @@
 from . import VMBaseClass
 from .releases import base_vm_classes as relbase
+from unittest import SkipTest
 
 import textwrap
 
@@ -38,16 +39,18 @@
         self.output_files_exist(
             ["fstab", "ls_dname"])
 
+    # FIXME(LP: #1523037): dname does not work on precise|trusty, so we cannot
+    # expect sda-part2 to exist in /dev/disk/by-dname as we can on other
+    # releases when dname works on trusty, then we need to re-enable by
+    # removing line.
+    def test_dname(self):
+        if self.release in ['precise', 'trusty']:
+            raise SkipTest("test_dname does not work for %s" % self.release)
+
 
 class PreciseTestLvm(relbase.precise, TestLvmAbs):
     __test__ = True
 
-    # FIXME(LP: #1523037): dname does not work on trusty, so we cannot expect
-    # sda-part2 to exist in /dev/disk/by-dname as we can on other releases
-    # when dname works on trusty, then we need to re-enable by removing line.
-    def test_dname(self):
-        print("test_dname does not work for Trusty")
-
 
 class PreciseHWETTestLvm(relbase.precise_hwe_t, PreciseTestLvm):
     __test__ = True
@@ -56,22 +59,10 @@
 class TrustyTestLvm(relbase.trusty, TestLvmAbs):
     __test__ = True
 
-    # FIXME(LP: #1523037): dname does not work on trusty, so we cannot expect
-    # sda-part2 to exist in /dev/disk/by-dname as we can on other releases
-    # when dname works on trusty, then we need to re-enable by removing line.
-    def test_dname(self):
-        print("test_dname does not work for Trusty")
-
 
 class TrustyHWEXTestLvm(relbase.trusty_hwe_x, TestLvmAbs):
     __test__ = True
 
-    # FIXME(LP: #1523037): dname does not work on trusty, so we cannot expect
-    # sda-part2 to exist in /dev/disk/by-dname as we can on other releases
-    # when dname works on trusty, then we need to re-enable by removing line.
-    def test_dname(self):
-        print("test_dname does not work for Trusty")
-
 
 class WilyTestLvm(relbase.wily, TestLvmAbs):
     # EOL - 2016-07-28

=== added file 'tests/vmtests/test_lvm_iscsi.py'
--- tests/vmtests/test_lvm_iscsi.py	1970-01-01 00:00:00 +0000
+++ tests/vmtests/test_lvm_iscsi.py	2017-05-19 00:57:33 +0000
@@ -0,0 +1,67 @@
+from .releases import base_vm_classes as relbase
+from .test_lvm import TestLvmAbs
+from .test_iscsi import TestBasicIscsiAbs
+
+import textwrap
+
+
+class TestLvmIscsiAbs(TestLvmAbs, TestBasicIscsiAbs):
+    interactive = False
+    iscsi_disks = [
+        {'size': '6G'},
+        {'size': '5G', 'auth': 'user:passw0rd', 'iauth': 'iuser:ipassw0rd'}]
+    conf_file = "examples/tests/lvm_iscsi.yaml"
+    nr_testfiles = 4
+
+    collect_scripts = TestLvmAbs.collect_scripts
+    collect_scripts += TestBasicIscsiAbs.collect_scripts + [textwrap.dedent(
+        """
+        cd OUTPUT_COLLECT_D
+        ls -al /sys/class/block/dm*/slaves/  > dm_slaves
+        """)]
+
+    fstab_expected = {
+        'UUID=6de56115-9500-424b-8151-221b270ec708': '/mnt/iscsi1',
+        'UUID=9604e4c4-e5ae-40dd-ab1f-940de6b59047': '/mnt/iscsi2',
+        'UUID=18bec31c-09a8-4a02-91c6-e9bf6efb6fad': '/mnt/iscsi3',
+        'UUID=a98f706b-b064-4682-8eb2-6c2c1284060c': '/mnt/iscsi4',
+    }
+    disk_to_check = [('main_disk', 1),
+                     ('main_disk', 5),
+                     ('main_disk', 6),
+                     ('vg1-lv1', 0),
+                     ('vg1-lv2', 0),
+                     ('vg2-lv3', 0),
+                     ('vg2-lv4', 0)]
+
+    def test_lvs(self):
+        self.check_file_strippedline("lvs", "lv1=vg1")
+        self.check_file_strippedline("lvs", "lv2=vg1")
+        self.check_file_strippedline("lvs", "lv3=vg2")
+        self.check_file_strippedline("lvs", "lv4=vg2")
+
+    def test_pvs(self):
+        self.check_file_strippedline("pvs", "vg1=/dev/sda5")
+        self.check_file_strippedline("pvs", "vg1=/dev/sda6")
+        self.check_file_strippedline("pvs", "vg2=/dev/sdb5")
+        self.check_file_strippedline("pvs", "vg2=/dev/sdb6")
+
+
+class PreciseTestIscsiLvm(relbase.precise, TestLvmIscsiAbs):
+    __test__ = True
+
+
+class TrustyTestIscsiLvm(relbase.trusty, TestLvmIscsiAbs):
+    __test__ = True
+
+
+class XenialTestIscsiLvm(relbase.xenial, TestLvmIscsiAbs):
+    __test__ = True
+
+
+class YakketyTestIscsiLvm(relbase.yakkety, TestLvmIscsiAbs):
+    __test__ = True
+
+
+class ZestyTestIscsiLvm(relbase.zesty, TestLvmIscsiAbs):
+    __test__ = True

=== added file 'tests/vmtests/test_mdadm_iscsi.py'
--- tests/vmtests/test_mdadm_iscsi.py	1970-01-01 00:00:00 +0000
+++ tests/vmtests/test_mdadm_iscsi.py	2017-05-19 00:57:33 +0000
@@ -0,0 +1,42 @@
+from .releases import base_vm_classes as relbase
+from .test_mdadm_bcache import TestMdadmAbs
+from .test_iscsi import TestBasicIscsiAbs
+
+import textwrap
+
+
+class TestMdadmIscsiAbs(TestMdadmAbs, TestBasicIscsiAbs):
+    interactive = False
+    iscsi_disks = [
+        {'size': '5G', 'auth': 'user:passw0rd'},
+        {'size': '5G', 'auth': 'user:passw0rd', 'iauth': 'iuser:ipassw0rd'},
+        {'size': '5G', 'iauth': 'iuser:ipassw0rd'}]
+    conf_file = "examples/tests/mdadm_iscsi.yaml"
+    nr_testfiles = 1
+
+    collect_scripts = TestMdadmAbs.collect_scripts
+    collect_scripts += TestBasicIscsiAbs.collect_scripts + [textwrap.dedent(
+        """
+        cd OUTPUT_COLLECT_D
+        ls -al /sys/class/block/md*/slaves/  > md_slaves
+        """)]
+
+
+class PreciseTestIscsiMdadm(relbase.precise, TestMdadmIscsiAbs):
+    __test__ = True
+
+
+class TrustyTestIscsiMdadm(relbase.trusty, TestMdadmIscsiAbs):
+    __test__ = True
+
+
+class XenialTestIscsiMdadm(relbase.xenial, TestMdadmIscsiAbs):
+    __test__ = True
+
+
+class YakketyTestIscsiMdadm(relbase.yakkety, TestMdadmIscsiAbs):
+    __test__ = True
+
+
+class ZestyTestIscsiMdadm(relbase.zesty, TestMdadmIscsiAbs):
+    __test__ = True