Macrosan driver: some improvements about variables and syntax

Change-Id: Id32da3b37186880cefdb5ff48d836c0a7f1ad656
(cherry picked from commit 4b83a6bd52)

manila/share/drivers/macrosan/macrosan_helper.py

@@ -61,7 +61,7 @@ class MacrosanHelper(object):
         cifs_status = self.rest._get_cifs_service_status()
         if cifs_status == constants.CIFS_EXCEPTION:
             raise exception.MacrosanBackendExeption(
-                reason=_("cifs service exception.Please check backend"))
+                reason=_("cifs service exception. Please check backend"))
         elif cifs_status == constants.CIFS_NON_CONFIG:
             """need config first, then start service"""
             self.rest._config_cifs_service()
@@ -81,17 +81,8 @@ class MacrosanHelper(object):
     def create_share(self, share, share_server=None):
         """Create a share"""
         pool_name, share_name, proto = self._get_share_instance_pnp(share)
-
         share_size = ''.join((str(share['size']), 'GB'))
 
-        # check pool available
-        storage_pools = self.rest._get_all_pool()
-        pool_info = self._find_pool_info(pool_name, storage_pools)
-        if not pool_info:
-            msg = f'Failed to find information regarding pool {pool_name}'
-            msg = _(msg)
-            raise exception.InvalidHost(reason=msg)
-
         # first create filesystem
         self.rest._create_filesystem(fs_name=share_name,
                                      pool_name=pool_name,
@@ -112,7 +103,7 @@ class MacrosanHelper(object):
                 self.rest._delete_filesystem(share_name)
                 raise exception.MacrosanBackendExeption(
                     reason=(_(
-                        'Failed to create share %(share)s.Reason:'
+                        'Failed to create share %(share)s. Reason: '
                         'username %(user_name)s error.')
                         % {'share': share_name, 'user_name': user_name}))
 
@@ -328,7 +319,8 @@ class MacrosanHelper(object):
 
         if proto == 'NFS':
             for share_access in access_list:
-                if share_access['access_to'] == '172.0.0.2':
+                # IPv4 Address Blocks Reserved for Documentation
+                if share_access['access_to'] == '192.0.2.0':
                     continue
                 self.rest._delete_nfs_access_rest(share_path,
                                                   share_access['access_to'])
@@ -400,13 +392,10 @@ class MacrosanHelper(object):
 
     def update_share_stats(self, dict_data):
         """Update pools info"""
-
         result = self.rest._get_all_pool()
         dict_data["pools"] = []
         for pool_name in self.pools:
-
             pool_capacity = self._get_pool_capacity(pool_name, result)
-
             if pool_capacity:
                 pool = {
                     'pool_name': pool_name,
@@ -414,9 +403,15 @@ class MacrosanHelper(object):
                     'free_capacity_gb': pool_capacity['freecapacity'],
                     'allocated_capacity_gb':
                         pool_capacity['allocatedcapacity'],
-                    'reserved_percentage': 0,
+                    'reserved_percentage':
-                    'reserved_snapshot_percentage': 0,
+                        self.configuration.reserved_share_percentage,
-                    'reserved_share_extend_percentage': 0,
+                    'reserved_snapshot_percentage':
+                        self.configuration
+                            .reserved_share_from_snapshot_percentage
+                        or self.configuration.reserved_share_percentage,
+                    'reserved_share_extend_percentage':
+                        self.configuration.reserved_share_extend_percentage
+                        or self.configuration.reserved_share_percentage,
                     'dedupe': False,
                     'compression': False,
                     'qos': False,

manila/share/drivers/macrosan/macrosan_nas.py

@@ -37,10 +37,14 @@ macrosan_opts = [
     cfg.StrOpt('macrosan_nas_password',
                default=None,
                secret=True,
-               help='Password for the Macrosan NAS server. '),
+               help='Password for the Macrosan NAS server.'),
     cfg.StrOpt('macrosan_nas_http_protocol',
                default='https',
+               choices=['http', 'https'],
                help='Http protocol for the Macrosan NAS server.'),
+    cfg.BoolOpt('macrosan_ssl_cert_verify',
+                default=False,
+                help='Defines whether the driver should check ssl cert.'),
     cfg.StrOpt('macrosan_nas_prefix',
                default='nas',
                help='Url prefix for the Macrosan NAS server.'),

manila/share/drivers/macrosan/rest_helper.py

@@ -36,6 +36,8 @@ class RestHelper(object):
         self._username = self.configuration.macrosan_nas_username
         self._password = self.configuration.macrosan_nas_password
         self.request_timeout = self.configuration.macrosan_timeout
+        self.ssl_verify = self.configuration.macrosan_ssl_cert_verify
+        if not self.ssl_verify:
             # Suppress the Insecure request warnings
             requests.packages.urllib3.disable_warnings(
                 requests.packages.urllib3.exceptions.InsecureRequestWarning)
@@ -72,16 +74,20 @@ class RestHelper(object):
 
         if method == 'POST':
             res = requests.post(final_url, data=data, headers=header,
-                                timeout=self.request_timeout, verify=False)
+                                timeout=self.request_timeout,
+                                verify=self.ssl_verify)
         elif method == 'GET':
             res = requests.get(final_url, data=data, headers=header,
-                               timeout=self.request_timeout, verify=False)
+                               timeout=self.request_timeout,
+                               verify=self.ssl_verify)
         elif method == 'PUT':
             res = requests.put(final_url, data=data, headers=header,
-                               timeout=self.request_timeout, verify=False)
+                               timeout=self.request_timeout,
+                               verify=self.ssl_verify)
         elif method == 'DELETE':
             res = requests.delete(final_url, data=data, headers=header,
-                                  timeout=self.request_timeout, verify=False)
+                                  timeout=self.request_timeout,
+                                  verify=self.ssl_verify)
         else:
             msg = (_("Request method %s invalid.") % method)
             raise exception.ShareBackendException(msg=msg)
