openstack
/
neutron
Code Issues Proposed changes

Update trunk metadata during wire/unwire operations 

Before this patch metadata on the OVS trunk bridge were
not updated during unwiring operations and upon subsequent
wire operations old subports were getting wiped from the
external_ids field of the OVS parent port. This could lead
to OVS ports left behind on the host on trunk removal.

This patch ensures that the subports stored as metadata
reflect the logical state of the trunk.

Closes-bug: #1625875

Change-Id: I9d885b10d84fad43c527d3ca7bc9a37439197fc9
This commit is contained in:
Armando Migliaccio 2016-09-20 19:34:32 -07:00
parent 4371ae010a
commit 29457631ad
3 changed files with 107 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py
View File

@ -195,6 +195,8 @@ class OVSDBHandler(object):
:param port: Parent port dict.
"""
try:
# TODO(jlibosva): Investigate how to proceed during removal of
# trunk bridge that doesn't have metadata stored.
parent_port_id, trunk_id, subport_ids = self._get_trunk_metadata(
port)
# NOTE(status_police): we do not report changes in trunk status on
@ -242,16 +244,14 @@ class OVSDBHandler(object):
subport_ids.append(subport.port_id)
try:
self._set_trunk_metadata(
self._update_trunk_metadata(
trunk_bridge, parent_port, trunk_id, subport_ids)
except RuntimeError:
LOG.error(_LE("Failed to set metadata for trunk %s"), trunk_id)
# NOTE(status_police): Trunk bridge has missing metadata now, it
# will cause troubles during deletion.
# TODO(jlibosva): Investigate how to proceed during removal of
# trunk bridge that doesn't have metadata stored and whether it's
# wise to report DEGRADED status in case we don't have metadata
# present on the bridge.
LOG.error(_LE("Failed to store metadata for trunk %s"), trunk_id)
# NOTE(status_police): Trunk bridge has stale metadata now, it
# might cause troubles during deletion. Signal a DEGRADED status;
# if the user undo/redo the operation things may go back to
# normal.
return constants.DEGRADED_STATUS
LOG.debug("Added trunk: %s", trunk_id)
@ -270,6 +270,20 @@ class OVSDBHandler(object):
{'subport_id': subport_id,
'trunk_id': trunk_id,
'err': te})
try:
# OVS bridge and port to be determined by _update_trunk_metadata
bridge = None
port = None
self._update_trunk_metadata(
bridge, port, trunk_id, subport_ids, wire=False)
except RuntimeError:
# NOTE(status_police): Trunk bridge has stale metadata now, it
# might cause troubles during deletion. Signal a DEGRADED status;
# if the user undo/redo the operation things may go back to
# normal.
LOG.error(_LE("Failed to store metadata for trunk %s"), trunk_id)
return constants.DEGRADED_STATUS
return self._get_current_status(subport_ids, ids)
def report_trunk_status(self, context, trunk_id, status):
@ -349,11 +363,32 @@ class OVSDBHandler(object):
"""Get trunk metadata from OVS port."""
parent_port_id = (
self.trunk_manager.get_port_uuid_from_external_ids(port))
trunk_id = port['external_ids']['trunk_id']
subport_ids = jsonutils.loads(port['external_ids']['subport_ids'])
trunk_id = port['external_ids'].get('trunk_id')
subport_ids = jsonutils.loads(
port['external_ids'].get('subport_ids', '[]'))
return parent_port_id, trunk_id, subport_ids
def _update_trunk_metadata(self, trunk_bridge, port,
trunk_id, subport_ids, wire=True):
"""Update trunk metadata.
:param trunk_bridge: OVS trunk bridge.
:param port: OVS parent port.
:param trunk_id: trunk ID.
:param subport_ids: subports affecting the metadata.
:param wire: if True subport_ids are added, otherwise removed.
"""
trunk_bridge = trunk_bridge or ovs_lib.OVSBridge(
utils.gen_trunk_br_name(trunk_id))
port = port or self._get_parent_port(trunk_bridge)
_port_id, _trunk_id, old_subports = self._get_trunk_metadata(port)
if wire:
new_subports = set(old_subports) | set(subport_ids)
else:
new_subports = set(old_subports) - set(subport_ids)
self._set_trunk_metadata(trunk_bridge, port, trunk_id, new_subports)
def _get_current_status(self, expected_subports, actual_subports):
"""Return the current status of the trunk.

3
neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_driver.py
View File

