Merge lp:~mpontillo/maas/fix-recommission-lvm-virtio-bug-1585016 into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Use the path instead of the id_path when recommissioning virtual disks (when the serial number is blank). This prevents issues where the LVM id_path is used instead of a correct ID.

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

=== modified file 'src/metadataserver/models/commissioningscript.py'
--- src/metadataserver/models/commissioningscript.py	2016-06-10 02:29:21 +0000
+++ src/metadataserver/models/commissioningscript.py	2016-06-14 15:22:18 +0000
@@ -333,8 +333,10 @@
         model = block_info.get("MODEL", "")
         serial = block_info.get("SERIAL", "")
         id_path = block_info.get("ID_PATH", "")
-        if not id_path:
-            # Fallback to the dev path if id_path missing.
+        if not id_path or not serial:
+            # Fallback to the dev path if id_path missing or there is no
+            # serial number. (No serial number is a strong indicator that this
+            # is a virtual disk, so it's unlikely that the ID_PATH would work.)
             id_path = block_info["PATH"]
         size = int(block_info["SIZE"])
         block_size = int(block_info["BLOCK_SIZE"])

=== modified file 'src/metadataserver/models/tests/test_commissioningscript.py'
--- src/metadataserver/models/tests/test_commissioningscript.py	2016-06-11 00:59:08 +0000
+++ src/metadataserver/models/tests/test_commissioningscript.py	2016-06-14 15:22:18 +0000
@@ -603,6 +603,24 @@
                 name=name, id_path='/dev/%s' % name, size=size,
                 block_size=block_size, model=model, serial=serial))
 
+    def test__creates_physical_block_device_with_path_for_missing_serial(self):
+        name = factory.make_name('name')
+        size = random.randint(MIN_BLOCK_DEVICE_SIZE, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        model = factory.make_name('model')
+        serial = ''
+        device = self.make_block_device(
+            name=name, size=size, block_size=block_size,
+            model=model, serial=serial, id_path='bogus')
+        node = factory.make_Node()
+        json_output = json.dumps([device]).encode('utf-8')
+        update_node_physical_block_devices(node, json_output, 0)
+        self.assertThat(
+            PhysicalBlockDevice.objects.filter(node=node).first(),
+            MatchesStructure.byEquality(
+                name=name, id_path='/dev/%s' % name, size=size,
+                block_size=block_size, model=model, serial=serial))
+
     def test__creates_physical_block_device_only_for_node(self):
         device = self.make_block_device()
         node = factory.make_Node(with_boot_disk=False)