From 6ac43c119bff9da93f438d10033b209819a32230 Mon Sep 17 00:00:00 2001 From: Eugene Nikanorov Date: Sun, 25 Oct 2015 15:47:38 +0400 Subject: [PATCH] Cleanup dhcp namespace upon dhcp setup. In some cases when more than 1 DHCP agents were assigned to a network and then they became dead, their DHCP ports become reserved. Later, when those agents revive or start again, they acquire reserved ports, but it's not guaranteed that they get exactly same ports. In such case DHCP agent may create interface in the namespaces despite that another interface already exist. In such case there will be two hosts with dhcp namespaces each containing duplicate ports, e.g. one port will be present on two hosts. This breaks DHCP. Closes-Bug: #1509959 Change-Id: I34eb1ad5c44dd3528c9910462e26536186e7a4fb --- neutron/agent/linux/dhcp.py | 16 ++++ .../tests/functional/agent/linux/test_dhcp.py | 88 +++++++++++++++++++ neutron/tests/unit/agent/dhcp/test_agent.py | 9 ++ 3 files changed, 113 insertions(+) create mode 100644 neutron/tests/functional/agent/linux/test_dhcp.py diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 7c67f18d6..bfa586e09 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -1165,6 +1165,16 @@ class DeviceManager(object): else: network.ports.append(port) + def _cleanup_stale_devices(self, network, dhcp_port): + LOG.debug("Cleaning stale devices for network %s", network.id) + dev_name = self.driver.get_device_name(dhcp_port) + ns_ip = ip_lib.IPWrapper(namespace=network.namespace) + for d in ns_ip.get_devices(exclude_loopback=True): + # delete all devices except current active DHCP port device + if d.name != dev_name: + LOG.debug("Found stale device %s, deleting", d.name) + self.driver.unplug(d.name, namespace=network.namespace) + def setup(self, network): """Create and initialize a device for network's DHCP on this host.""" port = self.setup_dhcp_port(network) @@ -1215,6 +1225,12 @@ class DeviceManager(object): if self.conf.use_namespaces: self._set_default_route(network, interface_name) + try: + self._cleanup_stale_devices(network, port) + except Exception: + # catch everything as we don't want to fail because of + # cleanup step + LOG.error(_LE("Exception during stale dhcp device cleanup")) return interface_name diff --git a/neutron/tests/functional/agent/linux/test_dhcp.py b/neutron/tests/functional/agent/linux/test_dhcp.py new file mode 100644 index 000000000..4ee433fa6 --- /dev/null +++ b/neutron/tests/functional/agent/linux/test_dhcp.py @@ -0,0 +1,88 @@ +# Copyright (c) 2015 Mirantis, Inc. +# +# 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. + +import mock + +from oslo_config import cfg + +from neutron.agent.common import config +from neutron.agent.dhcp import config as dhcp_conf +from neutron.agent.linux import dhcp +from neutron.agent.linux import interface +from neutron.agent.linux import ip_lib +from neutron.common import config as common_conf +from neutron.tests import base as tests_base +from neutron.tests.common import net_helpers +from neutron.tests.functional import base as functional_base + + +class TestDhcp(functional_base.BaseSudoTestCase): + def setUp(self): + super(TestDhcp, self).setUp() + conf = cfg.ConfigOpts() + conf.register_opts(config.INTERFACE_DRIVER_OPTS) + conf.register_opts(config.USE_NAMESPACES_OPTS) + conf.register_opts(interface.OPTS) + conf.register_opts(common_conf.core_opts) + conf.register_opts(dhcp_conf.DHCP_AGENT_OPTS) + conf.set_override('interface_driver', 'openvswitch') + conf.set_override('host', 'foo_host') + conf.set_override('use_namespaces', True) + self.conf = conf + br_int = self.useFixture(net_helpers.OVSBridgeFixture()).bridge + self.conf.set_override('ovs_integration_bridge', br_int.br_name) + + def test_cleanup_stale_devices(self): + plugin = mock.MagicMock() + dev_mgr = dhcp.DeviceManager(self.conf, plugin) + network = { + 'id': 'foo_id', + 'tenant_id': 'foo_tenant', + 'namespace': 'qdhcp-foo_id', + 'ports': [], + 'subnets': [tests_base.AttributeDict({'id': 'subnet_foo_id', + 'enable_dhcp': True, + 'ipv6_address_mode': None, + 'ipv6_ra_mode': None, + 'cidr': '10.0.0.0/24', + 'ip_version': 4, + 'gateway_ip': '10.0.0.1'})]} + dhcp_port = { + 'id': 'foo_port_id', + 'mac_address': '10:22:33:44:55:67', + 'fixed_ips': [tests_base.AttributeDict( + {'subnet_id': 'subnet_foo_id', 'ip_address': '10.0.0.1'})] + } + plugin.create_dhcp_port.return_value = tests_base.AttributeDict( + dhcp_port) + dev_mgr.driver.plug("foo_id", + "foo_id2", + "tapfoo_id2", + "10:22:33:44:55:68", + namespace="qdhcp-foo_id") + dev_mgr.driver.plug("foo_id", + "foo_id3", + "tapfoo_id3", + "10:22:33:44:55:69", + namespace="qdhcp-foo_id") + ipw = ip_lib.IPWrapper(namespace="qdhcp-foo_id") + devices = ipw.get_devices(exclude_loopback=True) + self.addCleanup(ipw.netns.delete, 'qdhcp-foo_id') + self.assertEqual(2, len(devices)) + # setting up dhcp for the network + dev_mgr.setup(tests_base.AttributeDict(network)) + devices = ipw.get_devices(exclude_loopback=True) + # only one non-loopback device should remain + self.assertEqual(1, len(devices)) + self.assertEqual("tapfoo_port_id", devices[0].name) diff --git a/neutron/tests/unit/agent/dhcp/test_agent.py b/neutron/tests/unit/agent/dhcp/test_agent.py index fb7355f14..1f2c2603c 100644 --- a/neutron/tests/unit/agent/dhcp/test_agent.py +++ b/neutron/tests/unit/agent/dhcp/test_agent.py @@ -230,6 +230,10 @@ class TestDhcpAgent(base.BaseTestCase): self.mock_makedirs_p = mock.patch("os.makedirs") self.mock_makedirs = self.mock_makedirs_p.start() + self.mock_ip_wrapper_p = mock.patch("neutron.agent.linux.ip_lib." + "IPWrapper") + self.mock_ip_wrapper = self.mock_ip_wrapper_p.start() + def test_init_host(self): dhcp = dhcp_agent.DhcpAgent(HOSTNAME) with mock.patch.object(dhcp, 'sync_state') as sync_state: @@ -1217,6 +1221,10 @@ class TestDeviceManager(base.BaseTestCase): self.mangle_inst = mock.Mock() self.iptables_inst.ipv4 = {'mangle': self.mangle_inst} + self.mock_ip_wrapper_p = mock.patch("neutron.agent.linux.ip_lib." + "IPWrapper") + self.mock_ip_wrapper = self.mock_ip_wrapper_p.start() + def _test_setup_helper(self, device_is_ready, net=None, port=None): net = net or fake_network port = port or fake_port1 @@ -1227,6 +1235,7 @@ class TestDeviceManager(base.BaseTestCase): dh = dhcp.DeviceManager(cfg.CONF, plugin) dh._set_default_route = mock.Mock() + dh._cleanup_stale_devices = mock.Mock() interface_name = dh.setup(net) self.assertEqual(interface_name, 'tap12345678-12') -- 2.45.2