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

[Ryan Harper]

block_meta: Attempt to mount by UUID= if possible

When generating fstab entries in mount_handler prefer UUID if the
underlying volume has an identifiable UUID available.

Update vmtests to check by UUID in test_basic.

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

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

vmtest run is clean:

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

[lp:~raharper/curtin/trunk.mount_fs_by_uuid]

485. By Ryan Harper on 2017-04-05

Refine uuid extraction from ls_uuid

486. By Ryan Harper on 2017-04-05

merge from trunk

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

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

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

I think this would presumably fix https://launchpad.net/bugs/1676991 .

[Scott Moser (smoser)]

i dont think we neeed to block your mp on this, but in general we should be striving to collect generic information in the guest rather than specific little things like 'by_uname' and 'by_dname' and such.

[Scott Moser (smoser)]

needs fixing or explanation as to make sure that multipath disks do not get written in /etc/fstab with UUID=. multipath disks need to have mounts done by path to the multipath device (which is consistent thansk to multipath).

review: Needs Fixing

[Ryan Harper (raharper)]

I think this just works, but follow my logic to confirm.

With this change, the test_multipath vmtest still passes. Here's what the fstab looks like now:

# note those UUID are different, but by only one char (4 vs. 5 a index 7)
(diglett) collect % cat fstab
UUID=b48f7bd4-1b26-11e7-a7b6-525400123456 / ext4 defaults 0 0
UUID=b48f7bd5-1b26-11e7-a7b6-525400123456 /home ext4 defaults 0 0

Here is what /dev/disk/by-uuid/ looks like

(diglett) collect % cat ls_uuid
total 0
drwxr-xr-x 2 root root 120 Apr 7 00:12 .
drwxr-xr-x 7 root root 140 Apr 7 00:12 ..
lrwxrwxrwx 1 root root 9 Apr 7 00:12 2017-04-06-19-10-58-00 -> ../../vdb
lrwxrwxrwx 1 root root 10 Apr 7 00:12 b48f7bd4-1b26-11e7-a7b6-525400123456 -> ../../dm-1
lrwxrwxrwx 1 root root 10 Apr 7 00:12 b48f7bd5-1b26-11e7-a7b6-525400123456 -> ../../dm-2
lrwxrwxrwx 1 root root 9 Apr 7 00:12 db8db497-6d9c-4c14-ad6b-5f615dbd96d3 -> ../../vda

The UUID points to a device mapper entry

Here's what /dev/disk/by-id shows:

(diglett) collect % cat ls_disk_id
total 0
drwxr-xr-x 2 root root 420 Apr 7 00:12 .
drwxr-xr-x 7 root root 140 Apr 7 00:12 ..
lrwxrwxrwx 1 root root 9 Apr 7 00:12 ata-QEMU_DVD-ROM_QM00003 -> ../../sr0
lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-name-mpath0 -> ../../dm-0
lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-name-mpath0-part1 -> ../../dm-1
lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-name-mpath0-part2 -> ../../dm-2

Note that dm-1 and dm-2 are mpath devices.

Below you can see underlying scsi devices that compose the mpath device.

lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-uuid-mpath-0QEMU_QEMU_HARDDISK_IPR-0_1234567890 -> ../../dm-0
lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-uuid-part1-mpath-0QEMU_QEMU_HARDDISK_IPR-0_1234567890 -> ../../dm-1
lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-uuid-part2-mpath-0QEMU_QEMU_HARDDISK_IPR-0_1234567890 -> ../../dm-2
lrwxrwxrwx 1 root root 9 Apr 7 00:12 scsi-0QEMU_QEMU_DVD-ROM_QM00003 -> ../../sr0
lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-0QEMU_QEMU_HARDDISK_IPR-0_1234567890 -> ../../dm-0
lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-0QEMU_QEMU_HARDDISK_IPR-0_1234567890-part1 -> ../../dm-1
lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-0QEMU_QEMU_HARDDISK_IPR-0_1234567890-part2 -> ../../dm-2
lrwxrwxrwx 1 root root 9 Apr 7 00:12 scsi-1ATA_QEMU_DVD-ROM_QM00003 -> ../../sr0
lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-mpath0 -> ../../dm-0
lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-mpath0-part1 -> ../../dm-1
lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-mpath0-part2 -> ../../dm-2
lrwxrwxrwx 1 root root 9 Apr 7 00:12 virtio-output_disk.img -> ../../vda
lrwxrwxrwx 1 root root 10 Apr 7 00:12 wwn-0xQEMU_QEMU_HARDDISK_IPR-0_1234567890 -> ../../dm-0
lrwxrwxrwx 1 root root 10 Apr 7 00:12 wwn-0xpath0-part1 -> ../../dm-1
lrwxrwxrwx 1 root root 10 Apr 7 00:12 wwn-0xpath0-part2 -> ../../dm-2

