Skip to content

Commit

Permalink
Improve BlockDeviceMapping object cells awareness
Browse files Browse the repository at this point in the history
This patch makes BlockDeviceMapping object methods that are creating and
updating the database more aware of the cells. There are 2 changes that
are good to distinguish between:

 - create() will _never_ work in the API cell and will raise an
   ObjectActionError.
 - save() and destroy() are now aware in which cell they are being
   called and do not attempt to propagate it upwards

This is currently a no-op change as we don not currently use objects in
any of the code that will run in the top cell. This is in fact
preparation for transiting the API code to using BlockDeviceMapping
objects.

Change-Id: Ic5806c844f2deab734011d68f5da316903000f67
  • Loading branch information
djipko committed Jul 4, 2014
1 parent 52b2633 commit c3e7c7b
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 20 deletions.
30 changes: 22 additions & 8 deletions nova/objects/block_device.py
Expand Up @@ -13,6 +13,7 @@
# under the License.

from nova import block_device
from nova.cells import opts as cells_opts
from nova.cells import rpcapi as cells_rpcapi
from nova import db
from nova import exception
Expand Down Expand Up @@ -81,6 +82,13 @@ def _from_db_object(context, block_device_obj,

@base.remotable
def create(self, context):
cell_type = cells_opts.get_cell_type()
if cell_type == 'api':
raise exception.ObjectActionError(
action='create',
reason='BlockDeviceMapping cannot be '
'created in the API cell.')

if self.obj_attr_is_set('id'):
raise exception.ObjectActionError(action='create',
reason='already created')
Expand All @@ -90,8 +98,9 @@ def create(self, context):
reason='instance assigned')

db_bdm = db.block_device_mapping_create(context, updates, legacy=False)
cells_api = cells_rpcapi.CellsAPI()
cells_api.bdm_update_or_create_at_top(context, db_bdm, create=True)
if cell_type == 'compute':
cells_api = cells_rpcapi.CellsAPI()
cells_api.bdm_update_or_create_at_top(context, db_bdm, create=True)
self._from_db_object(context, self, db_bdm)

@base.remotable
Expand All @@ -101,10 +110,13 @@ def destroy(self, context):
reason='already destroyed')
db.block_device_mapping_destroy(context, self.id)
delattr(self, base.get_attrname('id'))
cells_api = cells_rpcapi.CellsAPI()
cells_api.bdm_destroy_at_top(context, self.instance_uuid,
device_name=self.device_name,
volume_id=self.volume_id)

cell_type = cells_opts.get_cell_type()
if cell_type == 'compute':
cells_api = cells_rpcapi.CellsAPI()
cells_api.bdm_destroy_at_top(context, self.instance_uuid,
device_name=self.device_name,
volume_id=self.volume_id)

@base.remotable
def save(self, context):
Expand All @@ -115,8 +127,10 @@ def save(self, context):
updates.pop('id', None)
updated = db.block_device_mapping_update(self._context, self.id,
updates, legacy=False)
cells_api = cells_rpcapi.CellsAPI()
cells_api.bdm_update_or_create_at_top(context, updated)
cell_type = cells_opts.get_cell_type()
if cell_type == 'compute':
cells_api = cells_rpcapi.CellsAPI()
cells_api.bdm_update_or_create_at_top(context, updated)
self._from_db_object(context, self, updated)

@base.remotable_classmethod
Expand Down
81 changes: 69 additions & 12 deletions nova/tests/objects/test_block_device.py
Expand Up @@ -43,7 +43,12 @@ def fake_bdm(self, instance=None):
fake_bdm['instance'] = instance
return fake_bdm

def test_save(self):
def _test_save(self, cell_type=None):
if cell_type:
self.flags(enable=True, cell_type=cell_type, group='cells')
else:
self.flags(enable=False, group='cells')

