Fix manage existing MissingDependencies error
When attempting to manage an existing volume a MissingDependencies error is logged to c-vol.log. Some of the tasks defined in ManageExistingTask#get_flow now require a 'volume' parameter not the 'volume_ref' parameter. To fix this issue, convert all 'volume_ref' parameter to 'volume' parameter in managing existing volume task flow. Co-Authored-By: wanghao <sxmatch1986@gmail.com> Change-Id: I2b6ad5d86acba6c3a130275c9c3617ec70ae62e0 Closes-Bug: 1617442
This commit is contained in:
Chuck Fouts
parent
fa7cd27577
commit
b9371615d3
|
|
@ -4668,6 +4668,28 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
self.assertRaises(exception.VolumeNotFound, volume.refresh)
|
||||
self.assertRaises(exception.SnapshotNotFound, snapshot.refresh)
|
||||
|
||||
@mock.patch('cinder.volume.drivers.lvm.LVMVolumeDriver.'
|
||||
'manage_existing')
|
||||
@mock.patch('cinder.volume.drivers.lvm.LVMVolumeDriver.'
|
||||
'manage_existing_get_size')
|
||||
@mock.patch('cinder.volume.utils.notify_about_volume_usage')
|
||||
def test_manage_volume_with_notify(self, mock_notify, mock_size,
|
||||
mock_manage):
|
||||
elevated = context.get_admin_context()
|
||||
vol_type = db.volume_type_create(
|
||||
elevated, {'name': 'type1', 'extra_specs': {}})
|
||||
# create source volume
|
||||
volume_params = {'volume_type_id': vol_type.id, 'status': 'managing'}
|
||||
test_vol = tests_utils.create_volume(self.context, **volume_params)
|
||||
mock_size.return_value = 1
|
||||
mock_manage.return_value = None
|
||||
|
||||
self.volume.manage_existing(self.context, None, 'volume_ref',
|
||||
test_vol)
|
||||
mock_notify.assert_called_with(self.context, test_vol,
|
||||
'manage_existing.end',
|
||||
host=test_vol.host)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class VolumeMigrationTestCase(BaseVolumeTestCase):
|
||||
|
|
|
|||
|
|
@ -11,7 +11,9 @@
|
|||
# under the License.
|
||||
""" Tests for manage_existing TaskFlow """
|
||||
|
||||
import inspect
|
||||
import mock
|
||||
import taskflow.engines
|
||||
|
||||
from cinder import context
|
||||
from cinder import test
|
||||
|
|
@ -95,3 +97,59 @@ class ManageVolumeFlowTestCase(test.TestCase):
|
|||
|
||||
task.revert(self.ctxt, mock_result, mock_flow_failures, volume_ref)
|
||||
mock_error_out.assert_called_once_with(volume_ref, reason=mock.ANY)
|
||||
|
||||
def test_get_flow(self):
|
||||
mock_volume_flow = mock.Mock()
|
||||
mock_linear_flow = self.mock_object(manager.linear_flow, 'Flow')
|
||||
mock_linear_flow.return_value = mock_volume_flow
|
||||
mock_taskflow_engine = self.mock_object(taskflow.engines, 'load')
|
||||
expected_store = {
|
||||
'context': mock.sentinel.context,
|
||||
'volume': mock.sentinel.volume,
|
||||
'manage_existing_ref': mock.sentinel.ref,
|
||||
'optional_args': {'is_quota_committed': False},
|
||||
}
|
||||
|
||||
manager.get_flow(
|
||||
mock.sentinel.context, mock.sentinel.db, mock.sentinel.driver,
|
||||
mock.sentinel.host, mock.sentinel.volume, mock.sentinel.ref)
|
||||
|
||||
mock_linear_flow.assert_called_once_with(
|
||||
'volume_manage_existing_manager')
|
||||
mock_taskflow_engine.assert_called_once_with(
|
||||
mock_volume_flow, store=expected_store)
|
||||
|
||||
def test_get_flow_volume_flow_tasks(self):
|
||||
"""Test that all expected parameter names exist for added tasks."""
|
||||
mock_taskflow_engine = self.mock_object(taskflow.engines, 'load')
|
||||
mock_taskflow_engine.side_effect = self._verify_volume_flow_tasks
|
||||
|
||||
manager.get_flow(
|
||||
mock.sentinel.context, mock.sentinel.db, mock.sentinel.driver,
|
||||
mock.sentinel.host, mock.sentinel.volume, mock.sentinel.ref)
|
||||
|
||||
def _verify_volume_flow_tasks(self, volume_flow, store=None):
|
||||
param_names = [
|
||||
'context',
|
||||
'volume',
|
||||
'manage_existing_ref',
|
||||
'optional_args',
|
||||
]
|
||||
|
||||
provides = {'self'}
|
||||
revert_provides = ['self', 'result', 'flow_failures']
|
||||
for node in volume_flow.iter_nodes():
|
||||
task = node[0]
|
||||
# Subsequent tasks may use parameters defined in a previous task's
|
||||
# default_provides list. Add these names to the provides set.
|
||||
if task.default_provides:
|
||||
for p in task.default_provides:
|
||||
provides.add(p)
|
||||
|
||||
execute_args = inspect.getargspec(task.execute)[0]
|
||||
execute_args = [x for x in execute_args if x not in provides]
|
||||
[self.assertIn(arg, param_names) for arg in execute_args]
|
||||
|
||||
revert_args = inspect.getargspec(task.revert)[0]
|
||||
revert_args = [x for x in revert_args if x not in revert_provides]
|
||||
[self.assertIn(arg, param_names) for arg in revert_args]
|
||||
|
|
|
|||
|
|
@ -39,31 +39,29 @@ class PrepareForQuotaReservationTask(flow_utils.CinderTask):
|
|||
self.db = db
|
||||
self.driver = driver
|
||||
|
||||
def execute(self, context, volume_ref, manage_existing_ref):
|
||||
volume_id = volume_ref.id
|
||||
def execute(self, context, volume, manage_existing_ref):
|
||||
if not self.driver.initialized:
|
||||
driver_name = self.driver.__class__.__name__
|
||||
LOG.error(_LE("Unable to manage existing volume. "
|
||||
"Volume driver %s not initialized.") % driver_name)
|
||||
flow_common.error_out(volume_ref, _("Volume driver %s not "
|
||||
"initialized.") % driver_name)
|
||||
flow_common.error_out(volume, _("Volume driver %s not "
|
||||
"initialized.") % driver_name)
|
||||
raise exception.DriverNotInitialized()
|
||||
|
||||
size = self.driver.manage_existing_get_size(volume_ref,
|
||||
size = self.driver.manage_existing_get_size(volume,
|
||||
manage_existing_ref)
|
||||
|
||||
return {'size': size,
|
||||
'volume_type_id': volume_ref.volume_type_id,
|
||||
'volume_properties': volume_ref,
|
||||
'volume_spec': {'status': volume_ref.status,
|
||||
'volume_name': volume_ref.name,
|
||||
'volume_id': volume_id}}
|
||||
'volume_type_id': volume.volume_type_id,
|
||||
'volume_properties': volume,
|
||||
'volume_spec': {'status': volume.status,
|
||||
'volume_name': volume.name,
|
||||
'volume_id': volume.id}}
|
||||
|
||||
def revert(self, context, result, flow_failures, volume_ref, **kwargs):
|
||||
volume_id = volume_ref.id
|
||||
def revert(self, context, result, flow_failures, volume, **kwargs):
|
||||
reason = _('Volume manage failed.')
|
||||
flow_common.error_out(volume_ref, reason=reason)
|
||||
LOG.error(_LE("Volume %s: manage failed."), volume_id)
|
||||
flow_common.error_out(volume, reason=reason)
|
||||
LOG.error(_LE("Volume %s: manage failed."), volume.id)
|
||||
|
||||
|
||||
class ManageExistingTask(flow_utils.CinderTask):
|
||||
|
|
@ -76,24 +74,24 @@ class ManageExistingTask(flow_utils.CinderTask):
|
|||
self.db = db
|
||||
self.driver = driver
|
||||
|
||||
def execute(self, context, volume_ref, manage_existing_ref, size):
|
||||
model_update = self.driver.manage_existing(volume_ref,
|
||||
def execute(self, context, volume, manage_existing_ref, size):
|
||||
model_update = self.driver.manage_existing(volume,
|
||||
manage_existing_ref)
|
||||
|
||||
if not model_update:
|
||||
model_update = {}
|
||||
model_update.update({'size': size})
|
||||
try:
|
||||
volume_ref.update(model_update)
|
||||
volume_ref.save()
|
||||
volume.update(model_update)
|
||||
volume.save()
|
||||
except exception.CinderException:
|
||||
LOG.exception(_LE("Failed updating model of volume %(volume_id)s"
|
||||
" with creation provided model %(model)s") %
|
||||
{'volume_id': volume_ref['id'],
|
||||
{'volume_id': volume.id,
|
||||
'model': model_update})
|
||||
raise
|
||||
|
||||
return {'volume': volume_ref}
|
||||
return {'volume': volume}
|
||||
|
||||
|
||||
def get_flow(context, db, driver, host, volume, ref):
|
||||
|
|
@ -107,9 +105,9 @@ def get_flow(context, db, driver, host, volume, ref):
|
|||
# determined.
|
||||
create_what = {
|
||||
'context': context,
|
||||
'volume_ref': volume,
|
||||
'volume': volume,
|
||||
'manage_existing_ref': ref,
|
||||
'optional_args': {'is_quota_committed': False}
|
||||
'optional_args': {'is_quota_committed': False},
|
||||
}
|
||||
|
||||
volume_flow.add(create_mgr.NotifyVolumeActionTask(db,
|
||||
|
|
|
|||
Loading…
Reference in New Issue