x/vmware-nsx
Code Issues Proposed changes

Avoid performing extra query for fetching mac learning binding

Browse Source 
Bug 1214879

Add a relationship performing eager load in the Port model, thus preventing
the 'extend' function from performing an extra query.
This patch also replaces assertTrue with assertEqual in unit tests as it
needs to evaluate whether the value of the mac_learning_enabled attribute
is equal to the boolean literal True, whereas assertTrue verifies that the
expression passed to it evaluates to True. This means that any value but
False will be enough for the test to pass, which is not correct.

Change-Id: I03345ef3a125fdfa1ec7f6c3363d76efc12ced82
This commit is contained in:
Salvatore Orlando 2013-08-21 05:14:54 -07:00
parent 7095354c81
commit 2d0ac0c227
3 changed files with 41 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
neutron/plugins/nicira/NeutronPlugin.py
View File

@ -1228,8 +1228,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
with context.session.begin(subtransactions=True):
neutron_lports = super(NvpPluginV2, self).get_ports(
context, filters)
for neutron_lport in neutron_lports:
self._extend_port_mac_learning_state(context, neutron_lport)
if (filters.get('network_id') and len(filters.get('network_id')) and
self._network_is_external(context, filters['network_id'][0])):
# Do not perform check on NVP platform
@ -1422,6 +1420,9 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
with context.session.begin(subtransactions=True):
ret_port = super(NvpPluginV2, self).update_port(
context, id, port)
# Save current mac learning state to check whether it's
# being updated or not
old_mac_learning_state = ret_port.get(mac_ext.MAC_LEARNING)
# copy values over - except fixed_ips as
# they've alreaby been processed
port['port'].pop('fixed_ips', None)
@ -1459,15 +1460,11 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
context, ret_port)
# Populate the mac learning attribute
new_mac_learning_state = port['port'].get(mac_ext.MAC_LEARNING)
old_mac_learning_state = self._get_mac_learning_state(context, id)
if (new_mac_learning_state is not None and
old_mac_learning_state != new_mac_learning_state):
self._update_mac_learning_state(context, id,
new_mac_learning_state)
ret_port[mac_ext.MAC_LEARNING] = new_mac_learning_state
elif (new_mac_learning_state is None and
old_mac_learning_state is not None):
ret_port[mac_ext.MAC_LEARNING] = old_mac_learning_state
self._delete_port_queue_mapping(context, ret_port['id'])
self._process_port_queue_mapping(context, ret_port)
LOG.warn(_("Update port request: %s"), port)
@ -1561,7 +1558,6 @@ class NvpPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
neutron_db_port = super(NvpPluginV2, self).get_port(context,
id, fields)
self._extend_port_qos_queue(context, neutron_db_port)
self._extend_port_mac_learning_state(context, neutron_db_port)
if self._network_is_external(context,
neutron_db_port['network_id']):

29
neutron/plugins/nicira/dbexts/maclearning.py
View File

@ -16,9 +16,13 @@
#
import sqlalchemy as sa
from sqlalchemy import orm
from sqlalchemy.orm import exc
from neutron.api.v2 import attributes
from neutron.db import db_base_plugin_v2
from neutron.db import model_base
from neutron.db import models_v2
from neutron.openstack.common import log as logging
from neutron.plugins.nicira.extensions import maclearning as mac
@ -32,6 +36,13 @@ class MacLearningState(model_base.BASEV2):
primary_key=True)
mac_learning_enabled = sa.Column(sa.Boolean(), nullable=False)
# Add a relationship to the Port model using the backref attribute.
# This will instruct SQLAlchemy to eagerly load this association.
port = orm.relationship(
models_v2.Port,
backref=orm.backref("mac_learning_state", lazy='joined',
uselist=False, cascade='delete'))
class MacLearningDbMixin(object):
"""Mixin class for mac learning."""
@ -41,18 +52,14 @@ class MacLearningDbMixin(object):
mac.MAC_LEARNING: port[mac.MAC_LEARNING]}
return self._fields(res, fields)
def _get_mac_learning_state(self, context, port_id):
try:
query = self._model_query(context, MacLearningState)
state = query.filter(MacLearningState.port_id == port_id).one()
except exc.NoResultFound:
return None
return state[mac.MAC_LEARNING]
def _extend_port_mac_learning_state(self, port_res, port_db):
state = port_db.mac_learning_state
if state and state.mac_learning_enabled:
port_res[mac.MAC_LEARNING] = state.mac_learning_enabled
def _extend_port_mac_learning_state(self, context, port):
state = self._get_mac_learning_state(context, port['id'])
if state:
port[mac.MAC_LEARNING] = state
# Register dict extend functions for ports
db_base_plugin_v2.NeutronDbPluginV2.register_dict_extend_funcs(
attributes.PORTS, [_extend_port_mac_learning_state])
def _update_mac_learning_state(self, context, port_id, enabled):
try:

