From: changzhi Date: Thu, 16 Jul 2015 02:14:16 +0000 (+0800) Subject: Keep dns nameserver order consistency X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=cb60d0bb4e0cc0cba68f59fdf5f4e89d6ec52950;p=openstack-build%2Fneutron-build.git 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 --- diff --git a/doc/source/devref/dns_order.rst b/doc/source/devref/dns_order.rst new file mode 100644 index 000000000..bb8397081 --- /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 0ed2d3296..7ed6143c6 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 54257ed97..c68b25fbb 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 ca69f460b..b5034997f 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 = 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: + new_dns_addr_list = s["dns_nameservers"] + + # 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 4d31e0ce6..9fef83520 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 000000000..baeafa3f3 --- /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 8ba70db77..3e9f7c074 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 8c5a5d19a..10063601f 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 1a2a9bdca..93e57a16b 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}}