Dell PowerStore and PowerFlex Manila drivers enhancement

Addressed unresolved comments for Dell PowerStore Manila Driver and PowerFlex Manila Driver. 1. Updated Dell PowerFlex Driver configuration guide. 2. Improved code style. Implements: blueprint dell-powerstore-manila-driver Change-Id: I744279b77226a097f774d3ae0f9a77b6524db1d0
2023-09-01 04:26:31 +00:00 · 2023-09-01 04:26:31 +00:00 · bd471d5d9a
commit bd471d5d9a
parent 0b70c0de11
4 changed files with 44 additions and 62 deletions
--- a/doc/source/configuration/shared-file-systems/drivers/dell-emc-powerflex-driver.rst
+++ b/doc/source/configuration/shared-file-systems/drivers/dell-emc-powerflex-driver.rst
@ -141,13 +141,13 @@ creating share type:

 .. code-block:: console

-    $ openstack share type create --extra_specs snapshot_support=True ${share_type_name} False
+    $ openstack share type create --extra_specs snapshot_support=True powerflex False

 Or you can update already existing share type with command:

 .. code-block:: console

-    $ openstack share type set --extra_specs snapshot_support=True ${share_type_name}
+    $ openstack share type set --extra_specs snapshot_support=True powerflex


 Known restrictions
--- a/manila/share/drivers/dell_emc/plugins/powerflex/connection.py
+++ b/manila/share/drivers/dell_emc/plugins/powerflex/connection.py
@ -355,6 +355,7 @@ class PowerFlexStorageConnection(driver.StorageConnection):
            self.protection_domain,
            self.storage_pool
            )
+        total = free = used = provisioned = 0
        statistic = self.manager.get_storage_pool_statistic(storage_pool_id)
        if statistic:
            total = statistic.get('maxCapacityInKb') // units.Mi
--- a/manila/share/drivers/dell_emc/plugins/powerflex/object_manager.py
+++ b/manila/share/drivers/dell_emc/plugins/powerflex/object_manager.py
@ -184,7 +184,7 @@ class StorageObjectManager(object):
            "storage_pool_id": storage_pool_id,
            "nas_server_id": nas_server_id
            }
-        url = self.base_url + '/v1/file-systems'
+        url = f'{self.base_url}/v1/file-systems'
        res, response = self.execute_powerflex_post_request(url, params)
        if res.status_code == 201:
            return response["id"]
@ -202,7 +202,7 @@ class StorageObjectManager(object):
            "path": "/" + str(name),
            "name": name
            }
-        url = self.base_url + '/v1/nfs-exports'
+        url = f'{self.base_url}/v1/nfs-exports'
        res, response = self.execute_powerflex_post_request(url, params)
        if res.status_code == 201:
            return response["id"]
@ -213,9 +213,7 @@ class StorageObjectManager(object):
        :param filesystem_id: ID of the filesystem to delete
        :return: True if deleted successfully
        """
-        url = self.base_url + \
-            '/v1/file-systems/' + \
-            filesystem_id
+        url = f'{self.base_url}/v1/file-systems/{filesystem_id}'
        res = self.execute_powerflex_delete_request(url)
        return res.status_code == 204

@ -229,10 +227,7 @@ class StorageObjectManager(object):
        params = {
            "name": name
            }
-        url = self.base_url + \
-            '/v1/file-systems/' + \
-            filesystem_id + \
-            '/snapshot'
+        url = f'{self.base_url}/v1/file-systems/{filesystem_id}/snapshot'
        res, response = self.execute_powerflex_post_request(url, params)
        return res.status_code == 201

@ -242,9 +237,7 @@ class StorageObjectManager(object):
        :param nas_server: NAS server name
        :return: ID of the NAS server if success
        """
-        url = self.base_url + \
-            '/v1/nas-servers?select=id&name=eq.' + \
-            nas_server
+        url = f'{self.base_url}/v1/nas-servers?select=id&name=eq.{nas_server}'
        res, response = self.execute_powerflex_get_request(url)
        if res.status_code == 200:
            return response[0]['id']
@ -255,7 +248,7 @@ class StorageObjectManager(object):
        :param export_id: ID of the NFS export
        :return: path of the NFS export if success
        """
-        url = self.base_url + '/v1/nfs-exports/' + export_id + '?select=*'
+        url = f'{self.base_url}/v1/nfs-exports/{export_id}?select=*'
        res, response = self.execute_powerflex_get_request(url)
        if res.status_code == 200:
            return response["name"]
@ -266,9 +259,7 @@ class StorageObjectManager(object):
        :param name: name of the filesystem
        :return: ID of the filesystem if success
        """