[Ryan Harper (raharper)]

After much discussion in #curtin our conclusion
was that, while the risk is small, ideally /etc/fstab should use mpath's
for mpath
devices.

However since the ephemeral environment does not actually configure and
enable multipath; we don't quite know the paths here.

Also, we've been using UUID= mounts for partitions on multipath successfully
since we've supported multipath configurations.

The plan is to continue with using UUID= mounts for bcache and other devices
if a UUID is present and tackle switching to mpath paths for systems with
multipath later.

On Fri, Apr 7, 2017 at 11:05 AM, Ryan Harper <email address hidden>
wrote:

> I think this just works, but follow my logic to confirm.
>
> With this change, the test_multipath vmtest still passes. Here's what the
> fstab looks like now:
>
> # note those UUID are different, but by only one char (4 vs. 5 a index 7)
> (diglett) collect % cat fstab
> UUID=b48f7bd4-1b26-11e7-a7b6-525400123456 / ext4 defaults 0 0
> UUID=b48f7bd5-1b26-11e7-a7b6-525400123456 /home ext4 defaults 0 0
>
> Here is what /dev/disk/by-uuid/ looks like
>
> (diglett) collect % cat ls_uuid
> total 0
> drwxr-xr-x 2 root root 120 Apr 7 00:12 .
> drwxr-xr-x 7 root root 140 Apr 7 00:12 ..
> lrwxrwxrwx 1 root root 9 Apr 7 00:12 2017-04-06-19-10-58-00 -> ../../vdb
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 b48f7bd4-1b26-11e7-a7b6-525400123456
> -> ../../dm-1
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 b48f7bd5-1b26-11e7-a7b6-525400123456
> -> ../../dm-2
> lrwxrwxrwx 1 root root 9 Apr 7 00:12 db8db497-6d9c-4c14-ad6b-5f615dbd96d3
> -> ../../vda
>
> The UUID points to a device mapper entry
>
> Here's what /dev/disk/by-id shows:
>
> (diglett) collect % cat ls_disk_id
> total 0
> drwxr-xr-x 2 root root 420 Apr 7 00:12 .
> drwxr-xr-x 7 root root 140 Apr 7 00:12 ..
> lrwxrwxrwx 1 root root 9 Apr 7 00:12 ata-QEMU_DVD-ROM_QM00003 ->
> ../../sr0
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-name-mpath0 -> ../../dm-0
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-name-mpath0-part1 -> ../../dm-1
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-name-mpath0-part2 -> ../../dm-2
>
> Note that dm-1 and dm-2 are mpath devices.
>
> Below you can see underlying scsi devices that compose the mpath device.
>
>
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-uuid-mpath-0QEMU_QEMU_HARDDISK_IPR-0_1234567890
> -> ../../dm-0
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-uuid-part1-mpath-0QEMU_QEMU_HARDDISK_IPR-0_1234567890
> -> ../../dm-1
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 dm-uuid-part2-mpath-0QEMU_QEMU_HARDDISK_IPR-0_1234567890
> -> ../../dm-2
> lrwxrwxrwx 1 root root 9 Apr 7 00:12 scsi-0QEMU_QEMU_DVD-ROM_QM00003
> -> ../../sr0
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-0QEMU_QEMU_HARDDISK_IPR-0_1234567890
> -> ../../dm-0
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-0QEMU_QEMU_HARDDISK_IPR-0_1234567890-part1
> -> ../../dm-1
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-0QEMU_QEMU_HARDDISK_IPR-0_1234567890-part2
> -> ../../dm-2
> lrwxrwxrwx 1 root root 9 Apr 7 00:12 scsi-1ATA_QEMU_DVD-ROM_QM00003 ->
> ../../sr0
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-mpath0 -> ../../dm-0
> lrwxrwxrwx 1 root root 10 Apr 7 00:12 scsi-mpath0-...