26
neutron/tests/unit/nicira/test_maclearning.py
View File

@ -91,15 +91,18 @@ class MacLearningDBTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
def test_create_with_mac_learning(self):
with self.port(arg_list=('mac_learning_enabled',),
mac_learning_enabled=True) as port:
# Validate create operation response
self.assertEqual(True, port['port']['mac_learning_enabled'])
# Verify that db operation successfully set mac learning state
req = self.new_show_request('ports', port['port']['id'], self.fmt)
sport = self.deserialize(self.fmt, req.get_response(self.api))
self.assertTrue(sport['port']['mac_learning_enabled'])
self.assertEqual(True, sport['port']['mac_learning_enabled'])
def test_create_port_without_mac_learning(self):
def test_create_and_show_port_without_mac_learning(self):
with self.port() as port:
req = self.new_show_request('ports', port['port']['id'], self.fmt)
sport = self.deserialize(self.fmt, req.get_response(self.api))
self.assertNotIn('mac_learning', sport['port'])
self.assertNotIn('mac_learning_enabled', sport['port'])
def test_update_port_with_mac_learning(self):
with self.port(arg_list=('mac_learning_enabled',),
@ -107,7 +110,7 @@ class MacLearningDBTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
data = {'port': {'mac_learning_enabled': True}}
req = self.new_update_request('ports', data, port['port']['id'])
res = self.deserialize(self.fmt, req.get_response(self.api))
self.assertTrue(res['port']['mac_learning_enabled'])
self.assertEqual(True, res['port']['mac_learning_enabled'])
def test_update_preexisting_port_with_mac_learning(self):
with self.port() as port:
@ -116,8 +119,13 @@ class MacLearningDBTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
self.assertNotIn('mac_learning_enabled', sport['port'])
data = {'port': {'mac_learning_enabled': True}}
req = self.new_update_request('ports', data, port['port']['id'])
# Validate update operation response
res = self.deserialize(self.fmt, req.get_response(self.api))
self.assertTrue(res['port']['mac_learning_enabled'])
self.assertEqual(True, res['port']['mac_learning_enabled'])
# Verify that db operation successfully updated mac learning state
req = self.new_show_request('ports', port['port']['id'], self.fmt)
sport = self.deserialize(self.fmt, req.get_response(self.api))
self.assertEqual(True, sport['port']['mac_learning_enabled'])
def test_list_ports(self):
# for this test we need to enable overlapping ips
@ -129,4 +137,10 @@ class MacLearningDBTestCase(test_db_plugin.NeutronDbPluginV2TestCase):
self.port(arg_list=('mac_learning_enabled',),
mac_learning_enabled=True)):
for port in self._list('ports')['ports']:
self.assertTrue(port['mac_learning_enabled'])
self.assertEqual(True, port['mac_learning_enabled'])
def test_show_port(self):
with self.port(arg_list=('mac_learning_enabled',),
mac_learning_enabled=True) as p:
port_res = self._show('ports', p['port']['id'])['port']
self.assertEqual(True, port_res['mac_learning_enabled'])