Fix hostname roaming for ml2 tunnel endpoints.

Change I75c6581fcc9f47a68bde29cbefcaa1a2a082344e introduced
a bug where host name changes broke tunneling endpoint updates.
Tunneling endpoint updates roaming a hostname from IP to IP
are a common method for active/passive HA with pacemaker and
should happen automatically without the need for API/CLI calls [1].

delete_endpoint_by_host_or_ip is introduced to allow cleanup of
endpoints that potentially belonged to the newly registered agent,
while preventing the race condition found when deleting ip1 & ip2
in the next situation at step 4:

1) we have hostA: ip1
2) hostA goes offline
3) hostB goes online, with ip1, and registers
4) hostA goes online, with ip2, and registers

[1] https://bugs.launchpad.net/python-neutronclient/+bug/1381664

Change-Id: I04d08d5b82ce9911f3af555b5776fc9823e0e5b6
Closes-Bug: #1464178

diff --git a/neutron/plugins/ml2/drivers/type_tunnel.py b/neutron/plugins/ml2/drivers/type_tunnel.py
index a8ac2fccf9029c39200a124e5a6e0c85345807dd..82c035ca9006e4ded2b6416638f7012a5b0d0a77 100644 (file)
--- a/neutron/plugins/ml2/drivers/type_tunnel.py
+++ b/neutron/plugins/ml2/drivers/type_tunnel.py
@@ -21,6 +21,7 @@ from oslo_db import api as oslo_db_api
 from oslo_db import exception as db_exc
 from oslo_log import log
 from six import moves
+from sqlalchemy import or_
 
 from neutron.common import exceptions as exc
 from neutron.common import topics
@@ -103,6 +104,17 @@ class TunnelTypeDriver(helpers.SegmentTypeDriver):
         param ip: the IP address of the endpoint
         """
 
+    @abc.abstractmethod
+    def delete_endpoint_by_host_or_ip(self, host, ip):
+        """Delete the endpoint in the type_driver database.
+
+        This function will delete any endpoint matching the specified
+        ip or host.
+
+        param host: the host name of the endpoint
+        param ip: the IP address of the endpoint
+        """
+
     def _initialize(self, raw_tunnel_ranges):
         self.tunnel_ranges = []
         self._parse_tunnel_ranges(raw_tunnel_ranges, self.tunnel_ranges)
@@ -266,6 +278,15 @@ class EndpointTunnelTypeDriver(TunnelTypeDriver):
             (session.query(self.endpoint_model).
              filter_by(ip_address=ip).delete())
 
+    def delete_endpoint_by_host_or_ip(self, host, ip):
+        LOG.debug("delete_endpoint_by_host_or_ip() called for "
+                  "host %(host)s or %(ip)s", {'host': host, 'ip': ip})
+        session = db_api.get_session()
+        with session.begin(subtransactions=True):
+            session.query(self.endpoint_model).filter(
+                    or_(self.endpoint_model.host == host,
+                        self.endpoint_model.ip_address == ip)).delete()
+
     def _get_endpoints(self):
         LOG.debug("_get_endpoints() called")
         session = db_api.get_session()
@@ -322,6 +343,13 @@ class TunnelRpcCallbackMixin(object):
             #    found, delete the endpoint belonging to that host and
             #    add endpoint with latest (tunnel_ip, host), it is a case
             #    where local_ip of an agent got changed.
+            # 5. If the passed host had another ip in the DB the host-id has
+            #    roamed to a different IP then delete any reference to the new
+            #    local_ip or the host id. Don't notify tunnel_delete for the
+            #    old IP since that one could have been taken by a different
+            #    agent host-id (neutron-ovs-cleanup should be used to clean up
+            #    the stale endpoints).
+            #    Finally create a new endpoint for the (tunnel_ip, host).
             if host:
                 host_endpoint = driver.obj.get_endpoint_by_host(host)
                 ip_endpoint = driver.obj.get_endpoint_by_ip(tunnel_ip)
@@ -330,10 +358,14 @@ class TunnelRpcCallbackMixin(object):
                     and host_endpoint is None):
                     driver.obj.delete_endpoint(ip_endpoint.ip_address)
                 elif (ip_endpoint and ip_endpoint.host != host):
-                    msg = (_("Tunnel IP %(ip)s in use with host %(host)s"),
-                           {'ip': ip_endpoint.ip_address,
-                            'host': ip_endpoint.host})
-                    raise exc.InvalidInput(error_message=msg)
+                    LOG.info(
+                        _LI("Tunnel IP %(ip)s was used by host %(host)s and "
+                            "will be assigned to %(new_host)s"),
+                        {'ip': ip_endpoint.ip_address,
+                         'host': ip_endpoint.host,
+                         'new_host': host})
+                    driver.obj.delete_endpoint_by_host_or_ip(
+                        host, ip_endpoint.ip_address)
                 elif (host_endpoint and host_endpoint.ip_address != tunnel_ip):
                     # Notify all other listening agents to delete stale tunnels
                     self._notifier.tunnel_delete(rpc_context,

diff --git a/neutron/tests/unit/plugins/ml2/drivers/base_type_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/base_type_tunnel.py
index 5bbb3ec38dc93831d6b28a8f8ba8300b648194c3..d5619fdcf4c4214667776a5b813db6d14cd4f7f6 100644 (file)
--- a/neutron/tests/unit/plugins/ml2/drivers/base_type_tunnel.py
+++ b/neutron/tests/unit/plugins/ml2/drivers/base_type_tunnel.py
@@ -369,20 +369,20 @@ class TunnelRpcCallbackTestMixin(object):
                   'host': HOST_ONE}
         self._test_tunnel_sync(kwargs, True)
 
-    def test_tunnel_sync_called_with_used_tunnel_ip_case_one(self):
+    def test_tunnel_sync_called_with_used_tunnel_ip_host_roaming(self):
         self.driver.add_endpoint(TUNNEL_IP_ONE, HOST_ONE)
 
         kwargs = {'tunnel_ip': TUNNEL_IP_ONE, 'tunnel_type': self.TYPE,
                   'host': HOST_TWO}
-        self._test_tunnel_sync_raises(kwargs)
+        self._test_tunnel_sync(kwargs, False)
 
-    def test_tunnel_sync_called_with_used_tunnel_ip_case_two(self):
+    def test_tunnel_sync_called_with_used_tunnel_ip_roaming_case_two(self):
         self.driver.add_endpoint(TUNNEL_IP_ONE, None)
         self.driver.add_endpoint(TUNNEL_IP_TWO, HOST_TWO)
 
         kwargs = {'tunnel_ip': TUNNEL_IP_ONE, 'tunnel_type': self.TYPE,
                   'host': HOST_TWO}
-        self._test_tunnel_sync_raises(kwargs)
+        self._test_tunnel_sync(kwargs, False)
 
     def test_tunnel_sync_called_without_tunnel_ip(self):
         kwargs = {'tunnel_type': self.TYPE, 'host': None}