Merge ~ltrager/maas:lp1830303 into maas:master

[Lee Trager]

LP: #1830303 - Show the VMFS datastore mount point.

The UI and API now both show the VMFS datastore mount point. While VMware
ESXi's UI shows /vmfs/volumes/<UUID> MAAS shows
/vmfs/volumes/<datastore name>. This is because VMware ESXi does not allow
you to specify the UUID so there is no way to know what it is. On a deployed
VMware ESXi system /vmfs/volumes/<datastore name> is a symbolic link to
/vmfs/volumes/<UUID>.

[Anthony Dillon (ya-bo-ng)]

Lee, used_for is also used in the disks-partitions.html L886.

[Martin Storey (cassiocassio)]

Lee: if the user copies that path that we are able to show in the GUI

- does is reliably point to the real path post deploy? can they e.g. use it in scripts etc?

- is there any way, or any value in extracting the real path back into the MAAS UI post-deploy? That's not currently MAAS's business, right?

[Lee Trager (ltrager)]

Thanks for the review. I've fixed the remaining sections which contained used_for === "VMFS Datastore" and verified all messages, options are working.

What this change does is add a filesystem on top of the datastore which properly defines where the datastore will be mounted. The current path given(e.g /dev/disk/by-dname/sda) is the Linux block device path and does not exist in VMware. The new path(/vmfs/volumes/<datastore name>) does exist on a deployed ESXi host so scripts can use that path. There currently isn't a way for storage data to be sent back post-deployment but the new path is valid.

[Blake Rouse (blake-rouse)]

Looks good.

I like the addition of filesystem on top of the datastore, because that is true it is a filesystem.

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b lp1830303 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/5772/consoleText

5e0d730... by Lee Trager on 2019-05-28

Fix failing test

diff --git a/src/maasserver/api/tests/test_vmfs_datastores.py b/src/maasserver/api/tests/test_vmfs_datastores.py
index 15f405a..4eed96d 100644
--- a/src/maasserver/api/tests/test_vmfs_datastores.py
+++ b/src/maasserver/api/tests/test_vmfs_datastores.py
@@ -13,6 +13,7 @@ from maasserver.enum import (
     FILESYSTEM_GROUP_TYPE,
     NODE_STATUS,
 )
+from maasserver.models.filesystemgroup import VMFS
 from maasserver.models.partition import MIN_PARTITION_SIZE
 from maasserver.models.partitiontable import PARTITION_TABLE_EXTRA_SPACE
 from maasserver.storage_layouts import VMFS6StorageLayout
@@ -149,7 +150,9 @@ class TestVMFSDatastoreAPI(APITestCase.ForUser):
             self.get_vmfs_uri(vmfs))
 
     def test_GET(self):
-        vmfs = factory.make_VMFS()
+        part = factory.make_Partition()
+        name = factory.make_name('datastore')
+        vmfs = VMFS.objects.create_vmfs(name, [part])
 
         response = self.client.get(self.get_vmfs_uri(vmfs))
         self.assertThat(response, HasStatusCode(http.client.OK))
@@ -162,6 +165,10 @@ class TestVMFSDatastoreAPI(APITestCase.ForUser):
             'name': Equals(vmfs.name),
             'size': Equals(vmfs.get_size()),
             'human_size': Equals(human_readable_bytes(vmfs.get_size())),
+            'filesystem': Equals({
+                'fstype': 'vmfs6',
+                'mount_point': '/vmfs/volumes/%s' % name,
+            }),
         }))
         self.assertEquals(
             vmfs.filesystems.count(), len(parsed_result['devices']))
diff --git a/src/maasserver/api/vmfs_datastores.py b/src/maasserver/api/vmfs_datastores.py
index 07bc76c..e8c6a9a 100644
--- a/src/maasserver/api/vmfs_datastores.py
+++ b/src/maasserver/api/vmfs_datastores.py
@@ -34,6 +34,7 @@ DISPLAYED_VMFS_DATASTORE_FIELDS = (
     'devices',
     'size',
     'human_size',
+    'filesystem',
 )
 
 
@@ -160,6 +161,19 @@ class VmfsDatastoreHandler(OperationsHandler):
             for filesystem in vmfs.filesystems.all()
         ]
 
