Merge "Introduce status for floating IPs"

neutron/agent/l3_agent.py

@@ -37,6 +37,7 @@ from neutron.openstack.common import lockutils
 from neutron.openstack.common import log as logging
 from neutron.openstack.common import loopingcall
 from neutron.openstack.common import periodic_task
+from neutron.openstack.common import processutils
 from neutron.openstack.common.rpc import common as rpc_common
 from neutron.openstack.common.rpc import proxy
 from neutron.openstack.common import service
@@ -56,6 +57,7 @@ class L3PluginApi(proxy.RpcProxy):
 
     API version history:
         1.0 - Initial version.
+        1.1 - Floating IP operational status updates
 
     """
 
@@ -85,6 +87,15 @@ class L3PluginApi(proxy.RpcProxy):
                                        host=self.host),
                          topic=self.topic)
 
+    def update_floatingip_statuses(self, context, router_id, fip_statuses):
+        """Call the plugin update floating IPs's operational status."""
+        return self.call(context,
+                         self.make_msg('update_floatingip_statuses',
+                                       router_id=router_id,
+                                       fip_statuses=fip_statuses),
+                         topic=self.topic,
+                         version='1.1')
+
 
 class RouterInfo(object):
 
@@ -94,6 +105,7 @@ class RouterInfo(object):
         self._snat_enabled = None
         self._snat_action = None
         self.internal_ports = []
+        self.floating_ips = set()
         self.root_helper = root_helper
         self.use_namespaces = use_namespaces
         # Invoke the setter for establishing initial SNAT action
@@ -434,21 +446,41 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
             self.external_gateway_removed(ri, ri.ex_gw_port,
                                           interface_name, internal_cidrs)
 
+        # Process static routes for router
+        self.routes_updated(ri)
         # Process SNAT rules for external gateway
         ri.perform_snat_action(self._handle_router_snat_rules,
                                internal_cidrs, interface_name)
 
         # Process SNAT/DNAT rules for floating IPs
+        fip_statuses = {}
+        try:
             if ex_gw_port:
+                existing_floating_ips = ri.floating_ips
                 self.process_router_floating_ip_nat_rules(ri)
-
-        ri.ex_gw_port = ex_gw_port
-        self.routes_updated(ri)
                 ri.iptables_manager.defer_apply_off()
                 # Once NAT rules for floating IPs are safely in place
                 # configure their addresses on the external gateway port
+                fip_statuses = self.process_router_floating_ip_addresses(
+                    ri, ex_gw_port)
+        except Exception:
+            # TODO(salv-orlando): Less broad catching
+            # All floating IPs must be put in error state
+            for fip in ri.router.get(l3_constants.FLOATINGIP_KEY, []):
+                fip_statuses[fip] = l3_constants.FLOATINGIP_STATUS_ERROR
+
         if ex_gw_port:
-            self.process_router_floating_ip_addresses(ri, ex_gw_port)
+            # Identify floating IPs which were disabled
+            ri.floating_ips = set(fip_statuses.keys())
+            for fip_id in existing_floating_ips - ri.floating_ips:
+                fip_statuses[fip_id] = l3_constants.FLOATINGIP_STATUS_DOWN
+            # Update floating IP status on the neutron server
+            self.plugin_rpc.update_floatingip_statuses(
+                self.context, ri.router_id, fip_statuses)
+
+        # Update ex_gw_port and enable_snat on the router info cache
+        ri.ex_gw_port = ex_gw_port
+        ri.enable_snat = ri.router.get('enable_snat')
 
     def _handle_router_snat_rules(self, ri, ex_gw_port, internal_cidrs,
                                   interface_name, action):
