From 7674390a2f908bba21e46fd10d141b466aa087c6 Mon Sep 17 00:00:00 2001 From: Matt Dietz Date: Thu, 8 Oct 2015 02:14:08 -0500 Subject: [PATCH] Remove unused network_strategy code JIRA:NCP-1580 Children of parent networks was a feature that went unused in quark, and may as well be removed. Additionally, it changes the word "parent" to be "provider" whereever this code is used. This patch acts as a pre-cursor to the provider subnets implementation. --- quark/ipam.py | 2 +- quark/network_strategy.py | 29 +++------------------ quark/plugin_modules/ports.py | 9 ++++--- quark/plugin_modules/subnets.py | 5 ---- quark/plugin_views.py | 11 ++++---- quark/tests/plugin_modules/test_ports.py | 6 ++--- quark/tests/test_network_strategies.py | 32 +----------------------- 7 files changed, 18 insertions(+), 76 deletions(-) diff --git a/quark/ipam.py b/quark/ipam.py index 039a5f1..28fca5e 100644 --- a/quark/ipam.py +++ b/quark/ipam.py @@ -118,7 +118,7 @@ def rfc3041_ip(port_id, cidr): def ip_address_failure(network_id): - if STRATEGY.is_parent_network(network_id): + if STRATEGY.is_provider_network(network_id): return q_exc.ProviderNetworkOutOfIps(net_id=network_id) else: return exceptions.IpAddressGenerationFailure(net_id=network_id) diff --git a/quark/network_strategy.py b/quark/network_strategy.py index 36f2e65..159df5d 100644 --- a/quark/network_strategy.py +++ b/quark/network_strategy.py @@ -15,7 +15,6 @@ import json -from neutron.common import exceptions from oslo_config import cfg from oslo_log import log as logging @@ -39,18 +38,13 @@ class JSONStrategy(object): self._compile_strategy(strategy) def _compile_strategy(self, strategy): - strategy = json.loads(strategy) - for network, definition in strategy.iteritems(): - if "children" in definition: - for _, child_net in definition["children"].iteritems(): - self.reverse_strategy[child_net] = network - self.strategy = strategy + self.strategy = json.loads(strategy) def split_network_ids(self, context, net_ids): assignable = [] tenant = [] for net_id in net_ids: - if self.is_parent_network(net_id): + if self.is_provider_network(net_id): assignable.append(net_id) else: tenant.append(net_id) @@ -62,25 +56,8 @@ class JSONStrategy(object): def get_assignable_networks(self, context): return self.strategy.keys() - def is_parent_network(self, net_id): + def is_provider_network(self, net_id): return self.strategy.get(net_id) is not None - def get_parent_network(self, net_id): - net = self.reverse_strategy.get(net_id) - if net: - return net - - # No matches, this is the highest network - return net_id - - def best_match_network_id(self, context, net_id, key): - net = self.strategy.get(net_id) - if net: - child_net = net["children"].get(key) - if not child_net: - raise exceptions.NetworkNotFound(net_id=net_id) - return child_net - return net_id - STRATEGY = JSONStrategy() diff --git a/quark/plugin_modules/ports.py b/quark/plugin_modules/ports.py index 5140bcc..1d2206b 100644 --- a/quark/plugin_modules/ports.py +++ b/quark/plugin_modules/ports.py @@ -38,7 +38,7 @@ STRATEGY = network_strategy.STRATEGY # HACK(amir): RM9305: do not allow a tenant to associate a network to a port # that does not belong to them unless it is publicnet or servicenet def _raise_if_unauthorized(tenant_id, net): - if (not STRATEGY.is_parent_network(net["id"]) and + if (not STRATEGY.is_provider_network(net["id"]) and net["tenant_id"] != tenant_id): raise exceptions.NotAuthorized() @@ -130,7 +130,7 @@ def create_port(context, port): quota.QUOTAS.limit_check(context, context.tenant_id, fixed_ips_per_port=len(fixed_ips)) - if not STRATEGY.is_parent_network(net_id): + if not STRATEGY.is_provider_network(net_id): # We don't honor segmented networks when they aren't "shared" segment_id = None port_count = db_api.port_count_all(context, network_id=[net_id], @@ -164,7 +164,8 @@ def create_port(context, port): def _allocate_ips(fixed_ips, net, port_id, segment_id, mac): fixed_ip_kwargs = {} if fixed_ips: - if STRATEGY.is_parent_network(net_id) and not context.is_admin: + if (STRATEGY.is_provider_network(net_id) and + not context.is_admin): raise exceptions.NotAuthorized() ips, subnets = split_and_validate_requested_subnets(context, @@ -298,7 +299,7 @@ def update_port(context, id, port): new_security_groups = utils.pop_param(port_dict, "security_groups") if Capabilities.SECURITY_GROUPS not in CONF.QUARK.environment_capabilities: if new_security_groups is not None: - if not STRATEGY.is_parent_network(port_db["network_id"]): + if not STRATEGY.is_provider_network(port_db["network_id"]): raise q_exc.TenantNetworkSecurityGroupsNotImplemented() if new_security_groups is not None and not port_db["device_id"]: diff --git a/quark/plugin_modules/subnets.py b/quark/plugin_modules/subnets.py index 0f63f29..9ec97b8 100644 --- a/quark/plugin_modules/subnets.py +++ b/quark/plugin_modules/subnets.py @@ -340,11 +340,6 @@ def get_subnet(context, id, fields=None): if not subnet: raise exceptions.SubnetNotFound(subnet_id=id) - # Check the network_id against the strategies - net_id = subnet["network_id"] - net_id = STRATEGY.get_parent_network(net_id) - subnet["network_id"] = net_id - cache = subnet.get("_allocation_pool_cache") if not cache: new_cache = subnet.allocation_pools diff --git a/quark/plugin_views.py b/quark/plugin_views.py index 0dc186f..e13ea76 100644 --- a/quark/plugin_views.py +++ b/quark/plugin_views.py @@ -53,7 +53,7 @@ def _is_default_route(route): def _make_network_dict(network, fields=None): - shared_net = STRATEGY.is_parent_network(network["id"]) + shared_net = STRATEGY.is_provider_network(network["id"]) res = {"id": network["id"], "name": network.get("name"), "tenant_id": network.get("tenant_id"), @@ -77,7 +77,7 @@ def _make_network_dict(network, fields=None): def _make_subnet_dict(subnet, fields=None): dns_nameservers = [str(netaddr.IPAddress(dns["ip"])) for dns in subnet.get("dns_nameservers")] - net_id = STRATEGY.get_parent_network(subnet["network_id"]) + net_id = subnet["network_id"] res = {"id": subnet.get("id"), "name": subnet.get("name"), @@ -86,7 +86,7 @@ def _make_subnet_dict(subnet, fields=None): "ip_version": subnet.get("ip_version"), "dns_nameservers": dns_nameservers or [], "cidr": subnet.get("cidr"), - "shared": STRATEGY.is_parent_network(net_id), + "shared": STRATEGY.is_provider_network(net_id), "enable_dhcp": None} if CONF.QUARK.show_subnet_ip_policy_id: @@ -163,7 +163,7 @@ def _ip_port_dict(ip, port, fields=None): def _port_dict(port, fields=None): res = {"id": port.get("id"), "name": port.get("name"), - "network_id": STRATEGY.get_parent_network(port["network_id"]), + "network_id": port["network_id"], "tenant_id": port.get("tenant_id"), "mac_address": port.get("mac_address"), "admin_state_up": port.get("admin_state_up"), @@ -263,9 +263,8 @@ def _make_route_dict(route): def _make_ip_dict(address): - net_id = STRATEGY.get_parent_network(address["network_id"]) return {"id": address["id"], - "network_id": net_id, + "network_id": address["network_id"], "address": address.formatted(), "port_ids": [assoc.port_id for assoc in address["associations"]], diff --git a/quark/tests/plugin_modules/test_ports.py b/quark/tests/plugin_modules/test_ports.py index 5280cf2..6e40209 100644 --- a/quark/tests/plugin_modules/test_ports.py +++ b/quark/tests/plugin_modules/test_ports.py @@ -452,7 +452,7 @@ class TestQuarkCreatePortsSameDevBadRequest(test_quark_plugin.TestQuarkPlugin): for key in expected.keys(): self.assertEqual(result[key], expected[key]) - @mock.patch("quark.network_strategy.JSONStrategy.is_parent_network") + @mock.patch("quark.network_strategy.JSONStrategy.is_provider_network") def test_create_providernet_port_fixed_ip_not_authorized(self, is_parent): is_parent.return_value = True network = dict(id='1', tenant_id=self.context.tenant_id) @@ -474,7 +474,7 @@ class TestQuarkCreatePortsSameDevBadRequest(test_quark_plugin.TestQuarkPlugin): with self.assertRaises(exceptions.NotAuthorized): self.plugin.create_port(self.context, port) - @mock.patch("quark.network_strategy.JSONStrategy.is_parent_network") + @mock.patch("quark.network_strategy.JSONStrategy.is_provider_network") def test_create_providernet_port_fixed_ip_wrong_segment(self, is_parent): is_parent.return_value = True network = dict(id='1', tenant_id=self.context.tenant_id) @@ -773,7 +773,7 @@ class TestQuarkUpdatePortSecurityGroups(test_quark_plugin.TestQuarkPlugin): mock.patch("quark.ipam.QuarkIpam.deallocate_ips_by_port"), mock.patch("neutron.quota.QuotaEngine.limit_check"), mock.patch("quark.plugin_modules.ports.STRATEGY" - ".is_parent_network"), + ".is_provider_network"), mock.patch("quark.db.api.security_group_find"), mock.patch("quark.drivers.base.BaseDriver.update_port") ) as (port_find, port_update, alloc_ip, dealloc_ip, limit_check, diff --git a/quark/tests/test_network_strategies.py b/quark/tests/test_network_strategies.py index e521b69..5697841 100644 --- a/quark/tests/test_network_strategies.py +++ b/quark/tests/test_network_strategies.py @@ -15,7 +15,6 @@ import json -from neutron.common import exceptions from oslo_config import cfg from quark import network_strategy @@ -27,8 +26,7 @@ class TestJSONStrategy(test_base.TestBase): self.context = None self.strategy = {"public_network": {"required": True, - "bridge": "xenbr0", - "children": {"nova": "child_net"}}} + "bridge": "xenbr0"}} strategy_json = json.dumps(self.strategy) cfg.CONF.set_override("default_net_strategy", strategy_json, "QUARK") @@ -57,31 +55,3 @@ class TestJSONStrategy(test_base.TestBase): self.assertTrue("foo_net" not in assignable) self.assertTrue("public_network" not in tenant) self.assertTrue("public_network" in assignable) - - def test_get_parent_network(self): - json_strategy = network_strategy.JSONStrategy(None) - parent_net = json_strategy.get_parent_network("child_net") - self.assertEqual(parent_net, "public_network") - - def test_get_parent_network_no_parent(self): - json_strategy = network_strategy.JSONStrategy(None) - parent_net = json_strategy.get_parent_network("bar_network") - self.assertEqual(parent_net, "bar_network") - - def test_best_match_network_id(self): - json_strategy = network_strategy.JSONStrategy(None) - net = json_strategy.best_match_network_id(self.context, - "public_network", "nova") - self.assertEqual(net, "child_net") - - def test_best_match_network_net_not_in_strategy(self): - json_strategy = network_strategy.JSONStrategy(None) - net = json_strategy.best_match_network_id(self.context, - "foo_net", "nova") - self.assertEqual(net, "foo_net") - - def test_best_match_network_no_valid_child(self): - json_strategy = network_strategy.JSONStrategy(None) - with self.assertRaises(exceptions.NetworkNotFound): - json_strategy.best_match_network_id(self.context, - "public_network", "derpa")