limit the usage of connection_info

Related to blueprint xenapi-volume-drivers

The connection_info parameter, that is coming from volume contains only
two keys, 'driver_volume_type', and 'data'.
See:
https://github.com/openstack/nova/blob/master/nova/volume/manager.py#L375

parse_volume_info was checking the connection_info for 'auth_method',
and authentication parameters. These informations live inside
connection_data. This patch is fixing this, and covering with a test.

Also rename the generic data variable to connection_data, to make it
easier to read the code.

parse_volume_info thus no longer depends on the whole connection_info,
only on the connection_data.

Also clean-up some copy-paste test setup code.

Change-Id: I5289120d46176c8528c3317a650269c4f384f930

nova/tests/test_xenapi.py

@@ -176,18 +176,22 @@ class XenAPIVolumeTestCase(stubs.XenAPITestBase):
         return db.volume_create(self.context, vol)
 
     @staticmethod
-    def _make_info():
+    def _make_connection_data():
+        return {
+            'volume_id': 1,
+            'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001',
+            'target_portal': '127.0.0.1:3260,fake',
+            'target_lun': None,
+            'auth_method': 'CHAP',
+            'auth_username': 'username',
+            'auth_password': 'password',
+        }
+
+    @classmethod
+    def _make_connection_info(cls):
         return {
             'driver_volume_type': 'iscsi',
-            'data': {
-                'volume_id': 1,
-                'target_iqn': 'iqn.2010-10.org.openstack:volume-00000001',
-                'target_portal': '127.0.0.1:3260,fake',
-                'target_lun': None,
-                'auth_method': 'CHAP',
-                'auth_method': 'fake',
-                'auth_method': 'fake',
-            }
+            'data': cls._make_connection_data()
         }
 
     def test_mountpoint_to_number(self):
@@ -212,17 +216,18 @@ class XenAPIVolumeTestCase(stubs.XenAPITestBase):
                     '%s yielded %s, not %s' % (input, actual, expected))
 
     def test_parse_volume_info_raise_exception(self):
-        """This shows how to test helper classes' methods."""
-        stubs.stubout_session(self.stubs, stubs.FakeSessionForVolumeTests)
-        session = xenapi_conn.XenAPISession('test_url', 'root', 'test_pass')
-        vol = self._create_volume()
-        # oops, wrong mount point!
         self.assertRaises(volume_utils.StorageError,
                           volume_utils.parse_volume_info,
-                          self._make_info(),
+                          self._make_connection_data(),
                           'dev/sd'
                           )
-        db.volume_destroy(context.get_admin_context(), vol['id'])
+
+    def test_parse_volume_info_parsing_auth_details(self):
+        result = volume_utils.parse_volume_info(
+            self._make_connection_data(), '/dev/sda')
+
+        self.assertEquals('username', result['chapuser'])
+        self.assertEquals('password', result['chappassword'])
 
     def test_attach_volume(self):
         """This shows how to test Ops classes' methods."""
@@ -231,7 +236,7 @@ class XenAPIVolumeTestCase(stubs.XenAPITestBase):
         volume = self._create_volume()
         instance = db.instance_create(self.context, self.instance_values)
         vm = xenapi_fake.create_vm(instance.name, 'Running')
