Merge pull request #120 from Cerberus98/master

Fixes #118 again
This commit is contained in:
Jason Meridth 2014-04-25 11:23:26 -05:00
commit 3070e1805a
4 changed files with 72 additions and 13 deletions

View File

@ -37,6 +37,10 @@ class RouteConflict(exceptions.NeutronException):
message = _("Route overlaps existing route %(route_id)s with %(cidr)s")
class DuplicateRouteConflict(exceptions.NeutronException):
message = _("More than one default route found for subnet %(subnet_id)s")
class InvalidPhysicalNetworkType(exceptions.NeutronException):
message = _("Providernet type %(net_type)s is invalid")

View File

@ -26,6 +26,7 @@ from oslo.config import cfg
from quark.db import api as db_api
from quark.db import models as models
from quark import exceptions as q_exc
from quark import network_strategy
from quark.plugin_modules import routes
from quark import plugin_views as v
@ -127,6 +128,10 @@ def create_subnet(context, subnet):
for route in host_routes:
netaddr_route = netaddr.IPNetwork(route["destination"])
if netaddr_route.value == routes.DEFAULT_ROUTE.value:
if default_route:
raise q_exc.DuplicateRouteConflict(
subnet_id=new_subnet["id"])
default_route = route
gateway_ip = default_route["nexthop"]
new_subnet["routes"].append(db_api.route_create(

View File

@ -112,23 +112,23 @@ def _make_subnet_dict(subnet, default_route=None, fields=None):
#TODO(mdietz): really inefficient, should go away
res["gateway_ip"] = None
found_gateway = False
idx = -1
res["host_routes"] = []
default_found = False
for route in subnet["routes"]:
netroute = netaddr.IPNetwork(route["cidr"])
idx += 1
if netroute.value == default_route.value:
#NOTE(mdietz): This has the potential to find more than one default
# route. Quark normally won't allow you to create
# more than one, but it's plausible one exists
# regardless. As such, we're going to pretend
# it isn't possible, but log it anyway.
if default_found:
LOG.info(_("Default route %(gateway_ip)s already found for "
"subnet %(id)s") % res)
res["gateway_ip"] = route["gateway"]
found_gateway = True
break
#NOTE(mdietz): since we have the antiquated idea of a gateway_ip distinct
# from the routes, we need to not also present the route that
# we used to identify the default gateway_ip
if found_gateway:
subnet["routes"].pop(idx)
res["host_routes"] = [_host_route(r) for r in subnet["routes"]]
default_found = True
else:
res["host_routes"].append(_host_route(route))
return res

View File

@ -25,6 +25,7 @@ from neutron.openstack.common.notifier import api as notifier_api
from oslo.config import cfg
from quark.db import models
from quark import exceptions as q_exc
from quark.tests import test_quark_plugin
@ -84,6 +85,26 @@ class TestQuarkGetSubnets(test_quark_plugin.TestQuarkPlugin):
self.assertEqual(res[0][key], subnet[key])
self.assertEqual(len(routes), 0)
def test_subnets_list_two_default_routes_shows_last_one(self):
subnet_id = str(uuid.uuid4())
route = dict(id=1, cidr="0.0.0.0/0", gateway="192.168.0.1")
route2 = dict(id=1, cidr="0.0.0.0/0", gateway="192.168.0.2")
subnet = dict(id=subnet_id, network_id=1, name=subnet_id,
tenant_id=self.context.tenant_id, ip_version=4,
cidr="192.168.0.0/24", gateway_ip="192.168.0.1",
dns_nameservers=[],
enable_dhcp=None)
with self._stubs(subnets=[subnet], routes=[route, route2]):
res = self.plugin.get_subnets(self.context, {}, {})
# Don't want to test that LOG.info is called but we can
# know the case is covered by checking the gateway is the one
# we expect it to be
self.assertEqual(res[0]["gateway_ip"], "192.168.0.2")
self.assertEqual(len(res[0]["host_routes"]), 0)
def test_subnet_show_fail(self):
with self._stubs():
with self.assertRaises(exceptions.SubnetNotFound):
@ -573,6 +594,35 @@ class TestQuarkCreateSubnet(test_quark_plugin.TestQuarkPlugin):
else:
self.assertEqual(res[key], subnet["subnet"][key])
def test_create_subnet_two_default_routes_fails(self):
routes = [dict(cidr="0.0.0.0/0", gateway="172.16.0.4"),
dict(cidr="0.0.0.0/0", gateway="172.16.0.4")]
subnet = dict(
subnet=dict(network_id=1,
tenant_id=self.context.tenant_id, ip_version=4,
cidr="172.16.0.0/24",
gateway_ip=neutron_attrs.ATTR_NOT_SPECIFIED,
dns_nameservers=neutron_attrs.ATTR_NOT_SPECIFIED,
host_routes=[
{"destination": "0.0.0.0/0",
"nexthop": "172.16.0.4"},
{"destination": "0.0.0.0/0",
"nexthop": "172.16.0.4"}],
enable_dhcp=None))
network = dict(network_id=1)
with self._stubs(
subnet=subnet["subnet"],
network=network,
routes=routes
) as (subnet_create, dns_create, route_create):
dns_nameservers = subnet["subnet"].pop("dns_nameservers")
gateway_ip = subnet["subnet"].pop("gateway_ip")
subnet_request = copy.deepcopy(subnet)
subnet_request["subnet"]["dns_nameservers"] = dns_nameservers
subnet_request["subnet"]["gateway_ip"] = gateway_ip
with self.assertRaises(q_exc.DuplicateRouteConflict):
self.plugin.create_subnet(self.context, subnet_request)
def test_create_subnet_default_route_gateway_ip(self):
"""If default route (host_routes) and gateway_ip are both provided,
then host_route takes precedence.