fixes bug 919155 by tracking deallocated macs in the allocatable_macs table

Change-Id: I6d78d503393e9c3e584dffec9a7ec50c7247afdd

melange/db/sqlalchemy/api.py

@@ -185,14 +185,14 @@ def find_all_allocated_ips(model, used_by_device=None, used_by_tenant=None,
     return query
 
 
-def pop_allocatable_address(**conditions):
-    ip = _query_by(ipam.models.AllocatableIp, **conditions).\
-         with_lockmode('update').first()
-    if not ip:
+def pop_allocatable_address(address_model, **conditions):
+    address_rec = _query_by(address_model, **conditions).\
+                     with_lockmode('update').first()
+    if not address_rec:
         return None
 
-    delete(ip)
-    return ip.address
+    delete(address_rec)
+    return address_rec.address
 
 
 def save_allowed_ip(interface_id, ip_address_id):

melange/db/sqlalchemy/mappers.py

@@ -34,6 +34,7 @@ def map(engine, models):
     mac_addresses_table = Table('mac_addresses', meta, autoload=True)
     interfaces_table = Table('interfaces', meta, autoload=True)
     allowed_ips_table = Table('allowed_ips', meta, autoload=True)
+    allocatable_macs_table = Table('allocatable_macs', meta, autoload=True)
 
     orm.mapper(models["IpBlock"], Table('ip_blocks', meta, autoload=True))
     orm.mapper(models["IpAddress"], ip_addresses_table)
@@ -45,6 +46,7 @@ def map(engine, models):
     orm.mapper(models["MacAddressRange"], mac_address_ranges_table)
     orm.mapper(models["MacAddress"], mac_addresses_table)
     orm.mapper(models["Interface"], interfaces_table)
+    orm.mapper(models["AllocatableMac"], allocatable_macs_table)
 
     inside_global_join = (ip_nats_table.c.inside_global_address_id
                           == ip_addresses_table.c.id)

melange/db/sqlalchemy/migrate_repo/versions/001_base_schema.py

@@ -153,16 +153,24 @@ allowed_ips = Table('allowed_ips', meta,
                nullable=False),
         UniqueConstraint('ip_address_id', 'interface_id'))
 
+allocatable_macs = Table('allocatable_macs', meta,
+        Column('id', String(36), primary_key=True, nullable=False),
+        Column('mac_address_range_id', String(36),
+               ForeignKey('mac_address_ranges.id')),
+        Column('address', BigInteger(), nullable=False),
+        Column('created_at', DateTime()),
+        Column('updated_at', DateTime()))
+
 
 def upgrade(migrate_engine):
     meta.bind = migrate_engine
     create_tables([policies, ip_ranges, ip_octets, ip_blocks, ip_routes,
                    mac_address_ranges, interfaces, mac_addresses, ip_addresses,
-                   ip_nats, allocatable_ips, allowed_ips])
+                   ip_nats, allocatable_ips, allowed_ips, allocatable_macs])
 
 
 def downgrade(migrate_engine):
     meta.bind = migrate_engine
-    drop_tables([allowed_ips, allocatable_ips, ip_nats, ip_addresses,
-                 mac_addresses, interfaces, mac_address_ranges, ip_routes,
-                 ip_blocks, ip_ranges, ip_octets, policies])
+    drop_tables([allocatable_macs, allowed_ips, allocatable_ips, ip_nats,
+                 ip_addresses, mac_addresses, interfaces, mac_address_ranges,
+                 ip_routes, ip_blocks, ip_ranges, ip_octets, policies])

melange/ipam/models.py

@@ -707,19 +707,16 @@ class MacAddressRange(ModelBase):
             raise NoMoreMacAddressesError()
 
         max_retry_count = int(config.Config.get("mac_allocation_retries", 10))
-        next_address = self._next_eligible_address()
         for retries in range(max_retry_count):
+            next_address = self._next_eligible_address()
             try:
