From 610883ad3ede19083f460c85bd81fac11cc3a740 Mon Sep 17 00:00:00 2001 From: Yipei Niu Date: Mon, 28 Aug 2017 08:41:16 +0800 Subject: [PATCH] Improve get_subent in local neutron 1. What is the problem? When adding a member for a load balancer, neutron lbaas checks whether the members subnet exists. If it does not exist, local neutron creates one. Currently, local neutron creates subnet with assuming that the network already exists, leading failure of creating subent. 2. What is the solution to the problem? Before creating a subnet, local neutorn checks whether the network exist. If not, creating the network, then the subnet. 3. What the features need to be implemented to the Tricircle to realize the solution? No new features. Change-Id: I276445e53a1c35de9263c8c871e5bf3723b4bcfb --- tricircle/network/local_plugin.py | 48 ++++++-- .../tests/unit/network/test_local_plugin.py | 103 +++++++++++++++--- 2 files changed, 129 insertions(+), 22 deletions(-) diff --git a/tricircle/network/local_plugin.py b/tricircle/network/local_plugin.py index a978df99..7d5a7990 100644 --- a/tricircle/network/local_plugin.py +++ b/tricircle/network/local_plugin.py @@ -249,6 +249,30 @@ class TricirclePlugin(plugin.Ml2Plugin): {'network': net_body}) return b_network + def _is_valid_network(self, context, network_id): + try: + self.core_plugin.get_network(context, network_id) + except q_exceptions.NotFound: + if self._in_subnet_delete(context): + raise + t_ctx = t_context.get_context_from_neutron_context(context) + + t_network = self.neutron_handle.handle_get( + t_ctx, 'network', network_id) + region_name = self._get_neutron_region() + located = self._is_network_located_in_region(t_network, + region_name) + if not located: + LOG.error('network: %(network_id)s not located in current ' + 'region: %(region_name)s, ' + 'az_hints: %(az_hints)s', + {'network_id': t_network['id'], + 'region_name': region_name, + 'az_hints': t_network[az_def.AZ_HINTS]}) + return located + self._create_bottom_network(context, network_id) + return True + def _is_network_located_in_region(self, t_network, region_name): az_hints = t_network.get(az_def.AZ_HINTS) if not az_hints: @@ -268,12 +292,7 @@ class TricirclePlugin(plugin.Ml2Plugin): t_ctx = t_context.get_context_from_neutron_context(context) if self._skip_non_api_query(t_ctx): raise q_exceptions.NetworkNotFound(net_id=_id) - t_network = self.neutron_handle.handle_get(t_ctx, 'network', _id) - if not t_network: - raise q_exceptions.NetworkNotFound(net_id=_id) - self._adapt_network_body(t_network) - b_network = self.core_plugin.create_network(context, - {'network': t_network}) + t_network, b_network = self._create_bottom_network(context, _id) subnet_ids = self._ensure_subnet(context, t_network) if subnet_ids: b_network['subnets'] = subnet_ids @@ -363,10 +382,19 @@ class TricirclePlugin(plugin.Ml2Plugin): b_gateway_ip)['subnet'] t_subnet['gateway_ip'] = subnet_body['gateway_ip'] t_subnet['allocation_pools'] = subnet_body['allocation_pools'] - b_subnet = self.core_plugin.create_subnet(q_ctx, {'subnet': t_subnet}) return b_subnet + def _create_bottom_network(self, context, _id): + t_ctx = t_context.get_context_from_neutron_context(context) + t_network = self.neutron_handle.handle_get(t_ctx, 'network', _id) + if not t_network: + raise q_exceptions.NetworkNotFound(net_id=_id) + self._adapt_network_body(t_network) + b_network = self.core_plugin.create_network(context, + {'network': t_network}) + return t_network, b_network + def get_subnet(self, context, _id, fields=None): t_ctx = t_context.get_context_from_neutron_context(context) try: @@ -377,6 +405,9 @@ class TricirclePlugin(plugin.Ml2Plugin): t_subnet = self.neutron_handle.handle_get(t_ctx, 'subnet', _id) if not t_subnet: raise q_exceptions.SubnetNotFound(subnet_id=_id) + valid = self._is_valid_network(context, t_subnet['network_id']) + if not valid: + raise q_exceptions.SubnetNotFound(subnet_id=_id) b_subnet = self._create_bottom_subnet(t_ctx, context, t_subnet) if b_subnet['enable_dhcp']: self._ensure_subnet_dhcp_port(t_ctx, context, b_subnet) @@ -417,6 +448,9 @@ class TricirclePlugin(plugin.Ml2Plugin): missing_subnets = [subnet for subnet in t_subnets if ( subnet['id'] in missing_id_set)] for subnet in missing_subnets: + valid = self._is_valid_network(context, subnet['network_id']) + if not valid: + continue b_subnet = self._create_bottom_subnet(t_ctx, context, subnet) if b_subnet['enable_dhcp']: self._ensure_subnet_dhcp_port(t_ctx, context, b_subnet) diff --git a/tricircle/tests/unit/network/test_local_plugin.py b/tricircle/tests/unit/network/test_local_plugin.py index bcb640a7..3695c8b7 100644 --- a/tricircle/tests/unit/network/test_local_plugin.py +++ b/tricircle/tests/unit/network/test_local_plugin.py @@ -123,6 +123,10 @@ class FakeCorePlugin(object): def get_subnet(self, context, _id, fields=None): return get_resource('subnet', False, _id) + def get_subnets(self, context, filters=None, fields=None, sorts=None, + limit=None, marker=None, page_reverse=False): + return list_resource('subnet', False, filters) + def create_port(self, context, port): create_resource('port', False, port['port']) return port['port'] @@ -181,6 +185,9 @@ class FakeClient(object): def list_networks(self, **kwargs): return {'networks': list_resource('network', True, kwargs)} + def list_subnets(self, **kwargs): + return {'subnets': list_resource('subnet', True, kwargs)} + def create_port(self, port): if 'id' not in port['port']: port['port']['id'] = uuidutils.generate_uuid() @@ -313,18 +320,28 @@ class PluginTest(unittest.TestCase): TOP_SGS.append(t_sg) return t_net, t_subnet, t_port, t_sg - def _validate(self, net, subnet, port): - b_net = self.plugin.get_network(self.context, net['id']) - net.pop('provider:network_type') - net.pop('availability_zone_hints') - b_net_type = b_net.pop('provider:network_type') + def _get_bottom_resources_with_net(self, net, subnet, port): + b_net = get_resource('network', False, net['id']) b_subnet = get_resource('subnet', False, subnet['id']) b_port = get_resource('port', False, port['id']) b_net.pop('project_id') + return b_net, b_subnet, b_port + + def _get_bottom_resources_without_net(self, subnet, port): + b_net = get_resource('network', False, subnet['network_id']) + b_subnet = get_resource('subnet', False, subnet['id']) + b_port = get_resource('port', False, port['id']) + return b_net, b_subnet, b_port + + def _validate(self, b_net, b_subnet, b_port, t_net, t_subnet, t_port): + + t_net.pop('provider:network_type') + t_net.pop('availability_zone_hints') + b_net_type = b_net.pop('provider:network_type') b_subnet.pop('project_id') - pool = subnet.pop('allocation_pools')[0] + pool = t_subnet.pop('allocation_pools')[0] b_pools = b_subnet.pop('allocation_pools') - t_gateway_ip = subnet.pop('gateway_ip') + t_gateway_ip = t_subnet.pop('gateway_ip') b_gateway_ip = b_subnet.pop('gateway_ip') def ip_to_digit(ip): @@ -347,13 +364,13 @@ class PluginTest(unittest.TestCase): ip_to_digit(pool['end']))) b_pool_range = list(range(ip_to_digit(b_pools[0]['start']), ip_to_digit(b_pools[0]['end']))) - port.pop('name') + t_port.pop('name') b_port.pop('name') - self.assertDictEqual(net, b_net) - self.assertDictEqual(subnet, b_subnet) + self.assertDictEqual(t_net, b_net) + self.assertDictEqual(t_subnet, b_subnet) self.assertSetEqual(set(pool_range), set(b_pool_range)) self.assertEqual('vlan', b_net_type) - self.assertDictEqual(port, b_port) + self.assertDictEqual(t_port, b_port) def _prepare_vm_port(self, t_net, t_subnet, index, t_sgs=[]): port_id = uuidutils.generate_uuid() @@ -372,16 +389,39 @@ class PluginTest(unittest.TestCase): TOP_PORTS.append(t_port) return t_port + @patch.object(t_context, 'get_context_from_neutron_context', new=mock.Mock) + def test_get_subnet_no_bottom_network(self): + t_net, t_subnet, t_port, _ = self._prepare_resource() + self.plugin.get_subnet(self.context, t_subnet['id']) + b_net, b_subnet, b_port = self._get_bottom_resources_without_net( + t_subnet, t_port) + self._validate(b_net, b_subnet, b_port, t_net, t_subnet, t_port) + + @patch.object(t_context, 'get_context_from_neutron_context', new=mock.Mock) + def test_get_subnet(self): + t_net, t_subnet, t_port, _ = self._prepare_resource() + self.plugin.get_network(self.context, t_net['id']) + self.plugin.get_subnet(self.context, t_subnet['id']) + b_net, b_subnet, b_port = self._get_bottom_resources_with_net( + t_net, t_subnet, t_port) + self._validate(b_net, b_subnet, b_port, t_net, t_subnet, t_port) + @patch.object(t_context, 'get_context_from_neutron_context', new=mock.Mock) def test_get_network(self): t_net, t_subnet, t_port, _ = self._prepare_resource() - self._validate(t_net, t_subnet, t_port) + self.plugin.get_network(self.context, t_net['id']) + b_net, b_subnet, b_port = self._get_bottom_resources_with_net( + t_net, t_subnet, t_port) + self._validate(b_net, b_subnet, b_port, t_net, t_subnet, t_port) @patch.object(t_context, 'get_context_from_neutron_context', new=mock.Mock) def test_get_network_no_gateway(self): t_net, t_subnet, t_port, _ = self._prepare_resource() update_resource('subnet', True, t_subnet['id'], {'gateway_ip': None}) - self._validate(t_net, t_subnet, t_port) + self.plugin.get_network(self.context, t_net['id']) + b_net, b_subnet, b_port = self._get_bottom_resources_with_net( + t_net, t_subnet, t_port) + self._validate(b_net, b_subnet, b_port, t_net, t_subnet, t_port) @patch.object(t_context, 'get_context_from_neutron_context', new=mock.Mock) @patch.object(client.Client, 'get_admin_token', new=mock.Mock) @@ -393,8 +433,12 @@ class PluginTest(unittest.TestCase): self.plugin.get_networks(self.context, {'id': [t_net1['id'], t_net2['id'], 'fake_net_id']}) - self._validate(t_net1, t_subnet1, t_port1) - self._validate(t_net2, t_subnet2, t_port2) + b_net1, b_subnet1, b_port1 = self._get_bottom_resources_with_net( + t_net1, t_subnet1, t_port1) + b_net2, b_subnet2, b_port2 = self._get_bottom_resources_with_net( + t_net2, t_subnet2, t_port2) + self._validate(b_net1, b_subnet1, b_port1, t_net1, t_subnet1, t_port1) + self._validate(b_net2, b_subnet2, b_port2, t_net2, t_subnet2, t_port2) @patch.object(t_context, 'get_context_from_neutron_context', new=mock.Mock) @patch.object(client.Client, 'get_admin_token', new=mock.Mock) @@ -408,6 +452,35 @@ class PluginTest(unittest.TestCase): nets = self.plugin.get_networks(self.context, net_filter) six.assertCountEqual(self, nets, []) + @patch.object(t_context, 'get_context_from_neutron_context', new=mock.Mock) + @patch.object(client.Client, 'get_admin_token', new=mock.Mock) + def test_get_subnets(self): + az_hints = ['Pod1', 'Pod2'] + t_net1, t_subnet1, t_port1, _ = self._prepare_resource() + t_net2, t_subnet2, t_port2, _ = self._prepare_resource(az_hints) + cfg.CONF.set_override('region_name', 'Pod1', 'nova') + self.plugin.get_subnets(self.context, + {'id': [t_subnet1['id'], t_subnet2['id'], + 'fake_net_id']}) + b_net1, b_subnet1, b_port1 = self._get_bottom_resources_without_net( + t_subnet1, t_port1) + b_net2, b_subnet2, b_port2 = self._get_bottom_resources_without_net( + t_subnet2, t_port2) + self._validate(b_net1, b_subnet1, b_port1, t_net1, t_subnet1, t_port1) + self._validate(b_net2, b_subnet2, b_port2, t_net2, t_subnet2, t_port2) + + @patch.object(t_context, 'get_context_from_neutron_context', new=mock.Mock) + @patch.object(client.Client, 'get_admin_token', new=mock.Mock) + def test_get_invaild_subnets(self): + az_hints = ['Pod2', 'Pod3'] + t_net1, t_subnet1, t_port1, _ = self._prepare_resource(az_hints) + cfg.CONF.set_override('region_name', 'Pod1', 'nova') + net_filter = { + 'id': [t_subnet1.get('id')] + } + subnets = self.plugin.get_subnets(self.context, net_filter) + six.assertCountEqual(self, subnets, []) + @patch.object(t_context, 'get_context_from_neutron_context', new=mock.Mock) def test_create_port(self): t_net, t_subnet, t_port, _ = self._prepare_resource()