-        result = conn.attach_volume(self._make_info(),
+        result = conn.attach_volume(self._make_connection_info(),
                                     instance.name, '/dev/sdc')
 
         # check that the VM has a VBD attached to it

nova/virt/xenapi/volume_utils.py

@@ -275,7 +275,7 @@ def purge_sr(session, sr_ref):
     forget_sr(session, sr_rec['uuid'])
 
 
-def parse_volume_info(connection_info, mountpoint):
+def parse_volume_info(connection_data, mountpoint):
     """
     Parse device_path and mountpoint as they can be used by XenAPI.
     In particular, the mountpoint (e.g. /dev/sdc) must be translated
@@ -289,12 +289,11 @@ def parse_volume_info(connection_info, mountpoint):
     the iscsi driver to set them.
     """
     device_number = mountpoint_to_number(mountpoint)
-    data = connection_info['data']
-    volume_id = data['volume_id']
-    target_portal = data['target_portal']
+    volume_id = connection_data['volume_id']
+    target_portal = connection_data['target_portal']
     target_host = _get_target_host(target_portal)
     target_port = _get_target_port(target_portal)
-    target_iqn = data['target_iqn']
+    target_iqn = connection_data['target_iqn']
     LOG.debug('(vol_id,number,host,port,iqn): (%s,%s,%s,%s)',
               volume_id, target_host, target_port, target_iqn)
     if (device_number < 0 or
@@ -302,16 +301,16 @@ def parse_volume_info(connection_info, mountpoint):
         target_host is None or
         target_iqn is None):
         raise StorageError(_('Unable to obtain target information'
-                ' %(data)s, %(mountpoint)s') % locals())
+                ' %(connection_data)s, %(mountpoint)s') % locals())
     volume_info = {}
     volume_info['id'] = volume_id
     volume_info['target'] = target_host
     volume_info['port'] = target_port
     volume_info['targetIQN'] = target_iqn
-    if ('auth_method' in connection_info and
-        connection_info['auth_method'] == 'CHAP'):
-        volume_info['chapuser'] = connection_info['auth_username']
-        volume_info['chappassword'] = connection_info['auth_password']
+    if ('auth_method' in connection_data and
+        connection_data['auth_method'] == 'CHAP'):
+        volume_info['chapuser'] = connection_data['auth_username']
+        volume_info['chappassword'] = connection_data['auth_password']
 
     return volume_info
 

nova/virt/xenapi/volumeops.py

@@ -118,29 +118,29 @@ class VolumeOps(object):
         if driver_type not in ['iscsi', 'xensm']:
             raise exception.VolumeDriverNotFound(driver_type=driver_type)
 
-        data = connection_info['data']
-        if 'name_label' not in data:
-            label = 'tempSR-%s' % data['volume_id']
+        connection_data = connection_info['data']
+        if 'name_label' not in connection_data:
+            label = 'tempSR-%s' % connection_data['volume_id']
         else:
-            label = data['name_label']
-            del data['name_label']
+            label = connection_data['name_label']
+            del connection_data['name_label']
 
-        if 'name_description' not in data:
+        if 'name_description' not in connection_data:
             desc = 'Disk-for:%s' % instance_name
         else:
-            desc = data['name_description']
+            desc = connection_data['name_description']
 
         LOG.debug(connection_info)
         sr_params = {}
-        if u'sr_uuid' not in data:
-            sr_params = volume_utils.parse_volume_info(connection_info,
+        if u'sr_uuid' not in connection_data:
+            sr_params = volume_utils.parse_volume_info(connection_data,
                                                        mountpoint)
             uuid = "FA15E-D15C-" + str(sr_params['id'])
             sr_params['sr_type'] = 'iscsi'
         else:
-            uuid = data['sr_uuid']
-            for k in data['introduce_sr_keys']:
-                sr_params[k] = data[k]
+            uuid = connection_data['sr_uuid']
+            for k in connection_data['introduce_sr_keys']:
+                sr_params[k] = connection_data[k]
 
         sr_params['name_description'] = desc
 
@@ -155,10 +155,10 @@ class VolumeOps(object):
 
         vdi_uuid = None
         target_lun = None
-        if 'vdi_uuid' in data:
-            vdi_uuid = data['vdi_uuid']
-        elif 'target_lun' in data:
-            target_lun = data['target_lun']
+        if 'vdi_uuid' in connection_data:
+            vdi_uuid = connection_data['vdi_uuid']
+        elif 'target_lun' in connection_data:
+            target_lun = connection_data['target_lun']
         else:
             vdi_uuid = None