Brick connector revised fix for NFS drivers

This change fixes the error that comes
while getting brick connector and attaching volumes
in case of NFS drivers in cinder. The attribute for
mount point base was not passed to attach_volume method
as the connector initialization logic is common for all
types of protocols. It is fixed by populating
the required parameter in the RemoteFsConnector
by extracting it from connection properties. The infrastructure
for fixing this situation for remote fs drivers is implemented
in this fix. The drivers need to override the method
_get_mount_point_base in their implementation. NFS driver
implements it. GlusterFs implementation of the method to follow
in a separate patch. Hence marking it partial fix.

Change-Id: Id61a437fca1870529fa8b85f7fc2f73eae665294
Partial-Bug: #1238085

diff --git a/cinder/brick/initiator/connector.py b/cinder/brick/initiator/connector.py
index 0778f946bba114e67ea5657ceceb8be2ddd1825a..0f03fababd297d88bd41eb923889393ea7aea13d 100644 (file)
--- a/cinder/brick/initiator/connector.py
+++ b/cinder/brick/initiator/connector.py
@@ -797,6 +797,21 @@ class RemoteFsConnector(InitiatorConnector):
                  execute=putils.execute,
                  device_scan_attempts=DEVICE_SCAN_ATTEMPTS_DEFAULT,
                  *args, **kwargs):
+        kwargs = kwargs or {}
+        conn = kwargs.get('conn')
+        if conn:
+            mount_point_base = conn.get('mount_point_base')
+            if mount_type.lower() == 'nfs':
+                kwargs['nfs_mount_point_base'] =\
+                    kwargs.get('nfs_mount_point_base') or\
+                    mount_point_base
+            elif mount_type.lower() == 'glusterfs':
+                kwargs['glusterfs_mount_point_base'] =\
+                    kwargs.get('glusterfs_mount_point_base') or\
+                    mount_point_base
+        else:
+            LOG.warn(_("Connection details not present."
+                       " RemoteFsClient may not initialize properly."))
         self._remotefsclient = remotefs.RemoteFsClient(mount_type, root_helper,
                                                        execute=execute,
                                                        *args, **kwargs)
@@ -822,7 +837,7 @@ class RemoteFsConnector(InitiatorConnector):
         """
 
         mnt_flags = []
-        if 'options' in connection_properties:
+        if connection_properties.get('options'):
             mnt_flags = connection_properties['options'].split()
 
         nfs_share = connection_properties['export']

diff --git a/cinder/tests/test_coraid.py b/cinder/tests/test_coraid.py
index 277ba6ef7a88839224a1db48e223754ab3f89035..500cae7b21829e20edf1aea15e8ae65a26b628de 100644 (file)
--- a/cinder/tests/test_coraid.py
+++ b/cinder/tests/test_coraid.py
@@ -787,7 +787,8 @@ class CoraidDriverImageTestCases(CoraidDriverTestCase):
 
         utils.brick_get_connector('aoe',
                                   device_scan_attempts=3,
-                                  use_multipath=False).\
+                                  use_multipath=False,
+                                  conn=mox.IgnoreArg()).\
             AndReturn(aoe_initiator)
 
         aoe_initiator\

diff --git a/cinder/utils.py b/cinder/utils.py
index de3d2481defad02cfde10919766a652602ed4a24..6fac1d6fea74a789f79782eb2ec642a0aaa7f6fb 100644 (file)
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -795,7 +795,8 @@ def brick_get_connector_properties():
 def brick_get_connector(protocol, driver=None,
                         execute=processutils.execute,
                         use_multipath=False,
-                        device_scan_attempts=3):
+                        device_scan_attempts=3,
+                        *args, **kwargs):
     """Wrapper to get a brick connector object.
     This automatically populates the required protocol as well
     as the root_helper needed to execute commands.
@@ -807,7 +808,8 @@ def brick_get_connector(protocol, driver=None,
                                                 execute=execute,
                                                 use_multipath=use_multipath,
                                                 device_scan_attempts=
-                                                device_scan_attempts)
+                                                device_scan_attempts,
+                                                *args, **kwargs)
 
 
 def require_driver_initialized(func):

diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py
index a286b4b3b7d3d38975f274729acca830a43efb1f..2b149a10182bde327f4869fbecc8dc6054d27925 100644 (file)
--- a/cinder/volume/driver.py
+++ b/cinder/volume/driver.py
@@ -373,7 +373,8 @@ class VolumeDriver(object):
         connector = utils.brick_get_connector(protocol,
                                               use_multipath=use_multipath,
                                               device_scan_attempts=
-                                              device_scan_attempts)
+                                              device_scan_attempts,
+                                              conn=conn)
         device = connector.connect_volume(conn['data'])
         host_device = device['path']
 

diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py
index 277b173a6495772be6ab5b6b043f8fd37c0bc46a..c7312e72698f8c8bfc7cc0891d3c6d8ee0d0a712 100644 (file)
--- a/cinder/volume/drivers/nfs.py
+++ b/cinder/volume/drivers/nfs.py
@@ -92,9 +92,25 @@ class RemoteFsDriver(driver.VolumeDriver):
             data['options'] = self.shares[volume['provider_location']]
         return {
             'driver_volume_type': self.driver_volume_type,
-            'data': data
+            'data': data,
+            'mount_point_base': self._get_mount_point_base()
         }
 
+    def _get_mount_point_base(self):
+        """Returns the mount point base for the remote fs.
+
+           This method facilitates returning mount point base
+           for the specific remote fs. Override this method
+           in the respective driver to return the entry to be
+           used while attach/detach using brick in cinder.
+           If not overridden then it returns None without
+           raising exception to continue working for cases
+           when not used with brick.
+        """
+        LOG.debug(_("Driver specific implementation needs to return"
+                    " mount_point_base."))
+        return None
+
     def create_volume(self, volume):
         """Creates a volume.
 
@@ -373,15 +389,16 @@ class NfsDriver(RemoteFsDriver):
         super(NfsDriver, self).__init__(*args, **kwargs)
         self.configuration.append_config_values(volume_opts)
         root_helper = utils.get_root_helper()
-        base = getattr(self.configuration,
-                       'nfs_mount_point_base',
-                       CONF.nfs_mount_point_base)
+        # base bound to instance is used in RemoteFsConnector.
+        self.base = getattr(self.configuration,
+                            'nfs_mount_point_base',
+                            CONF.nfs_mount_point_base)
         opts = getattr(self.configuration,
                        'nfs_mount_options',
                        CONF.nfs_mount_options)
         self._remotefsclient = remotefs.RemoteFsClient(
             'nfs', root_helper, execute=execute,
-            nfs_mount_point_base=base,
+            nfs_mount_point_base=self.base,
             nfs_mount_options=opts)
 
     def set_execute(self, execute):
@@ -533,3 +550,6 @@ class NfsDriver(RemoteFsDriver):
                               '*snapshot*', mount_point, run_as_root=True)
         total_allocated = float(du.split()[0])
         return total_size, total_available, total_allocated
+
+    def _get_mount_point_base(self):
+        return self.base