Merge ~ogayot/curtin:wwn-match-with-extension into curtin:master

[Olivier Gayot]

block-meta: fix failed disk lookup when WWN includes extension

curtin's storage config extracts the WWN of a disk from one of the
following udev variables:

 * ID_WWN_WITH_EXTENSION (preferred)
 * ID_WWN

However, when trying to look the disk up later in the block-meta code,
curtin only tries to match the WWN value against the ID_WWN variable
(and another variable related to DM multipath).

This means that the disk can not be be found just based on the wwn if
the ID_WWN and ID_WWN_WITH_EXTENSION variables don't hold the same
value.

In the past, that was often okay because other fields (such as disk path
or serial) would still make the disk lookup succeed.

However, the following patch introduced a restriction. In v2, all fields
specified must now match for the disk lookup to be successful:

8c5f87ed block_meta: all fields on a disk action must match with v2 config

Fixed by matching against the ID_WWN_WITH_EXTENSION (preferred) and then
ID_WWN.

We got multiple reports of failed installs where curtin logs show the following pattern:

found candidate disks [set(), {'/dev/sda'}, {'/dev/sda'}]
        An error occured handling 'disk-sda': ValueError - Failed to find storage volume id='disk-sda' config: {'ptable': 'gpt', 'serial': '121212121212121212121212', 'wwn': '0x123456789abcdef0deadbeef', 'path': '/dev/sda', 'wipe': 'superblock-recursive', 'preserve': False, 'name': '', 'grub_device': False, 'type': 'disk', 'id': 'disk-sda'}

When trying to look up the disk, the value of the wwn field (i.e., 0x123456789abcdef0deadbeef here) is compared to the ID_WWN udev variable of available disks.

However, the wwn variable is not always initialized from the ID_WWN variable. If present, the ID_WWN_WITH_EXTENSION variable takes precedence:

source_keys = {
                'wwn': ['ID_WWN_WITH_EXTENSION', 'ID_WWN'],
                'serial': ['ID_SERIAL', 'ID_SERIAL_SHORT'],
            }

Example
-------

"ID_WWN": '0x123456789abcdef0'
"ID_WWN_VENDOR_EXTENSION": "0xdeadbeef",
"ID_WWN_WITH_EXTENSION": '0x123456789abcdef0deadbeef'

Since present, the wwn variable gets initialized from ID_WWN_WITH_EXTENSION.

Later on, we compare the wwn variable with ID_WWN and the lookup fails.

Fixed by making sure to compare the wwn value with the ID_WWN_WITH_EXTENSION first (if available), and only then ID_WWN.

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

PASSED: Continuous integration, rev:e8e59df25c9b193362576346a6275a6b1b91cef2
https://jenkins.canonical.com/server-team/job/curtin-ci/90/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-amd64/90/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-arm64/90/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-ppc64el/90/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-s390x/90/

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/curtin-ci/90//rebuild

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:d4f445a3418af09f966c791461c55d94bd2ff668
https://jenkins.canonical.com/server-team/job/curtin-ci/91/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-amd64/91/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-arm64/91/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-ppc64el/91/
    SUCCESS: https://jenkins.canonical.com/server-team/job/curtin-ci/nodes=metal-s390x/91/

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/curtin-ci/91//rebuild

review: Approve (continuous-integration)

[Olivier Gayot (ogayot)]

Note: I'm quite confident that the patch will help but I don't have a way to reproduce the bug in a real install so we might need to lean on the users who originally reported the issue to test again and confirm.

[Dan Bungert (dbungert)]

LGTM

review: Approve

diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index c0e595d..7988f3a 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -513,7 +513,8 @@ def v2_get_path_to_disk(vol):
         cands.append(set([dev['DEVNAME'] for dev in new_devs]))
 
     if 'wwn' in vol:
-        add_cands(*disk_by_keys(vol['wwn'], 'DM_WWN', 'ID_WWN'))
+        add_cands(*disk_by_keys(vol['wwn'],
+                                'DM_WWN', 'ID_WWN_WITH_EXTENSION', 'ID_WWN'))
     if 'serial' in vol:
         add_cands(
             *disk_by_keys(
diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
index 0c198e6..82530ff 100644
--- a/tests/unittests/test_commands_block_meta.py
+++ b/tests/unittests/test_commands_block_meta.py
@@ -222,6 +222,23 @@ class TestGetPathToStorageVolume(CiTestCase):
             devname,
             block_meta.get_path_to_storage_volume(disk_id, s_cfg))
 
+    def test_v2_match_wwn_with_extension(self):
+        wwn = self.random_string()
+        extension = self.random_string()
+        devname = self.random_string()
+        self.m_udev_all.return_value = [
+            {'DEVNAME': devname,
+             'ID_WWN': wwn,
+             'ID_WWN_WITH_EXTENSION': wwn + extension},
+            ]
+        disk_id = self.random_string()
+        cfg = {'id': disk_id, 'type': 'disk', 'wwn': wwn + extension}
+        s_cfg = OrderedDict({disk_id: cfg})
+        s_cfg.version = 2
+        self.assertEqual(
+            devname,
+            block_meta.get_path_to_storage_volume(disk_id, s_cfg))
+
     def test_v2_match_wwn_prefer_mpath(self):
         wwn = self.random_string()
         devname1 = self.random_string()