From cb60d0bb4e0cc0cba68f59fdf5f4e89d6ec52950 Mon Sep 17 00:00:00 2001 From: changzhi Date: Thu, 16 Jul 2015 10:14:16 +0800 Subject: [PATCH] Keep dns nameserver order consistency Currently, there is no dns servers prioritization for subnets for Neutron. Generally speaking, it is useful to keep the order of dns nameservers consistent. Add a new column named 'order' in table 'dnsnameservers' and add nameserver into DB one by one. Closes-Bug: #1218629 Implements: blueprint keep-dns-nameserver-orderconsistency Change-Id: Id937aea411397d39370368a4eb45be26c4eefa9e --- doc/source/devref/dns_order.rst | 74 +++++++++++++++++++ doc/source/devref/index.rst | 1 + neutron/db/db_base_plugin_common.py | 3 +- neutron/db/ipam_backend_mixin.py | 34 +++++---- .../alembic_migrations/versions/HEADS | 2 +- .../1c844d1677f7_dns_nameservers_order.py | 35 +++++++++ neutron/db/models_v2.py | 2 + neutron/tests/unit/agent/linux/test_dhcp.py | 31 ++++++++ .../tests/unit/db/test_db_base_plugin_v2.py | 25 ++++++- 9 files changed, 188 insertions(+), 19 deletions(-) create mode 100644 doc/source/devref/dns_order.rst create mode 100644 neutron/db/migration/alembic_migrations/versions/liberty/expand/1c844d1677f7_dns_nameservers_order.py diff --git a/doc/source/devref/dns_order.rst b/doc/source/devref/dns_order.rst new file mode 100644 index 00000000000..bb8397081c6 --- /dev/null +++ b/doc/source/devref/dns_order.rst @@ -0,0 +1,74 @@ +Keep DNS Nameserver Order Consistency In Neutron +================================================ + +In Neutron subnets, DNS nameservers are given priority when created or updated. +This means if you create a subnet with multiple DNS servers, the order will +be retained and guests will receive the DNS servers in the order you +created them in when the subnet was created. The same thing applies for update +operations on subnets to add, remove, or update DNS servers. + +Get Subnet Details Info +----------------------- +:: + + changzhi@stack:~/devstack$ neutron subnet-list + +--------------------------------------+------+-------------+--------------------------------------------+ + | id | name | cidr | allocation_pools | + +--------------------------------------+------+-------------+--------------------------------------------+ + | 1a2d261b-b233-3ab9-902e-88576a82afa6 | | 10.0.0.0/24 | {"start": "10.0.0.2", "end": "10.0.0.254"} | + +--------------------------------------+------+-------------+--------------------------------------------+ + + changzhi@stack:~/devstack$ neutron subnet-show 1a2d261b-b233-3ab9-902e-88576a82afa6 + +------------------+--------------------------------------------+ + | Field | Value | + +------------------+--------------------------------------------+ + | allocation_pools | {"start": "10.0.0.2", "end": "10.0.0.254"} | + | cidr | 10.0.0.0/24 | + | dns_nameservers | 1.1.1.1 | + | | 2.2.2.2 | + | | 3.3.3.3 | + | enable_dhcp | True | + | gateway_ip | 10.0.0.1 | + | host_routes | | + | id | 1a2d26fb-b733-4ab3-992e-88554a87afa6 | + | ip_version | 4 | + | name | | + | network_id | a404518c-800d-2353-9193-57dbb42ac5ee | + | tenant_id | 3868290ab10f417390acbb754160dbb2 | + +------------------+--------------------------------------------+ + +Update Subnet DNS Nameservers +----------------------------- +:: + + neutron subnet-update 1a2d261b-b233-3ab9-902e-88576a82afa6 \ + --dns_nameservers list=true 3.3.3.3 2.2.2.2 1.1.1.1 + + changzhi@stack:~/devstack$ neutron subnet-show 1a2d261b-b233-3ab9-902e-88576a82afa6 + +------------------+--------------------------------------------+ + | Field | Value | + +------------------+--------------------------------------------+ + | allocation_pools | {"start": "10.0.0.2", "end": "10.0.0.254"} | + | cidr | 10.0.0.0/24 | + | dns_nameservers | 3.3.3.3 | + | | 2.2.2.2 | + | | 1.1.1.1 | + | enable_dhcp | True | + | gateway_ip | 10.0.0.1 | + | host_routes | | + | id | 1a2d26fb-b733-4ab3-992e-88554a87afa6 | + | ip_version | 4 | + | name | | + | network_id | a404518c-800d-2353-9193-57dbb42ac5ee | + | tenant_id | 3868290ab10f417390acbb754160dbb2 | + +------------------+--------------------------------------------+ + +As shown in above output, the order of the DNS nameservers has been updated. +New virtual machines deployed to this subnet will receive the DNS nameservers +in this new priority order. Existing virtual machines that have already been +deployed will not be immediately affected by changing the DNS nameserver order +on the neutron subnet. Virtual machines that are configured to get their IP +address via DHCP will detect the DNS nameserver order change +when their DHCP lease expires or when the virtual machine is restarted. +Existing virtual machines configured with a static IP address will never +detect the updated DNS nameserver order. diff --git a/doc/source/devref/index.rst b/doc/source/devref/index.rst index 0ed2d3296f0..7ed6143c62c 100644 --- a/doc/source/devref/index.rst +++ b/doc/source/devref/index.rst @@ -53,6 +53,7 @@ Neutron Internals advanced_services oslo-incubator callbacks + dns_order Testing ------- diff --git a/neutron/db/db_base_plugin_common.py b/neutron/db/db_base_plugin_common.py index 54257ed971c..c68b25fbbff 100644 --- a/neutron/db/db_base_plugin_common.py +++ b/neutron/db/db_base_plugin_common.py @@ -172,7 +172,8 @@ class DbBasePluginCommon(common_db_mixin.CommonDbMixin): def _get_dns_by_subnet(self, context, subnet_id): dns_qry = context.session.query(models_v2.DNSNameServer) - return dns_qry.filter_by(subnet_id=subnet_id).all() + return dns_qry.filter_by(subnet_id=subnet_id).order_by( + models_v2.DNSNameServer.order).all() def _get_route_by_subnet(self, context, subnet_id): route_qry = context.session.query(models_v2.SubnetRoute) diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index ca69f460b78..b5034997f9f 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -138,22 +138,22 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): def _update_subnet_dns_nameservers(self, context, id, s): old_dns_list = self._get_dns_by_subnet(context, id) - new_dns_addr_set = set(s["dns_nameservers"]) - old_dns_addr_set = set([dns['address'] - for dns in old_dns_list]) + new_dns_addr_list = s["dns_nameservers"] - new_dns = list(new_dns_addr_set) - for dns_addr in old_dns_addr_set - new_dns_addr_set: - for dns in old_dns_list: - if dns['address'] == dns_addr: - context.session.delete(dns) - for dns_addr in new_dns_addr_set - old_dns_addr_set: + # NOTE(changzhi) delete all dns nameservers from db + # when update subnet's DNS nameservers. And store new + # nameservers with order one by one. + for dns in old_dns_list: + context.session.delete(dns) + + for order, server in enumerate(new_dns_addr_list): dns = models_v2.DNSNameServer( - address=dns_addr, + address=server, + order=order, subnet_id=id) context.session.add(dns) del s["dns_nameservers"] - return new_dns + return new_dns_addr_list def _update_subnet_allocation_pools(self, context, subnet_id, s): context.session.query(models_v2.IPAllocationPool).filter_by( @@ -424,11 +424,15 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): subnet = models_v2.Subnet(**subnet_args) context.session.add(subnet) + # NOTE(changzhi) Store DNS nameservers with order into DB one + # by one when create subnet with DNS nameservers if attributes.is_attr_set(dns_nameservers): - for addr in dns_nameservers: - ns = models_v2.DNSNameServer(address=addr, - subnet_id=subnet.id) - context.session.add(ns) + for order, server in enumerate(dns_nameservers): + dns = models_v2.DNSNameServer( + address=server, + order=order, + subnet_id=subnet.id) + context.session.add(dns) if attributes.is_attr_set(host_routes): for rt in host_routes: diff --git a/neutron/db/migration/alembic_migrations/versions/HEADS b/neutron/db/migration/alembic_migrations/versions/HEADS index 4d31e0ce6c7..9fef8352067 100644 --- a/neutron/db/migration/alembic_migrations/versions/HEADS +++ b/neutron/db/migration/alembic_migrations/versions/HEADS @@ -1,3 +1,3 @@ -2a16083502f3 +1c844d1677f7 45f955889773 kilo diff --git a/neutron/db/migration/alembic_migrations/versions/liberty/expand/1c844d1677f7_dns_nameservers_order.py b/neutron/db/migration/alembic_migrations/versions/liberty/expand/1c844d1677f7_dns_nameservers_order.py new file mode 100644 index 00000000000..baeafa3f3d7 --- /dev/null +++ b/neutron/db/migration/alembic_migrations/versions/liberty/expand/1c844d1677f7_dns_nameservers_order.py @@ -0,0 +1,35 @@ +# Copyright 2015 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""add order to dnsnameservers + +Revision ID: 1c844d1677f7 +Revises: 2a16083502f3 +Create Date: 2015-07-21 22:59:03.383850 + +""" + +# revision identifiers, used by Alembic. +revision = '1c844d1677f7' +down_revision = '2a16083502f3' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('dnsnameservers', + sa.Column('order', sa.Integer(), + server_default='0', nullable=False)) diff --git a/neutron/db/models_v2.py b/neutron/db/models_v2.py index 8ba70db7790..3e9f7c07434 100644 --- a/neutron/db/models_v2.py +++ b/neutron/db/models_v2.py @@ -178,6 +178,7 @@ class DNSNameServer(model_base.BASEV2): sa.ForeignKey('subnets.id', ondelete="CASCADE"), primary_key=True) + order = sa.Column(sa.Integer, nullable=False, server_default='0') class Subnet(model_base.BASEV2, HasId, HasTenant): @@ -201,6 +202,7 @@ class Subnet(model_base.BASEV2, HasId, HasTenant): dns_nameservers = orm.relationship(DNSNameServer, backref='subnet', cascade='all, delete, delete-orphan', + order_by=DNSNameServer.order, lazy='joined') routes = orm.relationship(SubnetRoute, backref='subnet', diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index 8c5a5d19a99..10063601f73 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -333,6 +333,17 @@ class FakeV4SubnetMultipleAgentsWithoutDnsProvided(object): host_routes = [] +class FakeV4SubnetAgentWithManyDnsProvided(object): + id = 'dddddddd-dddd-dddd-dddd-dddddddddddd' + ip_version = 4 + cidr = '192.168.0.0/24' + gateway_ip = '192.168.0.1' + enable_dhcp = True + dns_nameservers = ['2.2.2.2', '9.9.9.9', '1.1.1.1', + '3.3.3.3'] + host_routes = [] + + class FakeV4MultipleAgentsWithoutDnsProvided(object): id = 'ffffffff-ffff-ffff-ffff-ffffffffffff' subnets = [FakeV4SubnetMultipleAgentsWithoutDnsProvided()] @@ -341,6 +352,14 @@ class FakeV4MultipleAgentsWithoutDnsProvided(object): namespace = 'qdhcp-ns' +class FakeV4AgentWithManyDnsProvided(object): + id = 'ffffffff-ffff-ffff-ffff-ffffffffffff' + subnets = [FakeV4SubnetAgentWithManyDnsProvided()] + ports = [FakePort1(), FakePort2(), FakePort3(), FakeRouterPort(), + FakePortMultipleAgents1()] + namespace = 'qdhcp-ns' + + class FakeV4SubnetMultipleAgentsWithDnsProvided(object): id = 'dddddddd-dddd-dddd-dddd-dddddddddddd' ip_version = 4 @@ -1135,6 +1154,18 @@ class TestDnsmasq(TestBase): self._test_output_opts_file(expected, FakeV4MultipleAgentsWithoutDnsProvided()) + def test_output_opts_file_agent_with_many_dns_provided(self): + expected = ('tag:tag0,' + 'option:dns-server,2.2.2.2,9.9.9.9,1.1.1.1,3.3.3.3\n' + 'tag:tag0,option:classless-static-route,' + '169.254.169.254/32,192.168.0.1,0.0.0.0/0,192.168.0.1\n' + 'tag:tag0,249,169.254.169.254/32,192.168.0.1,0.0.0.0/0,' + '192.168.0.1\n' + 'tag:tag0,option:router,192.168.0.1').lstrip() + + self._test_output_opts_file(expected, + FakeV4AgentWithManyDnsProvided()) + def test_output_opts_file_multiple_agents_with_dns_provided(self): expected = ('tag:tag0,option:dns-server,8.8.8.8\n' 'tag:tag0,option:classless-static-route,' diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index 1a2a9bdcade..93e57a16b7b 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -3921,8 +3921,8 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): res = self.deserialize(self.fmt, req.get_response(self.api)) self.assertEqual(sorted(res['subnet']['host_routes']), sorted(host_routes)) - self.assertEqual(sorted(res['subnet']['dns_nameservers']), - sorted(dns_nameservers)) + self.assertEqual(res['subnet']['dns_nameservers'], + dns_nameservers) def test_update_subnet_shared_returns_400(self): with self.network(shared=True) as network: @@ -4463,6 +4463,27 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): self.assertEqual(res['subnet']['dns_nameservers'], data['subnet']['dns_nameservers']) + def test_subnet_lifecycle_dns_retains_order(self): + cfg.CONF.set_override('max_dns_nameservers', 3) + with self.subnet(dns_nameservers=['1.1.1.1', '2.2.2.2', + '3.3.3.3']) as subnet: + subnets = self._show('subnets', subnet['subnet']['id'], + expected_code=webob.exc.HTTPOk.code) + self.assertEqual(['1.1.1.1', '2.2.2.2', '3.3.3.3'], + subnets['subnet']['dns_nameservers']) + data = {'subnet': {'dns_nameservers': ['2.2.2.2', '3.3.3.3', + '1.1.1.1']}} + req = self.new_update_request('subnets', + data, + subnet['subnet']['id']) + res = self.deserialize(self.fmt, req.get_response(self.api)) + self.assertEqual(data['subnet']['dns_nameservers'], + res['subnet']['dns_nameservers']) + subnets = self._show('subnets', subnet['subnet']['id'], + expected_code=webob.exc.HTTPOk.code) + self.assertEqual(data['subnet']['dns_nameservers'], + subnets['subnet']['dns_nameservers']) + def test_update_subnet_dns_to_None(self): with self.subnet(dns_nameservers=['11.0.0.1']) as subnet: data = {'subnet': {'dns_nameservers': None}}