@@ -130,10 +136,11 @@ class RestHelper(object):
 
     def _create_nfs_share(self, share_path):
         url = 'rest/nfsShare'
+        # IPv4 Address Blocks Reserved for Documentation
         params = {
             'path': share_path,
             'authority': 'ro',
-            'accessClient': '172.0.0.2',
+            'accessClient': '192.0.2.0',
         }
         result = self.call(url, params, 'POST')
 

manila/tests/share/drivers/macrosan/test_macrosan_nas.py

@@ -16,7 +16,6 @@
 """
 Share driver test for Macrosan Storage Array.
 """
-
 import ddt
 import requests
 
@@ -25,14 +24,13 @@ from unittest import mock
 
 from manila import context
 from manila import exception
-from manila import test
-
 from manila.share import configuration
 from manila.share import driver
 from manila.share.drivers.macrosan import macrosan_constants as constants
 from manila.share.drivers.macrosan import macrosan_helper
 from manila.share.drivers.macrosan import macrosan_nas
 from manila.share.drivers.macrosan import rest_helper
+from manila import test
 from manila.tests import fake_share
 
 CONF = cfg.CONF
@@ -74,17 +72,20 @@ class MacrosanShareDriverTestCase(test.TestCase):
         self.configuration.macrosan_nas_prefix = 'nas'
         self.configuration.macrosan_share_pools = ['fake_pool']
         self.configuration.macrosan_timeout = 60
+        self.configuration.macrosan_ssl_cert_verify = False
 
         self.configuration.network_config_group = 'fake_network_config_group'
         self.configuration.admin_network_config_group = (
             'fake_admin_network_config_group')
         self.configuration.config_group = 'fake_config_group'
         self.configuration.reserved_share_percentage = 0
+        self.configuration.reserved_share_from_snapshot_percentage = 0
+        self.configuration.reserved_share_extend_percentage = 0
         self.configuration.filter_function = None
         self.configuration.goodness_function = None
         self.driver = macrosan_nas.MacrosanNasDriver(
             configuration=self.configuration)
