openstack
/
neutron
Code Issues Proposed changes

Simplify keepalived.virtual_routes 

keepalived.virtual_routes previously held one list of virtual
routes of different kinds, and the HA router class manipulated
that list directly. The list held both the default gateway
virtual route, and any extra routes. This means that when adding
extra routes for example, the HA router would first have to
remove all routes that are not default gateway routes, then add
the extra routes received via RPC.

This is messy because:
a) It's needlessly complicated
b) It's fragile
c) There's zero separation of concerns (HA router should not know
   how keepalived maintains its list of virtual routes)
d) It requires changes to the management of the default gateway
   and virtual routes just to add another type of extra routes

This patch solves these issues by separating the persistency of
virtual routes according to their role.

Co-Authored-By: gong yong sheng <gong.yongsheng@99cloud.net>
Related-Bug: 1414640
Change-Id: I1406b1876c3a47b110818686b42e5f2f688154fa
This commit is contained in:
Assaf Muller 2015-03-02 11:29:51 -05:00 committed by gong yong sheng
parent 949f6e23b3
commit e214b56da9
3 changed files with 76 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
neutron/agent/l3/ha_router.py
View File

@ -179,20 +179,15 @@ class HaRouter(router.RouterInfo):
new_routes = self.router['routes']
instance = self._get_keepalived_instance()
# Filter out all of the old routes while keeping only the default route
default_gw = (n_consts.IPv6_ANY, n_consts.IPv4_ANY)
instance.virtual_routes = [route for route in instance.virtual_routes
if route.destination in default_gw]
for route in new_routes:
instance.virtual_routes.append(keepalived.KeepalivedVirtualRoute(
route['destination'],
route['nexthop']))
instance.virtual_routes.extra_routes = [
keepalived.KeepalivedVirtualRoute(
route['destination'], route['nexthop'])
for route in new_routes]
self.routes = new_routes
def _add_default_gw_virtual_route(self, ex_gw_port, interface_name):
subnets = ex_gw_port.get('subnets', [])
default_gw_rts = []
for subnet in subnets:
gw_ip = subnet['gateway_ip']
if gw_ip:
@ -202,12 +197,9 @@ class HaRouter(router.RouterInfo):
netaddr.IPAddress(gw_ip).version == 4 else
n_consts.IPv6_ANY)
instance = self._get_keepalived_instance()
instance.virtual_routes = (
[route for route in instance.virtual_routes
if route.destination != default_gw])
instance.virtual_routes.append(
keepalived.KeepalivedVirtualRoute(
default_gw, gw_ip, interface_name))
default_gw_rts.append(keepalived.KeepalivedVirtualRoute(
default_gw, gw_ip, interface_name))
instance.virtual_routes.gateway_routes = default_gw_rts
def _should_delete_ipv6_lladdr(self, ipv6_lladdr):
"""Only the master should have any IP addresses configured.

35
neutron/agent/linux/keepalived.py
View File

@ -107,6 +107,32 @@ class KeepalivedVirtualRoute(object):
return output
class KeepalivedInstanceRoutes(object):
def __init__(self):
self.gateway_routes = []
self.extra_routes = []
def remove_routes_on_interface(self, interface_name):
self.gateway_routes = [gw_rt for gw_rt in self.gateway_routes
if gw_rt.interface_name != interface_name]
# NOTE(amuller): extra_routes are initialized from the router's
# 'routes' attribute. These routes do not have an interface
# parameter and so cannot be removed via an interface_name lookup.
@property
def routes(self):
return self.gateway_routes + self.extra_routes
def __len__(self):
return len(self.routes)
def build_config(self):
return itertools.chain([' virtual_routes {'],
(' %s' % route.build_config()
for route in self.routes),
[' }'])
class KeepalivedInstance(object):
"""Instance section of a keepalived configuration."""
@ -127,7 +153,7 @@ class KeepalivedInstance(object):
self.mcast_src_ip = mcast_src_ip
self.track_interfaces = []
self.vips = []
self.virtual_routes = []
self.virtual_routes = KeepalivedInstanceRoutes()
self.authentication = None
metadata_cidr = '169.254.169.254/32'
self.primary_vip_range = get_free_range(
@ -148,8 +174,7 @@ class KeepalivedInstance(object):
self.vips = [vip for vip in self.vips
if vip.interface_name != interface_name]
self.virtual_routes = [vroute for vroute in self.virtual_routes
if vroute.interface_name != interface_name]
self.virtual_routes.remove_routes_on_interface(interface_name)
def remove_vip_by_ip_address(self, ip_address):
self.vips = [vip for vip in self.vips
@ -243,8 +268,8 @@ class KeepalivedInstance(object):
config.extend(self._build_vips_config())
if self.virtual_routes:
config.extend(self._build_virtual_routes_config())
if len(self.virtual_routes):
config.extend(self.virtual_routes.build_config())
config.append('}')

39
neutron/tests/unit/agent/linux/test_keepalived.py
View File

@ -87,7 +87,7 @@ class KeepalivedConfBaseMixin(object):
virtual_route = keepalived.KeepalivedVirtualRoute(n_consts.IPv4_ANY,
"192.168.1.1",
"eth1")
instance1.virtual_routes.append(virtual_route)
instance1.virtual_routes.gateway_routes = [virtual_route]
instance2 = keepalived.KeepalivedInstance('MASTER', 'eth4', 2,
['169.254.192.0/18'],
@ -189,6 +189,43 @@ class KeepalivedStateExceptionTestCase(base.BaseTestCase):
invalid_auth_type, 'some_password')
class KeepalivedInstanceRoutesTestCase(base.BaseTestCase):
@classmethod
def _get_instance_routes(cls):
routes = keepalived.KeepalivedInstanceRoutes()
default_gw_eth0 = keepalived.KeepalivedVirtualRoute(
'0.0.0.0/0', '1.0.0.254', 'eth0')
default_gw_eth1 = keepalived.KeepalivedVirtualRoute(
'::/0', 'fe80::3e97:eff:fe26:3bfa/64', 'eth1')
routes.gateway_routes = [default_gw_eth0, default_gw_eth1]
extra_routes = [
keepalived.KeepalivedVirtualRoute('10.0.0.0/8', '1.0.0.1'),
keepalived.KeepalivedVirtualRoute('20.0.0.0/8', '2.0.0.2')]
routes.extra_routes = extra_routes
return routes
def test_routes(self):
routes = self._get_instance_routes()
self.assertEqual(len(routes.routes), 4)
def test_remove_routes_on_interface(self):
routes = self._get_instance_routes()
routes.remove_routes_on_interface('eth0')
self.assertEqual(len(routes.routes), 3)
routes.remove_routes_on_interface('eth1')
self.assertEqual(len(routes.routes), 2)
def test_build_config(self):
expected = """ virtual_routes {
0.0.0.0/0 via 1.0.0.254 dev eth0
::/0 via fe80::3e97:eff:fe26:3bfa/64 dev eth1
10.0.0.0/8 via 1.0.0.1
20.0.0.0/8 via 2.0.0.2
}"""
routes = self._get_instance_routes()
self.assertEqual(expected, '\n'.join(routes.build_config()))
class KeepalivedInstanceTestCase(base.BaseTestCase,
KeepalivedConfBaseMixin):
def test_get_primary_vip(self):