openstack/zun
Code Issues Proposed changes

Use volume uuid based mountpoint

Browse Source 
In before, we use the uuid of volume mapping as the basic to
compute the mountpoint. However, volume mapping represents
a mapping between a container and a local volume. If there is
multiple containers mapping to a volume, the computed mountpoint
is problematic because each container will compute a different
mountpoint.

This commits use the uuid of volume to compute the mountpoint.
As a result, in the shared volume case, all container will
compute the same mountpoint as long as they share the volume.

Change-Id: I0af30ecef043da000fea38776891feb19da018e6
This commit is contained in:
Hongbin Lu
2018-09-27 03:32:56 +00:00
parent 3ce95424b8
commit 1bb78daacd
2 changed files with 39 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
zun/tests/unit/volume/test_driver.py
View File

@@ -37,12 +37,12 @@ class CinderVolumeDriverTestCase(base.TestCase):
self.fake_conn_info = { self.fake_conn_info = {
'data': {'device_path': self.fake_devpath}, 'data': {'device_path': self.fake_devpath},
} }
self.volume = mock.MagicMock() self.volmap = mock.MagicMock()
self.volume.uuid = self.fake_uuid self.volmap.volume.uuid = self.fake_uuid
self.volume.volume_provider = 'cinder' self.volmap.volume_provider = 'cinder'
self.volume.volume_id = self.fake_volume_id self.volmap.volume_id = self.fake_volume_id
self.volume.container_path = self.fake_container_path self.volmap.container_path = self.fake_container_path
self.volume.connection_info = jsonutils.dumps(self.fake_conn_info) self.volmap.connection_info = jsonutils.dumps(self.fake_conn_info)
@mock.patch('zun.common.mount.do_mount') @mock.patch('zun.common.mount.do_mount')
@mock.patch('oslo_utils.fileutils.ensure_tree') @mock.patch('oslo_utils.fileutils.ensure_tree')
@@ -56,9 +56,9 @@ class CinderVolumeDriverTestCase(base.TestCase):
mock_get_mountpoint.return_value = self.fake_mountpoint mock_get_mountpoint.return_value = self.fake_mountpoint
volume_driver = driver.Cinder() volume_driver = driver.Cinder()
volume_driver.attach(self.context, self.volume) volume_driver.attach(self.context, self.volmap)
mock_cinder_workflow.attach_volume.assert_called_once_with(self.volume) mock_cinder_workflow.attach_volume.assert_called_once_with(self.volmap)
mock_get_mountpoint.assert_called_once_with(self.fake_uuid) mock_get_mountpoint.assert_called_once_with(self.fake_uuid)
mock_do_mount.assert_called_once_with( mock_do_mount.assert_called_once_with(
self.fake_devpath, self.fake_mountpoint, CONF.volume.fstype) self.fake_devpath, self.fake_mountpoint, CONF.volume.fstype)
@@ -72,9 +72,9 @@ class CinderVolumeDriverTestCase(base.TestCase):
mock_get_mountpoint, mock_ensure_tree, mock_get_mountpoint, mock_ensure_tree,
mock_do_mount): mock_do_mount):
volume_driver = driver.Cinder() volume_driver = driver.Cinder()
self.volume.volume_provider = 'unknown' self.volmap.volume_provider = 'unknown'
self.assertRaises(exception.ZunException, self.assertRaises(exception.ZunException,
volume_driver.attach, self.context, self.volume) volume_driver.attach, self.context, self.volmap)
@mock.patch('zun.common.mount.do_mount') @mock.patch('zun.common.mount.do_mount')
@mock.patch('oslo_utils.fileutils.ensure_tree') @mock.patch('oslo_utils.fileutils.ensure_tree')
@@ -91,9 +91,9 @@ class CinderVolumeDriverTestCase(base.TestCase):
volume_driver = driver.Cinder() volume_driver = driver.Cinder()
self.assertRaises(exception.ZunException, self.assertRaises(exception.ZunException,
volume_driver.attach, self.context, self.volume) volume_driver.attach, self.context, self.volmap)
mock_cinder_workflow.attach_volume.assert_called_once_with(self.volume) mock_cinder_workflow.attach_volume.assert_called_once_with(self.volmap)
mock_get_mountpoint.assert_not_called() mock_get_mountpoint.assert_not_called()
mock_do_mount.assert_not_called() mock_do_mount.assert_not_called()
mock_cinder_workflow.detach_volume.assert_not_called() mock_cinder_workflow.detach_volume.assert_not_called()
@@ -113,13 +113,13 @@ class CinderVolumeDriverTestCase(base.TestCase):
volume_driver = driver.Cinder() volume_driver = driver.Cinder()
self.assertRaises(exception.ZunException, self.assertRaises(exception.ZunException,
volume_driver.attach, self.context, self.volume) volume_driver.attach, self.context, self.volmap)
mock_cinder_workflow.attach_volume.assert_called_once_with(self.volume) mock_cinder_workflow.attach_volume.assert_called_once_with(self.volmap)
mock_get_mountpoint.assert_called_once_with(self.fake_uuid) mock_get_mountpoint.assert_called_once_with(self.fake_uuid)
mock_do_mount.assert_called_once_with( mock_do_mount.assert_called_once_with(
self.fake_devpath, self.fake_mountpoint, CONF.volume.fstype) self.fake_devpath, self.fake_mountpoint, CONF.volume.fstype)
mock_cinder_workflow.detach_volume.assert_called_once_with(self.volume) mock_cinder_workflow.detach_volume.assert_called_once_with(self.volmap)
@mock.patch('zun.common.mount.do_mount') @mock.patch('zun.common.mount.do_mount')
@mock.patch('oslo_utils.fileutils.ensure_tree') @mock.patch('oslo_utils.fileutils.ensure_tree')
@@ -144,13 +144,13 @@ class CinderVolumeDriverTestCase(base.TestCase):
volume_driver = driver.Cinder() volume_driver = driver.Cinder()
self.assertRaises(TestException1, self.assertRaises(TestException1,
volume_driver.attach, self.context, self.volume) volume_driver.attach, self.context, self.volmap)
mock_cinder_workflow.attach_volume.assert_called_once_with(self.volume) mock_cinder_workflow.attach_volume.assert_called_once_with(self.volmap)
mock_get_mountpoint.assert_called_once_with(self.fake_uuid) mock_get_mountpoint.assert_called_once_with(self.fake_uuid)
mock_do_mount.assert_called_once_with( mock_do_mount.assert_called_once_with(
self.fake_devpath, self.fake_mountpoint, CONF.volume.fstype) self.fake_devpath, self.fake_mountpoint, CONF.volume.fstype)
mock_cinder_workflow.detach_volume.assert_called_once_with(self.volume) mock_cinder_workflow.detach_volume.assert_called_once_with(self.volmap)
@mock.patch('shutil.rmtree') @mock.patch('shutil.rmtree')
@mock.patch('zun.common.mount.do_unmount') @mock.patch('zun.common.mount.do_unmount')
@@ -163,9 +163,9 @@ class CinderVolumeDriverTestCase(base.TestCase):
mock_get_mountpoint.return_value = self.fake_mountpoint mock_get_mountpoint.return_value = self.fake_mountpoint
volume_driver = driver.Cinder() volume_driver = driver.Cinder()
volume_driver.detach(self.context, self.volume) volume_driver.detach(self.context, self.volmap)
mock_cinder_workflow.detach_volume.\ mock_cinder_workflow.detach_volume.\
assert_called_once_with(self.context, self.volume) assert_called_once_with(self.context, self.volmap)
mock_get_mountpoint.assert_called_once_with(self.fake_uuid) mock_get_mountpoint.assert_called_once_with(self.fake_uuid)
mock_do_unmount.assert_called_once_with(self.fake_mountpoint) mock_do_unmount.assert_called_once_with(self.fake_mountpoint)
mock_rmtree.assert_called_once_with(self.fake_mountpoint) mock_rmtree.assert_called_once_with(self.fake_mountpoint)
@@ -179,7 +179,7 @@ class CinderVolumeDriverTestCase(base.TestCase):
volume_driver = driver.Cinder() volume_driver = driver.Cinder()
source, destination = volume_driver.bind_mount( source, destination = volume_driver.bind_mount(
self.context, self.volume) self.context, self.volmap)
self.assertEqual(self.fake_mountpoint, source) self.assertEqual(self.fake_mountpoint, source)
self.assertEqual(self.fake_container_path, destination) self.assertEqual(self.fake_container_path, destination)
@@ -196,9 +196,9 @@ class CinderVolumeDriverTestCase(base.TestCase):
mock_get_mountpoint.return_value = self.fake_mountpoint mock_get_mountpoint.return_value = self.fake_mountpoint
volume_driver = driver.Cinder() volume_driver = driver.Cinder()
volume_driver.delete(self.context, self.volume) volume_driver.delete(self.context, self.volmap)
mock_cinder_workflow.delete_volume.assert_called_once_with(self.volume) mock_cinder_workflow.delete_volume.assert_called_once_with(self.volmap)
class LocalVolumeDriverTestCase(base.TestCase): class LocalVolumeDriverTestCase(base.TestCase):
@@ -209,11 +209,11 @@ class LocalVolumeDriverTestCase(base.TestCase):
self.fake_mountpoint = '/fake-mountpoint' self.fake_mountpoint = '/fake-mountpoint'
self.fake_container_path = '/fake-container-path' self.fake_container_path = '/fake-container-path'
self.fake_contents = 'fake-contents' self.fake_contents = 'fake-contents'
self.volume = mock.MagicMock() self.volmap = mock.MagicMock()
self.volume.uuid = self.fake_uuid self.volmap.volume.uuid = self.fake_uuid
self.volume.volume_provider = 'local' self.volmap.volume_provider = 'local'
self.volume.container_path = self.fake_container_path self.volmap.container_path = self.fake_container_path
self.volume.contents = self.fake_contents self.volmap.contents = self.fake_contents
@mock.patch('oslo_utils.fileutils.ensure_tree') @mock.patch('oslo_utils.fileutils.ensure_tree')
@mock.patch('zun.common.mount.get_mountpoint') @mock.patch('zun.common.mount.get_mountpoint')
@@ -223,7 +223,7 @@ class LocalVolumeDriverTestCase(base.TestCase):
with mock.patch('zun.volume.driver.open', mock.mock_open() with mock.patch('zun.volume.driver.open', mock.mock_open()
) as mock_open: ) as mock_open:
volume_driver.attach(self.context, self.volume) volume_driver.attach(self.context, self.volmap)
expected_file_path = self.fake_mountpoint + '/' + self.fake_uuid expected_file_path = self.fake_mountpoint + '/' + self.fake_uuid
mock_open.assert_called_once_with(expected_file_path, 'wb') mock_open.assert_called_once_with(expected_file_path, 'wb')
@@ -236,7 +236,7 @@ class LocalVolumeDriverTestCase(base.TestCase):
def test_detach(self, mock_get_mountpoint, mock_rmtree): def test_detach(self, mock_get_mountpoint, mock_rmtree):
mock_get_mountpoint.return_value = self.fake_mountpoint mock_get_mountpoint.return_value = self.fake_mountpoint
volume_driver = driver.Local() volume_driver = driver.Local()
volume_driver.detach(self.context, self.volume) volume_driver.detach(self.context, self.volmap)
mock_get_mountpoint.assert_called_once_with(self.fake_uuid) mock_get_mountpoint.assert_called_once_with(self.fake_uuid)
mock_rmtree.assert_called_once_with(self.fake_mountpoint) mock_rmtree.assert_called_once_with(self.fake_mountpoint)
@@ -246,7 +246,7 @@ class LocalVolumeDriverTestCase(base.TestCase):
mock_get_mountpoint.return_value = self.fake_mountpoint mock_get_mountpoint.return_value = self.fake_mountpoint
volume_driver = driver.Local() volume_driver = driver.Local()
source, destination = volume_driver.bind_mount( source, destination = volume_driver.bind_mount(
self.context, self.volume) self.context, self.volmap)
expected_file_path = self.fake_mountpoint + '/' + self.fake_uuid expected_file_path = self.fake_mountpoint + '/' + self.fake_uuid
self.assertEqual(expected_file_path, source) self.assertEqual(expected_file_path, source)