-        self.resutl_success_storage_pools = {
+        self.result_success_storage_pools = {
             'code': 0,
             'message': 'success',
             'data': [{
@@ -202,17 +203,6 @@ class MacrosanShareDriverTestCase(test.TestCase):
     def test_create_share(self, share_proto):
         share = fake_share.fake_share(
             share_proto=share_proto, host="fake_host@fake_backend#fake_pool")
-        self.mock_object(rest_helper.RestHelper, '_get_all_pool')
-        self.mock_object(
-            macrosan_helper.MacrosanHelper, '_find_pool_info',
-            mock.Mock(return_value={
-                "name": "fake_pool",
-                "totalcapacity": "10G",
-                "allocatedcapacity": "0G",
-                "freecapacity": "10G",
-                "health": "ONLINE",
-                "rw": "off"
-            }))
         mock_cf = self.mock_object(rest_helper.RestHelper,
                                    '_create_filesystem')
         mock_cfd = self.mock_object(rest_helper.RestHelper,
@@ -245,36 +235,9 @@ class MacrosanShareDriverTestCase(test.TestCase):
         else:
             mock_ccs.assert_called_once()
 
-    def test_create_share_pool_fail(self):
-        share = fake_share.fake_share(
-            share_proto='nfs', host="fake_host@fake_backend#fake_pool")
-        self.mock_object(rest_helper.RestHelper, '_get_all_pool')
-        self.mock_object(macrosan_helper.MacrosanHelper, '_find_pool_info',
-                         mock.Mock(return_value=None))
-        self.mock_object(
-            rest_helper.RestHelper, '_create_filesystem')
-        self.mock_object(
-            rest_helper.RestHelper, '_create_nfs_share')
-
-        self.assertRaises(exception.InvalidHost,
-                          self.driver.create_share,
-                          self._context,
-                          share)
-
     def test_create_share_user_error(self):
         share = fake_share.fake_share(
             share_proto='cifs', host="fake_host@fake_backend#fake_pool")
-        self.mock_object(rest_helper.RestHelper, '_get_all_pool')
-        self.mock_object(
-            macrosan_helper.MacrosanHelper, '_find_pool_info',
-            mock.Mock(return_value={
-                "name": "fake_pool",
-                "totalcapacity": "10G",
-                "allocatedcapacity": "0G",
-                "freecapacity": "10G",
-                "health": "ONLINE",
-                "rw": "off"
-            }))
         mock_cf = self.mock_object(rest_helper.RestHelper,
                                    '_create_filesystem')
         mock_cfd = self.mock_object(rest_helper.RestHelper,
@@ -758,7 +721,6 @@ class MacrosanShareDriverTestCase(test.TestCase):
                 'access_type': 'ip',
                 'access_to': '172.0.0.1',
                 'access_level': 'rw',
-
             }
         else:
             access = {
@@ -1310,13 +1272,13 @@ class MacrosanShareDriverTestCase(test.TestCase):
     def test__find_pool_info(self):
         pool_info = self.driver.helper._find_pool_info(
             'fake_pool',
-            self.resutl_success_storage_pools)
+            self.result_success_storage_pools)
         self.assertIsNotNone(pool_info)
 
     def test__find_pool_info_fail(self):
         pool_info = self.driver.helper._find_pool_info(
             'error_pool',
-            self.resutl_success_storage_pools)
+            self.result_success_storage_pools)
         expect = {}
         self.assertEqual(expect, pool_info)
 
@@ -1340,6 +1302,7 @@ class RestHelperTestCase(test.TestCase):
         self.configuration.macrosan_nas_username = 'fake_username'
         self.configuration.macrosan_nas_password = 'fake_password'
         self.configuration.macrosan_timeout = 60
+        self.configuration.macrosan_ssl_cert_verify = False
         self.resthelper = rest_helper.RestHelper(
             configuration=self.configuration)
         self.post = 'POST'
@@ -1372,7 +1335,7 @@ class RestHelperTestCase(test.TestCase):
             'message': 'failed',
             'data': '',
         }
-        self.resutl_success_storage_pools = {
+        self.result_success_storage_pools = {
             'code': 0,
             'message': 'success',
             'data': [{
@@ -1532,7 +1495,7 @@ class RestHelperTestCase(test.TestCase):
         data = {
             'path': 'fake_path',
             'authority': 'ro',
-            'accessClient': '172.0.0.2',
+            'accessClient': '192.0.2.0',
         }
         mock_call.assert_called_once_with(url, data, self.post)
 
@@ -2229,11 +2192,11 @@ class RestHelperTestCase(test.TestCase):
         mock_call = self.mock_object(
             self.resthelper,
             'call',
-            mock.Mock(return_value=self.resutl_success_storage_pools))
+            mock.Mock(return_value=self.result_success_storage_pools))
         self.mock_object(self.resthelper,
                          '_assert_result_code')
         result = self.resthelper._get_all_pool()
-        self.assertEqual(self.resutl_success_storage_pools, result)
+        self.assertEqual(self.result_success_storage_pools, result)
         url = 'rest/storagepool'
         mock_call.assert_called_once_with(url, None, self.get)
 

releasenote/notes/macrosan-add-configuration-option-282fa1026748c4f9.yaml

@@ -0,0 +1,5 @@
+---
+upgrade:
+  - Added a new configuration option ``macrosan_ssl_cert_verfiy`` to configure
+    whether the driver should only allow verified ssl certificates. This option
+    defaults to ``False`` to allow backwards compatibility.