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

@@ -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
 

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)

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')