16
zun/volume/driver.py
View File

@@ -92,15 +92,15 @@ class Local(VolumeDriver):
@validate_volume_provider(supported_providers) @validate_volume_provider(supported_providers)
def attach(self, context, volmap): def attach(self, context, volmap):
mountpoint = mount.get_mountpoint(volmap.uuid) mountpoint = mount.get_mountpoint(volmap.volume.uuid)
fileutils.ensure_tree(mountpoint) fileutils.ensure_tree(mountpoint)
filename = '/'.join([mountpoint, volmap.uuid]) filename = '/'.join([mountpoint, volmap.volume.uuid])
with open(filename, 'wb') as fd: with open(filename, 'wb') as fd:
content = utils.decode_file_data(volmap.contents) content = utils.decode_file_data(volmap.contents)
fd.write(content) fd.write(content)
def _remove_local_file(self, volmap): def _remove_local_file(self, volmap):
mountpoint = mount.get_mountpoint(volmap.uuid) mountpoint = mount.get_mountpoint(volmap.volume.uuid)
shutil.rmtree(mountpoint) shutil.rmtree(mountpoint)
@validate_volume_provider(supported_providers) @validate_volume_provider(supported_providers)
@@ -113,8 +113,8 @@ class Local(VolumeDriver):
@validate_volume_provider(supported_providers) @validate_volume_provider(supported_providers)
def bind_mount(self, context, volmap): def bind_mount(self, context, volmap):
mountpoint = mount.get_mountpoint(volmap.uuid) mountpoint = mount.get_mountpoint(volmap.volume.uuid)
filename = '/'.join([mountpoint, volmap.uuid]) filename = '/'.join([mountpoint, volmap.volume.uuid])
return filename, volmap.container_path return filename, volmap.container_path
def is_volume_available(self, context, volmap): def is_volume_available(self, context, volmap):
@@ -142,7 +142,7 @@ class Cinder(VolumeDriver):
LOG.exception("Failed to detach volume") LOG.exception("Failed to detach volume")
def _mount_device(self, volmap, devpath): def _mount_device(self, volmap, devpath):
mountpoint = mount.get_mountpoint(volmap.uuid) mountpoint = mount.get_mountpoint(volmap.volume.uuid)
fileutils.ensure_tree(mountpoint) fileutils.ensure_tree(mountpoint)
mount.do_mount(devpath, mountpoint, CONF.volume.fstype) mount.do_mount(devpath, mountpoint, CONF.volume.fstype)
@@ -159,13 +159,13 @@ class Cinder(VolumeDriver):
def _unmount_device(self, volmap): def _unmount_device(self, volmap):
if hasattr(volmap, 'connection_info'): if hasattr(volmap, 'connection_info'):
mountpoint = mount.get_mountpoint(volmap.uuid) mountpoint = mount.get_mountpoint(volmap.volume.uuid)
mount.do_unmount(mountpoint) mount.do_unmount(mountpoint)
shutil.rmtree(mountpoint) shutil.rmtree(mountpoint)
@validate_volume_provider(supported_providers) @validate_volume_provider(supported_providers)
def bind_mount(self, context, volmap): def bind_mount(self, context, volmap):
mountpoint = mount.get_mountpoint(volmap.uuid) mountpoint = mount.get_mountpoint(volmap.volume.uuid)
return mountpoint, volmap.container_path return mountpoint, volmap.container_path
@validate_volume_provider(supported_providers) @validate_volume_provider(supported_providers)