-        url = self.base_url + \
-            '/v1/file-systems?select=id&name=eq.' + \
-            name
+        url = f'{self.base_url}/v1/file-systems?select=id&name=eq.{name}'
        res, response = self.execute_powerflex_get_request(url)
        if res.status_code == 200:
            return response[0]['id']
@ -279,9 +270,7 @@ class StorageObjectManager(object):
        :param name: name of the NFS export
        :return: id of the NFS export if success
        """
-        url = self.base_url + \
-            '/v1/nfs-exports?select=id&name=eq.' + \
-            name
+        url = f'{self.base_url}/v1/nfs-exports?select=id&name=eq.{name}'
        res, response = self.execute_powerflex_get_request(url)
        if res.status_code == 200:
            return response[0]['id']
@ -297,8 +286,8 @@ class StorageObjectManager(object):
            "protectionDomainName": protection_domain,
            "name": storage_pool
            }
-        url = self.host_url + \
-            '/api/types/StoragePool/instances/action/queryIdByKey'
+        url = (f'{self.host_url}/api/types/StoragePool/instances/'
+               'action/queryIdByKey')
        res, response = self.execute_powerflex_post_request(url, params)
        if res.status_code == 200:
            return response
@ -315,9 +304,7 @@ class StorageObjectManager(object):
            "read_only_hosts": list(ro_hosts),
            "read_write_root_hosts": list(rw_hosts)
            }
-        url = self.base_url + \
-            '/v1/nfs-exports/' + \
-            export_id
+        url = f'{self.base_url}/v1/nfs-exports/{export_id}'
        res = self.execute_powerflex_patch_request(url, params)
        return res.status_code == 204

@ -331,9 +318,7 @@ class StorageObjectManager(object):
        params = {
            "size_total": new_size
            }
-        url = self.base_url + \
-            '/v1/file-systems/' + \
-            export_id
+        url = f'{self.base_url}/v1/file-systems/{export_id}'
        res = self.execute_powerflex_patch_request(url, params)
        return res.status_code == 204

@ -343,9 +328,8 @@ class StorageObjectManager(object):
        :param name: name of the export
        :return: ID of the Filesystem which owns the export
        """
-        url = self.base_url + \
-            '/v1/nfs-exports?select=file_system_id&name=eq.' + \
-            name
+        url = (f'{self.base_url}/v1/nfs-exports'
+               f'?select=file_system_id&name=eq.{name}')
        res, response = self.execute_powerflex_get_request(url)
        if res.status_code == 200:
            return response[0]['file_system_id']
@ -356,9 +340,8 @@ class StorageObjectManager(object):
        :param snapshot_name: Name of the snapshot
        :return: ID of the parent filesystem of the snapshot
        """
-        url = self.base_url + \
-            '/v1/file-systems?select=id&name=eq.' + \
-            snapshot_name
+        url = (f'{self.base_url}/v1/file-systems'
+               f'?select=id&name=eq.{snapshot_name}')
        res, response = self.execute_powerflex_get_request(url)
        if res.status_code == 200:
            return response[0]['id']
@ -369,9 +352,7 @@ class StorageObjectManager(object):
        :param storage_pool_id: ID of the storage pool
        :return: Spare capacity percentage of the storage pool
        """
-        url = self.host_url + \
-            '/api/instances/StoragePool::' + \
-            storage_pool_id
+        url = f'{self.host_url}/api/instances/StoragePool::{storage_pool_id}'
        res, response = self.execute_powerflex_get_request(url)
        if res.status_code == 200:
            return response['sparePercentage']
@ -382,9 +363,8 @@ class StorageObjectManager(object):
        :param storage_pool_id: ID of the storage pool
        :return: Statistics of the storage pool
        """
-        url = self.host_url + \
-            '/api/instances/StoragePool::' + \
-            storage_pool_id + '/relationships/Statistics'
+        url = (f'{self.host_url}/api/instances/StoragePool::{storage_pool_id}'
+               '/relationships/Statistics')
        res, response = self.execute_powerflex_get_request(url)
        if res.status_code == 200:
            statistics = {
@ -401,9 +381,8 @@ class StorageObjectManager(object):
        :param nas_server_id: ID of the NAS server
        :return: file interfaces of the NAS server
        """
-        url = self.base_url + \
-            '/v1/file-interfaces?select=ip_address&nas_server_id=eq.' + \
-            nas_server_id
+        url = (f'{self.base_url}/v1/file-interfaces'
+               f'?select=ip_address&nas_server_id=eq.{nas_server_id}')
        res, response = self.execute_powerflex_get_request(url)
        if res.status_code == 200:
            return [i['ip_address'] for i in response]