@@ -497,6 +529,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
         Ensures addresses for existing floating IPs and cleans up
         those that should not longer be configured.
         """
+        fip_statuses = {}
         interface_name = self.get_external_device_name(ex_gw_port['id'])
         device = ip_lib.IPDevice(interface_name, self.root_helper,
                                  namespace=ri.ns_name())
@@ -512,14 +545,30 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
 
             if ip_cidr not in existing_cidrs:
                 net = netaddr.IPNetwork(ip_cidr)
+                try:
                     device.addr.add(net.version, ip_cidr, str(net.broadcast))
-                self._send_gratuitous_arp_packet(ri, interface_name, fip_ip)
+                except (processutils.UnknownArgumentError,
+                        processutils.ProcessExecutionError):
+                    # any exception occurred here should cause the floating IP
+                    # to be set in error state
+                    fip_statuses[fip['id']] = (
+                        l3_constants.FLOATINGIP_STATUS_ERROR)
+                    LOG.warn(_("Unable to configure IP address for "
+                               "floating IP: %s"), fip['id'])
+                    continue
+                # As GARP is processed in a distinct thread the call below
+                # won't raise an exception to be handled.
+                self._send_gratuitous_arp_packet(
+                    ri, interface_name, fip_ip)
+            fip_statuses[fip['id']] = (
+                l3_constants.FLOATINGIP_STATUS_ACTIVE)
 
         # Clean up addresses that no longer belong on the gateway interface.
         for ip_cidr in existing_cidrs - new_cidrs:
             if ip_cidr.endswith(FLOATING_IP_CIDR_SUFFIX):
                 net = netaddr.IPNetwork(ip_cidr)
                 device.addr.delete(net.version, ip_cidr)
+        return fip_statuses
 
     def _get_ex_gw_port(self, ri):
         return ri.router.get('gw_port')

neutron/common/constants.py

@@ -13,6 +13,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# TODO(salv-orlando): Verify if a single set of operational
+# status constants is achievable
 NET_STATUS_ACTIVE = 'ACTIVE'
 NET_STATUS_BUILD = 'BUILD'
 NET_STATUS_DOWN = 'DOWN'
@@ -23,6 +25,10 @@ PORT_STATUS_BUILD = 'BUILD'
 PORT_STATUS_DOWN = 'DOWN'
 PORT_STATUS_ERROR = 'ERROR'
 
+FLOATINGIP_STATUS_ACTIVE = 'ACTIVE'
+FLOATINGIP_STATUS_DOWN = 'DOWN'
+FLOATINGIP_STATUS_ERROR = 'ERROR'
+
 DEVICE_OWNER_ROUTER_INTF = "network:router_interface"
 DEVICE_OWNER_ROUTER_GW = "network:router_gateway"
 DEVICE_OWNER_FLOATINGIP = "network:floatingip"

neutron/db/l3_db.py

@@ -68,6 +68,11 @@ class FloatingIP(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant):
     fixed_port_id = sa.Column(sa.String(36), sa.ForeignKey('ports.id'))
     fixed_ip_address = sa.Column(sa.String(64))
     router_id = sa.Column(sa.String(36), sa.ForeignKey('routers.id'))
+    # Additional attribute for keeping track of the router where the floating
+    # ip was associated in order to be able to ensure consistency even if an
+    # aysnchronous backend is unavailable when the floating IP is disassociated
+    last_known_router_id = sa.Column(sa.String(36))
+    status = sa.Column(sa.String(16))
 
 
 class L3_NAT_db_mixin(l3.RouterPluginBase):
@@ -450,7 +455,8 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                'floating_network_id': floatingip['floating_network_id'],
                'router_id': floatingip['router_id'],
                'port_id': floatingip['fixed_port_id'],
-               'fixed_ip_address': floatingip['fixed_ip_address']}
+               'fixed_ip_address': floatingip['fixed_ip_address'],
+               'status': floatingip['status']}
         return self._fields(res, fields)
 
     def _get_router_for_floatingip(self, context, internal_port,
@@ -564,6 +570,7 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
         return (fip['port_id'], internal_ip_address, router_id)
 
     def _update_fip_assoc(self, context, fip, floatingip_db, external_port):
+        previous_router_id = floatingip_db.router_id
         port_id = internal_ip_address = router_id = None
         if (('fixed_ip_address' in fip and fip['fixed_ip_address']) and
             not ('port_id' in fip and fip['port_id'])):
@@ -590,9 +597,12 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                 pass
         floatingip_db.update({'fixed_ip_address': internal_ip_address,
                               'fixed_port_id': port_id,
-                              'router_id': router_id})
+                              'router_id': router_id,
+                              'last_known_router_id': previous_router_id})
 
-    def create_floatingip(self, context, floatingip):
+    def create_floatingip(
+        self, context, floatingip,
+        initial_status=l3_constants.FLOATINGIP_STATUS_ACTIVE):
         fip = floatingip['floatingip']
         tenant_id = self._get_tenant_id_for_create(context, fip)
         fip_id = uuidutils.generate_uuid()
@@ -625,6 +635,7 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
             floatingip_db = FloatingIP(
                 id=fip_id,
                 tenant_id=tenant_id,
+                status=initial_status,
                 floating_network_id=fip['floating_network_id'],
                 floating_ip_address=floating_ip_address,
                 floating_port_id=external_port['id'])
@@ -664,6 +675,13 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                 context, router_ids, 'update_floatingip')
         return self._make_floatingip_dict(floatingip_db)
 
+    def update_floatingip_status(self, context, floatingip_id, status):
+        """Update operational status for floating IP in neutron DB."""
+        # TODO(salv-orlando): Optimize avoiding fetch before update
+        with context.session.begin(subtransactions=True):
+            floatingip_db = self._get_floatingip(context, floatingip_id)
+            floatingip_db['status'] = status
+
     def delete_floatingip(self, context, id):
         floatingip = self._get_floatingip(context, id)
         router_id = floatingip['router_id']

neutron/db/l3_rpc_base.py

@@ -94,3 +94,30 @@ class L3RpcCallbackMixin(object):
         LOG.debug(_("External network ID returned to l3 agent: %s"),
                   net_id)
         return net_id
+
+    def update_floatingip_statuses(self, context, router_id, fip_statuses):
+        """Update operational status for a floating IP."""
+        l3_plugin = manager.NeutronManager.get_service_plugins()[
+            plugin_constants.L3_ROUTER_NAT]
+        with context.session.begin(subtransactions=True):
+            for (floatingip_id, status) in fip_statuses.iteritems():
+                LOG.debug(_("New status for floating IP %(floatingip_id)s: "
+                            "%(status)s"), {'floatingip_id': floatingip_id,
+                                            'status': status})
+                l3_plugin.update_floatingip_status(context,
+                                                   floatingip_id,
+                                                   status)
+            # Find all floating IPs known to have been the given router
+            # for which an update was not received. Set them DOWN mercilessly
+            # This situation might occur for some asynchronous backends if
+            # notifications were missed
+            known_router_fips = l3_plugin.get_floatingips(
+                context, {'last_known_router_id': [router_id]})
+            # Consider only floating ips which were disassociated in the API
+            # FIXME(salv-orlando): Filtering in code should be avoided.
+            # the plugin should offer a way to specify a null filter
+            fips_to_disable = (fip['id'] for fip in known_router_fips
+                               if not fip['router_id'])
+            for fip_id in fips_to_disable:
+                l3_plugin.update_floatingip_status(
+                    context, fip_id, constants.FLOATINGIP_STATUS_DOWN)

neutron/db/migration/alembic_migrations/versions/2eeaf963a447_floatingip_status.py

@@ -0,0 +1,59 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+#
+# Copyright 2014 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.
+#
+
+"""floatingip_status
+
+Revision ID: 2eeaf963a447
+Revises: f44ab9871cd6
+Create Date: 2014-01-14 11:58:13.754747
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '2eeaf963a447'
+down_revision = 'f44ab9871cd6'
+
+# Change to ['*'] if this migration applies to all plugins
+
+migration_for_plugins = [
+    '*'
+]
+
+from alembic import op
+import sqlalchemy as sa
+
+from neutron.db import migration
+
+
+def upgrade(active_plugins=None, options=None):
+    if not migration.should_run(active_plugins, migration_for_plugins):
+        return
+    op.add_column('floatingips',
+                  sa.Column('last_known_router_id',
+                            sa.String(length=36),
+                            nullable=True))
+    op.add_column('floatingips',
+                  sa.Column('status',
+                            sa.String(length=16),
+                            nullable=True))
+
+
+def downgrade(active_plugins=None, options=None):
+    if not migration.should_run(active_plugins, migration_for_plugins):
+        return
+    op.drop_column('floatingips', 'last_known_router_id')
+    op.drop_column('floatingips', 'status')