+    @classmethod
+    def filesystem(cls, vmfs):
+        # XXX: This is almost the same as
+        # m.api.partitions.PartitionHandler.filesystem.
+        filesystem = vmfs.virtual_device.get_effective_filesystem()
+        if filesystem is not None:
+            return {
+                'fstype': filesystem.fstype,
+                'mount_point': filesystem.mount_point,
+            }
+        else:
+            return None
+
     def read(self, request, system_id, id):
         """@description-title Read a VMFS datastore.
         @description Read a VMFS datastore with the given id on the machine
diff --git a/src/maasserver/models/filesystemgroup.py b/src/maasserver/models/filesystemgroup.py
index 38edce7..3993d8e 100644
--- a/src/maasserver/models/filesystemgroup.py
+++ b/src/maasserver/models/filesystemgroup.py
@@ -292,6 +292,9 @@ class VMFSManager(BaseFilesystemGroupManager):
                 fstype=FILESYSTEM_TYPE.VMFS6, partition=partition,
                 filesystem_group=vmfs)
         vmfs.save(force_update=True)
+        vmfs.virtual_device.filesystem_set.create(
+            fstype=FILESYSTEM_TYPE.VMFS6,
+            mount_point='/vmfs/volumes/%s' % name)
         return vmfs
 
 
diff --git a/src/maasserver/models/tests/test_filesystemgroup.py b/src/maasserver/models/tests/test_filesystemgroup.py
index c815abf..ce2cb7d 100644
--- a/src/maasserver/models/tests/test_filesystemgroup.py
+++ b/src/maasserver/models/tests/test_filesystemgroup.py
@@ -32,6 +32,7 @@ from maasserver.models.filesystemgroup import (
     RAID,
     RAID_SUPERBLOCK_OVERHEAD,
     RAIDManager,
+    VMFS,
     VolumeGroup,
     VolumeGroupManager,
 )
@@ -878,6 +879,14 @@ class TestFilesystemGroup(MAASServerTestCase):
         vmfs = factory.make_VMFS()
         self.assertTrue(vmfs.is_vmfs())
 
+    def test_creating_vmfs_automatically_creates_mounted_fs(self):
+        part = factory.make_Partition()
+        name = factory.make_name('datastore')
+        vmfs = VMFS.objects.create_vmfs(name, [part])
+        self.assertEquals(
+            '/vmfs/volumes/%s' % name,
+            vmfs.virtual_device.get_effective_filesystem().mount_point)
+
     def test_can_save_new_filesystem_group_without_filesystems(self):
         fsgroup = FilesystemGroup(
             group_type=FILESYSTEM_GROUP_TYPE.LVM_VG,
diff --git a/src/maasserver/static/js/angular/controllers/node_details_storage.js b/src/maasserver/static/js/angular/controllers/node_details_storage.js
index 749abca..440ff2b 100644
--- a/src/maasserver/static/js/angular/controllers/node_details_storage.js
+++ b/src/maasserver/static/js/angular/controllers/node_details_storage.js
@@ -48,7 +48,7 @@ export function removeAvailableByNew() {
 export function datastoresOnly() {
   return function(filesystems) {
     return filesystems.filter(filesystem => {
-      return filesystem.used_for === "VMFS Datastore";
+      return filesystem.parent_type == "vmfs6";
     });
   };
 }
@@ -2396,7 +2396,7 @@ export function NodeStorageController(
   // Returns warning text based on number of datastores
   $scope.getRemoveDatastoreWarningText = function(disks) {
     var datastores = disks.filter(function(disk) {
-      return disk.used_for === "VMFS Datastore";
+      return disk.parent_type === "vmfs6";
     });
     var warningText = "Are you sure you want to remove this datastore?";
 
diff --git a/src/maasserver/static/js/angular/controllers/tests/test_node_details_storage.js b/src/maasserver/static/js/angular/controllers/tests/test_node_details_storage.js
index 4428c2b..0b4799c 100644
--- a/src/maasserver/static/js/angular/controllers/tests/test_node_details_storage.js
+++ b/src/maasserver/static/js/angular/controllers/tests/test_node_details_storage.js
@@ -5384,14 +5384,12 @@ describe("NodeStorageController", function() {
       makeController();
       var disks = [
         {
-          used_for: "VMFS Datastore"
+          parent_type: "vmfs6"
         },
         {
-          used_for: "VMFS Datastore"
+          parent_type: "vmfs6"
         },
-        {
-          used_for: "GPT partitioned with 1 partition"
-        }
+        {}
       ];
       expect($scope.getRemoveDatastoreWarningText(disks)).toBe(
         "Are you sure you want to remove this datastore?"
@@ -5402,11 +5400,9 @@ describe("NodeStorageController", function() {
       makeController();
       var disks = [
         {
-          used_for: "VMFS Datastore"
+          parent_type: "vmfs6"
         },
-        {
-          used_for: "GPT partitioned with 1 partition"
-        }
+        {}
       ];
       expect($scope.getRemoveDatastoreWarningText(disks)).toBe(
         "Are you sure you want to remove this datastore? " +
diff --git a/src/maasserver/static/partials/nodedetails/storage/datastores.html b/src/maasserver/static/partials/nodedetails/storage/datastores.html
index 26cea59..0308eaf 100644
--- a/src/maasserver/static/partials/nodedetails/storage/datastores.html
+++ b/src/maasserver/static/partials/nodedetails/storage/datastores.html
@@ -20,11 +20,11 @@
           No datastores defined.
         </td>
       </tr>
-      <tr class="p-table__row" data-ng-repeat="filesystem in node.disks" data-ng-class="{ 'is-active': filesystem.$selected }" data-ng-if="filesystem.used_for == 'VMFS Datastore'">
+      <tr class="p-table__row" data-ng-repeat="filesystem in node.disks" data-ng-class="{ 'is-active': filesystem.$selected }" data-ng-if="filesystem.parent_type == 'vmfs6'">
         <td role="gridcell" class="p-table__cell" aria-label="Name" title="{$ filesystem.name $}">{$ filesystem.name $}</td>
         <td role="gridcell" class="p-table__cell" aria-label="Filesystem" title="VMFS6">VMFS6</td>
         <td role="gridcell" class="p-table__cell" aria-label="Size" title="{$ filesystem.size_human $}">{$ filesystem.size_human $}</td>
-        <td role="gridcell" class="p-table__cell" aria-label="Mount point" title="{$ filesystem.path $}">{$ filesystem.path $}</td>
+        <td role="gridcell" class="p-table__cell" aria-label="Mount point" title="{$ filesystem.filesystem.mount_point $}">{$ filesystem.filesystem.mount_point $}</td>
         <td role="gridcell" class="p-table__cell p-table--action-cell u-align--right">
           <div class="p-contextual-menu" toggle-ctrl data-ng-if="!isAllStorageDisabled()">
             <button class="p-button--base p-contextual-menu__toggle" aria-controls="#{$ item.name $}-menu"
@@ -108,4 +108,4 @@
       </tr>
     </tbody>
   </table>
-</div>
\ No newline at end of file
+</div>
diff --git a/src/maasserver/static/partials/nodedetails/storage/disks-partitions.html b/src/maasserver/static/partials/nodedetails/storage/disks-partitions.html
index 4661058..6100bbc 100644
--- a/src/maasserver/static/partials/nodedetails/storage/disks-partitions.html
+++ b/src/maasserver/static/partials/nodedetails/storage/disks-partitions.html
@@ -848,10 +848,10 @@
                                 <p>{$ datastores.new.filesystem $}</p>
                             </div>
                         </div>
-                        <div class="p-form__group" data-ng-if="datastores.new.path">
+                        <div class="p-form__group" data-ng-if="datastores.new.filesystem.mount_point">
                             <label for="datastore-mountpoint" class="p-form__label">Mount point</label>
                             <div class="p-form__control">
-                                <p>{$ datastores.new.path $}</p>
+                                <p>{$ datastores.new.filesystem.mount_point $}</p>
                             </div>
                         </div>
                     </div>
@@ -895,7 +895,7 @@
                     <div class="p-form__group">
                         <label for="datastore-mountpoint" class="p-form__label">Mount point</label>
                         <div class="p-form__control">
-                            <p>{$ datastores.old.path $}</p>
+                            <p>{$ datastores.old.filesystem.mount_point $}</p>
                         </div>
                     </div>
                 </div>
diff --git a/src/maasserver/tests/test_preseed_storage.py b/src/maasserver/tests/test_preseed_storage.py
index 2fb4771..0cec35b 100644
--- a/src/maasserver/tests/test_preseed_storage.py
+++ b/src/maasserver/tests/test_preseed_storage.py
@@ -2194,6 +2194,24 @@ class TestVMFS(MAASServerTestCase, AssertStorageConfigMixin):
             type: vmfs6
             devices:
               - sdc-part1
+          - id: datastore1_format
+            type: format
+            fstype: vmfs6
+            volume: datastore1
+            label: null
+          - id: datastore2_format
+            type: format
+            fstype: vmfs6
+            volume: datastore2
+            label: null
+          - id: datastore1_mount
+            type: mount
+            device: datastore1_format
+            path: /vmfs/volumes/datastore1
+          - id: datastore2_mount
+            type: mount
+            device: datastore2_format
+            path: /vmfs/volumes/datastore2
     """)
 
     def test__renders_expected_output(self):
diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py
index f9fd714..6334ece 100644
--- a/src/maasserver/websockets/handlers/node.py
+++ b/src/maasserver/websockets/handlers/node.py
@@ -334,17 +334,7 @@ class NodeHandler(TimestampedModelHandler):
                 # UI knows that these partitions are in use.
                 if detected_layout == "vmfs6":
                     for disk in data["disks"]:
-                        if disk.get("parent", {}).get("type") == "vmfs6":
-                            disk["used_for"] = "VMFS Datastore"
-                            disk["filesystem"] = {
-                                "id": -1,
-                                "label": "RESERVED",
-                                "mount_point": "RESERVED",
-                                "mount_options": None,
-                                "fstype": None,
-                                "is_format_fstype": False
-                            }
-                        elif disk["id"] == layout_bd.id:
+                        if disk["id"] == layout_bd.id:
                             for partition in disk["partitions"]:
                                 partition["used_for"] = (
                                     "VMware ESXi OS partition")
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index 53b12b5..8f295c5 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -745,16 +745,6 @@ class TestMachineHandler(MAASServerTestCase):
                         "fstype": None,
                         "is_format_fstype": False,
                         }, partition["filesystem"])
-            else:
-                self.assertEquals("VMFS Datastore", disk["used_for"])
-                self.assertDictEqual({
-                    "id": -1,
-                    "label": "RESERVED",
-                    "mount_point": "RESERVED",
-                    "mount_options": None,
-                    "fstype": None,
-                    "is_format_fstype": False,
-                    }, disk["filesystem"])
 
     def test_dehydrate_power_parameters_returns_None_when_empty(self):
         owner = factory.make_User()