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

diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py
index 7c67f18d6d4efbd227996d84bc5117abe57b5b26..bfa586e092e2a8b35ff512061ef7130cb8838c4e 100644 (file)
--- 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 (file)
index 0000000..4ee433f
--- /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 fb7355f149ed253304811d5b6a13f44fe63468a7..1f2c2603cd251bf8cf7a7049746fe9e0f6b6771c 100644 (file)
--- 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')