neutron/extensions/l3.py

@@ -125,7 +125,9 @@ RESOURCE_ATTRIBUTE_MAP = {
         'tenant_id': {'allow_post': True, 'allow_put': False,
                       'required_by_policy': True,
                       'validate': {'type:string': None},
-                      'is_visible': True}
+                      'is_visible': True},
+        'status': {'allow_post': False, 'allow_put': False,
+                   'is_visible': True},
     },
 }
 

neutron/services/l3_router/l3_router_plugin.py

@@ -37,8 +37,7 @@ from neutron.plugins.common import constants
 
 class L3RouterPluginRpcCallbacks(l3_rpc_base.L3RpcCallbackMixin):
 
-    # Set RPC API version to 1.0 by default.
-    RPC_API_VERSION = '1.0'
+    RPC_API_VERSION = '1.1'
 
     def create_rpc_dispatcher(self):
         """Get the rpc dispatcher for this manager.
@@ -91,3 +90,18 @@ class L3RouterPlugin(db_base_plugin_v2.CommonDbMixin,
         return ("L3 Router Service Plugin for basic L3 forwarding"
                 " between (L2) Neutron networks and access to external"
                 " networks via a NAT gateway.")
+
+    def create_floatingip(self, context, floatingip):
+        """Create floating IP.
+
+        :param context: Neutron request context
+        :param floatingip: data fo the floating IP being created
+        :returns: A floating IP object on success
+
+        AS the l3 router plugin aysnchrounously creates floating IPs
+        leveraging tehe l3 agent, the initial status fro the floating
+        IP object will be DOWN.
+        """
+        return super(L3RouterPlugin, self).create_floatingip(
+            context, floatingip,
+            initial_status=q_const.FLOATINGIP_STATUS_DOWN)

neutron/tests/unit/test_l3_agent.py

@@ -25,6 +25,7 @@ from neutron.agent.linux import interface
 from neutron.common import config as base_config
 from neutron.common import constants as l3_constants
 from neutron.common import exceptions as n_exc
+from neutron.openstack.common import processutils
 from neutron.openstack.common import uuidutils
 from neutron.tests import base
 
@@ -382,11 +383,14 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
     def test_process_router(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        fake_fip_id = 'fake_fip_id'
         agent.process_router_floating_ip_addresses = mock.Mock()
         agent.process_router_floating_ip_nat_rules = mock.Mock()
+        agent.process_router_floating_ip_addresses.return_value = {
+            fake_fip_id: 'ACTIVE'}
         router = self._prepare_router_data()
         fake_floatingips1 = {'floatingips': [
-            {'id': _uuid(),
+            {'id': fake_fip_id,
              'floating_ip_address': '8.8.8.8',
              'fixed_ip_address': '7.7.7.7',
              'port_id': _uuid()}]}
@@ -433,8 +437,9 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
     @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
     def test_process_router_floating_ip_addresses_add(self, IPDevice):
+        fip_id = _uuid()
         fip = {
-            'id': _uuid(), 'port_id': _uuid(),
+            'id': fip_id, 'port_id': _uuid(),
             'floating_ip_address': '15.1.2.3',
             'fixed_ip_address': '192.168.0.1'
         }
@@ -447,8 +452,10 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
 
-        agent.process_router_floating_ip_addresses(ri, {'id': _uuid()})
-
+        fip_statuses = agent.process_router_floating_ip_addresses(
+            ri, {'id': _uuid()})
+        self.assertEqual({fip_id: l3_constants.FLOATINGIP_STATUS_ACTIVE},
+                         fip_statuses)
         device.addr.add.assert_called_once_with(4, '15.1.2.3/32', '15.1.2.3')
 
     def test_process_router_floating_ip_nat_rules_add(self):
@@ -481,8 +488,9 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
 
-        agent.process_router_floating_ip_addresses(ri, {'id': _uuid()})
-
+        fip_statuses = agent.process_router_floating_ip_addresses(
+            ri, {'id': _uuid()})
+        self.assertEqual({}, fip_statuses)
         device.addr.delete.assert_called_once_with(4, '15.1.2.3/32')
 
     def test_process_router_floating_ip_nat_rules_remove(self):
@@ -494,13 +502,14 @@ class TestBasicRouterOperations(base.BaseTestCase):
         agent.process_router_floating_ip_nat_rules(ri)
 
         nat = ri.iptables_manager.ipv4['nat']
-        nat = ri.iptables_manager.ipv4['nat']
+        nat = ri.iptables_manager.ipv4['nat`']
         nat.clear_rules_by_tag.assert_called_once_with('floating_ip')
 
     @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
     def test_process_router_floating_ip_addresses_remap(self, IPDevice):
+        fip_id = _uuid()
         fip = {
-            'id': _uuid(), 'port_id': _uuid(),
+            'id': fip_id, 'port_id': _uuid(),
             'floating_ip_address': '15.1.2.3',
             'fixed_ip_address': '192.168.0.2'
         }
@@ -513,31 +522,55 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
 
-        agent.process_router_floating_ip_addresses(ri, {'id': _uuid()})
+        fip_statuses = agent.process_router_floating_ip_addresses(
+            ri, {'id': _uuid()})
+        self.assertEqual({fip_id: l3_constants.FLOATINGIP_STATUS_ACTIVE},
+                         fip_statuses)
 
         self.assertFalse(device.addr.add.called)
         self.assertFalse(device.addr.delete.called)
 
-    def test_process_router_floating_ip_nat_rules_remap(self):
+    @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
+    def test_process_router_with_disabled_floating_ip(self, IPDevice):
+        fip_id = _uuid()
         fip = {
-            'id': _uuid(), 'port_id': _uuid(),
+            'id': fip_id, 'port_id': _uuid(),
             'floating_ip_address': '15.1.2.3',
             'fixed_ip_address': '192.168.0.2'
         }
 
         ri = mock.MagicMock()
+        ri.floating_ips = [fip]
+        ri.router.get.return_value = []
 
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+
+        fip_statuses = agent.process_router_floating_ip_addresses(
+            ri, {'id': _uuid()})
+
+        self.assertIsNone(fip_statuses.get(fip_id))
+
+    @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
+    def test_process_router_floating_ip_with_device_add_error(self, IPDevice):
+        IPDevice.return_value = device = mock.Mock()
+        device.addr.add.side_effect = processutils.ProcessExecutionError
+        device.addr.list.return_value = []
+        fip_id = _uuid()
+        fip = {
+            'id': fip_id, 'port_id': _uuid(),
+            'floating_ip_address': '15.1.2.3',
+            'fixed_ip_address': '192.168.0.2'
+        }
+        ri = mock.MagicMock()
         ri.router.get.return_value = [fip]
 
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
 
-        agent.process_router_floating_ip_nat_rules(ri)
+        fip_statuses = agent.process_router_floating_ip_addresses(
+            ri, {'id': _uuid()})
 
-        nat = ri.iptables_manager.ipv4['nat']
-        nat.clear_rules_by_tag.assert_called_once_with('floating_ip')
-        rules = agent.floating_forward_rules('15.1.2.3', '192.168.0.2')
-        for chain, rule in rules:
-            nat.add_rule.assert_any_call(chain, rule, tag='floating_ip')
+        self.assertEqual({fip_id: l3_constants.FLOATINGIP_STATUS_ERROR},
+                         fip_statuses)
 
     def test_process_router_snat_disabled(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
@@ -686,6 +719,36 @@ class TestBasicRouterOperations(base.BaseTestCase):
             self.assertNotIn(
                 router[l3_constants.INTERFACE_KEY][0], ri.internal_ports)
 
+    def test_process_router_floatingip_disabled(self):
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        with mock.patch.object(
+            agent.plugin_rpc,
+            'update_floatingip_statuses') as mock_update_fip_status:
+            fip_id = _uuid()
+            router = self._prepare_router_data(num_internal_ports=1)
+            router[l3_constants.FLOATINGIP_KEY] = [
+                {'id': fip_id,
+                 'floating_ip_address': '8.8.8.8',
+                 'fixed_ip_address': '7.7.7.7',
+                 'port_id': router[l3_constants.INTERFACE_KEY][0]['id']}]
+
+            ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
+                                     self.conf.use_namespaces, router=router)
+            agent.process_router(ri)
+            # Assess the call for putting the floating IP up was performed
+            mock_update_fip_status.assert_called_once_with(
+                mock.ANY, ri.router_id,
+                {fip_id: l3_constants.FLOATINGIP_STATUS_ACTIVE})
+            mock_update_fip_status.reset_mock()
+            # Process the router again, this time without floating IPs
+            router[l3_constants.FLOATINGIP_KEY] = []
+            ri.router = router
+            agent.process_router(ri)
+            # Assess the call for putting the floating IP up was performed
+            mock_update_fip_status.assert_called_once_with(
+                mock.ANY, ri.router_id,
+                {fip_id: l3_constants.FLOATINGIP_STATUS_DOWN})
+
     def test_handle_router_snat_rules_add_back_jump(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         ri = mock.MagicMock()

neutron/tests/unit/test_l3_plugin.py

@@ -1195,7 +1195,8 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                                                   private_sub['subnet']['id'],
                                                   None)
 
-    def test_floatingip_update(self):
+    def test_floatingip_update(
+        self, expected_status=l3_constants.FLOATINGIP_STATUS_ACTIVE):
         with self.port() as p:
             private_sub = {'subnet': {'id':
                                       p['port']['fixed_ips'][0]['subnet_id']}}
@@ -1203,6 +1204,7 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                 body = self._show('floatingips', fip['floatingip']['id'])
                 self.assertIsNone(body['floatingip']['port_id'])
                 self.assertIsNone(body['floatingip']['fixed_ip_address'])
+                self.assertEqual(body['floatingip']['status'], expected_status)
 
                 port_id = p['port']['id']
                 ip_address = p['port']['fixed_ips'][0]['ip_address']