openstack/cinder
Code Issues Proposed changes

Merge "Fail manage operations if service is down"

Browse Source
This commit is contained in:
Jenkins 2016-08-06 15:26:06 +00:00 committed by Gerrit Code Review
commit 8c847e677b
4 changed files with 104 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
cinder/tests/unit/api/contrib/test_snapshot_manage.py
View File

@ -112,7 +112,7 @@ class SnapshotManageTest(test.TestCase):
@mock.patch('cinder.db.service_get')
def test_manage_snapshot_ok(self, mock_db,
mock_create_snapshot, mock_rpcapi):
"""Test successful manage volume execution.
"""Test successful manage snapshot execution.
Tests for correct operation when valid arguments are passed in the
request body. We ensure that cinder.volume.api.API.manage_existing got
@ -144,6 +144,41 @@ class SnapshotManageTest(test.TestCase):
args = mock_rpcapi.call_args[0]
self.assertEqual('fake_ref', args[2])
@mock.patch('cinder.utils.service_is_up', return_value=True)
@mock.patch('cinder.volume.rpcapi.VolumeAPI.manage_existing_snapshot')
@mock.patch('cinder.volume.api.API.create_snapshot_in_db')
@mock.patch('cinder.db.service_get')
def test_manage_snapshot_disabled(self, mock_db, mock_create_snapshot,
mock_rpcapi, mock_is_up):
"""Test manage snapshot failure due to disabled service."""
mock_db.return_value = fake_service.fake_service_obj(self._admin_ctxt,
disabled=True)
body = {'snapshot': {'volume_id': fake.VOLUME_ID, 'ref': 'fake_ref'}}
res = self._get_resp_post(body)
self.assertEqual(400, res.status_int, res)
self.assertEqual(exception.ServiceUnavailable.message,
res.json['badRequest']['message'])
mock_create_snapshot.assert_not_called()
mock_rpcapi.assert_not_called()
mock_is_up.assert_not_called()
@mock.patch('cinder.utils.service_is_up', return_value=False)
@mock.patch('cinder.volume.rpcapi.VolumeAPI.manage_existing_snapshot')
@mock.patch('cinder.volume.api.API.create_snapshot_in_db')
@mock.patch('cinder.db.service_get')
def test_manage_snapshot_is_down(self, mock_db, mock_create_snapshot,
mock_rpcapi, mock_is_up):
"""Test manage snapshot failure due to down service."""
mock_db.return_value = fake_service.fake_service_obj(self._admin_ctxt)
body = {'snapshot': {'volume_id': fake.VOLUME_ID, 'ref': 'fake_ref'}}
res = self._get_resp_post(body)
self.assertEqual(400, res.status_int, res)
self.assertEqual(exception.ServiceUnavailable.message,
res.json['badRequest']['message'])
mock_create_snapshot.assert_not_called()
mock_rpcapi.assert_not_called()
self.assertTrue(mock_is_up.called)
def test_manage_snapshot_missing_volume_id(self):
"""Test correct failure when volume_id is not specified."""
body = {'snapshot': {'ref': 'fake_ref'}}
@ -240,3 +275,24 @@ class SnapshotManageTest(test.TestCase):
mock_api_manageable.assert_called_once_with(
self._admin_ctxt, 'fakehost', limit=10, marker='1234', offset=4,
sort_dirs=['asc'], sort_keys=['reference'])
@mock.patch('cinder.utils.service_is_up', return_value=True)
@mock.patch('cinder.db.service_get')
def test_get_manageable_snapshots_disabled(self, mock_db, mock_is_up):
mock_db.return_value = fake_service.fake_service_obj(self._admin_ctxt,
disabled=True)
res = self._get_resp_get('host_ok', False, True)
self.assertEqual(400, res.status_int, res)
self.assertEqual(exception.ServiceUnavailable.message,
res.json['badRequest']['message'])
mock_is_up.assert_not_called()
@mock.patch('cinder.utils.service_is_up', return_value=False)
@mock.patch('cinder.db.service_get')
def test_get_manageable_snapshots_is_down(self, mock_db, mock_is_up):
mock_db.return_value = fake_service.fake_service_obj(self._admin_ctxt)
res = self._get_resp_get('host_ok', False, True)
self.assertEqual(400, res.status_int, res)
self.assertEqual(exception.ServiceUnavailable.message,
res.json['badRequest']['message'])
self.assertTrue(mock_is_up.called)

