openstack
/
neutron
Code Issues Proposed changes

OVS: UnboundLocalError on switch timeout fixed 

In case there is some cached datapath id in OVSAgentBridge
and openflow switch does not respond in time for some
reason, a call to OVSAgentBridge._dpid() method will result
in UnboundLocalError.
This patch addresses the issue by calculating dpid_str
value from cached dpid instead of referencing unassigned
variable.
Some minor refactor also took place.

Closes-Bug: #1588042

Change-Id: If50183bf95cbe50c3a2393be8c2ab913c9715a10
This commit is contained in:
Inessa Vasilevskaya 2016-06-01 20:54:43 +03:00
parent e0c2122222
commit b22c7ae428
2 changed files with 43 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py
View File

@ -42,33 +42,31 @@ class OVSAgentBridge(ofswitch.OpenFlowSwitchMixin,
A convenient method for openflow message composers.
"""
while True:
dpid_int = self._cached_dpid
if dpid_int is None:
if self._cached_dpid is None:
dpid_str = self.get_datapath_id()
LOG.info(_LI("Bridge %(br_name)s has datapath-ID %(dpid)s"),
{"br_name": self.br_name, "dpid": dpid_str})
dpid_int = int(dpid_str, 16)
self._cached_dpid = int(dpid_str, 16)
try:
dp = self._get_dp_by_dpid(dpid_int)
dp = self._get_dp_by_dpid(self._cached_dpid)
return dp, dp.ofproto, dp.ofproto_parser
except RuntimeError:
with excutils.save_and_reraise_exception() as ctx:
self._cached_dpid = None
# Retry if dpid has been changed.
# NOTE(yamamoto): Open vSwitch change its dpid on
# some events.
# REVISIT(yamamoto): Consider to set dpid statically.
old_dpid_str = format(self._cached_dpid, '0x')
new_dpid_str = self.get_datapath_id()
if new_dpid_str != dpid_str:
if new_dpid_str != old_dpid_str:
LOG.info(_LI("Bridge %(br_name)s changed its "
"datapath-ID from %(old)s to %(new)s"), {
"br_name": self.br_name,
"old": dpid_str,
"old": old_dpid_str,
"new": new_dpid_str,
})
ctx.reraise = False
else:
self._cached_dpid = dpid_int
return dp, dp.ofproto, dp.ofproto_parser
self._cached_dpid = int(new_dpid_str, 16)
def setup_controllers(self, conf):
controllers = [

35
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_ovs_bridge.py Normal file
View File

@ -0,0 +1,35 @@
# Copyright (c) 2016 Mirantis, Inc.
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import mock
from neutron.tests.unit.plugins.ml2.drivers.openvswitch.agent \
import ovs_test_base
class OVSAgentBridgeTestCase(ovs_test_base.OVSRyuTestBase):
def test__get_dp(self):
mock.patch(
'neutron.agent.common.ovs_lib.OVSBridge.get_datapath_id',
return_value="3e9").start()
mock.patch(
"neutron.plugins.ml2.drivers.openvswitch.agent.openflow.native."
"ofswitch.OpenFlowSwitchMixin._get_dp_by_dpid",
side_effect=RuntimeError).start()
br = self.br_int_cls('br-int')
br._cached_dpid = int("3e9", 16)
# make sure it correctly raises RuntimeError, not UnboundLocalError as
# in LP https://bugs.launchpad.net/neutron/+bug/1588042
self.assertRaises(RuntimeError, br._get_dp)