fake_bdm = self.fake_bdm()
with contextlib.nested(
mock.patch.object(
Expand All @@ -59,7 +64,20 @@ def test_save(self):
bdm_update_mock.assert_called_once_with(
self.context, 123, {'volume_id': 'fake_volume_id'},
legacy=False)
cells_update_mock.assert_called_once_with(self.context, fake_bdm)
if cell_type != 'compute':
self.assertFalse(cells_update_mock.called)
else:
cells_update_mock.assert_called_once_with(
self.context, fake_bdm)

def test_save_nocells(self):
self._test_save()

def test_save_apicell(self):
self._test_save(cell_type='api')

def test_save_computecell(self):
self._test_save(cell_type='compute')

def test_save_instance_changed(self):
bdm_object = objects.BlockDeviceMapping()
Expand Down Expand Up @@ -108,7 +126,11 @@ def test_get_by_volume_id_with_expected(self, get_by_vol_id):
['instance'])
self.assertRemotes()

def test_create_mocked(self):
def _test_create_mocked(self, cell_type=None):
if cell_type:
self.flags(enable=True, cell_type=cell_type, group='cells')
else:
self.flags(enable=False, group='cells')
values = {'source_type': 'volume', 'volume_id': 'fake-vol-id',
'destination_type': 'volume',
'instance_uuid': 'fake-instance'}
Expand All @@ -121,11 +143,30 @@ def test_create_mocked(self):
'bdm_update_or_create_at_top')
) as (bdm_create_mock, cells_update_mock):
bdm = objects.BlockDeviceMapping(**values)
bdm.create(self.context)
bdm_create_mock.assert_called_once_with(

if cell_type == 'api':
self.assertRaises(exception.ObjectActionError,
bdm.create, self.context)
elif cell_type == 'compute':
bdm.create(self.context)
bdm_create_mock.assert_called_once_with(
self.context, values, legacy=False)
cells_update_mock.assert_called_once_with(
self.context, fake_bdm, create=True)
else:
bdm.create(self.context)
self.assertFalse(cells_update_mock.called)
bdm_create_mock.assert_called_once_with(
self.context, values, legacy=False)
cells_update_mock.assert_called_once_with(
self.context, fake_bdm, create=True)

def test_create_nocells(self):
self._test_create_mocked()

def test_create_apicell(self):
self._test_create_mocked(cell_type='api')

def test_create_computecell(self):
self._test_create_mocked(cell_type='compute')

def test_create(self):
values = {'source_type': 'volume', 'volume_id': 'fake-vol-id',
Expand Down Expand Up @@ -158,21 +199,37 @@ def test_create_fails_instance(self):
self.assertRaises(exception.ObjectActionError,
bdm.create, self.context)

def test_destroy_mocked(self):
def _test_destroy_mocked(self, cell_type=None):
values = {'source_type': 'volume', 'volume_id': 'fake-vol-id',
'destination_type': 'volume', 'id': 1,
'instance_uuid': 'fake-instance', 'device_name': 'fake'}
if cell_type:
self.flags(enable=True, cell_type=cell_type, group='cells')
else:
self.flags(enable=False, group='cells')
with contextlib.nested(
mock.patch.object(db, 'block_device_mapping_destroy'),
mock.patch.object(cells_rpcapi.CellsAPI, 'bdm_destroy_at_top')
) as (bdm_del, cells_destroy):
bdm = objects.BlockDeviceMapping(**values)
bdm.destroy(self.context)
bdm_del.assert_called_once_with(self.context, values['id'])
cells_destroy.assert_called_once_with(
self.context, values['instance_uuid'],
device_name=values['device_name'],
volume_id=values['volume_id'])
if cell_type != 'compute':
self.assertFalse(cells_destroy.called)
else:
cells_destroy.assert_called_once_with(
self.context, values['instance_uuid'],
device_name=values['device_name'],
volume_id=values['volume_id'])

def test_destroy_nocells(self):
self._test_destroy_mocked()

def test_destroy_apicell(self):
self._test_destroy_mocked(cell_type='api')

def test_destroy_computecell(self):
self._test_destroy_mocked(cell_type='compute')


class TestBlockDeviceMappingObject(test_objects._LocalTest,
Expand Down

0 comments on commit c3e7c7b

Please sign in to comment.