Add lease expiration management to ip recycling

Fixes bug 1022804

This is the 3rd and final patch for this bug.  This patch alters ip allocation
recycling to honor lease expiration. Allocations that are in the
expiration wait state have null port_ids.

Change-Id: Ib7960b142eb15733c6418b01973d02a827634cb6

quantum/db/db_base_plugin_v2.py

@@ -191,7 +191,49 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
         return False
 
     @staticmethod
-    def _recycle_ip(context, network_id, subnet_id, port_id, ip_address):
+    def _hold_ip(context, network_id, subnet_id, port_id, ip_address):
+        alloc_qry = context.session.query(models_v2.IPAllocation)
+        allocated = alloc_qry.filter_by(network_id=network_id,
+                                        port_id=port_id,
+                                        ip_address=ip_address,
+                                        subnet_id=subnet_id).one()
+
+        if not allocated:
+            return
+        if allocated.expiration < timeutils.utcnow():
+            # immediately delete expired allocations
+            QuantumDbPluginV2._recycle_ip(
+                context, network_id, subnet_id, ip_address)
+        else:
+            LOG.debug("Hold allocated IP %s (%s/%s/%s)", ip_address,
+                      network_id, subnet_id, port_id)
+            allocated.port_id = None
+
+    @staticmethod
+    def _recycle_expired_ip_allocations(context, network_id):
+        """Return held ip allocations with expired leases back to the pool."""
+        if network_id in getattr(context, '_recycled_networks', set()):
+            return
+
+        expired_qry = context.session.query(models_v2.IPAllocation)
+        expired_qry = expired_qry.filter_by(network_id=network_id,
+                                            port_id=None)
+        expired_qry = expired_qry.filter(
+            models_v2.IPAllocation.expiration <= timeutils.utcnow())
+
+        for expired in expired_qry.all():
+            QuantumDbPluginV2._recycle_ip(context,
+                                          network_id,
+                                          expired['subnet_id'],
+                                          expired['ip_address'])
+
+        if hasattr(context, '_recycled_networks'):
+            context._recycled_networks.add(network_id)
+        else:
+            context._recycled_networks = set([network_id])
+
+    @staticmethod
+    def _recycle_ip(context, network_id, subnet_id, ip_address):
         """Return an IP address to the pool of free IP's on the network
         subnet.
         """
@@ -266,7 +308,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
             context.session.add(ip_range)
             LOG.debug("Recycle: created new %s-%s", ip_address, ip_address)
         QuantumDbPluginV2._delete_ip_allocation(context, network_id, subnet_id,
-                                                port_id, ip_address)
+                                                ip_address)
 
     @staticmethod
     def _default_allocation_expiration():
@@ -289,15 +331,13 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
                       "ip address %s.", network_id, ip_address)
 
     @staticmethod
-    def _delete_ip_allocation(context, network_id, subnet_id, port_id,
+    def _delete_ip_allocation(context, network_id, subnet_id, ip_address):
-                              ip_address):
 
         # Delete the IP address from the IPAllocate table
-        LOG.debug("Delete allocated IP %s (%s/%s/%s)", ip_address,
+        LOG.debug("Delete allocated IP %s (%s/%s)", ip_address,
-                  network_id, subnet_id, port_id)
+                  network_id, subnet_id)
         alloc_qry = context.session.query(models_v2.IPAllocation)
         allocated = alloc_qry.filter_by(network_id=network_id,
-                                        port_id=port_id,
                                         ip_address=ip_address,
                                         subnet_id=subnet_id).delete()
 
@@ -474,6 +514,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
                              new_ips):
         """Add or remove IPs from the port."""
         ips = []
+
         # Remove all of the intersecting elements
         for original_ip in original_ips[:]:
             for new_ip in new_ips[:]:
@@ -487,12 +528,12 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
         # Check if the IP's to add are OK
         to_add = self._test_fixed_ips_for_port(context, network_id, new_ips)
         for ip in original_ips:
-            LOG.debug("Port update. Deleting %s", ip)
+            LOG.debug("Port update. Hold %s", ip)
-            QuantumDbPluginV2._recycle_ip(context,
+            QuantumDbPluginV2._hold_ip(context,
-                                          network_id=network_id,
+                                       network_id,
-                                          subnet_id=ip['subnet_id'],
+                                       ip['subnet_id'],
-                                          ip_address=ip['ip_address'],
+                                       port_id,
-                                          port_id=port_id)
+                                       ip['ip_address'])
 
         if to_add:
             LOG.debug("Port update. Adding %s", to_add)