[Scott Moser (smoser)]

review: Approve

=== modified file 'curtin/commands/block_meta.py'
--- curtin/commands/block_meta.py	2017-03-22 21:03:58 +0000
+++ curtin/commands/block_meta.py	2017-04-05 20:49:33 +0000
@@ -633,15 +633,11 @@
     # Add volume to fstab
     if state['fstab']:
         with open(state['fstab'], "a") as fp:
-            if volume.get('type') in ["raid", "bcache",
-                                      "disk", "lvm_partition"]:
-                location = get_path_to_storage_volume(volume.get('id'),
-                                                      storage_config)
-            elif volume.get('type') in ["partition", "dm_crypt"]:
-                location = "UUID=%s" % block.get_volume_uuid(volume_path)
-            else:
-                raise ValueError("cannot write fstab for volume type '%s'" %
-                                 volume.get("type"))
+            location = get_path_to_storage_volume(volume.get('id'),
+                                                  storage_config)
+            uuid = block.get_volume_uuid(volume_path)
+            if len(uuid) > 0:
+                location = "UUID=%s" % uuid
 
             if filesystem.get('fstype') == "swap":
                 path = "none"

=== modified file 'tests/vmtests/test_basic.py'
--- tests/vmtests/test_basic.py	2017-02-22 01:30:50 +0000
+++ tests/vmtests/test_basic.py	2017-04-05 20:49:33 +0000
@@ -33,6 +33,20 @@
         echo "$v" > apt-proxy
         """)]
 
+    def _kname_to_uuid(self, kname):
+        # extract uuid from /dev/disk/by-uuid on /dev/<kname>
+        # parsing ls -al output on /dev/disk/by-uuid:
+        # lrwxrwxrwx 1 root root   9 Dec  4 20:02
+        #  d591e9e9-825a-4f0a-b280-3bfaf470b83c -> ../../vdg
+        ls_uuid = self.load_collect_file("ls_uuid")
+        uuid = [line.split()[8] for line in ls_uuid.split('\n')
+                if ("../../" + kname) in line.split()]
+        self.assertEqual(len(uuid), 1)
+        uuid = uuid.pop()
+        self.assertTrue(uuid is not None)
+        self.assertEqual(len(uuid), 36)
+        return uuid
+
     def test_output_files_exist(self):
         self.output_files_exist(
             ["blkid_output_vda", "blkid_output_vda1", "blkid_output_vda2",
@@ -79,9 +93,10 @@
         self.assertEqual(fstab_entry.split(' ')[1], "/home")
 
         # Test whole disk vdd is mounted at /btrfs
+        uuid = self._kname_to_uuid('vdd')
         fstab_entry = None
         for line in fstab_lines:
-            if "/dev/vdd" in line:
+            if uuid in line:
                 fstab_entry = line
                 break
         self.assertIsNotNone(fstab_entry)
@@ -90,24 +105,16 @@
     def test_whole_disk_format(self):
         # confirm the whole disk format is the expected device
         btrfs_uuid = self.load_collect_file('btrfs_uuid_vdd').strip()
-        ls_uuid = self.load_collect_file("ls_uuid")
 
         # extract uuid from btrfs superblock
         self.assertTrue(btrfs_uuid is not None)
         self.assertEqual(len(btrfs_uuid), 36)
 
-        # extract uuid from /dev/disk/by-uuid on /dev/vdd
-        # parsing ls -al output on /dev/disk/by-uuid:
-        # lrwxrwxrwx 1 root root   9 Dec  4 20:02
-        #  d591e9e9-825a-4f0a-b280-3bfaf470b83c -> ../../vdg
-        vdd_uuid = [line.split()[8] for line in ls_uuid.split('\n')
-                    if 'vdd' in line]
-        self.assertEqual(len(vdd_uuid), 1)
-        vdd_uuid = vdd_uuid.pop()
-        self.assertTrue(vdd_uuid is not None)
+        # extract uuid from ls_uuid by kname
+        kname_uuid = self._kname_to_uuid('vdd')
 
         # compare them
-        self.assertEqual(vdd_uuid, btrfs_uuid)
+        self.assertEqual(kname_uuid, btrfs_uuid)
 
     def test_proxy_set(self):
         expected = get_apt_proxy()
@@ -153,23 +160,15 @@
     def test_whole_disk_format(self):
         # confirm the whole disk format is the expected device
         btrfs_uuid = self.load_collect_file("btrfs_uuid_vdd").strip()
-        ls_uuid = self.load_collect_file("ls_uuid")
 
         self.assertTrue(btrfs_uuid is not None)
         self.assertEqual(len(btrfs_uuid), 36)
 
-        # extract uuid from /dev/disk/by-uuid on /dev/vdd
-        # parsing ls -al output on /dev/disk/by-uuid:
-        # lrwxrwxrwx 1 root root   9 Dec  4 20:02
-        #  d591e9e9-825a-4f0a-b280-3bfaf470b83c -> ../../vdg
-        vdd_uuid = [line.split()[8] for line in ls_uuid.split('\n')
-                    if 'vdd' in line]
-        self.assertEqual(len(vdd_uuid), 1)
-        vdd_uuid = vdd_uuid.pop()
-        self.assertTrue(vdd_uuid is not None)
+        # extract uuid from ls_uuid by kname
+        kname_uuid = self._kname_to_uuid('vdd')
 
         # compare them
-        self.assertEqual(vdd_uuid, btrfs_uuid)
+        self.assertEqual(kname_uuid, btrfs_uuid)
 
     def test_ptable(self):
         print("test_ptable does not work for Precise")
@@ -289,9 +288,10 @@
         self.assertEqual(fstab_entry.split(' ')[1], "/home")
 
         # Test whole disk sdc is mounted at /btrfs
+        uuid = self._kname_to_uuid('sdc')
         fstab_entry = None
         for line in fstab_lines:
-            if "/dev/sdc" in line:
+            if uuid in line:
                 fstab_entry = line
                 break
         self.assertIsNotNone(fstab_entry)
@@ -300,24 +300,16 @@
     def test_whole_disk_format(self):
         # confirm the whole disk format is the expected device
         btrfs_uuid = self.load_collect_file("btrfs_uuid_sdc").strip()
-        ls_uuid = self.load_collect_file("ls_uuid")
 
         # extract uuid from btrfs superblock
         self.assertTrue(btrfs_uuid is not None)
         self.assertEqual(len(btrfs_uuid), 36)
 
-        # extract uuid from /dev/disk/by-uuid on /dev/sdc
-        # parsing ls -al output on /dev/disk/by-uuid:
-        # lrwxrwxrwx 1 root root   9 Dec  4 20:02
-        #  d591e9e9-825a-4f0a-b280-3bfaf470b83c -> ../../vdg
-        uuid = [line.split()[8] for line in ls_uuid.split('\n')
-                if 'sdc' in line]
-        self.assertEqual(len(uuid), 1)
-        uuid = uuid.pop()
-        self.assertTrue(uuid is not None)
+        # extract uuid from ls_uuid by kname
+        kname_uuid = self._kname_to_uuid('sdc')
 
         # compare them
-        self.assertEqual(uuid, btrfs_uuid)
+        self.assertEqual(kname_uuid, btrfs_uuid)
 
 
 class XenialTestScsiBasic(relbase.xenial, TestBasicScsiAbs):