Merge ~ltrager/maas:lp1825241 into maas:master

[Lee Trager]

LP: #1825241 - Allow VMFS6 layout to be applied to any disk

[Newell Jensen (newell-jensen)]

You should fix the docstring as mentioned inline. You should also update line 212 in maasserver.storage_layouts if you are doing to keep the change that you added for get_root_device as this will not be returning None anymore.

review: Needs Fixing

4f42134... by Lee Trager on 2019-04-19

Merge branch 'master' into lp1825241

fd38000... by Lee Trager on 2019-04-19

newell-jensen fixes

[Lee Trager (ltrager)]

Thanks for the review. I forgot about the docstring, and I updated line 212 as requested.

[Newell Jensen (newell-jensen)]

Thanks for the fixes. LGTM.

review: Approve

diff --git a/src/maasserver/storage_layouts.py b/src/maasserver/storage_layouts.py
index 2bb5b34..dcf4c34 100644
--- a/src/maasserver/storage_layouts.py
+++ b/src/maasserver/storage_layouts.py
@@ -138,13 +138,15 @@ class StorageLayoutBase(Form):
     def get_root_device(self):
         """Get the device that should be the root partition.
 
-        Return of None means to use the boot disk.
+        Return the boot_disk if no root_device was defined.
         """
         if self.cleaned_data.get('root_device'):
             root_id = self.cleaned_data['root_device']
             return self.node.physicalblockdevice_set.get(id=root_id)
         else:
-            return None
+            # User didn't specify a root disk so use the currently defined
+            # boot disk.
+            return self.boot_disk
 
     def get_boot_size(self):
         """Get the size of the boot partition."""
@@ -209,7 +211,7 @@ class StorageLayoutBase(Form):
                 mount_point="/boot")
         root_device = self.get_root_device()
         root_size = self.get_root_size()
-        if root_device is None or root_device == self.boot_disk:
+        if root_device == self.boot_disk:
             partition_table = boot_partition_table
             root_device = self.boot_disk
         else:
@@ -566,7 +568,7 @@ class VMFS6Layout(StorageLayoutBase):
         from maasserver.models.partitiontable import PartitionTable
 
         boot_partition_table = PartitionTable.objects.create(
-            block_device=self.boot_disk,
+            block_device=self.get_root_device(),
             table_type=PARTITION_TABLE_TYPE.GPT)
         now = datetime.now()
         new_part = partial(
@@ -598,7 +600,11 @@ class VMFS6Layout(StorageLayoutBase):
             new_part(size=2560 * 1024 ** 2),
         ])
         vmfs_part = boot_partition_table.partitions.get(size=0)
-        vmfs_part.size = boot_partition_table.get_available_size()
+        root_size = self.get_root_size()
+        if root_size is not None:
+            vmfs_part.size = root_size
+        else:
+            vmfs_part.size = boot_partition_table.get_available_size()
         vmfs_part.save()
         # datastore1 is the default name VMware uses.
         VMFS.objects.create_vmfs(name="datastore1", partitions=[vmfs_part])
diff --git a/src/maasserver/tests/test_storage_layouts.py b/src/maasserver/tests/test_storage_layouts.py
index ef2df86..8daa2ee 100644
--- a/src/maasserver/tests/test_storage_layouts.py
+++ b/src/maasserver/tests/test_storage_layouts.py
@@ -391,14 +391,14 @@ class TestStorageLayoutBase(MAASServerTestCase):
         self.assertTrue(layout.is_valid(), layout.errors)
         self.assertEqual(boot_size, layout.get_boot_size())
 
-    def test_get_root_device_returns_None_if_not_set(self):
+    def test_get_root_device_returns_boot_disk_if_not_set(self):
         node = make_Node_with_uefi_boot_method()
         factory.make_PhysicalBlockDevice(
             node=node, size=LARGE_BLOCK_DEVICE)
         layout = StorageLayoutBase(node, {
             })
         self.assertTrue(layout.is_valid(), layout.errors)
-        self.assertIsNone(layout.get_root_device())
+        self.assertEquals(node.get_boot_disk(), layout.get_root_device())
 
     def test_get_root_device_returns_root_device_if_set(self):
         node = make_Node_with_uefi_boot_method()
@@ -1652,6 +1652,48 @@ class TestVMFS6Layout(MAASServerTestCase):
             "size": ["Boot disk must be atleast 10G."],
             }, error.message_dict)
 
+    def test__accepts_root_device_param(self):
+        # Regression test for LP:1825241
+        node = factory.make_Node(with_boot_disk=False)
+        node.boot_disk = factory.make_PhysicalBlockDevice(
+            node=node, size=LARGE_BLOCK_DEVICE)
+        root_disk = factory.make_PhysicalBlockDevice(
+            node=node, size=LARGE_BLOCK_DEVICE)
+        layout = VMFS6Layout(node, {'root_device': root_disk.id})
+        self.assertEqual('VMFS6', layout.configure())
+        pt = root_disk.get_partitiontable()
+        self.assertDictEqual({
+            '%s-part1' % root_disk.name: 3 * 1024 ** 2,
+            '%s-part2' % root_disk.name: 4 * 1024 ** 3,
+            '%s-part3' % root_disk.name: (
+                root_disk.size - 3 * 1024 ** 2 - 4 * 1024 ** 3 -
+                249 * 1024 ** 2 - 249 * 1024 ** 2 - 109 * 1024 ** 2 -
+                285 * 1024 ** 2 - 2560 * 1024 ** 2 - 5 * 1024 ** 2),
+            '%s-part4' % root_disk.name: 249 * 1024 ** 2,
+            '%s-part5' % root_disk.name: 249 * 1024 ** 2,
+            '%s-part6' % root_disk.name: 109 * 1024 ** 2,
+            '%s-part7' % root_disk.name: 285 * 1024 ** 2,
+            '%s-part8' % root_disk.name: 2560 * 1024 ** 2,
+        }, {part.name: part.size for part in pt.partitions.all()})
+
+    def test__accepts_root_size_param(self):
+        node = factory.make_Node(with_boot_disk=False)
+        node.boot_disk = factory.make_PhysicalBlockDevice(
+            node=node, size=LARGE_BLOCK_DEVICE)
+        layout = VMFS6Layout(node, {'root_size': 10 * 1024 ** 3})
+        self.assertEqual('VMFS6', layout.configure())
+        pt = node.boot_disk.get_partitiontable()
+        self.assertDictEqual({
+            '%s-part1' % node.boot_disk.name: 3 * 1024 ** 2,
+            '%s-part2' % node.boot_disk.name: 4 * 1024 ** 3,
+            '%s-part3' % node.boot_disk.name: 10 * 1024 ** 3,
+            '%s-part4' % node.boot_disk.name: 249 * 1024 ** 2,
+            '%s-part5' % node.boot_disk.name: 249 * 1024 ** 2,
+            '%s-part6' % node.boot_disk.name: 109 * 1024 ** 2,
+            '%s-part7' % node.boot_disk.name: 285 * 1024 ** 2,
+            '%s-part8' % node.boot_disk.name: 2560 * 1024 ** 2,
+        }, {part.name: part.size for part in pt.partitions.all()})
+
 
 class TestBlankStorageLayout(MAASServerTestCase):