openstack
/
manila
Code Issues Proposed changes

Merge "Fix concurrency issues in container driver"

This commit is contained in:
Jenkins 2016-11-01 15:00:32 +00:00 committed by Gerrit Code Review
parent 63423ec0f7 a915b75d00
commit 6cd83cd148
8 changed files with 111 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
contrib/ci/post_test_hook.sh
View File

@ -212,7 +212,8 @@ elif [[ "$DRIVER" == "dummy" ]]; then
iniset $TEMPEST_CONFIG share create_networks_when_multitenancy_enabled False
iniset $TEMPEST_CONFIG share multi_backend True
elif [[ "$DRIVER" == "container" ]]; then
MANILA_TEMPEST_CONCURRENCY=1
MANILA_TESTS="(^manila_tempest_tests.tests.api)(?=.*\[.*\bbackend\b.*\])"
MANILA_TEMPEST_CONCURRENCY=8
RUN_MANILA_CG_TESTS=False
RUN_MANILA_MANAGE_TESTS=False
iniset $TEMPEST_CONFIG share run_host_assisted_migration_tests False

2
contrib/ci/pre_test_hook.sh
View File

@ -157,7 +157,7 @@ elif [[ "$DRIVER" == "zfsonlinux" ]]; then
save_configuration "MANILA_ZFSONLINUX_USE_SSH" "True"
elif [[ "$DRIVER" == "container" ]]; then
save_configuration "SHARE_DRIVER" "manila.share.drivers.container.driver.ContainerShareDriver"
save_configuration "SHARE_BACKING_FILE_SIZE" "32000M"
save_configuration "SHARE_BACKING_FILE_SIZE" "64000M"
save_configuration "MANILA_DEFAULT_SHARE_TYPE_EXTRA_SPECS" "'snapshot_support=false'"
fi

32
manila/share/drivers/container/driver.py
View File

@ -132,6 +132,7 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin):
location = self._get_helper(share).create_share(server_id)
return location
@utils.synchronized('container_driver_delete_share_lock', external=True)
def delete_share(self, context, share, share_server=None):
LOG.debug("Deleting share %(share)s on server '%(server)s'." %
{"server": share_server["id"],
@ -143,10 +144,21 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin):
server_id,
["umount", "/shares/%s" % share.share_id]
)
self.container.execute(
server_id,
["rm", "-fR", "/shares/%s" % share.share_id]
)
# (aovchinnikov): bug 1621784 manifests itself here as well as in
# storage helper. There is a chance that we won't be able to remove
# this directory, despite the fact that it is not shared anymore and
# already contains nothing. In such case the driver should not fail
# share deletion, but issue a warning.
try:
self.container.execute(
server_id,
["rm", "-fR", "/shares/%s" % share.share_id]
)
except exception.ProcessExecutionError as e:
LOG.warning(_LW("Failed to remove /shares/%(share)s directory in "
"container %(cont)s."), {"share": share.share_id,
"cont": server_id})
LOG.error(e)
self.storage.remove_storage(share)
LOG.debug("Deletion of share %s is completed!", share.share_id)
@ -243,6 +255,7 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin):
LOG.debug("Now container %(id)s should be accessible from network "
"%(subnet)s by address %(ip)s." % msg_helper)
@utils.synchronized("container_driver_teardown_lock", external=True)
def _teardown_server(self, *args, **kwargs):
server_id = self._get_container_name(kwargs["server_details"]["id"])
self.container.stop_container(server_id)
@ -259,9 +272,14 @@ class ContainerShareDriver(driver.ShareDriver, driver.ExecuteMixin):
continue
elif container_id[0] == server_id:
LOG.debug("Deleting veth %s.", veth)
self._execute("ovs-vsctl", "--", "del-port",
self.configuration.container_ovs_bridge_name,
veth, run_as_root=True)
try:
self._execute("ovs-vsctl", "--", "del-port",
self.configuration.container_ovs_bridge_name,
veth, run_as_root=True)
except exception.ProcessExecutionError as e:
LOG.warning(_LW("Failed to delete port %s: port "
"vanished."), veth)
LOG.error(e)
def _get_veth_state(self):
result = self._execute("brctl", "show",

21
manila/share/drivers/container/storage_helper.py
View File

@ -17,9 +17,10 @@ import os
import re
from oslo_config import cfg
from oslo_log import log
from manila import exception
from manila.i18n import _
from manila.i18n import _, _LW
from manila.share import driver
@ -34,6 +35,7 @@ lv_opts = [
]
CONF.register_opts(lv_opts)
LOG = log.getLogger(__name__)
class LVMHelper(driver.ExecuteMixin):
@ -79,8 +81,21 @@ class LVMHelper(driver.ExecuteMixin):
run_as_root=True)
def remove_storage(self, share):
self._execute("lvremove", "-f", "--autobackup", "n",
self._get_lv_device(share), run_as_root=True)
to_remove = self._get_lv_device(share)
try:
self._execute("umount", to_remove, run_as_root=True)
except exception.ProcessExecutionError as e:
LOG.warning(_LW("Failed to umount helper directory %s."),
to_remove)
LOG.error(e)
# (aovchinnikov): bug 1621784 manifests itself in jamming logical
# volumes, so try removing once and issue warning until it is fixed.
try:
self._execute("lvremove", "-f", "--autobackup", "n",
to_remove, run_as_root=True)
except exception.ProcessExecutionError as e:
LOG.warning(_LW("Failed to remove logical volume %s.") % to_remove)
LOG.error(e)
def extend_share(self, share, new_size, share_server=None):
lv_device = self._get_lv_device(share)