@@ -968,7 +1009,8 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
             allocated_qry = allocated_qry.options(orm.joinedload('ports'))
             allocated = allocated_qry.filter_by(subnet_id=id).all()
 
-            only_svc = all(a.ports.device_owner.startswith(AGENT_OWNER_PREFIX)
+            only_svc = all(not a.port_id or
+                           a.ports.device_owner.startswith(AGENT_OWNER_PREFIX)
                            for a in allocated)
             if not only_svc:
                 raise q_exc.NetworkInUse(subnet_id=id)
@@ -998,6 +1040,7 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
         tenant_id = self._get_tenant_id_for_create(context, p)
 
         with context.session.begin(subtransactions=True):
+            self._recycle_expired_ip_allocations(context, p['network_id'])
             network = self._get_network(context, p["network_id"])
 
             # Ensure that a MAC address is defined and it is unique on the
@@ -1051,6 +1094,8 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
             port = self._get_port(context, id)
             # Check if the IPs need to be updated
             if 'fixed_ips' in p:
+                self._recycle_expired_ip_allocations(context,
+                                                     port['network_id'])
                 original = self._make_port_dict(port)
                 ips = self._update_ips_for_port(context,
                                                 port["network_id"],
@@ -1091,18 +1136,17 @@ class QuantumDbPluginV2(quantum_plugin_base_v2.QuantumPluginBaseV2):
                     # Gateway address will not be recycled, but we do
                     # need to delete the allocation from the DB
                     QuantumDbPluginV2._delete_ip_allocation(
-                        context,
+                        context, a['network_id'],
-                        a['network_id'], a['subnet_id'],
+                        a['subnet_id'], a['ip_address'])
-                        id, a['ip_address'])
                     LOG.debug("Gateway address (%s/%s) is not recycled",
                               a['ip_address'], a['subnet_id'])
                     continue
 
-                QuantumDbPluginV2._recycle_ip(context,
+                QuantumDbPluginV2._hold_ip(context,
-                                              network_id=a['network_id'],
+                                           a['network_id'],
-                                              subnet_id=a['subnet_id'],
+                                           a['subnet_id'],
-                                              ip_address=a['ip_address'],
+                                           id,
-                                              port_id=id)
+                                           a['ip_address'])
         context.session.delete(port)
 
     def get_port(self, context, id, fields=None):

quantum/db/models_v2.py

@@ -76,7 +76,7 @@ class IPAllocation(model_base.BASEV2):
     """
     port_id = sa.Column(sa.String(36), sa.ForeignKey('ports.id',
                                                      ondelete="CASCADE"),
-                        nullable=False, primary_key=True)
+                        nullable=True)
     ip_address = sa.Column(sa.String(64), nullable=False, primary_key=True)
     subnet_id = sa.Column(sa.String(36), sa.ForeignKey('subnets.id',
                                                        ondelete="CASCADE"),

quantum/plugins/cisco/network_plugin.py

@@ -256,8 +256,8 @@ class PluginV2(db_base_plugin_v2.QuantumDbPluginV2):
             allocated = allocated_qry.filter_by(subnet_id=id).all()
 
             prefix = db_base_plugin_v2.AGENT_OWNER_PREFIX
-            if not all(a.ports.device_owner.startswith(prefix) for a in
+            if not all(not a.port_id or a.ports.device_owner.startswith(prefix)
-                       allocated):
+                       for a in allocated):
                 raise exc.SubnetInUse(subnet_id=id)
         context.session.close()
         try:

quantum/tests/unit/test_db_plugin.py

@@ -856,7 +856,7 @@ class TestPortsV2(QuantumDbPluginV2TestCase):
                                  data['port']['admin_state_up'])
                 ips = res['port']['fixed_ips']
                 self.assertEquals(len(ips), 1)
-                self.assertEquals(ips[0]['ip_address'], '10.0.0.2')
+                self.assertEquals(ips[0]['ip_address'], '10.0.0.3')
                 self.assertEquals(ips[0]['subnet_id'], subnet['subnet']['id'])
 
     def test_update_port_add_additional_ip(self):
@@ -875,9 +875,9 @@ class TestPortsV2(QuantumDbPluginV2TestCase):
                                  data['port']['admin_state_up'])
                 ips = res['port']['fixed_ips']
                 self.assertEquals(len(ips), 2)
-                self.assertEquals(ips[0]['ip_address'], '10.0.0.2')
+                self.assertEquals(ips[0]['ip_address'], '10.0.0.3')
                 self.assertEquals(ips[0]['subnet_id'], subnet['subnet']['id'])
-                self.assertEquals(ips[1]['ip_address'], '10.0.0.3')
+                self.assertEquals(ips[1]['ip_address'], '10.0.0.4')
                 self.assertEquals(ips[1]['subnet_id'], subnet['subnet']['id'])
 
     def test_requested_duplicate_mac(self):
@@ -1192,30 +1192,38 @@ class TestPortsV2(QuantumDbPluginV2TestCase):
                     self._delete('ports', p['port']['id'])
 
     def test_recycling(self):
+        # set expirations to past so that recycling is checked
+        reference = datetime.datetime(2012, 8, 13, 23, 11, 0)
+        cfg.CONF.set_override('dhcp_lease_duration', 0)
         fmt = 'json'
+
         with self.subnet(cidr='10.0.1.0/24') as subnet:
             with self.port(subnet=subnet) as port:
-                ips = port['port']['fixed_ips']
+                with mock.patch.object(timeutils, 'utcnow') as mock_utcnow:
-                self.assertEquals(len(ips), 1)
+                    mock_utcnow.return_value = reference
-                self.assertEquals(ips[0]['ip_address'], '10.0.1.2')
+                    ips = port['port']['fixed_ips']
-                self.assertEquals(ips[0]['subnet_id'], subnet['subnet']['id'])
+                    self.assertEquals(len(ips), 1)
-                net_id = port['port']['network_id']
+                    self.assertEquals(ips[0]['ip_address'], '10.0.1.2')
-                ports = []
+                    self.assertEquals(ips[0]['subnet_id'],
-                for i in range(16 - 3):
+                                      subnet['subnet']['id'])
+                    net_id = port['port']['network_id']
+                    ports = []
+                    for i in range(16 - 3):
+                        res = self._create_port(fmt, net_id=net_id)
+                        p = self.deserialize(fmt, res)
+                        ports.append(p)
+                    for i in range(16 - 3):
+                        x = random.randrange(0, len(ports), 1)
+                        p = ports.pop(x)
+                        self._delete('ports', p['port']['id'])
                     res = self._create_port(fmt, net_id=net_id)
-                    p = self.deserialize(fmt, res)
+                    port = self.deserialize(fmt, res)
-                    ports.append(p)
+                    ips = port['port']['fixed_ips']
-                for i in range(16 - 3):
+                    self.assertEquals(len(ips), 1)
-                    x = random.randrange(0, len(ports), 1)
+                    self.assertEquals(ips[0]['ip_address'], '10.0.1.3')
-                    p = ports.pop(x)
+                    self.assertEquals(ips[0]['subnet_id'],
-                    self._delete('ports', p['port']['id'])
+                                      subnet['subnet']['id'])
-                res = self._create_port(fmt, net_id=net_id)
+                    self._delete('ports', port['port']['id'])
-                port = self.deserialize(fmt, res)
-                ips = port['port']['fixed_ips']
-                self.assertEquals(len(ips), 1)
-                self.assertEquals(ips[0]['ip_address'], '10.0.1.3')
-                self.assertEquals(ips[0]['subnet_id'], subnet['subnet']['id'])
-                self._delete('ports', port['port']['id'])
 
     def test_invalid_admin_state(self):
         with self.network() as network:
@@ -1239,15 +1247,16 @@ class TestPortsV2(QuantumDbPluginV2TestCase):
             self.assertEquals(res.status_int, 422)
 
     def test_default_allocation_expiration(self):
-        reference = datetime.datetime(2012, 8, 13, 23, 11, 0)
-        timeutils.utcnow.override_time = reference
-
         cfg.CONF.set_override('dhcp_lease_duration', 120)
-        expires = QuantumManager.get_plugin()._default_allocation_expiration()
+        reference = datetime.datetime(2012, 8, 13, 23, 11, 0)
-        timeutils.utcnow
+
-        cfg.CONF.reset()
+        with mock.patch.object(timeutils, 'utcnow') as mock_utcnow:
-        timeutils.utcnow.override_time = None
+            mock_utcnow.return_value = reference
-        self.assertEqual(expires, reference + datetime.timedelta(seconds=120))
+
+            plugin = QuantumManager.get_plugin()
+            expires = plugin._default_allocation_expiration()
+            self.assertEqual(expires,
+                             reference + datetime.timedelta(seconds=120))
 
     def test_update_fixed_ip_lease_expiration(self):
         cfg.CONF.set_override('dhcp_lease_duration', 10)
@@ -1272,7 +1281,22 @@ class TestPortsV2(QuantumDbPluginV2TestCase):
                     ip_allocation.expiration - timeutils.utcnow(),
                     datetime.timedelta(seconds=10))
 
-        cfg.CONF.reset()
+    def test_port_delete_holds_ip(self):
+        plugin = QuantumManager.get_plugin()
+        base_class = db_base_plugin_v2.QuantumDbPluginV2
+        with mock.patch.object(base_class, '_hold_ip') as hold_ip:
+            with self.subnet() as subnet:
+                with self.port(subnet=subnet, no_delete=True) as port:
+                    req = self.new_delete_request('ports', port['port']['id'])
+                    res = req.get_response(self.api)
+                    self.assertEquals(res.status_int, 204)
+
+                    hold_ip.assert_called_once_with(
+                        mock.ANY,
+                        port['port']['network_id'],
+                        port['port']['fixed_ips'][0]['subnet_id'],
+                        port['port']['id'],
+                        port['port']['fixed_ips'][0]['ip_address'])
 
     def test_update_fixed_ip_lease_expiration_invalid_address(self):
         cfg.CONF.set_override('dhcp_lease_duration', 10)
@@ -1287,7 +1311,68 @@ class TestPortsV2(QuantumDbPluginV2TestCase):
                         '255.255.255.0',
                         120)
                     self.assertTrue(log.mock_calls)
-        cfg.CONF.reset()
+
+    def test_hold_ip_address(self):
+        plugin = QuantumManager.get_plugin()
+        with self.subnet() as subnet:
+            with self.port(subnet=subnet) as port:
+                update_context = context.Context('', port['port']['tenant_id'])
+                port_id = port['port']['id']
+                with mock.patch.object(db_base_plugin_v2, 'LOG') as log:
+                    ip_address = port['port']['fixed_ips'][0]['ip_address']
+                    plugin._hold_ip(
+                        update_context,
+                        subnet['subnet']['network_id'],
+                        subnet['subnet']['id'],
+                        port_id,
+                        ip_address)
+                    self.assertTrue(log.mock_calls)
+
+                    q = update_context.session.query(models_v2.IPAllocation)
+                    q = q.filter_by(port_id=None, ip_address=ip_address)
+
+                self.assertEquals(len(q.all()), 1)
+
+    def test_recycle_held_ip_address(self):
+        plugin = QuantumManager.get_plugin()
+        with self.subnet() as subnet:
+            with self.port(subnet=subnet) as port:
+                update_context = context.Context('', port['port']['tenant_id'])
+                port_id = port['port']['id']
+                port_obj = plugin._get_port(update_context, port_id)
+
+                for fixed_ip in port_obj.fixed_ips:
+                    fixed_ip.active = False
+                    fixed_ip.expiration = datetime.datetime.utcnow()
+
+                with mock.patch.object(plugin, '_recycle_ip') as rc:
+                    plugin._recycle_expired_ip_allocations(
+                        update_context, subnet['subnet']['network_id'])
+                    rc.assertEquals(len(rc.mock_calls), 1)
+                    self.assertEquals(update_context._recycled_networks,
+                                      set([subnet['subnet']['network_id']]))
+
+    def test_recycle_expired_previously_run_within_context(self):
+        plugin = QuantumManager.get_plugin()
+        with self.subnet() as subnet:
+            with self.port(subnet=subnet) as port:
+                update_context = context.Context('', port['port']['tenant_id'])
+                port_id = port['port']['id']
+                port_obj = plugin._get_port(update_context, port_id)
+
+                update_context._recycled_networks = set(
+                    [subnet['subnet']['network_id']])
+
+                for fixed_ip in port_obj.fixed_ips:
+                    fixed_ip.active = False
+                    fixed_ip.expiration = datetime.datetime.utcnow()
+
+                with mock.patch.object(plugin, '_recycle_ip') as rc:
+                    plugin._recycle_expired_ip_allocations(
+                        update_context, subnet['subnet']['network_id'])
+                    rc.assertFalse(rc.called)
+                    self.assertEquals(update_context._recycled_networks,
+                                      set([subnet['subnet']['network_id']]))
 
 
 class TestNetworksV2(QuantumDbPluginV2TestCase):