Merge ~ltrager/maas:vmfs_websocket into maas:master

[Lee Trager]

Add VMFS websocket operations

Create, update, and delete websocket operations have been added for VMFS
datastores. Additionally a new websocket operation, apply_storage_layout,
allows the websocket so select a storage layout for an individual machine.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b vmfs_websocket lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 138afd9cbc2ce582abccfd7672a51678b3af9cb6

review: Approve

[Björn Tillenius (bjornt)]

The code itself looks good. But I'd like to see some more testing being added. First of all, for all the methods, there should be a test showing that a non-admin user can't use them.

But I also added comments for parts of the code that are still untested.

+1, assuming that you'll add those tests.

review: Approve

91efe01... by Lee Trager on 2019-03-20

Merge branch 'master' into vmfs_websocket

6484175... by Lee Trager on 2019-03-21

Add tests for error cases

[Newell Jensen (newell-jensen)]

Looks good. All tests look to be there as well. +1

review: Approve

diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py
index 8a2b95f..754f5df 100644
--- a/src/maasserver/websockets/handlers/machine.py
+++ b/src/maasserver/websockets/handlers/machine.py
@@ -10,7 +10,10 @@ __all__ = [
 from functools import partial
 from operator import itemgetter
 
-from django.core.exceptions import ValidationError
+from django.core.exceptions import (
+    ObjectDoesNotExist,
+    ValidationError,
+)
 from maasserver.enum import (
     BMC_TYPE,
     INTERFACE_LINK_TYPE,
@@ -30,11 +33,13 @@ from maasserver.forms import (
     CreateCacheSetForm,
     CreateLogicalVolumeForm,
     CreateRaidForm,
+    CreateVMFSForm,
     CreateVolumeGroupForm,
     FormatBlockDeviceForm,
     FormatPartitionForm,
     UpdatePhysicalBlockDeviceForm,
     UpdateVirtualBlockDeviceForm,
+    UpdateVMFSForm,
 )
 from maasserver.forms.filesystem import (
     MountFilesystemForm,
@@ -64,9 +69,15 @@ from maasserver.models.partition import Partition
 from maasserver.models.subnet import Subnet
 from maasserver.node_action import compile_node_actions
 from maasserver.permissions import NodePermission
+from maasserver.storage_layouts import (
+    StorageLayoutError,
+    StorageLayoutForm,
+    StorageLayoutMissingBootDiskError,
+)
 from maasserver.utils.orm import transactional
 from maasserver.utils.threads import deferToDatabase
 from maasserver.websockets.base import (
+    HandlerDoesNotExistError,
     HandlerError,
     HandlerPermissionError,
     HandlerValidationError,
@@ -138,13 +149,17 @@ class MachineHandler(NodeHandler):
             'delete_volume_group',
             'delete_cache_set',
             'delete_filesystem',
+            'delete_vmfs_datastore',
+            'update_vmfs_datastore',
             'create_partition',
             'create_cache_set',
             'create_bcache',
             'create_raid',
             'create_volume_group',
             'create_logical_volume',
+            'create_vmfs_datastore',
             'set_boot_disk',
+            'apply_storage_layout',
             'default_user',
             'get_summary_xml',
             'get_summary_yaml',
@@ -620,6 +635,34 @@ class MachineHandler(NodeHandler):
             fs = Filesystem.objects.get(partition=partition, id=filesystem_id)
         fs.delete()
 
+    def _get_vmfs_datastore(self, params):
+        """Get the VMFS datastore from the given system_id and id."""
+        node = self._get_node_or_permission_error(
+            params, permission=self._meta.edit_permission)
+        vmfs_datastore_id = params.get('vmfs_datastore_id')
+        try:
+            vbd = node.virtualblockdevice_set.get(
+                filesystem_group_id=vmfs_datastore_id)
+        except ObjectDoesNotExist:
+            raise HandlerDoesNotExistError(vmfs_datastore_id)
+        if not vbd.filesystem_group:
+            raise HandlerDoesNotExistError(vmfs_datastore_id)
+        return vbd.filesystem_group
+
+    def delete_vmfs_datastore(self, params):
+        """Delete a VMFS datastore."""
+        vmfs = self._get_vmfs_datastore(params)
+        vmfs.delete()
+
+    def update_vmfs_datastore(self, params):
+        """Add or remove block devices or partitions from a datastore."""
+        vmfs = self._get_vmfs_datastore(params)
+        form = UpdateVMFSForm(vmfs, data=params)
+        if not form.is_valid():
+            raise HandlerError(form.errors)
+        else:
+            form.save()
+
     def create_partition(self, params):
         """Create a partition."""
         node = self._get_node_or_permission_error(
@@ -744,6 +787,16 @@ class MachineHandler(NodeHandler):
                 logical_volume, params.get("fstype"),
                 params.get("mount_point"), params.get("mount_options"))
 
+    def create_vmfs_datastore(self, params):
+        """Create a VMFS datastore."""
+        node = self._get_node_or_permission_error(
+            params, permission=self._meta.edit_permission)
+        form = CreateVMFSForm(node, data=params)
+        if not form.is_valid():
+            raise HandlerError(form.errors)
+        else:
+            form.save()
+
     def set_boot_disk(self, params):
         """Set the disk as the boot disk."""
         node = self._get_node_or_permission_error(
@@ -756,6 +809,25 @@ class MachineHandler(NodeHandler):
         node.boot_disk = device
         node.save()
 
+    def apply_storage_layout(self, params):
+        """Apply the specified storage layout."""
+        node = self._get_node_or_permission_error(
+            params, permission=self._meta.edit_permission)
+        form = StorageLayoutForm(required=True, data=params)
+        if not form.is_valid():
+            raise HandlerError(form.errors)
+        storage_layout = params.get('storage_layout')
+        try:
+            node.set_storage_layout(storage_layout)
+        except StorageLayoutMissingBootDiskError:
+            raise HandlerError(
+                "Machine is missing a boot disk; no storage layout can be "
+                "applied.")
+        except StorageLayoutError as e:
+            raise HandlerError(
+                "Failed to configure storage layout '%s': %s" % (
+                    storage_layout, str(e)))
+
     def action(self, params):
         """Perform the action on the object."""
         # `compile_node_actions` handles the permission checking internally
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index ea646ca..7c91872 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -66,6 +66,7 @@ from maasserver.rbac import (
     FakeRBACClient,
     rbac,
 )
+from maasserver.storage_layouts import get_storage_layout_choices
 from maasserver.testing.architecture import make_usable_architecture
 from maasserver.testing.factory import factory
 from maasserver.testing.fixtures import (
@@ -2432,6 +2433,78 @@ class TestMachineHandler(MAASServerTestCase):
         self.assertRaises(
             HandlerPermissionError, handler.delete_filesystem, params)
 
+    def test_delete_vmfs_datastore(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node()
+        vmfs = factory.make_VMFS(node=node)
+        params = {
+            'system_id': node.system_id,
+            'vmfs_datastore_id': vmfs.id,
+        }
+        handler.delete_vmfs_datastore(params)
+        self.assertIsNone(reload_object(vmfs))
+
+    def test_delete_vmfs_datastore_invalid_id(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node()
+        vmfs = factory.make_VMFS(node=node)
+        params = {
+            'system_id': node.system_id,
+            'vmfs_datastore_id': 999,
+        }
+        self.assertRaises(
+            HandlerDoesNotExistError,
+            handler.delete_vmfs_datastore, params)
+        self.assertIsNotNone(reload_object(vmfs))
+
+    def test_update_vmfs_datastore(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node()
+        vmfs = factory.make_VMFS(node=node)
+        block_device = factory.make_PhysicalBlockDevice(node=node)
+        partition_table = factory.make_PartitionTable(
+            block_device=block_device)
+        partition = factory.make_Partition(partition_table=partition_table)
+        factory.make_Filesystem(
+            fstype=FILESYSTEM_TYPE.LVM_PV, partition=partition,
+            filesystem_group=vmfs)
+        params = {
+            'system_id': node.system_id,
+            'vmfs_datastore_id': vmfs.id,
+            'remove_partitions': [partition.id],
+        }
+        handler.update_vmfs_datastore(params)
+        self.assertIsNone(partition.get_effective_filesystem())
+
+    def test_update_vmfs_datastore_invalid_id(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node()
+        vmfs = factory.make_VMFS(node=node)
+        params = {
+            'system_id': node.system_id,
+            'vmfs_datastore_id': 999,
+        }
+        self.assertRaises(
+            HandlerDoesNotExistError,
+            handler.update_vmfs_datastore, params)
+        self.assertIsNotNone(reload_object(vmfs))
+
+    def test_update_vmfs_datastore_raises_errors(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node()
+        vmfs = factory.make_VMFS(node=node)
+        params = {
+            'system_id': node.system_id,
+            'vmfs_datastore_id': vmfs.id,
+            'remove_partitions': [999],
+        }
+        self.assertRaises(HandlerError, handler.update_vmfs_datastore, params)
+
     def test_create_partition(self):
         user = factory.make_admin()
         handler = MachineHandler(user, {}, None)
@@ -2881,6 +2954,37 @@ class TestMachineHandler(MAASServerTestCase):
         self.assertRaises(
             HandlerPermissionError, handler.create_logical_volume, params)
 
+    def test_create_vmfs_datastore(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node()
+        bd_ids = [
+            factory.make_PhysicalBlockDevice(node=node).id
+            for _ in range(3)
+        ]
+        params = {
+            'system_id': node.system_id,
+            'name': 'datastore1',
+            'block_devices': bd_ids,
+        }
+        handler.create_vmfs_datastore(params)
+        vbd = node.virtualblockdevice_set.get(name='datastore1')
+        vmfs = vbd.filesystem_group
+        self.assertItemsEqual(
+            bd_ids,
+            [
+                fs.get_parent().partition_table.block_device.id
+                for fs in vmfs.filesystems.all()
+            ])
+
+    def test_create_vmfs_datastore_raises_errors(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node()
+        self.assertRaises(
+            HandlerError, handler.create_vmfs_datastore,
+            {'system_id': node.system_id})
+
     def test_set_boot_disk(self):
         user = factory.make_admin()
         handler = MachineHandler(user, {}, None)
@@ -2917,6 +3021,54 @@ class TestMachineHandler(MAASServerTestCase):
         self.assertRaises(
             HandlerPermissionError, handler.set_boot_disk, params)
 
+    def test_apply_storage_layout(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node(with_boot_disk=False)
+        node.boot_disk = factory.make_PhysicalBlockDevice(
+            node=node, size=10 * 1024 ** 3)
+        factory.make_PhysicalBlockDevice(node=node, size=10 * 2024 ** 3)
+        params = {
+            'system_id': node.system_id,
+            'storage_layout': factory.pick_choice(get_storage_layout_choices())
+        }
+        handler.apply_storage_layout(params)
+        self.assertTrue(node.boot_disk.partitiontable_set.exists())
+
+    def test_apply_storage_layout_validates_layout_name(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node()
+        params = {
+            'system_id': node.system_id,
+            'storage_layout': factory.make_name('storage_layout'),
+        }
+        self.assertRaises(
+            HandlerError, handler.apply_storage_layout, params)
+
+    def test_apply_storage_layout_raises_missing_boot_disk_error(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node(with_boot_disk=False)
+        params = {
+            'system_id': node.system_id,
+            'storage_layout': factory.pick_choice(get_storage_layout_choices())
+        }
+        self.assertRaises(
+            HandlerError, handler.apply_storage_layout, params)
+
+    def test_apply_storage_layout_raises_errors(self):
+        user = factory.make_admin()
+        handler = MachineHandler(user, {}, None)
+        node = factory.make_Node(with_boot_disk=False)
+        factory.make_PhysicalBlockDevice(size=1024 ** 3)
+        params = {
+            'system_id': node.system_id,
+            'storage_layout': factory.pick_choice(get_storage_layout_choices())
+        }
+        self.assertRaises(
+            HandlerError, handler.apply_storage_layout, params)
+
     def test_update_raise_HandlerError_if_tag_has_definition(self):
         user = factory.make_admin()
         handler = MachineHandler(user, {}, None)