37
manila/tests/share/drivers/container/test_driver.py
View File

@ -119,7 +119,7 @@ class ContainerShareDriverTestCase(test.TestCase):
self._driver.create_share(self._context, self.share,
{'id': 'fake'}))
def test_delete_share(self):
def test_delete_share_ok(self):
helper = mock.Mock()
self.mock_object(self._driver, "_get_helper",
mock.Mock(return_value=helper))
@ -133,6 +133,19 @@ class ContainerShareDriverTestCase(test.TestCase):
['rm', '-fR', '/shares/fakeshareid']
)
def test_delete_share_rm_fails(self):
def fake_execute(*args):
if 'rm' in args[1]:
raise exception.ProcessExecutionError()
self.mock_object(driver.LOG, "warning")
self.mock_object(self._driver, "_get_helper")
self.mock_object(self._driver.container, "execute", fake_execute)
self.mock_object(self._driver.storage, 'remove_storage')
self._driver.delete_share(self._context, self.share, {'id': 'fake'})
self.assertTrue(driver.LOG.warning.called)
def test_extend_share(self):
share = cont_fakes.fake_share()
actual_arguments = []
@ -193,7 +206,7 @@ class ContainerShareDriverTestCase(test.TestCase):
setattr(self._driver.configuration.local_conf,
'network_api_class',
'manila.share.drivers.container.driver.ContainerShareDriver')
driver.LOG.warning = mock.Mock()
self.mock_object(driver.LOG, "warning")
self._driver.check_for_setup_error()
@ -239,6 +252,26 @@ class ContainerShareDriverTestCase(test.TestCase):
self.assertEqual(expected_arguments.sort(), actual_arguments.sort())
@ddt.data(['veth0000000'], ['veth0000000' * 2])
def test__teardown_server_veth_disappeared_mysteriously(self,
list_of_veths):
def fake_ovs_execute(*args, **kwargs):
if len(args) == 3:
return list_of_veths
if len(args) == 4:
return ('fake:manila_b5afb5c1_6011_43c4_8a37_29820e6951a7', '')
if 'del-port' in args:
raise exception.ProcessExecutionError()
else:
return 0
self.mock_object(driver.LOG, "warning")
self.mock_object(self._driver, "_execute", fake_ovs_execute)
self._driver._teardown_server(
server_details={"id": "b5afb5c1-6011-43c4-8a37-29820e6951a7"})
self.assertTrue(driver.LOG.warning.called)
@ddt.data(['veth0000000'], ['veth0000000' * 2])
def test__teardown_server_check_continuation(self, list_of_veths):
def fake_ovs_execute(*args, **kwargs):

2
manila/tests/share/drivers/container/test_protocol_helper.py
View File

@ -216,7 +216,7 @@ class DockerCIFSHelperTestCase(test.TestCase):
self.DockerCIFSHelper._get_existing_users = mock.Mock()
self.DockerCIFSHelper._get_existing_users.side_effect = TypeError
self.DockerCIFSHelper._set_users = mock.Mock()
protocol_helper.LOG.warning = mock.Mock()
self.mock_object(protocol_helper.LOG, "warning")
self.DockerCIFSHelper._deny_access("fake_share", "fake_server_id",
"fake_user2", "rw")

25
manila/tests/share/drivers/container/test_storage_helper.py
View File

@ -85,6 +85,7 @@ class LVMHelperTestCase(test.TestCase):
def test_remove_storage(self):
actual_arguments = []
expected_arguments = [
('umount', '/dev/manila_docker_volumes/fakeshareid'),
('lvremove', '-f', '--autobackup', 'n',
'/dev/manila_docker_volumes/fakeshareid')
]
@ -96,6 +97,30 @@ class LVMHelperTestCase(test.TestCase):
self.assertEqual(expected_arguments, actual_arguments)
def test_remove_storage_umount_failed(self):
def fake_execute(*args, **kwargs):
if 'umount' in args:
raise exception.ProcessExecutionError()
self.mock_object(storage_helper.LOG, "warning")
self.mock_object(self.LVMHelper, "_execute", fake_execute)
self.LVMHelper.remove_storage(self.share)
self.assertTrue(storage_helper.LOG.warning.called)
def test_remove_storage_lvremove_failed(self):
def fake_execute(*args, **kwargs):
if 'lvremove' in args:
raise exception.ProcessExecutionError()
self.mock_object(storage_helper.LOG, "warning")
self.mock_object(self.LVMHelper, "_execute", fake_execute)
self.LVMHelper.remove_storage(self.share)
self.assertTrue(storage_helper.LOG.warning.called)
def test_extend_share(self):
actual_arguments = []
expected_arguments = [

4
releasenotes/notes/container-driver-hardening-against-races-30c9f517a6392b9d.yaml Normal file
View File

@ -0,0 +1,4 @@
---
fixes:
- Container driver. Fixed share and share server deletion concurrencies
by adding shared external lock.