Merge ~ltrager/maas:lp1825240 into maas:master

[Lee Trager]

LP: #1825240 - Do not allow datastores to be created unless the VMFS6 storage layout has been applied.

VMFS datastores only work with VMware ESXi. Prevent them from being created
unless the VMFS6 storage layout has been previously applied. This prevents
users from thinking we support VMFS Datastores on other operating systems.

To be backwards compatible with previous versions of MAAS VMware ESXi can
still be deployed without applying the VMFS6 storage layout. In that case
the default layout, including a datastore will be applied on deployment.
If the VMFS6 storage layout is applied make sure a datastore is defined
as one will always be created.

[Blake Rouse (blake-rouse)]

Looks good, minus the very minor nitpick.

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b lp1825240 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/5539/consoleText

ab519ac... by Lee Trager on 2019-04-23

Merge branch 'master' into lp1825240

1f26fb3... by Lee Trager on 2019-04-23

Fix failing tests

diff --git a/src/maasserver/api/tests/test_vmfs_datastores.py b/src/maasserver/api/tests/test_vmfs_datastores.py
index 167d7bc..15f405a 100644
--- a/src/maasserver/api/tests/test_vmfs_datastores.py
+++ b/src/maasserver/api/tests/test_vmfs_datastores.py
@@ -15,9 +15,11 @@ from maasserver.enum import (
 )
 from maasserver.models.partition import MIN_PARTITION_SIZE
 from maasserver.models.partitiontable import PARTITION_TABLE_EXTRA_SPACE
+from maasserver.storage_layouts import VMFS6StorageLayout
 from maasserver.testing.api import APITestCase
 from maasserver.testing.factory import factory
 from maasserver.testing.matchers import HasStatusCode
