diff --git a/melange/db/sqlalchemy/api.py b/melange/db/sqlalchemy/api.py index 2b00ea75..3a7e371b 100644 --- a/melange/db/sqlalchemy/api.py +++ b/melange/db/sqlalchemy/api.py @@ -146,11 +146,10 @@ def find_all_blocks_with_deallocated_ips(): filter(ipam.models.IpAddress.marked_for_deallocation == True) -def delete_deallocated_ips(deallocated_by, **kwargs): +def find_deallocated_ips(deallocated_by, **kwargs): return _query_by(ipam.models.IpAddress, **kwargs).\ filter_by(marked_for_deallocation=True).\ - filter(ipam.models.IpAddress.deallocated_at <= deallocated_by).\ - delete() + filter(ipam.models.IpAddress.deallocated_at <= deallocated_by).all() def find_all_top_level_blocks_in_network(network_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 ce85bf48..2793cf0d 100644 --- a/melange/db/sqlalchemy/migrate_repo/versions/001_base_schema.py +++ b/melange/db/sqlalchemy/migrate_repo/versions/001_base_schema.py @@ -53,6 +53,7 @@ ip_addresses = Table('ip_addresses', meta, Column('address', String(255), nullable=False), Column('interface_id', String(255), ForeignKey('interfaces.id')), Column('ip_block_id', String(36), ForeignKey('ip_blocks.id')), + Column('used_by_tenant_id', String(255)), Column('created_at', DateTime()), Column('updated_at', DateTime()), Column('marked_for_deallocation', Boolean()), diff --git a/melange/ipam/models.py b/melange/ipam/models.py index 72ae9dcf..63815241 100644 --- a/melange/ipam/models.py +++ b/melange/ipam/models.py @@ -327,7 +327,7 @@ class IpBlock(ModelBase): if self.is_full: raise exception.NoMoreAddressesError(_("IpBlock is full")) if address: - return self._allocate_specific_ip(address, interface) + return self._allocate_specific_ip(interface, address) return self._allocate_available_ip(interface, **kwargs) def _allocate_available_ip(self, interface, **kwargs): @@ -341,6 +341,7 @@ class IpBlock(ModelBase): try: return IpAddress.create(address=address, ip_block_id=self.id, + used_by_tenant_id=interface.tenant_id, interface_id=interface.id) except exception.DBConstraintError as error: LOG.debug("IP allocation retry count :{0}".format(retries + 1)) @@ -370,7 +371,7 @@ class IpBlock(ModelBase): self.update(is_full=True) raise exception.NoMoreAddressesError(_("IpBlock is full")) - def _allocate_specific_ip(self, address, interface): + def _allocate_specific_ip(self, interface, address): if not self.contains(address): raise AddressDoesNotBelongError( @@ -386,6 +387,7 @@ class IpBlock(ModelBase): return IpAddress.create(address=address, ip_block_id=self.id, + used_by_tenant_id=interface.tenant_id, interface_id=interface.id) def _address_is_allocatable(self, policy, address): @@ -414,8 +416,9 @@ class IpBlock(ModelBase): def delete_deallocated_ips(self): self.update(is_full=False) - db_api.delete_deallocated_ips( - deallocated_by=self._deallocated_by_date(), ip_block_id=self.id) + for ip in db_api.find_deallocated_ips( + deallocated_by=self._deallocated_by_date(), ip_block_id=self.id): + ip.delete() def _deallocated_by_date(self): days = config.Config.get('keep_deallocated_ips_for_days', 2) @@ -552,7 +555,6 @@ class IpAddress(ModelBase): _data_fields = ['ip_block_id', 'address', 'version'] def _validate(self): - self._validate_presence_of("interface_id") self._validate_existence_of("interface_id", Interface) @classmethod @@ -581,10 +583,20 @@ class IpAddress(ModelBase): **conditions) def delete(self): + if self._explicitly_allowed_on_interfaces(): + return self.update(marked_for_deallocation=False, + deallocated_at=None, + interface_id=None) + AllocatableIp.create(ip_block_id=self.ip_block_id, address=self.address) super(IpAddress, self).delete() + def _explicitly_allowed_on_interfaces(self): + return Query(IpAddress, + query_func=db_api.find_allowed_ips, + ip_address_id=self.id).count() > 0 + def _before_save(self): self.address = self._formatted(self.address) diff --git a/melange/tests/unit/test_ipam_models.py b/melange/tests/unit/test_ipam_models.py index 17eb0974..d7254658 100644 --- a/melange/tests/unit/test_ipam_models.py +++ b/melange/tests/unit/test_ipam_models.py @@ -495,13 +495,27 @@ class TestIpBlock(tests.BaseTest): def test_allocate_ip(self): block = factory_models.PrivateIpBlockFactory(cidr="10.0.0.0/31") - block = models.IpBlock.find(block.id) interface = factory_models.InterfaceFactory() ip = block.allocate_ip(interface) saved_ip = models.IpAddress.find(ip.id) self.assertEqual(ip.address, saved_ip.address) + self.assertTrue(netaddr.IPAddress(ip.address) + in netaddr.IPNetwork("10.0.0.0/31")) self.assertEqual(ip.interface_id, interface.id) + self.assertEqual(ip.ip_block_id, block.id) + self.assertEqual(ip.used_by_tenant_id, interface.tenant_id) + + def test_allocate_specific_address(self): + block = factory_models.PrivateIpBlockFactory(cidr="10.0.0.0/24") + interface = factory_models.InterfaceFactory(tenant_id="tnt_id") + ip = block.allocate_ip(interface, address="10.0.0.2") + + expected_ip = models.IpAddress.find(ip.id) + self.assertEqual(expected_ip.address, "10.0.0.2") + self.assertEqual(ip.interface_id, interface.id) + self.assertEqual(ip.ip_block_id, block.id) + self.assertEqual(ip.used_by_tenant_id, "tnt_id") def test_allocate_ip_from_non_leaf_block_fails(self): parent_block = factory_models.IpBlockFactory(cidr="10.0.0.0/28") @@ -670,6 +684,7 @@ class TestIpBlock(tests.BaseTest): def _mock_ip_creation(self): return models.IpAddress.create(address=mox.IgnoreArg(), interface_id=mox.IgnoreArg(), + used_by_tenant_id=mox.IgnoreArg(), ip_block_id=mox.IgnoreArg()) def test_ip_block_is_not_full(self): @@ -716,7 +731,7 @@ class TestIpBlock(tests.BaseTest): self.assertEqual(ip.address, "00ff:0000:0000:0000:0000:0000:0000:0002") - def test_allocate_ip_for_for_given_ipv6_address(self): + def test_allocate_ip_for_given_ipv6_address(self): block = factory_models.IpV6IpBlockFactory(cidr="ff::/120") interface = factory_models.InterfaceFactory() @@ -957,6 +972,12 @@ class TestIpBlock(tests.BaseTest): ip_block.delete_deallocated_ips() self.assertEqual(ip_block.addresses(), [ip2]) + allocatable_ips = [(ip.address, ip.ip_block_id) for ip in + models.AllocatableIp.find_all()] + self.assertItemsEqual(allocatable_ips, [(ip1.address, ip1.ip_block_id), + (ip3.address, ip2.ip_block_id), + (ip4.address, ip3.ip_block_id), + ]) def test_is_full_flag_reset_when_addresses_are_deleted(self): interface = factory_models.InterfaceFactory() @@ -1260,12 +1281,6 @@ class TestIpAddress(tests.BaseTest): self.assertEqual(ip.mac_address, mac_address) - def test_validates_presence_of_inteface(self): - ip = factory_models.IpAddressFactory.build(interface_id=None) - self.assertFalse(ip.is_valid()) - self.assertEqual(ip.errors['interface_id'], - ["interface_id should be present"]) - def test_validates_existance_of_inteface(self): ip = factory_models.IpAddressFactory.build(interface_id="bad_id") self.assertFalse(ip.is_valid()) @@ -2119,9 +2134,9 @@ class TestInterface(tests.BaseTest): self.assertModelsEqual(interface.ip_addresses, [ip1, ip2]) -class TestAllowedIps(tests.BaseTest): +class TestAllowedIp(tests.BaseTest): - def test_allow_an_ip_on_an_interface(self): + def test_allow_ip_on_an_interface(self): interface = factory_models.InterfaceFactory() ip1 = factory_models.IpAddressFactory() ip2 = factory_models.IpAddressFactory() @@ -2174,17 +2189,23 @@ class TestAllowedIps(tests.BaseTest): self.assertEqual(interface.ips_allowed(), []) self.assertEqual(other_interface.ips_allowed(), [ip]) - def skip_deallocating_allowed_ip_only_disassociates_from_interface(self): + def test_deallocating_allowed_ip_only_disassociates_from_interface(self): interface = factory_models.InterfaceFactory() block = factory_models.IpBlockFactory() ip = block.allocate_ip(interface=interface) other_interface = factory_models.InterfaceFactory() other_interface.allow_ip(ip) + current_time = datetime.datetime.now() + two_days_before = current_time - datetime.timedelta(days=2) - block.deallocate_ip(ip.address) + with unit.StubTime(time=two_days_before): + block.deallocate_ip(ip.address) + with unit.StubTime(time=current_time): + block.delete_deallocated_ips() reloaded_ip = models.IpAddress.find(ip.id) - self.assertFalse(reloaded_ip.marked_for_deallocation is True) + self.assertFalse(reloaded_ip.marked_for_deallocation) + self.assertIsNone(reloaded_ip.deallocated_at) self.assertIsNone(reloaded_ip.interface_id) def test_can_explicitly_allow_allocated_ip_on_same_interface(self): @@ -2208,6 +2229,5 @@ class TestAllowedIps(tests.BaseTest): def _allocate_ip(block, interface=None, **kwargs): - if interface is None: - interface = factory_models.InterfaceFactory() + interface = interface or factory_models.InterfaceFactory() return block.allocate_ip(interface=interface, **kwargs)