From 8c61076a2d0285556a9addf33092e30946a381a0 Mon Sep 17 00:00:00 2001 From: insequent Date: Thu, 18 Sep 2014 04:47:13 +0000 Subject: [PATCH] Removing all references to validate_gateway_excluded Why? RM9134. We've decided to follow upstream's example and allow gateway_ip for subnets to exist inside of the allocation pool. --- quark/allocation_pool.py | 10 --------- quark/plugin_modules/ip_policies.py | 19 ----------------- quark/plugin_modules/routes.py | 6 ------ quark/plugin_modules/subnets.py | 6 ------ .../tests/plugin_modules/test_ip_policies.py | 12 ----------- quark/tests/plugin_modules/test_routes.py | 11 ---------- quark/tests/plugin_modules/test_subnets.py | 21 ------------------- 7 files changed, 85 deletions(-) diff --git a/quark/allocation_pool.py b/quark/allocation_pool.py index ff3c7b7..916f9bf 100644 --- a/quark/allocation_pool.py +++ b/quark/allocation_pool.py @@ -145,16 +145,6 @@ class AllocationPools(object): self._exclude_cidrs = None self._policies.append(policy) - def validate_gateway_excluded(self, gateway_ip): - self._refresh_excludes() - gateway_ip_addr = netaddr.IPAddress(gateway_ip) - if gateway_ip_addr in self._subnet_cidr: - if (not self._exclude_cidrs or - (self._exclude_cidrs and gateway_ip_addr - not in self._exclude_cidrs)): - raise exceptions.GatewayConflictWithAllocationPools( - ip_address=gateway_ip, pool=self._alloc_pools) - def get_policy_cidrs(self): self._refresh_excludes() return [str(c) for c in self._exclude_cidrs.iter_cidrs()] diff --git a/quark/plugin_modules/ip_policies.py b/quark/plugin_modules/ip_policies.py index bbef491..2f46de3 100644 --- a/quark/plugin_modules/ip_policies.py +++ b/quark/plugin_modules/ip_policies.py @@ -18,7 +18,6 @@ from neutron.common import exceptions from neutron.openstack.common import log as logging from oslo.config import cfg -from quark import allocation_pool from quark.db import api as db_api from quark import exceptions as quark_exceptions from quark import plugin_views as v @@ -27,22 +26,6 @@ CONF = cfg.CONF LOG = logging.getLogger(__name__) -def _validate_policy_with_routes(context, policies, subnets): - pools = {} - policy_networks = [netaddr.IPNetwork(p) for p in policies] - for subnet in subnets: - pool = allocation_pool.AllocationPools(subnet["cidr"], - policies=policy_networks) - pools[subnet["id"]] = pool - - subnet_ids = [subnet["id"] for subnet in subnets] - - routes = db_api.route_find(context, subnet_id=subnet_ids) - for route in routes: - subnet_pool = pools[route["subnet_id"]] - subnet_pool.validate_gateway_excluded(route["gateway"]) - - def create_ip_policy(context, ip_policy): LOG.info("create_ip_policy for tenant %s" % context.tenant_id) @@ -178,8 +161,6 @@ def update_ip_policy(context, id, ip_policy): id=model["ip_policy"]["id"], n_id=model["id"]) model["ip_policy"] = ipp_db - if ip_policy_cidrs: - _validate_policy_with_routes(context, ip_policy_cidrs, all_subnets) ipp_db = db_api.ip_policy_update(context, ipp_db, **ipp) return v._make_ip_policy_dict(ipp_db) diff --git a/quark/plugin_modules/routes.py b/quark/plugin_modules/routes.py index ca6e20c..e09f70c 100644 --- a/quark/plugin_modules/routes.py +++ b/quark/plugin_modules/routes.py @@ -20,9 +20,7 @@ from neutron.openstack.common import log as logging from neutron import quota from oslo.config import cfg -from quark import allocation_pool from quark.db import api as db_api -from quark.db import models as db_models from quark import exceptions as quark_exceptions from quark import plugin_views as v @@ -60,10 +58,6 @@ def create_route(context, route): subnet = db_api.subnet_find(context, id=subnet_id, scope=db_api.ONE) if not subnet: raise exceptions.SubnetNotFound(subnet_id=subnet_id) - policies = db_models.IPPolicy.get_ip_policy_cidrs(subnet) - alloc_pools = allocation_pool.AllocationPools(subnet["cidr"], - policies=policies) - alloc_pools.validate_gateway_excluded(route["gateway"]) # TODO(anyone): May want to denormalize the cidr values into columns # to achieve single db lookup on conflict check diff --git a/quark/plugin_modules/subnets.py b/quark/plugin_modules/subnets.py index 0d47932..48959b6 100644 --- a/quark/plugin_modules/subnets.py +++ b/quark/plugin_modules/subnets.py @@ -138,7 +138,6 @@ def create_subnet(context, subnet): default_route = None for route in host_routes: netaddr_route = netaddr.IPNetwork(route["destination"]) - alloc_pools.validate_gateway_excluded(route["nexthop"]) if netaddr_route.value == routes.DEFAULT_ROUTE.value: if default_route: raise q_exc.DuplicateRouteConflict( @@ -153,10 +152,7 @@ def create_subnet(context, subnet): new_subnet["dns_nameservers"].append(db_api.dns_create( context, ip=netaddr.IPAddress(dns_ip))) - # if the gateway_ip is IN the cidr for the subnet and NOT excluded by - # policies, we should raise a 409 conflict if gateway_ip and default_route is None: - alloc_pools.validate_gateway_excluded(gateway_ip) new_subnet["routes"].append(db_api.route_create( context, cidr=str(routes.DEFAULT_ROUTE), gateway=gateway_ip)) @@ -207,7 +203,6 @@ def update_subnet(context, id, subnet): allocation_pools) if gateway_ip: - alloc_pools.validate_gateway_excluded(gateway_ip) default_route = None for route in host_routes: netaddr_route = netaddr.IPNetwork(route["destination"]) @@ -237,7 +232,6 @@ def update_subnet(context, id, subnet): if host_routes: subnet_db["routes"] = [] for route in host_routes: - alloc_pools.validate_gateway_excluded(route["nexthop"]) subnet_db["routes"].append(db_api.route_create( context, cidr=route["destination"], gateway=route["nexthop"])) diff --git a/quark/tests/plugin_modules/test_ip_policies.py b/quark/tests/plugin_modules/test_ip_policies.py index da591bd..889b577 100644 --- a/quark/tests/plugin_modules/test_ip_policies.py +++ b/quark/tests/plugin_modules/test_ip_policies.py @@ -483,18 +483,6 @@ class TestQuarkUpdatePolicySubnetWithRoutes(test_quark_plugin.TestQuarkPlugin): route_find.return_value = routes yield ip_policy_update - def test_update_ip_policy_has_route_conflict_raises(self): - subnet = dict(id=1, cidr="192.168.0.0/24") - ipp = dict(id=1, subnets=[subnet], exclude=["192.168.0.1/32"], - name="foo", tenant_id=1) - route = {"gateway": "192.168.0.1", "subnet_id": subnet["id"]} - with self._stubs(ipp, subnets=[subnet], routes=[route]): - with self.assertRaises( - exceptions.GatewayConflictWithAllocationPools): - self.plugin.update_ip_policy( - self.context, 1, - dict(ip_policy=dict(subnet_ids=[1], exclude=[]))) - def test_update_ip_policy_no_route_conflict(self): subnet = dict(id=1, cidr="192.168.0.0/24") ipp = dict(id=1, subnets=[subnet], exclude=["192.168.0.1/32"], diff --git a/quark/tests/plugin_modules/test_routes.py b/quark/tests/plugin_modules/test_routes.py index 9df0da5..6d98293 100644 --- a/quark/tests/plugin_modules/test_routes.py +++ b/quark/tests/plugin_modules/test_routes.py @@ -124,17 +124,6 @@ class TestQuarkCreateRoutes(test_quark_plugin.TestQuarkPlugin): self.plugin.create_route(self.context, dict(route=create_route)) - def test_create_route_gateway_conflict_raises(self): - subnet = dict(id=2, ip_policy=[], cidr="192.168.0.0/24") - create_route = dict(id=1, cidr="192.168.0.0/24", gateway="192.168.0.1", - subnet_id=subnet["id"]) - with self._stubs(create_route=create_route, find_routes=[], - subnet=subnet): - with self.assertRaises( - exceptions.GatewayConflictWithAllocationPools): - self.plugin.create_route(self.context, - dict(route=create_route)) - def test_create_route_no_gateway_raises(self): subnet = dict(id=2, ip_policy=[], cidr="192.168.0.0/24") create_route = dict(id=1, cidr="192.168.0.0/24", diff --git a/quark/tests/plugin_modules/test_subnets.py b/quark/tests/plugin_modules/test_subnets.py index 7e4186e..9f2abe2 100644 --- a/quark/tests/plugin_modules/test_subnets.py +++ b/quark/tests/plugin_modules/test_subnets.py @@ -273,17 +273,6 @@ class TestQuarkCreateSubnetAllocationPools(test_quark_plugin.TestQuarkPlugin): self.assertEqual(subnet_create.call_count, 1) self.assertEqual(resp["allocation_pools"], pools) - def test_create_subnet_allocation_pools_gateway_conflict(self): - pools = [dict(start="192.168.1.1", end="192.168.1.20")] - s = dict(subnet=dict(allocation_pools=pools, - cidr="192.168.1.1/24", - gateway_ip="192.168.1.1", - network_id=1)) - with self._stubs(s["subnet"]): - with self.assertRaises( - exceptions.GatewayConflictWithAllocationPools): - self.plugin.create_subnet(self.context, s) - def test_create_subnet_allocation_pools_invalid_outside(self): pools = [dict(start="192.168.0.10", end="192.168.0.20")] s = dict(subnet=dict( @@ -984,16 +973,6 @@ class TestQuarkUpdateSubnet(test_quark_plugin.TestQuarkPlugin): expected_pools = [] self.assertEqual(resp["allocation_pools"], expected_pools) - def test_update_subnet_conflicting_gateway(self): - pools = [dict(start="172.16.0.1", end="172.16.0.254")] - s = dict(subnet=dict(allocation_pools=pools, gateway_ip="172.16.0.1")) - with self._stubs( - new_ip_policy=['172.16.0.0/30', '172.16.0.4/32', '172.16.0.255/32'] - ) as (dns_create, route_update, route_create): - with self.assertRaises( - exceptions.GatewayConflictWithAllocationPools): - self.plugin.update_subnet(self.context, 1, s) - class TestQuarkDeleteSubnet(test_quark_plugin.TestQuarkPlugin): @contextlib.contextmanager