diff --git a/releasenotes/notes/network-subnet-update-baed5ded548f7269.yaml b/releasenotes/notes/network-subnet-update-baed5ded548f7269.yaml new file mode 100644 index 00000000..7834dc8e --- /dev/null +++ b/releasenotes/notes/network-subnet-update-baed5ded548f7269.yaml @@ -0,0 +1,21 @@ +--- +features: + - | + Network + + * Update networks + + * qos-policy not supported + + - | + Subnet + + * Update subnets + +issues: + - | + Update network or subnet may not lead to the expected result if an + instance is being booted at the same time. You can redo the update + operation later to make it execute correctly. + + diff --git a/tricircle/common/constants.py b/tricircle/common/constants.py index c332c505..eaea3d41 100644 --- a/tricircle/common/constants.py +++ b/tricircle/common/constants.py @@ -82,7 +82,7 @@ JT_ROUTER_SETUP = 'router_setup' JT_PORT_DELETE = 'port_delete' JT_SEG_RULE_SETUP = 'seg_rule_setup' JT_NETWORK_UPDATE = 'update_network' - +JT_SUBNET_UPDATE = 'subnet_update' # network type NT_LOCAL = 'local' NT_VLAN = 'vlan' diff --git a/tricircle/common/xrpcapi.py b/tricircle/common/xrpcapi.py index 53615a74..45c9327d 100644 --- a/tricircle/common/xrpcapi.py +++ b/tricircle/common/xrpcapi.py @@ -102,3 +102,9 @@ class XJobAPI(object): self.client.prepare(exchange='openstack').cast( ctxt, 'update_network', payload={constants.JT_NETWORK_UPDATE: combine_id}) + + def update_subnet(self, ctxt, subnet_id, pod_id): + combine_id = '%s#%s' % (pod_id, subnet_id) + self.client.prepare(exchange='openstack').cast( + ctxt, 'update_subnet', + payload={constants.JT_SUBNET_UPDATE: combine_id}) diff --git a/tricircle/network/central_plugin.py b/tricircle/network/central_plugin.py index 5b740df8..5a5d3362 100644 --- a/tricircle/network/central_plugin.py +++ b/tricircle/network/central_plugin.py @@ -441,8 +441,39 @@ class TricirclePlugin(db_base_plugin_v2.NeutronDbPluginV2, super(TricirclePlugin, self).delete_subnet(context, subnet_id) def update_subnet(self, context, subnet_id, subnet): - return super(TricirclePlugin, self).update_subnet( - context, subnet_id, subnet) + """update top subnet + + update top subnet and trigger asynchronous job via RPC to update + bottom subnet. + :param context: neutron context + :param subnet_id: top subnet id + :param subnet: updated subnet body + :return: updated subnet + """ + with context.session.begin(): + subnet_data = subnet[attributes.SUBNET] + t_ctx = t_context.get_context_from_neutron_context(context) + # update top subnet + result = super(TricirclePlugin, self).update_subnet( + context, subnet_id, subnet) + # prepare top dhcp port if user enables dhcp, + # the top pre-created dhcp port will not be deleted even + # "enable_dhcp" is updated from True to False + enable_dhcp = subnet_data.get('enable_dhcp', False) + if enable_dhcp: + subnet = super(TricirclePlugin, self).get_subnet( + context, subnet_id) + self.helper.prepare_top_dhcp_port(t_ctx, context, + t_ctx.project_id, + subnet['network_id'], + subnet_id) + # update bottom pod subnet if exist + mappings = db_api.get_bottom_mappings_by_top_id( + t_ctx, subnet_id, t_constants.RT_SUBNET) + if mappings: + self.xjob_handler.update_subnet(t_ctx, subnet_id, + t_constants.POD_NOT_SPECIFIED) + return result def create_port(self, context, port): port_body = port['port'] diff --git a/tricircle/network/helper.py b/tricircle/network/helper.py index 6b032331..44ded263 100644 --- a/tricircle/network/helper.py +++ b/tricircle/network/helper.py @@ -227,14 +227,12 @@ class NetworkHelper(object): return body @staticmethod - def get_create_subnet_body(project_id, t_subnet, b_net_id, gateway_ip): - """Get request body to create bottom subnet + def get_bottom_subnet_pools(t_subnet, gateway_ip): + """Get bottom subnet allocation pools - :param project_id: project id - :param t_subnet: top subnet dict - :param b_net_id: bottom network id - :param gateway_ip: bottom gateway ip - :return: request body to create bottom subnet + :param t_subnet: top subnet + :param gateway_ip: bottom subnet gateway ip + :return: bottom subnet allocation pools """ pools = t_subnet['allocation_pools'] t_gateway_ip = t_subnet['gateway_ip'] @@ -263,6 +261,24 @@ class NetworkHelper(object): new_pools.append( {'start': ip_range[i + 1].format(), 'end': ip_range[ip_num - 1].format()}) + break + if not ip_found: + new_pools.extend(pools) + if not ip_merged: + new_pools.insert(0, {'start': t_gateway_ip, 'end': t_gateway_ip}) + return new_pools + + @staticmethod + def get_create_subnet_body(project_id, t_subnet, b_net_id, gateway_ip): + """Get request body to create bottom subnet + + :param project_id: project id + :param t_subnet: top subnet dict + :param b_net_id: bottom network id + :param gateway_ip: bottom gateway ip + :return: request body to create bottom subnet + """ + new_pools = NetworkHelper.get_bottom_subnet_pools(t_subnet, gateway_ip) body = { 'subnet': { 'network_id': b_net_id, @@ -287,7 +303,7 @@ class NetworkHelper(object): :param subnet_map: dict with top subnet id as key and bottom subnet id as value :param b_net_id: bottom network id - :param security_group_ids: list of bottom security group id + :param b_security_group_ids: list of bottom security group id :return: request body to create bottom port """ b_fixed_ips = [] diff --git a/tricircle/network/local_plugin.py b/tricircle/network/local_plugin.py index ca566413..f45fe4af 100644 --- a/tricircle/network/local_plugin.py +++ b/tricircle/network/local_plugin.py @@ -210,7 +210,7 @@ class TricirclePlugin(plugin.Ml2Plugin): def get_network(self, context, _id, fields=None): try: - b_network = self.core_plugin.get_network(context, _id, fields) + b_network = self.core_plugin.get_network(context, _id) subnet_ids = self._ensure_subnet(context, b_network, False) except q_exceptions.NotFound: t_ctx = t_context.get_context_from_neutron_context(context) @@ -301,7 +301,7 @@ class TricirclePlugin(plugin.Ml2Plugin): def get_subnet(self, context, _id, fields=None): t_ctx = t_context.get_context_from_neutron_context(context) try: - b_subnet = self.core_plugin.get_subnet(context, _id, fields) + b_subnet = self.core_plugin.get_subnet(context, _id) except q_exceptions.NotFound: if self._skip_non_api_query(t_ctx): raise q_exceptions.SubnetNotFound(subnet_id=_id) @@ -354,6 +354,32 @@ class TricirclePlugin(plugin.Ml2Plugin): b_subnets.append(self._fields(b_subnet, fields)) return b_subnets + def update_subnet(self, context, _id, subnet): + """update bottom subnet + + Can not directly use ML2 plugin's update_subnet function, + because it will call local plugin's get_subnet in a transaction, + the local plugin's get_subnet will create a dhcp port when subnet's + enable_dhcp attribute is changed from False to True, but neutron + doesn't allow calling create_port in a transaction and will raise an + exception. + + :param context: neutron context + :param _id: subnet_id + :param subnet: update body + :return: updated subnet + """ + t_ctx = t_context.get_context_from_neutron_context(context) + b_subnet = self.core_plugin.get_subnet(context, _id) + origin_enable_dhcp = b_subnet['enable_dhcp'] + req_enable_dhcp = subnet['subnet']['enable_dhcp'] + # when request enable dhcp, and origin dhcp is disabled, + # ensure subnet dhcp port is created + if req_enable_dhcp and not origin_enable_dhcp: + self._ensure_subnet_dhcp_port(t_ctx, context, b_subnet) + res = self.core_plugin.update_subnet(context, _id, subnet) + return res + @staticmethod def _is_special_port(port): return port.get('device_owner') in ( diff --git a/tricircle/tests/unit/network/test_central_plugin.py b/tricircle/tests/unit/network/test_central_plugin.py index 04396aad..59e434ab 100644 --- a/tricircle/tests/unit/network/test_central_plugin.py +++ b/tricircle/tests/unit/network/test_central_plugin.py @@ -49,6 +49,7 @@ from neutron.ipam import driver from neutron.ipam import exceptions as ipam_exc from neutron.ipam import requests import neutron.ipam.utils as ipam_utils + from neutron import manager import neutronclient.common.exceptions as q_exceptions @@ -232,7 +233,7 @@ class FakePool(driver.Pool): return FakeIpamSubnet(subnet_request) raise ipam_exc.InvalidSubnetRequest( - reason=_("updated subnet id no not found")) + reason=_("updated subnet id not found")) def remove_subnet(self, subnet_id): pass @@ -473,7 +474,20 @@ class FakeClient(object): return def update_subnets(self, ctx, subnet_id, body): - pass + subnet_data = body[neutron_attributes.SUBNET] + if self.region_name == 'pod_1': + subnets = BOTTOM1_SUBNETS + else: + subnets = BOTTOM2_SUBNETS + + for subnet in subnets: + if subnet['id'] == subnet_id: + for key in subnet_data: + subnet[key] = subnet_data[key] + return + + raise ipam_exc.InvalidSubnetRequest( + reason=_("updated subnet id not found")) def create_ports(self, ctx, body): return self.create_resources('port', ctx, body) @@ -988,6 +1002,11 @@ class FakeBaseRPCAPI(object): self.xmanager.update_network( ctxt, payload={constants.JT_NETWORK_UPDATE: combine_id}) + def update_subnet(self, ctxt, subnet_id, pod_id): + combine_id = '%s#%s' % (pod_id, subnet_id) + self.xmanager.update_subnet( + ctxt, payload={constants.JT_SUBNET_UPDATE: combine_id}) + class FakeRPCAPI(FakeBaseRPCAPI): def __init__(self, fake_plugin): @@ -1719,7 +1738,10 @@ class PluginTest(unittest.TestCase, 'gateway_ip': '10.0.%d.1' % index, 'ipv6_address_mode': '', 'ipv6_ra_mode': '', - 'tenant_id': tenant_id + 'tenant_id': tenant_id, + 'description': 'description', + 'host_routes': [], + 'dns_nameservers': [] } TOP_NETS.append(DotDict(t_net)) TOP_SUBNETS.append(DotDict(t_subnet)) @@ -1740,10 +1762,13 @@ class PluginTest(unittest.TestCase, 'cidr': '10.0.%d.0/24' % index, 'allocation_pools': [], 'enable_dhcp': enable_dhcp, - 'gateway_ip': '10.0.%d.1' % index, + 'gateway_ip': '10.0.%d.25' % index, 'ipv6_address_mode': '', 'ipv6_ra_mode': '', - 'tenant_id': tenant_id + 'tenant_id': tenant_id, + 'description': 'description', + 'host_routes': [], + 'dns_nameservers': [] } if region_name == 'pod_1': BOTTOM1_NETS.append(DotDict(b_net)) @@ -1828,6 +1853,125 @@ class PluginTest(unittest.TestCase, # check pre-created ports are all deleted self.assertEqual(port_num - pre_created_port_num, len(TOP_PORTS)) + @patch.object(directory, 'get_plugin', new=fake_get_plugin) + @patch.object(driver.Pool, 'get_instance', new=fake_get_instance) + @patch.object(context, 'get_context_from_neutron_context') + def test_update_subnet(self, mock_context): + tenant_id = TEST_TENANT_ID + self._basic_pod_route_setup() + neutron_context = FakeNeutronContext() + t_ctx = context.get_db_context() + _, t_subnet_id, _, b_subnet_id = self._prepare_network_test( + tenant_id, t_ctx, 'pod_1', 1) + + fake_plugin = FakePlugin() + fake_client = FakeClient('pod_1') + mock_context.return_value = t_ctx + update_body = { + 'subnet': + {'name': 'new_name', + 'description': 'new_description', + 'allocation_pools': [{"start": "10.0.1.10", + "end": "10.0.1.254"}], + 'gateway_ip': '10.0.1.2', + 'host_routes': [{"nexthop": "10.1.0.1", + "destination": "10.1.0.0/24"}, + {"nexthop": "10.2.0.1", + "destination": "10.2.0.0/24"}], + 'dns_nameservers': ['114.114.114.114', '8.8.8.8']} + } + body_copy = copy.deepcopy(update_body) + fake_plugin.update_subnet(neutron_context, t_subnet_id, update_body) + top_subnet = fake_plugin.get_subnet(neutron_context, t_subnet_id) + self.assertEqual(top_subnet['name'], body_copy['subnet']['name']) + self.assertEqual(top_subnet['description'], + body_copy['subnet']['description']) + self.assertEqual(top_subnet['allocation_pools'], + body_copy['subnet']['allocation_pools']) + six.assertCountEqual(self, top_subnet['host_routes'], + body_copy['subnet']['host_routes']) + six.assertCountEqual(self, top_subnet['dns_nameservers'], + body_copy['subnet']['dns_nameservers']) + self.assertEqual(top_subnet['gateway_ip'], + body_copy['subnet']['gateway_ip']) + + bottom_subnet = fake_client.get_subnets(t_ctx, b_subnet_id) + # name is set to top resource id, which is used by lock_handle to + # retrieve bottom/local resources that have been created but not + # registered in the resource routing table, so it's not allowed + # to be updated + self.assertEqual(bottom_subnet['name'], b_subnet_id) + self.assertEqual(bottom_subnet['description'], + body_copy['subnet']['description']) + bottom_allocation_pools = [{'start': '10.0.1.2', 'end': '10.0.1.2'}, + {'start': '10.0.1.10', 'end': '10.0.1.24'}, + {'start': '10.0.1.26', 'end': '10.0.1.254'}] + self.assertEqual(bottom_subnet['allocation_pools'], + bottom_allocation_pools) + six.assertCountEqual(self, + bottom_subnet['host_routes'], + body_copy['subnet']['host_routes']) + six.assertCountEqual(self, + bottom_subnet['dns_nameservers'], + body_copy['subnet']['dns_nameservers']) + # gateway ip is set to origin gateway ip ,because it is reserved + # by top pod, so it's not allowed to be updated + self.assertEqual(bottom_subnet['gateway_ip'], '10.0.1.25') + + @patch.object(directory, 'get_plugin', new=fake_get_plugin) + @patch.object(driver.Pool, 'get_instance', new=fake_get_instance) + @patch.object(ipam_pluggable_backend.IpamPluggableBackend, + '_allocate_ips_for_port', new=fake_allocate_ips_for_port) + @patch.object(context, 'get_context_from_neutron_context') + def test_update_subnet_enable_disable_dhcp(self, mock_context): + + tenant_id = TEST_TENANT_ID + self._basic_pod_route_setup() + neutron_context = FakeNeutronContext() + t_ctx = context.get_db_context() + _, t_subnet_id, _, b_subnet_id = self._prepare_network_test( + tenant_id, t_ctx, 'pod_1', 1, enable_dhcp=False) + + fake_plugin = FakePlugin() + fake_client = FakeClient('pod_1') + mock_context.return_value = t_ctx + + self.assertEqual(0, len(TOP_PORTS)) + self.assertEqual(0, len(BOTTOM1_PORTS)) + + update_body = { + 'subnet': + {'enable_dhcp': 'True'} + } + body_copy = copy.deepcopy(update_body) + # from disable dhcp to enable dhcp, create a new dhcp port + fake_plugin.update_subnet(neutron_context, t_subnet_id, update_body) + top_subnet = fake_plugin.get_subnet(neutron_context, t_subnet_id) + self.assertEqual(top_subnet['enable_dhcp'], + body_copy['subnet']['enable_dhcp']) + self.assertEqual(1, len(TOP_PORTS)) + + bottom_subnet = fake_client.get_subnets(t_ctx, b_subnet_id) + self.assertEqual(bottom_subnet['enable_dhcp'], + body_copy['subnet']['enable_dhcp']) + + update_body = { + 'subnet': + {'enable_dhcp': 'False'} + } + body_copy = copy.deepcopy(update_body) + # from enable dhcp to disable dhcp, reserved dhcp port + # previously created + fake_plugin.update_subnet(neutron_context, t_subnet_id, update_body) + top_subnet = fake_plugin.get_subnet(neutron_context, t_subnet_id) + self.assertEqual(top_subnet['enable_dhcp'], + body_copy['subnet']['enable_dhcp']) + self.assertEqual(1, len(TOP_PORTS)) + + bottom_subnet = fake_client.get_subnets(t_ctx, b_subnet_id) + self.assertEqual(bottom_subnet['enable_dhcp'], + body_copy['subnet']['enable_dhcp']) + @patch.object(directory, 'get_plugin', new=fake_get_plugin) @patch.object(driver.Pool, 'get_instance', new=fake_get_instance) @patch.object(_utils, 'filter_non_model_columns', diff --git a/tricircle/tests/unit/network/test_helper.py b/tricircle/tests/unit/network/test_helper.py index 3cb3e3dd..adfbdbaa 100644 --- a/tricircle/tests/unit/network/test_helper.py +++ b/tricircle/tests/unit/network/test_helper.py @@ -37,32 +37,42 @@ class HelperTest(unittest.TestCase): 'ip_version': 4, 'cidr': '10.0.1.0/24', 'gateway_ip': '10.0.1.1', - 'allocation_pools': [{'start': '10.0.1.2', 'end': '10.0.1.254'}], + 'allocation_pools': [{'start': '10.0.1.10', 'end': '10.0.1.254'}], 'enable_dhcp': True, 'tenant_id': project_id } body = self.helper.get_create_subnet_body(project_id, t_subnet, - b_net_id, '10.0.1.2') + b_net_id, '10.0.1.10') six.assertCountEqual(self, [{'start': '10.0.1.1', 'end': '10.0.1.1'}, - {'start': '10.0.1.3', 'end': '10.0.1.254'}], + {'start': '10.0.1.11', 'end': '10.0.1.254'}], body['subnet']['allocation_pools']) - self.assertEqual('10.0.1.2', body['subnet']['gateway_ip']) + self.assertEqual('10.0.1.10', body['subnet']['gateway_ip']) body = self.helper.get_create_subnet_body(project_id, t_subnet, b_net_id, '10.0.1.254') six.assertCountEqual(self, - [{'start': '10.0.1.1', 'end': '10.0.1.253'}], + [{'start': '10.0.1.1', 'end': '10.0.1.1'}, + {'start': '10.0.1.10', 'end': '10.0.1.253'}], body['subnet']['allocation_pools']) self.assertEqual('10.0.1.254', body['subnet']['gateway_ip']) + body = self.helper.get_create_subnet_body(project_id, t_subnet, + b_net_id, '10.0.1.8') + six.assertCountEqual(self, + [{'start': '10.0.1.1', 'end': '10.0.1.1'}, + {'start': '10.0.1.10', 'end': '10.0.1.254'}], + body['subnet']['allocation_pools']) + self.assertEqual('10.0.1.8', body['subnet']['gateway_ip']) + t_subnet['allocation_pools'] = [ {'start': '10.0.1.2', 'end': '10.0.1.10'}, {'start': '10.0.1.20', 'end': '10.0.1.254'}] body = self.helper.get_create_subnet_body(project_id, t_subnet, b_net_id, '10.0.1.5') - six.assertCountEqual(self, [{'start': '10.0.1.1', 'end': '10.0.1.4'}, - {'start': '10.0.1.6', 'end': '10.0.1.10'}, - {'start': '10.0.1.20', 'end': '10.0.1.254'}], + six.assertCountEqual(self, + [{'start': '10.0.1.1', 'end': '10.0.1.4'}, + {'start': '10.0.1.6', 'end': '10.0.1.10'}, + {'start': '10.0.1.20', 'end': '10.0.1.254'}], body['subnet']['allocation_pools']) self.assertEqual('10.0.1.5', body['subnet']['gateway_ip']) diff --git a/tricircle/xjob/xmanager.py b/tricircle/xjob/xmanager.py index 88910ac8..40760efb 100644 --- a/tricircle/xjob/xmanager.py +++ b/tricircle/xjob/xmanager.py @@ -150,7 +150,8 @@ class XManager(PeriodicTasks): constants.JT_ROUTER_SETUP: self.setup_bottom_router, constants.JT_PORT_DELETE: self.delete_server_port, constants.JT_SEG_RULE_SETUP: self.configure_security_group_rules, - constants.JT_NETWORK_UPDATE: self.update_network} + constants.JT_NETWORK_UPDATE: self.update_network, + constants.JT_SUBNET_UPDATE: self.update_subnet} self.helper = helper.NetworkHelper() self.xjob_handler = xrpcapi.XJobAPI() super(XManager, self).__init__() @@ -849,3 +850,64 @@ class XManager(PeriodicTasks): LOG.error(_LE('network: %(net_id)s not found,' 'pod name: %(name)s'), {'net_id': b_network_id, 'name': b_region_name}) + + @_job_handle(constants.JT_SUBNET_UPDATE) + def update_subnet(self, ctx, payload): + """update bottom subnet + + if bottom pod id equal to POD_NOT_SPECIFIED, dispatch jobs for every + mapped bottom pod via RPC, otherwise update subnet in the specified + pod. + + :param ctx: tricircle context + :param payload: dict whose key is JT_SUBNET_UPDATE and value + is "top_subnet_id#bottom_pod_id" + :return: None + """ + (b_pod_id, t_subnet_id) = payload[ + constants.JT_SUBNET_UPDATE].split('#') + if b_pod_id == constants.POD_NOT_SPECIFIED: + mappings = db_api.get_bottom_mappings_by_top_id( + ctx, t_subnet_id, constants.RT_SUBNET) + b_pods = [mapping[0] for mapping in mappings] + for b_pod in b_pods: + self.xjob_handler.update_subnet(ctx, t_subnet_id, + b_pod['pod_id']) + return + + t_client = self._get_client() + t_subnet = t_client.get_subnets(ctx, t_subnet_id) + if not t_subnet: + return + b_pod = db_api.get_pod(ctx, b_pod_id) + b_region_name = b_pod['region_name'] + b_subnet_id = db_api.get_bottom_id_by_top_id_region_name( + ctx, t_subnet_id, b_region_name, constants.RT_SUBNET) + b_client = self._get_client(region_name=b_region_name) + b_subnet = b_client.get_subnets(ctx, b_subnet_id) + b_gateway_ip = b_subnet['gateway_ip'] + + # we need to remove the bottom subnet gateway ip from the top subnet + # allaction pools + b_allocation_pools = helper.NetworkHelper.get_bottom_subnet_pools( + t_subnet, b_gateway_ip) + + # bottom gateway_ip doesn't need to be updated, because it is reserved + # by top pod. + # name is not allowed to be updated, because it is used by + # lock_handle to retrieve bottom/local resources that have been + # created but not registered in the resource routing table + body = { + 'subnet': + {'description': t_subnet['description'], + 'enable_dhcp': t_subnet['enable_dhcp'], + 'allocation_pools': b_allocation_pools, + 'host_routes': t_subnet['host_routes'], + 'dns_nameservers': t_subnet['dns_nameservers']} + } + try: + b_client.update_subnets(ctx, b_subnet_id, body) + except q_cli_exceptions.NotFound: + LOG.error(_LE('subnet: %(subnet_id)s not found, ' + 'pod name: %(name)s'), + {'subnet_id': b_subnet_id, 'name': b_region_name})