openstack
/
cinder
Code Issues Proposed changes

Revert status to previous state on extend failure 

When Cinder fails to schedule extend volume action, the
status will be set to 'available' no matter what the
previous status is.

Change-Id: I830c11cda252fe25e1c069a425bdebe8c98f7a8d
Closes-Bug: 1718374
This commit is contained in:
TommyLike 2017-09-20 16:29:10 +08:00
parent 6c236bca3d
commit ce3830a01a
4 changed files with 19 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
cinder/scheduler/manager.py
View File

@ -339,7 +339,8 @@ class SchedulerManager(manager.CleanableManager, manager.Manager):
request_spec=None, filter_properties=None): request_spec=None, filter_properties=None):
def _extend_volume_set_error(self, context, ex, request_spec): def _extend_volume_set_error(self, context, ex, request_spec):
volume_state = {'volume_state': {'status': 'available'}} volume_state = {'volume_state': {'status': volume.previous_status,
'previous_status': None}}
self._set_volume_state_and_notify('extend_volume', volume_state, self._set_volume_state_and_notify('extend_volume', volume_state,
context, ex, request_spec) context, ex, request_spec)

16
cinder/tests/unit/scheduler/test_scheduler.py
View File

@ -19,7 +19,7 @@ Tests For Scheduler
import collections import collections
from datetime import datetime from datetime import datetime
import ddt
import mock import mock
from oslo_config import cfg from oslo_config import cfg
@ -38,6 +38,7 @@ from cinder.tests.unit import utils as tests_utils
CONF = cfg.CONF CONF = cfg.CONF
@ddt.ddt
class SchedulerManagerTestCase(test.TestCase): class SchedulerManagerTestCase(test.TestCase):
"""Test case for scheduler manager.""" """Test case for scheduler manager."""
@ -117,14 +118,18 @@ class SchedulerManagerTestCase(test.TestCase):
mock_extend.assert_called_once_with( mock_extend.assert_called_once_with(
self.context, volume, 2, 'fake_reservation') self.context, volume, 2, 'fake_reservation')
@ddt.data('available', 'in-use')
@mock.patch('cinder.scheduler.driver.Scheduler.backend_passes_filters') @mock.patch('cinder.scheduler.driver.Scheduler.backend_passes_filters')
@mock.patch( @mock.patch(
'cinder.scheduler.host_manager.BackendState.consume_from_volume') 'cinder.scheduler.host_manager.BackendState.consume_from_volume')
@mock.patch('cinder.volume.rpcapi.VolumeAPI.extend_volume') @mock.patch('cinder.volume.rpcapi.VolumeAPI.extend_volume')
@mock.patch('cinder.quota.QUOTAS.rollback') @mock.patch('cinder.quota.QUOTAS.rollback')
def test_extend_volume_no_valid_host(self, mock_rollback, mock_extend, def test_extend_volume_no_valid_host(self, status, mock_rollback,
mock_consume, mock_backend_passes): mock_extend, mock_consume,
volume = fake_volume.fake_volume_obj(self.context, **{'size': 1}) mock_backend_passes):
volume = fake_volume.fake_volume_obj(self.context,
**{'size': 1,
'previous_status': status})
no_valid_backend = exception.NoValidBackend(reason='') no_valid_backend = exception.NoValidBackend(reason='')
mock_backend_passes.side_effect = [no_valid_backend] mock_backend_passes.side_effect = [no_valid_backend]
@ -133,7 +138,8 @@ class SchedulerManagerTestCase(test.TestCase):
self.manager.extend_volume(self.context, volume, 2, self.manager.extend_volume(self.context, volume, 2,
'fake_reservation') 'fake_reservation')
mock_notify.assert_called_once_with( mock_notify.assert_called_once_with(
'extend_volume', {'volume_state': {'status': 'available'}}, 'extend_volume', {'volume_state': {'status': status,
'previous_status': None}},
self.context, no_valid_backend, None) self.context, no_valid_backend, None)
mock_rollback.assert_called_once_with( mock_rollback.assert_called_once_with(
self.context, 'fake_reservation', project_id=volume.project_id) self.context, 'fake_reservation', project_id=volume.project_id)

4
cinder/tests/unit/volume/test_volume.py
View File

@ -2051,10 +2051,12 @@ class VolumeTestCase(base.BaseVolumeTestCase):
volume, 3, attached=True) volume, 3, attached=True)
db.volume_update(self.context, volume.id, {'status': 'in-use'}) db.volume_update(self.context, volume.id, {'status': 'in-use'})
volume.refresh()
reserve.return_value = ["RESERVATION"] reserve.return_value = ["RESERVATION"]
volume_api._extend(self.context, volume, 3, attached=True) volume_api._extend(self.context, volume, 3, attached=True)
volume.refresh() volume.refresh()
self.assertEqual('extending', volume.status) self.assertEqual('extending', volume.status)
self.assertEqual('in-use', volume.previous_status)
reserve.assert_called_once_with(self.context, gigabytes=1, reserve.assert_called_once_with(self.context, gigabytes=1,
project_id=volume.project_id) project_id=volume.project_id)
limit_check.side_effect = None limit_check.side_effect = None
@ -2091,6 +2093,7 @@ class VolumeTestCase(base.BaseVolumeTestCase):
3) 3)
db.volume_update(self.context, volume.id, {'status': 'available'}) db.volume_update(self.context, volume.id, {'status': 'available'})
volume.refresh()
# Extend fails when new_size < orig_size # Extend fails when new_size < orig_size
self.assertRaises(exception.InvalidInput, self.assertRaises(exception.InvalidInput,
volume_api._extend, volume_api._extend,
@ -2110,6 +2113,7 @@ class VolumeTestCase(base.BaseVolumeTestCase):
volume_api._extend(self.context, volume, 3) volume_api._extend(self.context, volume, 3)
volume.refresh() volume.refresh()
self.assertEqual('extending', volume.status) self.assertEqual('extending', volume.status)
self.assertEqual('available', volume.previous_status)
reserve.assert_called_once_with(self.context, gigabytes=1, reserve.assert_called_once_with(self.context, gigabytes=1,
project_id=volume.project_id) project_id=volume.project_id)

3
cinder/volume/api.py
View File

@ -1301,7 +1301,8 @@ class API(base.Base):
return response return response
def _extend(self, context, volume, new_size, attached=False): def _extend(self, context, volume, new_size, attached=False):
value = {'status': 'extending'} value = {'status': 'extending',
'previous_status': volume.status}
if attached: if attached:
expected = {'status': 'in-use'} expected = {'status': 'in-use'}
else: else: