openstack
/
neutron
Code Issues Proposed changes

Merge "lb: avoid doing nova VIF work plumbing tap to qbr" into stable/liberty

This commit is contained in:
Jenkins 2016-03-30 20:57:57 +00:00 committed by Gerrit Code Review
parent 056a048766 4cb9062319
commit d1871946e3
3 changed files with 165 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
neutron/common/constants.py
View File

@ -30,6 +30,10 @@ FLOATINGIP_STATUS_ACTIVE = 'ACTIVE'
FLOATINGIP_STATUS_DOWN = 'DOWN' FLOATINGIP_STATUS_DOWN = 'DOWN'
FLOATINGIP_STATUS_ERROR = 'ERROR' FLOATINGIP_STATUS_ERROR = 'ERROR'
DEVICE_OWNER_COMPUTE_PREFIX = "compute:"
DEVICE_OWNER_NETWORK_PREFIX = "network:"
DEVICE_OWNER_NEUTRON_PREFIX = "neutron:"
DEVICE_OWNER_ROUTER_HA_INTF = "network:router_ha_interface" DEVICE_OWNER_ROUTER_HA_INTF = "network:router_ha_interface"
DEVICE_OWNER_ROUTER_INTF = "network:router_interface" DEVICE_OWNER_ROUTER_INTF = "network:router_interface"
DEVICE_OWNER_ROUTER_GW = "network:router_gateway" DEVICE_OWNER_ROUTER_GW = "network:router_gateway"
@ -38,11 +42,11 @@ DEVICE_OWNER_DHCP = "network:dhcp"
DEVICE_OWNER_DVR_INTERFACE = "network:router_interface_distributed" DEVICE_OWNER_DVR_INTERFACE = "network:router_interface_distributed"
DEVICE_OWNER_AGENT_GW = "network:floatingip_agent_gateway" DEVICE_OWNER_AGENT_GW = "network:floatingip_agent_gateway"
DEVICE_OWNER_ROUTER_SNAT = "network:router_centralized_snat" DEVICE_OWNER_ROUTER_SNAT = "network:router_centralized_snat"
DEVICE_OWNER_LOADBALANCER = "neutron:LOADBALANCER" DEVICE_OWNER_LOADBALANCER = DEVICE_OWNER_NEUTRON_PREFIX + "LOADBALANCER"
DEVICE_OWNER_LOADBALANCERV2 = "neutron:LOADBALANCERV2" DEVICE_OWNER_LOADBALANCERV2 = DEVICE_OWNER_NEUTRON_PREFIX + "LOADBALANCERV2"
DEVICE_OWNER_COMPUTE_PREFIX = "compute:" DEVICE_OWNER_PREFIXES = (DEVICE_OWNER_NETWORK_PREFIX,
DEVICE_OWNER_PREFIXES = ["network:", "neutron:"] DEVICE_OWNER_NEUTRON_PREFIX)
# Collection used to identify devices owned by router interfaces. # Collection used to identify devices owned by router interfaces.
# DEVICE_OWNER_ROUTER_HA_INTF is a special case and so is not included. # DEVICE_OWNER_ROUTER_HA_INTF is a special case and so is not included.

117
neutron/plugins/ml2/drivers/linuxbridge/agent/linuxbridge_neutron_agent.py
View File

@ -465,12 +465,12 @@ class LinuxBridgeManager(object):
network_id: network_id}) network_id: network_id})
def add_tap_interface(self, network_id, network_type, physical_network, def add_tap_interface(self, network_id, network_type, physical_network,
segmentation_id, tap_device_name): segmentation_id, tap_device_name, device_owner):
"""Add tap interface and handle interface missing exeptions.""" """Add tap interface and handle interface missing exeptions."""
try: try:
return self._add_tap_interface(network_id, network_type, return self._add_tap_interface(network_id, network_type,
physical_network, segmentation_id, physical_network, segmentation_id,
tap_device_name) tap_device_name, device_owner)
except Exception: except Exception:
with excutils.save_and_reraise_exception() as ctx: with excutils.save_and_reraise_exception() as ctx:
if not ip_lib.device_exists(tap_device_name): if not ip_lib.device_exists(tap_device_name):
@ -481,7 +481,7 @@ class LinuxBridgeManager(object):
return False return False
def _add_tap_interface(self, network_id, network_type, physical_network, def _add_tap_interface(self, network_id, network_type, physical_network,
segmentation_id, tap_device_name): segmentation_id, tap_device_name, device_owner):
"""Add tap interface. """Add tap interface.
If a VIF has been plugged into a network, this function will If a VIF has been plugged into a network, this function will
@ -507,20 +507,24 @@ class LinuxBridgeManager(object):
return False return False
self.ensure_tap_mtu(tap_device_name, phy_dev_name) self.ensure_tap_mtu(tap_device_name, phy_dev_name)
# Check if device needs to be added to bridge # Avoid messing with plugging devices into a bridge that the agent
tap_device_in_bridge = self.get_bridge_for_tap_device(tap_device_name) # does not own
if not tap_device_in_bridge: if device_owner.startswith(constants.DEVICE_OWNER_PREFIXES):
data = {'tap_device_name': tap_device_name, # Check if device needs to be added to bridge
'bridge_name': bridge_name} if not self.get_bridge_for_tap_device(tap_device_name):
LOG.debug("Adding device %(tap_device_name)s to bridge " data = {'tap_device_name': tap_device_name,
"%(bridge_name)s", data) 'bridge_name': bridge_name}
if bridge_lib.BridgeDevice(bridge_name).addif(tap_device_name): LOG.debug("Adding device %(tap_device_name)s to bridge "
return False "%(bridge_name)s", data)
if bridge_lib.BridgeDevice(bridge_name).addif(tap_device_name):
return False
else: else:
data = {'tap_device_name': tap_device_name, data = {'tap_device_name': tap_device_name,
'device_owner': device_owner,
'bridge_name': bridge_name} 'bridge_name': bridge_name}
LOG.debug("%(tap_device_name)s already exists on bridge " LOG.debug("Skip adding device %(tap_device_name)s to "
"%(bridge_name)s", data) "%(bridge_name)s. It is owned by %(device_owner)s and "
"thus added elsewhere.", data)
return True return True
def ensure_tap_mtu(self, tap_dev_name, phy_dev_name): def ensure_tap_mtu(self, tap_dev_name, phy_dev_name):
@ -529,14 +533,14 @@ class LinuxBridgeManager(object):
ip_lib.IPDevice(tap_dev_name).link.set_mtu(phy_dev_mtu) ip_lib.IPDevice(tap_dev_name).link.set_mtu(phy_dev_mtu)
def add_interface(self, network_id, network_type, physical_network, def add_interface(self, network_id, network_type, physical_network,
segmentation_id, port_id): segmentation_id, port_id, device_owner):
self.network_map[network_id] = NetworkSegment(network_type, self.network_map[network_id] = NetworkSegment(network_type,
physical_network, physical_network,
segmentation_id) segmentation_id)
tap_device_name = self.get_tap_device_name(port_id) tap_device_name = self.get_tap_device_name(port_id)
return self.add_tap_interface(network_id, network_type, return self.add_tap_interface(network_id, network_type,
physical_network, segmentation_id, physical_network, segmentation_id,
tap_device_name) tap_device_name, device_owner)
def delete_bridge(self, bridge_name): def delete_bridge(self, bridge_name):
if ip_lib.device_exists(bridge_name): if ip_lib.device_exists(bridge_name):
@ -982,12 +986,14 @@ class LinuxBridgeNeutronAgentRPC(service.Service):
def setup_linux_bridge(self, bridge_mappings, interface_mappings): def setup_linux_bridge(self, bridge_mappings, interface_mappings):
self.br_mgr = LinuxBridgeManager(bridge_mappings, interface_mappings) self.br_mgr = LinuxBridgeManager(bridge_mappings, interface_mappings)
def remove_port_binding(self, network_id, physical_network, interface_id): def _ensure_port_admin_state(self, port_id, admin_state_up):
bridge_name = self.br_mgr.get_existing_bridge_name(physical_network) LOG.debug("Setting admin_state_up to %s for port %s",
if not bridge_name: admin_state_up, port_id)
bridge_name = self.br_mgr.get_bridge_name(network_id) tap_name = self.br_mgr.get_tap_device_name(port_id)
tap_device_name = self.br_mgr.get_tap_device_name(interface_id) if admin_state_up:
return self.br_mgr.remove_interface(bridge_name, tap_device_name) ip_lib.IPDevice(tap_name).link.set_up()
else:
ip_lib.IPDevice(tap_name).link.set_down()
def process_network_devices(self, device_info): def process_network_devices(self, device_info):
resync_a = False resync_a = False
@ -1031,18 +1037,58 @@ class LinuxBridgeNeutronAgentRPC(service.Service):
device_details['port_id']) device_details['port_id'])
arp_protect.setup_arp_spoofing_protection(port, arp_protect.setup_arp_spoofing_protection(port,
device_details) device_details)
# create the networking for the port
network_type = device_details.get('network_type')
segmentation_id = device_details.get('segmentation_id')
tap_in_bridge = self.br_mgr.add_interface(
device_details['network_id'], network_type,
device_details['physical_network'], segmentation_id,
device_details['port_id'], device_details['device_owner'])
# REVISIT(scheuran): Changed the way how ports admin_state_up
# is implemented.
#
# Old lb implementation:
# - admin_state_up: ensure that tap is plugged into bridge
# - admin_state_down: remove tap from bridge
# New lb implementation:
# - admin_state_up: set tap device state to up
# - admin_state_down: set tap device stae to down
#
# However both approaches could result in races with
# nova/libvirt and therefore to an invalid system state in the
# scenario, where an instance is booted with a port configured
# with admin_state_up = False:
#
# Libvirt does the following actions in exactly
# this order (see libvirt virnetdevtap.c)
# 1) Create the tap device, set its MAC and MTU
# 2) Plug the tap into the bridge
# 3) Set the tap online
#
# Old lb implementation:
# A race could occur, if the lb agent removes the tap device
# right after step 1). Then libvirt will add it to the bridge
# again in step 2).
# New lb implementation:
# The race could occur if the lb-agent sets the taps device
# state to down right after step 2). In step 3) libvirt
# might set it to up again.
#
# This is not an issue if an instance is booted with a port
# configured with admin_state_up = True. Libvirt would just
# set the tap device up again.
#
# This refactoring is recommended for the following reasons:
# 1) An existing race with libvirt caused by the behavior of
# the old implementation. See Bug #1312016
# 2) The new code is much more readable
if tap_in_bridge:
self._ensure_port_admin_state(
device_details['port_id'],
device_details['admin_state_up'])
# update plugin about port status if admin_state is up
if device_details['admin_state_up']: if device_details['admin_state_up']:
# create the networking for the port if tap_in_bridge:
network_type = device_details.get('network_type')
segmentation_id = device_details.get('segmentation_id')
if self.br_mgr.add_interface(
device_details['network_id'],
network_type,
device_details['physical_network'],
segmentation_id,
device_details['port_id']):
# update plugin about port status
self.plugin_rpc.update_device_up(self.context, self.plugin_rpc.update_device_up(self.context,
device, device,
self.agent_id, self.agent_id,
@ -1052,11 +1098,6 @@ class LinuxBridgeNeutronAgentRPC(service.Service):
device, device,
self.agent_id, self.agent_id,
cfg.CONF.host) cfg.CONF.host)
else:
physical_network = device_details['physical_network']
self.remove_port_binding(device_details['network_id'],
physical_network,
device_details['port_id'])
else: else:
LOG.info(_LI("Device %s not defined on plugin"), device) LOG.info(_LI("Device %s not defined on plugin"), device)
return False return False

115
neutron/tests/unit/plugins/ml2/drivers/linuxbridge/agent/test_linuxbridge_neutron_agent.py
View File

@ -147,6 +147,7 @@ class TestLinuxBridgeAgent(base.BaseTestCase):
def test_treat_devices_removed_with_existed_device(self): def test_treat_devices_removed_with_existed_device(self):
agent = self.agent agent = self.agent
agent._ensure_port_admin_state = mock.Mock()
devices = [DEVICE_1] devices = [DEVICE_1]
with mock.patch.object(agent.plugin_rpc, with mock.patch.object(agent.plugin_rpc,
"update_device_down") as fn_udd,\ "update_device_down") as fn_udd,\
@ -405,6 +406,25 @@ class TestLinuxBridgeAgent(base.BaseTestCase):
'tap4'])) 'tap4']))
agent.treat_devices_removed.assert_called_with(set(['tap1'])) agent.treat_devices_removed.assert_called_with(set(['tap1']))
def test_treat_devices_added_updated_no_local_interface(self):
agent = self.agent
mock_details = {'device': 'dev123',
'port_id': 'port123',
'network_id': 'net123',
'admin_state_up': True,
'network_type': 'vlan',
'segmentation_id': 100,
'physical_network': 'physnet1',
'device_owner': constants.DEVICE_OWNER_NETWORK_PREFIX}
agent.ext_manager = mock.Mock()
agent.plugin_rpc = mock.Mock()
agent.plugin_rpc.get_devices_details_list.return_value = [mock_details]
agent.mgr = mock.Mock()
agent.mgr.plug_interface.return_value = False
agent.mgr.ensure_port_admin_state = mock.Mock()
agent.treat_devices_added_updated(set(['tap1']))
self.assertFalse(agent.mgr.ensure_port_admin_state.called)
def test_treat_devices_added_updated_admin_state_up_true(self): def test_treat_devices_added_updated_admin_state_up_true(self):
agent = self.agent agent = self.agent
mock_details = {'device': 'dev123', mock_details = {'device': 'dev123',
@ -413,39 +433,22 @@ class TestLinuxBridgeAgent(base.BaseTestCase):
'admin_state_up': True, 'admin_state_up': True,
'network_type': 'vlan', 'network_type': 'vlan',
'segmentation_id': 100, 'segmentation_id': 100,
'physical_network': 'physnet1'} 'physical_network': 'physnet1',
'device_owner': constants.DEVICE_OWNER_NETWORK_PREFIX}
agent.plugin_rpc = mock.Mock() agent.plugin_rpc = mock.Mock()
agent.plugin_rpc.get_devices_details_list.return_value = [mock_details] agent.plugin_rpc.get_devices_details_list.return_value = [mock_details]
agent.br_mgr = mock.Mock() agent.br_mgr = mock.Mock()
agent.br_mgr.add_interface.return_value = True agent.br_mgr.add_interface.return_value = True
agent._ensure_port_admin_state = mock.Mock()
resync_needed = agent.treat_devices_added_updated(set(['tap1'])) resync_needed = agent.treat_devices_added_updated(set(['tap1']))
self.assertFalse(resync_needed) self.assertFalse(resync_needed)
agent.br_mgr.add_interface.assert_called_with('net123', 'vlan', agent.br_mgr.add_interface.assert_called_with(
'physnet1', 100, 'net123', 'vlan', 'physnet1',
'port123') 100, 'port123',
constants.DEVICE_OWNER_NETWORK_PREFIX)
self.assertTrue(agent.plugin_rpc.update_device_up.called) self.assertTrue(agent.plugin_rpc.update_device_up.called)
def test_treat_devices_added_updated_admin_state_up_false(self):
agent = self.agent
mock_details = {'device': 'dev123',
'port_id': 'port123',
'network_id': 'net123',
'admin_state_up': False,
'network_type': 'vlan',
'segmentation_id': 100,
'physical_network': 'physnet1'}
agent.plugin_rpc = mock.Mock()
agent.plugin_rpc.get_devices_details_list.return_value = [mock_details]
agent.remove_port_binding = mock.Mock()
resync_needed = agent.treat_devices_added_updated(set(['tap1']))
self.assertFalse(resync_needed)
agent.remove_port_binding.assert_called_with('net123',
'physnet1',
'port123')
self.assertFalse(agent.plugin_rpc.update_device_up.called)
def test_treat_devices_added_updated_prevent_arp_spoofing_true(self): def test_treat_devices_added_updated_prevent_arp_spoofing_true(self):
agent = self.agent agent = self.agent
agent.prevent_arp_spoofing = True agent.prevent_arp_spoofing = True
@ -490,6 +493,23 @@ class TestLinuxBridgeAgent(base.BaseTestCase):
self.agent._report_state() self.agent._report_state()
self.assertTrue(self.agent.fullsync) self.assertTrue(self.agent.fullsync)
def _test_ensure_port_admin_state(self, admin_state):
port_id = 'fake_id'
with mock.patch.object(ip_lib, 'IPDevice') as dev_mock:
self.agent._ensure_port_admin_state(port_id, admin_state)
tap_name = self.agent.br_mgr.get_tap_device_name(port_id)
self.assertEqual(admin_state,
dev_mock(tap_name).link.set_up.called)
self.assertNotEqual(admin_state,
dev_mock(tap_name).link.set_down.called)
def test_ensure_port_admin_state_up(self):
self._test_ensure_port_admin_state(True)
def test_ensure_port_admin_state_down(self):
self._test_ensure_port_admin_state(False)
class TestLinuxBridgeManager(base.BaseTestCase): class TestLinuxBridgeManager(base.BaseTestCase):
def setUp(self): def setUp(self):
@ -925,22 +945,31 @@ class TestLinuxBridgeManager(base.BaseTestCase):
self.assertFalse(self.lbm.add_tap_interface("123", self.assertFalse(self.lbm.add_tap_interface("123",
p_const.TYPE_VLAN, p_const.TYPE_VLAN,
"physnet1", None, "physnet1", None,
"tap1")) "tap1", "foo"))
@mock.patch.object(ip_lib, "device_exists", return_value=True) @mock.patch.object(ip_lib, "device_exists", return_value=True)
def test_add_tap_interface_with_other_error(self, exists): def test_add_tap_interface_with_other_error(self, exists):
with mock.patch.object(self.lbm, "_add_tap_interface", with mock.patch.object(self.lbm, "_add_tap_interface",
side_effect=RuntimeError("No more fuel")): side_effect=RuntimeError("No more fuel")):
self.assertRaises(RuntimeError, self.lbm.add_tap_interface, "123", self.assertRaises(RuntimeError, self.lbm.add_tap_interface, "123",
p_const.TYPE_VLAN, "physnet1", None, "tap1") p_const.TYPE_VLAN, "physnet1", None, "tap1",
"foo")
def test_add_tap_interface(self): def test_add_tap_interface_owner_other(self):
with mock.patch.object(ip_lib, "device_exists"):
with mock.patch.object(self.lbm, "ensure_local_bridge"):
self.assertTrue(self.lbm.add_tap_interface("123",
p_const.TYPE_LOCAL,
"physnet1", None,
"tap1", "foo"))
def _test_add_tap_interface(self, dev_owner_prefix):
with mock.patch.object(ip_lib, "device_exists") as de_fn: with mock.patch.object(ip_lib, "device_exists") as de_fn:
de_fn.return_value = False de_fn.return_value = False
self.assertFalse( self.assertFalse(
self.lbm.add_tap_interface("123", p_const.TYPE_VLAN, self.lbm.add_tap_interface("123", p_const.TYPE_VLAN,
"physnet1", "1", "tap1") "physnet1", "1", "tap1",
) dev_owner_prefix))
de_fn.return_value = True de_fn.return_value = True
bridge_device = mock.Mock() bridge_device = mock.Mock()
@ -954,14 +983,16 @@ class TestLinuxBridgeManager(base.BaseTestCase):
self.assertTrue(self.lbm.add_tap_interface("123", self.assertTrue(self.lbm.add_tap_interface("123",
p_const.TYPE_LOCAL, p_const.TYPE_LOCAL,
"physnet1", None, "physnet1", None,
"tap1")) "tap1",
dev_owner_prefix))
en_fn.assert_called_with("123", "brq123") en_fn.assert_called_with("123", "brq123")
self.lbm.bridge_mappings = {"physnet1": "brq999"} self.lbm.bridge_mappings = {"physnet1": "brq999"}
self.assertTrue(self.lbm.add_tap_interface("123", self.assertTrue(self.lbm.add_tap_interface("123",
p_const.TYPE_LOCAL, p_const.TYPE_LOCAL,
"physnet1", None, "physnet1", None,
"tap1")) "tap1",
dev_owner_prefix))
en_fn.assert_called_with("123", "brq999") en_fn.assert_called_with("123", "brq999")
get_br.return_value = False get_br.return_value = False
@ -969,8 +1000,8 @@ class TestLinuxBridgeManager(base.BaseTestCase):
self.assertFalse(self.lbm.add_tap_interface("123", self.assertFalse(self.lbm.add_tap_interface("123",
p_const.TYPE_LOCAL, p_const.TYPE_LOCAL,
"physnet1", None, "physnet1", None,
"tap1")) "tap1",
dev_owner_prefix))
with mock.patch.object(self.lbm, with mock.patch.object(self.lbm,
"ensure_physical_in_bridge") as ens_fn,\ "ensure_physical_in_bridge") as ens_fn,\
mock.patch.object(self.lbm, mock.patch.object(self.lbm,
@ -981,21 +1012,31 @@ class TestLinuxBridgeManager(base.BaseTestCase):
self.assertFalse(self.lbm.add_tap_interface("123", self.assertFalse(self.lbm.add_tap_interface("123",
p_const.TYPE_VLAN, p_const.TYPE_VLAN,
"physnet1", "1", "physnet1", "1",
"tap1")) "tap1",
dev_owner_prefix))
ens_fn.return_value = "eth0.1" ens_fn.return_value = "eth0.1"
get_br.return_value = "brq123" get_br.return_value = "brq123"
self.lbm.add_tap_interface("123", p_const.TYPE_VLAN, self.lbm.add_tap_interface("123", p_const.TYPE_VLAN,
"physnet1", "1", "tap1") "physnet1", "1", "tap1",
dev_owner_prefix)
en_mtu_fn.assert_called_once_with("tap1", "eth0.1") en_mtu_fn.assert_called_once_with("tap1", "eth0.1")
bridge_device.addif.assert_called_once_with("tap1") bridge_device.addif.assert_called_once_with("tap1")
def test_add_tap_interface_owner_network(self):
self._test_add_tap_interface(constants.DEVICE_OWNER_NETWORK_PREFIX)
def test_add_tap_interface_owner_neutron(self):
self._test_add_tap_interface(constants.DEVICE_OWNER_NEUTRON_PREFIX)
def test_add_interface(self): def test_add_interface(self):
with mock.patch.object(self.lbm, "add_tap_interface") as add_tap: with mock.patch.object(self.lbm, "add_tap_interface") as add_tap:
self.lbm.add_interface("123", p_const.TYPE_VLAN, "physnet-1", self.lbm.add_interface("123", p_const.TYPE_VLAN, "physnet-1",
"1", "234") "1", "234",
constants.DEVICE_OWNER_NETWORK_PREFIX)
add_tap.assert_called_with("123", p_const.TYPE_VLAN, "physnet-1", add_tap.assert_called_with("123", p_const.TYPE_VLAN, "physnet-1",
"1", "tap234") "1", "tap234",
constants.DEVICE_OWNER_NETWORK_PREFIX)
def test_delete_bridge(self): def test_delete_bridge(self):
bridge_device = mock.Mock() bridge_device = mock.Mock()