From 3aec5274da29c81c76bb7f87317160e32f9780ff Mon Sep 17 00:00:00 2001 From: TommyLike Date: Tue, 29 May 2018 12:17:39 +0800 Subject: [PATCH] Add user messages for extend volume operation Add user messages for these cases below when performing extend volume operation. 1. When no valid host is found. 2. When nova failed to perform extend volume event. 3. When backend failed to extend block device. Change-Id: I7d2061ac13b2c74745da56feecfe6ed53b284fc4 Closes-Bug: #1773833 --- cinder/compute/nova.py | 20 +++++++++- cinder/message/message_field.py | 12 +++++- cinder/scheduler/manager.py | 6 +++ cinder/tests/unit/compute/test_nova.py | 40 ++++++++++++++++++- cinder/tests/unit/scheduler/test_scheduler.py | 9 ++++- cinder/tests/unit/volume/test_volume.py | 27 ++++++++----- cinder/volume/manager.py | 5 +++ 7 files changed, 104 insertions(+), 15 deletions(-) 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'})