From 0894193d12584cc62b55d27e84aeb04a25b5a382 Mon Sep 17 00:00:00 2001 From: rajarammallya Date: Fri, 20 Jan 2012 17:05:19 +0530 Subject: [PATCH] fixes bug 919155 by tracking deallocated macs in the allocatable_macs table Change-Id: I6d78d503393e9c3e584dffec9a7ec50c7247afdd --- melange/db/sqlalchemy/api.py | 12 +++---- melange/db/sqlalchemy/mappers.py | 2 ++ .../migrate_repo/versions/001_base_schema.py | 16 ++++++--- melange/ipam/models.py | 35 ++++++++++++++----- melange/ipv4/db_based_ip_generator.py | 3 +- melange/tests/unit/test_ipam_models.py | 19 ++++++++++ 6 files changed, 67 insertions(+), 20 deletions(-) diff --git a/melange/db/sqlalchemy/api.py b/melange/db/sqlalchemy/api.py index fe723606..e0e1071c 100644 --- a/melange/db/sqlalchemy/api.py +++ b/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): diff --git a/melange/db/sqlalchemy/mappers.py b/melange/db/sqlalchemy/mappers.py index fce8bbf4..31a8d2d8 100644 --- a/melange/db/sqlalchemy/mappers.py +++ b/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) diff --git a/melange/db/sqlalchemy/migrate_repo/versions/001_base_schema.py b/melange/db/sqlalchemy/migrate_repo/versions/001_base_schema.py index 21b8ea04..104d1102 100644 --- a/melange/db/sqlalchemy/migrate_repo/versions/001_base_schema.py +++ b/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]) diff --git a/melange/ipam/models.py b/melange/ipam/models.py index ffc9a6ca..4ccfb44d 100644 --- a/melange/ipam/models.py +++ b/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 } diff --git a/melange/ipv4/db_based_ip_generator.py b/melange/ipv4/db_based_ip_generator.py index 0bc06948..c10b37c4 100644 --- a/melange/ipv4/db_based_ip_generator.py +++ b/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 diff --git a/melange/tests/unit/test_ipam_models.py b/melange/tests/unit/test_ipam_models.py index 991a5c2b..9074ab09 100644 --- a/melange/tests/unit/test_ipam_models.py +++ b/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):