45
cinder/tests/unit/api/contrib/test_volume_manage.py
View File

@ -41,7 +41,7 @@ def app():
return mapper
def service_get_by_host_and_topic(context, host, topic):
def service_get(context, host, binary):
"""Replacement for Service.service_get_by_host_and_topic.
We mock the Service.service_get_by_host_and_topic method to return
@ -50,7 +50,9 @@ def service_get_by_host_and_topic(context, host, topic):
check for existence of a host, so the content returned doesn't matter.
"""
if host == 'host_ok':
return {}
return {'disabled': False}
if host == 'host_disabled':
return {'disabled': True}
raise exception.ServiceNotFound(service_id=host)
# Some of the tests check that volume types are correctly validated during a
@ -128,8 +130,7 @@ def api_get_manageable_volumes(*args, **kwargs):
@ddt.ddt
@mock.patch('cinder.objects.service.Service.get_by_host_and_topic',
service_get_by_host_and_topic)
@mock.patch('cinder.db.service_get', service_get)
@mock.patch('cinder.volume.volume_types.get_volume_type_by_name',
vt_get_volume_type_by_name)
@mock.patch('cinder.volume.volume_types.get_volume_type',
@ -210,6 +211,26 @@ class VolumeManageTest(test.TestCase):
res = self._get_resp_post(body)
self.assertEqual(400, res.status_int)
@mock.patch('cinder.utils.service_is_up', return_value=True)
def test_manage_volume_disabled(self, mock_is_up):
"""Test manage volume failure due to disabled service."""
body = {'volume': {'host': 'host_disabled', 'ref': 'fake_ref'}}
res = self._get_resp_post(body)
self.assertEqual(400, res.status_int, res)
self.assertEqual(exception.ServiceUnavailable.message,
res.json['badRequest']['message'])
mock_is_up.assert_not_called()
@mock.patch('cinder.utils.service_is_up', return_value=False)
def test_manage_volume_is_down(self, mock_is_up):
"""Test manage volume failure due to down service."""
body = {'volume': {'host': 'host_ok', 'ref': 'fake_ref'}}
res = self._get_resp_post(body)
self.assertEqual(400, res.status_int, res)
self.assertEqual(exception.ServiceUnavailable.message,
res.json['badRequest']['message'])
self.assertTrue(mock_is_up.called)
@mock.patch('cinder.volume.api.API.manage_existing', api_manage)
@mock.patch(
'cinder.api.openstack.wsgi.Controller.validate_name_and_description')
@ -335,3 +356,19 @@ class VolumeManageTest(test.TestCase):
"metadata": value}}
res = self._get_resp_post(body)
self.assertEqual(400, res.status_int)
@mock.patch('cinder.utils.service_is_up', return_value=True)
def test_get_manageable_volumes_disabled(self, mock_is_up):
res = self._get_resp_get('host_disabled', False, True)
self.assertEqual(400, res.status_int, res)
self.assertEqual(exception.ServiceUnavailable.message,
res.json['badRequest']['message'])
mock_is_up.assert_not_called()
@mock.patch('cinder.utils.service_is_up', return_value=False)
def test_get_manageable_volumes_is_down(self, mock_is_up):
res = self._get_resp_get('host_ok', False, True)
self.assertEqual(400, res.status_int, res)
self.assertEqual(exception.ServiceUnavailable.message,
res.json['badRequest']['message'])
self.assertTrue(mock_is_up.called)

2
cinder/tests/unit/api/v3/test_volume_manage.py
View File

@ -37,7 +37,7 @@ def app():
@mock.patch('cinder.objects.service.Service.get_by_host_and_topic',
test_contrib.service_get_by_host_and_topic)
test_contrib.service_get)
@mock.patch('cinder.volume.volume_types.get_volume_type_by_name',
test_contrib.vt_get_volume_type_by_name)
@mock.patch('cinder.volume.volume_types.get_volume_type',

5
cinder/volume/api.py
View File

@ -1485,6 +1485,11 @@ class API(base.Base):
'service.'), resource)
raise exception.ServiceUnavailable()
if not utils.service_is_up(service):
LOG.error(_LE('Unable to manage existing %s on a service that is '
'down.'), resource)
raise exception.ServiceUnavailable()
return service
def manage_existing(self, context, host, ref, name=None, description=None,