--- a/manila/share/drivers/dell_emc/plugins/powerstore/connection.py
+++ b/manila/share/drivers/dell_emc/plugins/powerstore/connection.py
@ -181,12 +181,12 @@ class PowerStoreStorageConnection(driver.StorageConnection):
        return locations

    def _create_share_NFS_CIFS(self, nas_server_id, filesystem_id, share_name,
-                               protocal):
+                               protocol):
        LOG.debug(f"Get file interfaces of {nas_server_id}")
        file_interfaces = self.client.get_nas_server_interfaces(
            nas_server_id)
-        LOG.debug(f"Creating {protocal} export {share_name}")
-        if protocal == 'NFS':
+        LOG.debug(f"Creating {protocol} export {share_name}")
+        if protocol == 'NFS':
            export_id = self.client.create_nfs_export(filesystem_id,
                                                      share_name)
            if not export_id:
@ -197,7 +197,7 @@ class PowerStoreStorageConnection(driver.StorageConnection):
                LOG.error(message)
                raise exception.ShareBackendException(msg=message)
            locations = self._get_nfs_location(file_interfaces, share_name)
-        elif protocal == 'CIFS':
+        elif protocol == 'CIFS':
            export_id = self.client.create_smb_share(filesystem_id,
                                                     share_name)
            if not export_id:
@ -243,8 +243,8 @@ class PowerStoreStorageConnection(driver.StorageConnection):
        LOG.debug(f"Retrieving filesystem ID for filesystem {share['name']}")
        filesystem_id = self.client.get_filesystem_id(share['name'])
        if not filesystem_id:
-            LOG.warning(f'Filesystem with share name {share["name"]} \
-                        is not found.')
+            LOG.warning(
+                f'Filesystem with share name {share["name"]} is not found.')
        else:
            LOG.debug(f"Deleting filesystem ID {filesystem_id}")
            share_deleted = self.client.delete_filesystem(filesystem_id)
@ -293,11 +293,11 @@ class PowerStoreStorageConnection(driver.StorageConnection):
    def update_access(self, context, share, access_rules, add_rules,
                      delete_rules, share_server=None):
        """Is called to update share access."""
-        protocal = share['share_proto'].upper()
-        LOG.debug(f'Updating access to {protocal} share.')
-        if protocal == 'NFS':
+        protocol = share['share_proto'].upper()
+        LOG.debug(f'Updating access to {protocol} share.')
+        if protocol == 'NFS':
            return self._update_nfs_access(share, access_rules)
-        elif protocal == 'CIFS':
+        elif protocol == 'CIFS':
            return self._update_cifs_access(share, access_rules)

    def _update_nfs_access(self, share, access_rules):
@ -350,8 +350,10 @@ class PowerStoreStorageConnection(driver.StorageConnection):
                access_updates.update({rule['access_id']: {'state': 'error'}})

            else:
-                prefix = self.ad_domain or \
+                prefix = (
+                    self.ad_domain or
                    self.client.get_nas_server_smb_netbios(self.nas_server)
+                )
                if not prefix:
                    message = (
                        _('Failed to get daomain/netbios name of '
@ -386,16 +388,16 @@ class PowerStoreStorageConnection(driver.StorageConnection):
        stats_dict['driver_version'] = VERSION
        stats_dict['storage_protocol'] = 'NFS_CIFS'
        stats_dict['reserved_percentage'] = self.reserved_percentage
-        stats_dict['reserved_snapshot_percentage'] = \
-            self.reserved_snapshot_percentage
-        stats_dict['reserved_share_extend_percentage'] = \
-            self.reserved_share_extend_percentage
-        stats_dict['max_over_subscription_ratio'] = \
-            self.max_over_subscription_ratio
+        stats_dict['reserved_snapshot_percentage'] = (
+            self.reserved_snapshot_percentage)
+        stats_dict['reserved_share_extend_percentage'] = (
+            self.reserved_share_extend_percentage)
+        stats_dict['max_over_subscription_ratio'] = (
+            self.max_over_subscription_ratio)

        cluster_id = self.client.get_cluster_id()
        total, used = self.client.retreive_cluster_capacity_metrics(cluster_id)
-        if(total and used):
+        if total and used:
            free = total - used
            stats_dict['total_capacity_gb'] = total // units.Gi
            stats_dict['free_capacity_gb'] = free // units.Gi