diff --git a/cinder/compute/nova.py b/cinder/compute/nova.py index d81def661b5..13c0a51c349 100644 --- a/cinder/compute/nova.py +++ b/cinder/compute/nova.py @@ -27,6 +27,8 @@ from requests import exceptions as request_exceptions from cinder.db import base from cinder import exception +from cinder.message import api as message_api +from cinder.message import message_field from cinder import service_auth nova_opts = [ @@ -131,6 +133,9 @@ def novaclient(context, privileged_user=False, timeout=None, api_version=None): class API(base.Base): """API for interacting with novaclient.""" + def __init__(self): + self.message_api = message_api.API() + def _get_volume_extended_event(self, server_id, volume_id): return {'name': 'volume-extended', 'server_uuid': server_id, @@ -143,12 +148,14 @@ class API(base.Base): response = nova.server_external_events.create(events) except nova_exceptions.NotFound: LOG.warning('Nova returned NotFound for events: %s.', events) + return False except Exception: LOG.exception('Failed to notify nova on events: %s.', events) + return False else: if not isinstance(response, list): LOG.error('Error response returned from nova: %s.', response) - return + return False response_error = False for event in response: code = event.get('code') @@ -162,6 +169,8 @@ class API(base.Base): LOG.info('Nova event response: %s.', event) if response_error: LOG.error('Error response returned from nova: %s.', response) + return False + return True def has_extension(self, context, extension, timeout=None): try: @@ -207,4 +216,11 @@ class API(base.Base): api_version = '2.51' events = [self._get_volume_extended_event(server_id, volume_id) for server_id in server_ids] - self._send_events(context, events, api_version=api_version) + result = self._send_events(context, events, api_version=api_version) + if not result: + self.message_api.create( + context, + message_field.Action.EXTEND_VOLUME, + resource_uuid=volume_id, + detail=message_field.Detail.NOTIFY_COMPUTE_SERVICE_FAILED) + return result diff --git a/cinder/message/message_field.py b/cinder/message/message_field.py index 8f846575de2..19bc7bc94b4 100644 --- a/cinder/message/message_field.py +++ b/cinder/message/message_field.py @@ -37,13 +37,15 @@ class Action(object): UPDATE_ATTACHMENT = ('004', _('update attachment')) COPY_IMAGE_TO_VOLUME = ('005', _('copy image to volume')) UNMANAGE_VOLUME = ('006', _('unmanage volume')) + EXTEND_VOLUME = ('007', _('extend volume')) ALL = (SCHEDULE_ALLOCATE_VOLUME, ATTACH_VOLUME, COPY_VOLUME_TO_IMAGE, UPDATE_ATTACHMENT, COPY_IMAGE_TO_VOLUME, - UNMANAGE_VOLUME + UNMANAGE_VOLUME, + EXTEND_VOLUME ) @@ -68,6 +70,12 @@ class Detail(object): UNMANAGE_ENC_NOT_SUPPORTED = ( '008', _("Unmanaging encrypted volumes is not supported.")) + NOTIFY_COMPUTE_SERVICE_FAILED = ( + '009', + _("Compute service failed to extend volume.")) + DRIVER_FAILED_EXTEND = ( + '010', + _("Volume Driver failed to extend volume.")) ALL = (UNKNOWN_ERROR, DRIVER_NOT_INITIALIZED, @@ -77,6 +85,8 @@ class Detail(object): QUOTA_EXCEED, NOT_ENOUGH_SPACE_FOR_IMAGE, UNMANAGE_ENC_NOT_SUPPORTED, + NOTIFY_COMPUTE_SERVICE_FAILED, + DRIVER_FAILED_EXTEND ) # Exception and detail mappings diff --git a/cinder/scheduler/manager.py b/cinder/scheduler/manager.py index 85e6f6fe84e..9ee445fbb31 100644 --- a/cinder/scheduler/manager.py +++ b/cinder/scheduler/manager.py @@ -40,6 +40,7 @@ from cinder import flow_utils from cinder.i18n import _ from cinder import manager from cinder.message import api as mess_api +from cinder.message import message_field from cinder import objects from cinder.objects import fields from cinder import quota @@ -450,6 +451,11 @@ class SchedulerManager(manager.CleanableManager, manager.Manager): QUOTAS.rollback(context, reservations, project_id=volume.project_id) _extend_volume_set_error(self, context, ex, request_spec) + self.message_api.create( + context, + message_field.Action.EXTEND_VOLUME, + resource_uuid=volume.id, + exception=ex) def _set_volume_state_and_notify(self, method, updates, context, ex, request_spec, msg=None): diff --git a/cinder/tests/unit/compute/test_nova.py b/cinder/tests/unit/compute/test_nova.py index 2eb6bc79ad1..89239cd41e1 100644 --- a/cinder/tests/unit/compute/test_nova.py +++ b/cinder/tests/unit/compute/test_nova.py @@ -12,10 +12,12 @@ # License for the specific language governing permissions and limitations # under the License. +import ddt import mock from cinder.compute import nova from cinder import context +from cinder.message import message_field from cinder import test from keystoneauth1 import loading as ks_loading from novaclient import exceptions as nova_exceptions @@ -196,6 +198,7 @@ class FakeNovaClient(object): pass +@ddt.ddt class NovaApiTestCase(test.TestCase): def setUp(self): super(NovaApiTestCase, self).setUp() @@ -228,8 +231,10 @@ class NovaApiTestCase(test.TestCase): mock.patch.object(self.novaclient.server_external_events, 'create') as mock_create_event: mock_novaclient.return_value = self.novaclient + mock_create_event.return_value = [] - self.api.extend_volume(self.ctx, server_ids, 'volume_id') + result = self.api.extend_volume(self.ctx, server_ids, 'volume_id') + self.assertTrue(result) mock_novaclient.assert_called_once_with(self.ctx, privileged_user=True, @@ -242,3 +247,36 @@ class NovaApiTestCase(test.TestCase): 'server_uuid': 'server-id-2', 'tag': 'volume_id'}, ]) + + @ddt.data(nova_exceptions.NotFound, + Exception, + 'illegal_list', + [{'code': None}]) + @mock.patch('cinder.message.api.API.create') + def test_extend_volume_failed(self, nova_result, mock_create): + server_ids = ['server-id-1', 'server-id-2'] + with mock.patch.object(nova, 'novaclient') as mock_novaclient, \ + mock.patch.object(self.novaclient.server_external_events, + 'create') as mock_create_event: + mock_novaclient.return_value = self.novaclient + mock_create_event.side_effect = [nova_result] + + result = self.api.extend_volume(self.ctx, server_ids, 'volume_id') + self.assertFalse(result) + + mock_novaclient.assert_called_once_with(self.ctx, + privileged_user=True, + api_version='2.51') + mock_create.assert_called_once_with( + self.ctx, + message_field.Action.EXTEND_VOLUME, + resource_uuid='volume_id', + detail=message_field.Detail.NOTIFY_COMPUTE_SERVICE_FAILED) + mock_create_event.assert_called_once_with([ + {'name': 'volume-extended', + 'server_uuid': 'server-id-1', + 'tag': 'volume_id'}, + {'name': 'volume-extended', + 'server_uuid': 'server-id-2', + 'tag': 'volume_id'}, + ]) diff --git a/cinder/tests/unit/scheduler/test_scheduler.py b/cinder/tests/unit/scheduler/test_scheduler.py index 4fb8fc40ce4..ff082649611 100644 --- a/cinder/tests/unit/scheduler/test_scheduler.py +++ b/cinder/tests/unit/scheduler/test_scheduler.py @@ -181,7 +181,9 @@ class SchedulerManagerTestCase(test.TestCase): 'cinder.scheduler.host_manager.BackendState.consume_from_volume') @mock.patch('cinder.volume.rpcapi.VolumeAPI.extend_volume') @mock.patch('cinder.quota.QUOTAS.rollback') - def test_extend_volume_no_valid_host(self, status, mock_rollback, + @mock.patch('cinder.message.api.API.create') + def test_extend_volume_no_valid_host(self, status, mock_create, + mock_rollback, mock_extend, mock_consume, mock_backend_passes): volume = fake_volume.fake_volume_obj(self.context, @@ -202,6 +204,11 @@ class SchedulerManagerTestCase(test.TestCase): self.context, 'fake_reservation', project_id=volume.project_id) mock_consume.assert_not_called() mock_extend.assert_not_called() + mock_create.assert_called_once_with( + self.context, + message_field.Action.EXTEND_VOLUME, + resource_uuid=volume.id, + exception=no_valid_backend) @mock.patch('cinder.quota.QuotaEngine.expire') def test_clean_expired_reservation(self, mock_clean): diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index ec34c0b50ef..f88af88f0f9 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -36,6 +36,7 @@ from cinder import context from cinder import coordination from cinder import db from cinder import exception +from cinder.message import message_field from cinder import objects from cinder.objects import fields from cinder.policies import volumes as vol_policy @@ -2441,16 +2442,22 @@ class VolumeTestCase(base.BaseVolumeTestCase): fake_reservations = ['RESERVATION'] # Test driver exception - with mock.patch.object(self.volume.driver, - 'extend_volume') as extend_volume: - extend_volume.side_effect =\ - exception.CinderException('fake exception') - volume['status'] = 'extending' - self.volume.extend_volume(self.context, volume, '4', - fake_reservations) - volume.refresh() - self.assertEqual(2, volume.size) - self.assertEqual('error_extending', volume.status) + with mock.patch.object( + self.volume.driver, 'extend_volume', + side_effect=exception.CinderException('fake exception')): + with mock.patch.object( + self.volume.message_api, 'create') as mock_create: + volume['status'] = 'extending' + self.volume.extend_volume(self.context, volume, '4', + fake_reservations) + volume.refresh() + self.assertEqual(2, volume.size) + self.assertEqual('error_extending', volume.status) + mock_create.assert_called_once_with( + self.context, + message_field.Action.EXTEND_VOLUME, + resource_uuid=volume.id, + detail=message_field.Detail.DRIVER_FAILED_EXTEND) @mock.patch('cinder.compute.API') def _test_extend_volume_manager_successful(self, volume, nova_api): diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 91ab0d2c58d..332ade86ad6 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -2605,6 +2605,11 @@ class VolumeManager(manager.CleanableManager, except Exception: LOG.exception("Extend volume failed.", resource=volume) + self.message_api.create( + context, + message_field.Action.EXTEND_VOLUME, + resource_uuid=volume.id, + detail=message_field.Detail.DRIVER_FAILED_EXTEND) try: self.db.volume_update(context, volume.id, {'status': 'error_extending'})