+from maasserver.tests.test_storage_layouts import LARGE_BLOCK_DEVICE
 from maasserver.utils.converters import (
     human_readable_bytes,
     json_load_bytes,
@@ -89,7 +91,12 @@ class TestVMFSDatastoresAPI(APITestCase.ForUser):
 
     def test_POST_creates_with_block_devices_and_partitions(self):
         self.become_admin()
-        node = factory.make_Machine(status=NODE_STATUS.READY)
+        node = factory.make_Machine(
+            status=NODE_STATUS.READY, with_boot_disk=False)
+        node.boot_disk = factory.make_PhysicalBlockDevice(
+            node=node, size=LARGE_BLOCK_DEVICE)
+        layout = VMFS6StorageLayout(node)
+        layout.configure()
         block_devices = [
             factory.make_PhysicalBlockDevice(node=node)
             for _ in range(3)
diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
index 3a25954..021acdd 100644
--- a/src/maasserver/forms/__init__.py
+++ b/src/maasserver/forms/__init__.py
@@ -166,6 +166,7 @@ from maasserver.permissions import (
     NodePermission,
     ResourcePoolPermission,
 )
+from maasserver.storage_layouts import VMFS6StorageLayout
 from maasserver.utils.converters import machine_readable_bytes
 from maasserver.utils.forms import (
     compose_invalid_choice_text,
@@ -3793,6 +3794,17 @@ class CreateLogicalVolumeForm(Form):
 class CreateVMFSForm(CreateVolumeGroupForm):
     """For validating and saving a new VMFS group."""
 
+    def clean(self):
+        """Validate that the VMFS6 storage layout is applied."""
+        cleaned_data = super().clean()
+        vmfs_layout = VMFS6StorageLayout(self.node)
+        vmfs_bd = vmfs_layout.is_layout()
+        if vmfs_bd is None:
+            set_form_error(
+                self, 'VMFS6', 'VMFS Datastores may only be created after the '
+                'VMFS6 storage layout has been applied.')
+        return cleaned_data
+
     def save(self):
         """Persist the `VMFS` into the database."""
         block_devices = list(BlockDevice.objects.filter(
diff --git a/src/maasserver/forms/tests/test_vmfs.py b/src/maasserver/forms/tests/test_vmfs.py
index 233cc6c..ccff190 100644
--- a/src/maasserver/forms/tests/test_vmfs.py
+++ b/src/maasserver/forms/tests/test_vmfs.py
@@ -14,20 +14,32 @@ from maasserver.forms import (
 )
 from maasserver.models.blockdevice import MIN_BLOCK_DEVICE_SIZE
 from maasserver.models.partitiontable import PARTITION_TABLE_EXTRA_SPACE
+from maasserver.storage_layouts import VMFS6StorageLayout
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
+from maasserver.tests.test_storage_layouts import LARGE_BLOCK_DEVICE
+
+
+def make_Node_with_VMFS6_layout(*args, **kwargs):
+    """Create a node with the VMFS6 storage layout applied."""
+    kwargs['with_boot_disk'] = False
+    node = factory.make_Node(*args, **kwargs)
+    factory.make_PhysicalBlockDevice(node=node, size=LARGE_BLOCK_DEVICE)
+    layout = VMFS6StorageLayout(node)
+    layout.configure()
+    return node
 
 
 class TestCreateVMFSForm(MAASServerTestCase):
 
     def test_requires_fields(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         form = CreateVMFSForm(node, data={})
         self.assertFalse(form.is_valid(), form.errors)
         self.assertItemsEqual(['name'], form.errors.keys())
 
     def test_is_not_valid_if_invalid_uuid(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         block_device = factory.make_PhysicalBlockDevice(node=node)
         data = {
             'name': factory.make_name("name"),
@@ -41,7 +53,7 @@ class TestCreateVMFSForm(MAASServerTestCase):
         self.assertEqual({'uuid': ['Enter a valid value.']}, form._errors)
 
     def test_is_not_valid_missing_block_devices_and_partitions(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         data = {
             'name': factory.make_name('name'),
             'uuid': uuid.uuid4(),
@@ -57,7 +69,7 @@ class TestCreateVMFSForm(MAASServerTestCase):
                 ]}, form._errors)
 
     def test_is_not_valid_if_block_device_does_not_belong_to_node(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         block_device = factory.make_PhysicalBlockDevice()
         data = {
             'name': factory.make_name('name'),
@@ -75,7 +87,7 @@ class TestCreateVMFSForm(MAASServerTestCase):
                 ]}, form._errors)
 
     def test_is_not_valid_if_partition_does_not_belong_to_node(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         partition = factory.make_Partition()
         data = {
             'name': factory.make_name('name'),
@@ -92,8 +104,19 @@ class TestCreateVMFSForm(MAASServerTestCase):
                 'choices.' % partition.id,
                 ]}, form._errors)
 
+    def test_is_not_valid_if_vmfs_layout_is_not_applied(self):
+        node = factory.make_Node(with_boot_disk=False)
+        block_device = factory.make_PhysicalBlockDevice(node=node)
+        data = {
+            'name': factory.make_name('name'),
+            'block_devices': [block_device.id],
+        }
+        form = CreateVMFSForm(node, data=data)
+        self.assertFalse(form.is_valid())
+        self.assertIn('VMFS6', form.errors)
+
     def test_creates_volume_group_with_block_devices(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         block_devices = [
             factory.make_PhysicalBlockDevice(node=node)
             for _ in range(3)
@@ -117,7 +140,7 @@ class TestCreateVMFSForm(MAASServerTestCase):
             ])
 
     def test_creates_with_block_devices_by_name(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         block_devices = [
             factory.make_PhysicalBlockDevice(node=node)
             for _ in range(3)
@@ -142,7 +165,7 @@ class TestCreateVMFSForm(MAASServerTestCase):
             ])
 
     def test_creates_with_partitions(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         block_device = factory.make_PhysicalBlockDevice(
             node=node,
             size=(MIN_BLOCK_DEVICE_SIZE * 3) + PARTITION_TABLE_EXTRA_SPACE)
@@ -168,7 +191,7 @@ class TestCreateVMFSForm(MAASServerTestCase):
             [fs.get_parent().id for fs in vmfs.filesystems.all()])
 
     def test_creates_with_partitions_by_name(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         block_device = factory.make_PhysicalBlockDevice(
             node=node,
             size=(MIN_BLOCK_DEVICE_SIZE * 3) + PARTITION_TABLE_EXTRA_SPACE)
@@ -228,7 +251,7 @@ class TestUpdateVMFSForm(MAASServerTestCase):
         self.assertEqual(new_uuid, vmfs.uuid)
 
     def test_adds_block_device(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         vmfs = factory.make_VMFS(node=node)
         block_device = factory.make_PhysicalBlockDevice(node=node)
         data = {
@@ -243,7 +266,7 @@ class TestUpdateVMFSForm(MAASServerTestCase):
             part.get_effective_filesystem().filesystem_group_id)
 
     def test_adds_block_device_by_name(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         vmfs = factory.make_VMFS(node=node)
         block_device = factory.make_PhysicalBlockDevice(node=node)
         data = {
@@ -258,7 +281,7 @@ class TestUpdateVMFSForm(MAASServerTestCase):
             part.get_effective_filesystem().filesystem_group_id)
 
     def test_adds_partition(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         vmfs = factory.make_VMFS(node=node)
         block_device = factory.make_PhysicalBlockDevice(node=node)
         partition_table = factory.make_PartitionTable(
@@ -275,7 +298,7 @@ class TestUpdateVMFSForm(MAASServerTestCase):
             partition.get_effective_filesystem().filesystem_group.id)
 
     def test_adds_partition_by_name(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         vmfs = factory.make_VMFS(node=node)
         block_device = factory.make_PhysicalBlockDevice(node=node)
         partition_table = factory.make_PartitionTable(
@@ -292,7 +315,7 @@ class TestUpdateVMFSForm(MAASServerTestCase):
             partition.get_effective_filesystem().filesystem_group.id)
 
     def test_removes_partition(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         vmfs = factory.make_VMFS(node=node)
         block_device = factory.make_PhysicalBlockDevice(node=node)
         partition_table = factory.make_PartitionTable(
@@ -310,7 +333,7 @@ class TestUpdateVMFSForm(MAASServerTestCase):
         self.assertIsNone(partition.get_effective_filesystem())
 
     def test_removes_partition_by_name(self):
-        node = factory.make_Node()
+        node = make_Node_with_VMFS6_layout()
         vmfs = factory.make_VMFS(node=node)
         block_device = factory.make_PhysicalBlockDevice(node=node)
         partition_table = factory.make_PartitionTable(
diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index 84fcfa3..4225ce5 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -78,6 +78,7 @@ from maasserver.enum import (
     ALLOCATED_NODE_STATUSES,
     BMC_TYPE,
     FILESYSTEM_FORMAT_TYPE_CHOICES_DICT,
+    FILESYSTEM_GROUP_TYPE,
     FILESYSTEM_TYPE,
     INTERFACE_LINK_TYPE,
     INTERFACE_TYPE,
@@ -159,6 +160,7 @@ from maasserver.storage_layouts import (
     get_storage_layout_for_node,
     StorageLayoutError,
     StorageLayoutMissingBootDiskError,
+    VMFS6StorageLayout,
 )
 from maasserver.utils import synchronised
 from maasserver.utils.dns import validate_hostname
@@ -1367,6 +1369,22 @@ class Node(CleanSave, TimestampedModel):
         if not has_boot:
             issues.append(
                 "Specify a storage device to be able to deploy this node.")
+        if self.osystem == 'esxi':
+            # MAAS 2.6 added VMware ESXi storage support. To be backwards
+            # compatible with previous versions of MAAS deploying with a Linux
+            # layout is fine. In this case the default VMware ESXi storage
+            # layout is created with a datastore. If the user applied the VMFS
+            # storage layout a datastore must be defined as one will always be
+            # created.
+            vmfs_layout = VMFS6StorageLayout(self)
+            if vmfs_layout.is_layout() is not None:
+                fs_groups = self.virtualblockdevice_set.filter(
+                    filesystem_group__group_type=FILESYSTEM_GROUP_TYPE.VMFS6)
+                if not fs_groups.exists():
+                    issues.append(
+                        "A datastore must be defined when deploying "
+                        "VMware ESXi.")
+                    return issues
         # The remaining storage issue checks are only for Ubuntu, CentOS, and
         # RHEL. All other osystems storage isn't supported or in ESXi's case
         # we ignore unknown filesystems given.
diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
index 0c88692..a367a78 100644
--- a/src/maasserver/models/tests/test_node.py
+++ b/src/maasserver/models/tests/test_node.py
@@ -7201,6 +7201,17 @@ class TestNode_Start(MAASTransactionServerTestCase):
         layout.configure()
         self.assertItemsEqual([], node.storage_layout_issues())
 
+    def test_storage_layout_issues_is_invalid_no_datastore_on_esxi(self):
+        node = factory.make_Node(
+            osystem='esxi', distro_series='6.7', with_boot_disk=False)
+        factory.make_PhysicalBlockDevice(node=node, size=(100 * 1024 ** 3))
+        layout = VMFS6StorageLayout(node)
+        layout.configure()
+        node.virtualblockdevice_set.delete()
+        self.assertEqual(
+            ["A datastore must be defined when deploying VMware ESXi."],
+            node.storage_layout_issues())
+
     def test_storage_layout_issues_vmfs_not_esxi(self):
         node = factory.make_Node(
             osystem=random.choice(['ubuntu', 'centos', 'rhel']),
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index 352fab6..5257e2a 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -69,6 +69,7 @@ from maasserver.rbac import (
 from maasserver.storage_layouts import (
     get_applied_storage_layout_for_node,
     get_storage_layout_choices,
+    VMFS6StorageLayout,
 )
 from maasserver.testing.architecture import make_usable_architecture
 from maasserver.testing.factory import factory
@@ -81,6 +82,7 @@ from maasserver.testing.testcase import (
     MAASServerTestCase,
     MAASTransactionServerTestCase,
 )
+from maasserver.tests.test_storage_layouts import LARGE_BLOCK_DEVICE
 from maasserver.third_party_drivers import get_third_party_driver
 from maasserver.utils.converters import (
     human_readable_bytes,
@@ -2966,18 +2968,22 @@ class TestMachineHandler(MAASServerTestCase):
     def test_create_vmfs_datastore(self):
         user = factory.make_admin()
         handler = MachineHandler(user, {}, None)
-        node = factory.make_Node()
+        node = factory.make_Node(with_boot_disk=False)
+        node.boot_disk = factory.make_PhysicalBlockDevice(
+            node=node, size=LARGE_BLOCK_DEVICE)
+        layout = VMFS6StorageLayout(node)
+        layout.configure()
         bd_ids = [
             factory.make_PhysicalBlockDevice(node=node).id
             for _ in range(3)
         ]
         params = {
             'system_id': node.system_id,
-            'name': 'datastore1',
+            'name': 'datastore2',
             'block_devices': bd_ids,
         }
         handler.create_vmfs_datastore(params)
-        vbd = node.virtualblockdevice_set.get(name='datastore1')
+        vbd = node.virtualblockdevice_set.get(name='datastore2')
         vmfs = vbd.filesystem_group
         self.assertItemsEqual(
             bd_ids,