@ -76,7 +76,8 @@ class OvsTrunkSkeletonTest(base.BaseTestCase):
for subport in self.subports]
self.trunk_manager.add_sub_port.assert_has_calls(expected_calls)
def test_handle_subports_deleted(self):
@mock.patch('neutron.agent.common.ovs_lib.OVSBridge')
def test_handle_subports_deleted(self, br):
"""Test handler calls into trunk manager for deleting subports."""
self.skeleton.handle_subports(self.subports, events.DELETED)
expected_calls = [

75
neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py
View File

@ -176,22 +176,26 @@ class TestOVSDBHandler(base.BaseTestCase):
side_effect=trunk_manager.TrunkManagerError(error='error')):
self.ovsdb_handler.handle_trunk_remove('foo', self.fake_port)
def test_handle_trunk_remove_rpc_failure(self):
@mock.patch('neutron.agent.common.ovs_lib.OVSBridge')
def test_handle_trunk_remove_rpc_failure(self, br):
self.ovsdb_handler.trunk_rpc.update_trunk_status = (
oslo_messaging.MessagingException)
self.ovsdb_handler.handle_trunk_remove('foo', self.fake_port)
@mock.patch('neutron.agent.common.ovs_lib.OVSBridge')
def test_wire_subports_for_trunk_trunk_manager_failure(self, br):
trunk_rpc = self.ovsdb_handler.trunk_rpc
trunk_rpc.update_subport_bindings.return_value = self.subport_bindings
self.trunk_manager.add_sub_port.side_effect = (
trunk_manager.TrunkManagerError(error='error'))
with mock.patch.object(
self.ovsdb_handler, '_update_trunk_metadata') as f:
trunk_rpc = self.ovsdb_handler.trunk_rpc
trunk_rpc.update_subport_bindings.return_value = (
self.subport_bindings)
self.trunk_manager.add_sub_port.side_effect = (
trunk_manager.TrunkManagerError(error='error'))
status = self.ovsdb_handler.wire_subports_for_trunk(
None, 'trunk_id', self.fake_subports)
self.assertEqual(constants.DEGRADED_STATUS, status)
status = self.ovsdb_handler.wire_subports_for_trunk(
None, 'trunk_id', self.fake_subports)
self.assertTrue(f.call_count)
self.assertEqual(constants.DEGRADED_STATUS, status)
@mock.patch('neutron.agent.common.ovs_lib.OVSBridge')
def test_wire_subports_for_trunk_ovsdb_failure(self, br):
@ -203,12 +207,16 @@ class TestOVSDBHandler(base.BaseTestCase):
None, 'trunk_id', self.fake_subports)
self.assertEqual(constants.DEGRADED_STATUS, status)
def test_unwire_subports_for_trunk_trunk_manager_failure(self):
self.trunk_manager.remove_sub_port.side_effect = (
trunk_manager.TrunkManagerError(error='error'))
status = self.ovsdb_handler.unwire_subports_for_trunk(
mock.ANY, ['subport_id'])
self.assertEqual(constants.DEGRADED_STATUS, status)
@mock.patch('neutron.agent.common.ovs_lib.OVSBridge')
def test_unwire_subports_for_trunk_trunk_manager_failure(self, br):
with mock.patch.object(
self.ovsdb_handler, '_update_trunk_metadata') as f:
self.trunk_manager.remove_sub_port.side_effect = (
trunk_manager.TrunkManagerError(error='error'))
status = self.ovsdb_handler.unwire_subports_for_trunk(
'foo_trunk_id', ['subport_id'])
self.assertTrue(f.call_count)
self.assertEqual(constants.DEGRADED_STATUS, status)
def test__wire_trunk_get_trunk_details_failure(self):
self.trunk_manager.get_port_uuid_from_external_ids.side_effect = (
@ -259,3 +267,40 @@ class TestOVSDBHandler(base.BaseTestCase):
self.assertEqual(constants.DEGRADED_STATUS,
self.ovsdb_handler._get_current_status(
[mock.ANY], []))
def _test__update_trunk_metadata_wire_flag(self, mock_br, wire,
external_ids,
subport_ids,
expected_subport_ids):
with mock.patch.object(
self.ovsdb_handler, "_get_parent_port",
return_value={'name': 'foo', 'external_ids': external_ids}):
self.ovsdb_handler._update_trunk_metadata(
None, None, 'foo', subport_ids, wire=wire)
external_ids = mock_br.set_db_attribute.call_args[0][3]
self.assertEqual(1, mock_br.set_db_attribute.call_count)
self.assertEqual(
sorted(expected_subport_ids),
sorted(external_ids['subport_ids']))
@mock.patch('neutron.agent.common.ovs_lib.OVSBridge')
def test__update_trunk_metadata_wire(self, br):
mock_br = br.return_value
external_ids = {
'subport_ids': '["foo_subport_1"]'
}
subport_ids = ['foo_subport_2']
expected_subport_ids = '["foo_subport_1", "foo_subport_2"]'
self._test__update_trunk_metadata_wire_flag(
mock_br, True, external_ids, subport_ids, expected_subport_ids)
@mock.patch('neutron.agent.common.ovs_lib.OVSBridge')
def test__update_trunk_metadata_unwire(self, br):
mock_br = br.return_value
external_ids = {
'subport_ids': '["foo_subport_1", "foo_subport_2"]'
}
subport_ids = ['foo_subport_2']
expected_subport_ids = '["foo_subport_1"]'
self._test__update_trunk_metadata_wire_flag(
mock_br, False, external_ids, subport_ids, expected_subport_ids)