-                mac = MacAddress.create(address=next_address,
-                                        mac_address_range_id=self.id,
-                                        **kwargs)
-                self.update(next_address=next_address + 1)
-                return mac
+                return MacAddress.create(address=next_address,
+                                         mac_address_range_id=self.id,
+                                         **kwargs)
             except exception.DBConstraintError as error:
                 LOG.debug("MAC allocation retry count:{0}".format(retries + 1))
                 LOG.exception(error)
-                next_address = next_address + 1
-                if not self.contains(next_address):
+                if not self.contains(next_address + 1):
                     raise NoMoreMacAddressesError()
 
         raise ConcurrentAllocationError(
@@ -731,7 +728,7 @@ class MacAddressRange(ModelBase):
                 address <= self._last_address())
 
     def is_full(self):
-        return self._next_eligible_address() > self._last_address()
+        return self._get_next_address() > self._last_address()
 
     def length(self):
         base_address, slash, prefix_length = self.cidr.partition("/")
@@ -749,6 +746,16 @@ class MacAddressRange(ModelBase):
         return self._first_address() + self.length() - 1
 
     def _next_eligible_address(self):
+        allocatable_address = db.db_api.pop_allocatable_address(
+                AllocatableMac, mac_address_range_id=self.id)
+        if allocatable_address is not None:
+                return allocatable_address
+
+        address = self._get_next_address()
+        self.update(next_address=address + 1)
+        return address
+
+    def _get_next_address(self):
         return self.next_address or self._first_address()
 
 
@@ -770,6 +777,15 @@ class MacAddress(ModelBase):
     def _validate(self):
         self._validate_belongs_to_mac_address_range()
 
+    def delete(self):
+        AllocatableMac.create(mac_address_range_id=self.mac_address_range_id,
+                              address=self.address)
+        super(MacAddress, self).delete()
+
+
+class AllocatableMac(ModelBase):
+    pass
+
 
 class Interface(ModelBase):
 
@@ -1047,6 +1063,7 @@ def persisted_models():
         'MacAddressRange': MacAddressRange,
         'MacAddress': MacAddress,
         'Interface': Interface,
+        'AllocatableMac': AllocatableMac
         }
 
 

melange/ipv4/db_based_ip_generator.py

@@ -19,6 +19,7 @@ import netaddr
 
 from melange.common import exception
 from melange.db import db_api
+from melange import ipam
 
 
 class DbBasedIpGenerator(object):
@@ -28,7 +29,7 @@ class DbBasedIpGenerator(object):
 
     def next_ip(self):
         allocatable_address = db_api.pop_allocatable_address(
-            ip_block_id=self.ip_block.id)
+             ipam.models.AllocatableIp, ip_block_id=self.ip_block.id)
 
         if allocatable_address is not None:
                 return allocatable_address

melange/tests/unit/test_ipam_models.py

@@ -1631,6 +1631,14 @@ class TestMacAddressRange(tests.BaseTest):
     def test_mac_allocation_disabled_when_no_ranges_exist(self):
         self.assertFalse(models.MacAddressRange.mac_allocation_enabled())
 
+    def test_deallocated_macs_are_allocated_again(self):
+        rng = factory_models.MacAddressRangeFactory(cidr="BC:76:4E:20:0:0/40")
+        mac = rng.allocate_mac()
+
+        mac.delete()
+
+        self.assertEqual(rng.allocate_mac().address, mac.address)
+
     def test_contains_mac_address(self):
         rng = factory_models.MacAddressRangeFactory(cidr="BC:76:4E:20:0:0/40")
 
@@ -1682,6 +1690,17 @@ class TestMacAddress(tests.BaseTest):
         self.assertEqual(mac.errors['address'],
                          ["address does not belong to range"])
 
+    def test_delete_pushes_mac_address_on_allocatable_mac_list(self):
+        rng = factory_models.MacAddressRangeFactory(cidr="BC:76:4E:20:0:0/40")
+        mac = rng.allocate_mac()
+
+        mac.delete()
+
+        self.assertIsNone(models.MacAddress.get(mac.id))
+        allocatable_mac = models.AllocatableMac.get_by(
+                                mac_address_range_id=rng.id)
+        self.assertEqual(mac.address, allocatable_mac.address)
+
 
 class TestPolicy(tests.BaseTest):