Merge pull request #309 from Cerberus98/rm11043

Delete MAC addresses in ranges marked do_not_use

quark/db/api.py

@@ -325,6 +325,10 @@ def mac_address_find(context, lock_mode=False, **filters):
     return query.filter(*model_filters)
 
 
+def mac_address_delete(context, mac_address):
+    context.session.delete(mac_address)
+
+
 def mac_address_range_find_allocation_counts(context, address=None):
     count = sql_func.count(models.MacAddress.address)
     query = context.session.query(models.MacAddressRange,
@@ -336,6 +340,7 @@ def mac_address_range_find_allocation_counts(context, address=None):
         query = query.filter(models.MacAddressRange.last_address >= address)
         query = query.filter(models.MacAddressRange.first_address <= address)
     query = query.filter(models.MacAddressRange.next_auto_assign_mac != -1)
+    query = query.filter(models.MacAddressRange.do_not_use == False)  # noqa
     query = query.limit(1)
     return query.first()
 

quark/ipam.py

@@ -113,6 +113,18 @@ def generate_v6(mac, port_id, cidr):
 
 
 class QuarkIpam(object):
+    def _delete_mac_if_do_not_use(self, context, mac):
+        # NOTE(mdietz): This is a HACK. Please see RM11043 for details
+        admin_ctx = context.elevated()
+        mac_range = db_api.mac_address_range_find(
+            admin_ctx, id=mac["mac_address_range_id"], scope=db_api.ONE)
+
+        if mac_range and mac_range["do_not_use"]:
+            # NOTE(mdietz): This is a HACK. Please see RM11043 for details
+            db_api.mac_address_delete(context, mac)
+            return True
+        return False
+
     @synchronized(named("allocate_mac_address"))
     def allocate_mac_address(self, context, net_id, port_id, reuse_after,
                              mac_address=None):
@@ -136,6 +148,13 @@ class QuarkIpam(object):
                         address=mac_address, order_by="address ASC")
 
                     if deallocated_mac:
+                        if self._delete_mac_if_do_not_use(context,
+                                                          deallocated_mac):
+                            LOG.debug("Found a deallocated MAC in a do_not_use"
+                                      " mac_address_range and deleted it. "
+                                      "Retrying...")
+                            continue
+
                         dealloc = netaddr.EUI(deallocated_mac["address"])
                         LOG.info("Found a suitable deallocated MAC {0}".format(
                             str(dealloc)))
@@ -655,6 +674,8 @@ class QuarkIpam(object):
         if not mac:
             raise exceptions.NotFound(
                 message="No MAC address %s found" % netaddr.EUI(address))
+
+        if not self._delete_mac_if_do_not_use(context, mac):
             db_api.mac_address_update(context, mac, deallocated=True,
                                       deallocated_at=timeutils.utcnow())
 

quark/tests/functional/db/test_api.py

@@ -187,3 +187,39 @@ class QuarkFindSubnetAllocationCount(QuarkIpamBaseFunctionalTest):
                 self.assertEqual(
                     netaddr.IPAddress(subnet["next_auto_assign_ip"]).ipv4(),
                     net4[1])
+
+
+class QuarkFindMacAddressRangeAllocationCount(QuarkIpamBaseFunctionalTest):
+    @contextlib.contextmanager
+    def _fixtures(self, mac_ranges):
+        self.ipam = quark.ipam.QuarkIpamANY()
+        for mar in mac_ranges:
+            with self.context.session.begin():
+                db_api.mac_address_range_create(self.context,
+                                                **mar)
+        yield
+
+    def test_mac_address_ranges(self):
+        mr1_mac = netaddr.EUI("AA:AA:AA:00:00:00")
+        mr1 = {"cidr": "AA:AA:AA/24", "do_not_use": False,
+               "first_address": mr1_mac.value,
+               "last_address": netaddr.EUI("AA:AA:AA:FF:FF:FF").value,
+               "next_auto_assign_mac": mr1_mac.value}
+        with self._fixtures([mr1]):
+            with self.context.session.begin():
+                ranges = db_api.mac_address_range_find_allocation_counts(
+                    self.context)
+                self.assertTrue(ranges[0]["cidr"], mr1["cidr"])
+
+    def test_mac_address_ranges_do_not_use_returns_nothing(self):
+        mr1_mac = netaddr.EUI("AA:AA:AA:00:00:00")
+        mr1 = {"cidr": "AA:AA:AA/24", "do_not_use": True,
+               "first_address": mr1_mac.value,
+               "last_address": netaddr.EUI("AA:AA:AA:FF:FF:FF").value,
+               "next_auto_assign_mac": mr1_mac.value}
+
+        with self._fixtures([mr1]):
+            with self.context.session.begin():
+                ranges = db_api.mac_address_range_find_allocation_counts(
+                    self.context)
+                self.assertTrue(ranges is None)

quark/tests/test_ipam.py

@@ -99,17 +99,21 @@ class QuarkIpamBaseTest(test_base.TestBase):
 
 class QuarkMacAddressAllocateDeallocated(QuarkIpamBaseTest):
     @contextlib.contextmanager
-    def _stubs(self, mac_find=True):
+    def _stubs(self, mac_find=True, do_not_use=False):
         address = dict(address=0)
         mac_range = dict(id=1, first_address=0, last_address=255,
-                         next_auto_assign_mac=0)
+                         next_auto_assign_mac=0, do_not_use=do_not_use,
+                         cidr="AA:BB:CC/24")
         with contextlib.nested(
             mock.patch("quark.db.api.mac_address_find"),
             mock.patch("quark.db.api."
                        "mac_address_range_find_allocation_counts"),
             mock.patch("quark.db.api.mac_address_update"),
-            mock.patch("quark.db.api.mac_address_create")
+            mock.patch("quark.db.api.mac_address_create"),
-        ) as (addr_find, mac_range_count, mac_update, mac_create):
+            mock.patch("quark.db.api.mac_address_range_find"),
+            mock.patch("quark.db.api.mac_address_delete")
+        ) as (addr_find, mac_range_count, mac_update, mac_create,
+              range_find, mac_delete):
             address_mod = models.MacAddress(**address)
             range_mod = models.MacAddressRange(**mac_range)
             if mac_find:
@@ -118,19 +122,66 @@ class QuarkMacAddressAllocateDeallocated(QuarkIpamBaseTest):
                 addr_find.side_effect = [None, None]
             mac_range_count.return_value = (range_mod, 0)
             mac_create.return_value = address_mod
-            yield mac_update, mac_create
+            range_find.return_value = range_mod
+            yield mac_update, mac_create, mac_delete
 
     def test_allocate_mac_address_find_deallocated(self):
-        with self._stubs(True) as (mac_update, mac_create):
+        with self._stubs(True) as (mac_update, mac_create, mac_delete):
             self.ipam.allocate_mac_address(self.context, 0, 0, 0)
             self.assertTrue(mac_update.called)
             self.assertFalse(mac_create.called)
+            self.assertFalse(mac_delete.called)
 
     def test_allocate_mac_address_creates_new_mac(self):
-        with self._stubs(False) as (mac_update, mac_create):
+        with self._stubs(False) as (mac_update, mac_create, mac_delete):
             self.ipam.allocate_mac_address(self.context, 0, 0, 0)
             self.assertFalse(mac_update.called)
             self.assertTrue(mac_create.called)
+            self.assertFalse(mac_delete.called)
+
+
+class QuarkMacAddressAllocateDeallocatedDoNotUse(QuarkIpamBaseTest):
+    @contextlib.contextmanager
+    def _stubs(self, address, address2, mac_range, mac_range2):
+
+        with contextlib.nested(
+            mock.patch("quark.db.api.mac_address_find"),
+            mock.patch("quark.db.api."
+                       "mac_address_range_find_allocation_counts"),
+            mock.patch("quark.db.api.mac_address_update"),
+            mock.patch("quark.db.api.mac_address_create"),
+            mock.patch("quark.db.api.mac_address_range_find"),
+            mock.patch("quark.db.api.mac_address_delete")
+        ) as (addr_find, mac_range_count, mac_update, mac_create,
+              range_find, mac_delete):
+            address_mod = models.MacAddress(**address)
+            address_mod2 = models.MacAddress(**address2)
+            range_mod = models.MacAddressRange(**mac_range)
+            range_mod2 = models.MacAddressRange(**mac_range2)
+
+            addr_find.side_effect = [address_mod, address_mod2]
+            mac_range_count.return_value = (range_mod, 0)
+            mac_create.return_value = address_mod
+            range_find.side_effect = [range_mod, range_mod2]
+            mac_update.return_value = address2
+            yield mac_update, mac_create, mac_delete
+
+    def test_allocate_deallocated_do_not_use_range_deletes(self):
+        address = dict(address=0)
+        mac_range = dict(id=1, first_address=0, last_address=255,
+                         next_auto_assign_mac=0, do_not_use=True,
+                         cidr="AA:BB:CC/24")
+        address2 = dict(address=1)
+        mac_range2 = dict(id=1, first_address=0, last_address=255,
+                          next_auto_assign_mac=0, do_not_use=False,
+                          cidr="AA:BB:CC/24")
+        with self._stubs(address, address2, mac_range, mac_range2) as (
+                mac_update, mac_create, mac_delete):
+            addr = self.ipam.allocate_mac_address(self.context, 0, 0, 0)
+            self.assertTrue(mac_update.called)
+            self.assertFalse(mac_create.called)
+            self.assertTrue(mac_delete.called)
+            self.assertEqual(addr["address"], 1)
 
 
 class QuarkNewMacAddressAllocation(QuarkIpamBaseTest):
@@ -260,24 +311,37 @@ class QuarkNewMacAddressReallocationDeadlocks(QuarkIpamBaseTest):
 
 class QuarkMacAddressDeallocation(QuarkIpamBaseTest):
     @contextlib.contextmanager
-    def _stubs(self, mac):
+    def _stubs(self, mac, mac_range):
         with contextlib.nested(
             mock.patch("quark.db.api.mac_address_find"),
-            mock.patch("quark.db.api.mac_address_update")
+            mock.patch("quark.db.api.mac_address_update"),
-        ) as (mac_find,
+            mock.patch("quark.db.api.mac_address_range_find"),
-              mac_update):
+            mock.patch("quark.db.api.mac_address_delete")
+        ) as (mac_find, mac_update, range_find, mac_delete):
             mac_update.return_value = mac
             mac_find.return_value = mac
-            yield mac_update
+            range_find.return_value = mac_range
+            yield mac_update, mac_delete
 
     def test_deallocate_mac(self):
-        mac = dict(id=1, address=1)
+        mac_range = dict(id=2, do_not_use=False)
-        with self._stubs(mac=mac) as mac_update:
+        mac = dict(id=1, address=1, mac_address_range_id=mac_range["id"])
+        with self._stubs(mac=mac, mac_range=mac_range) as (mac_update,
+                                                           mac_delete):
             self.ipam.deallocate_mac_address(self.context, mac["address"])
             self.assertTrue(mac_update.called)
 
+    def test_deallocate_mac_do_not_use_range_deletes_mac(self):
+        mac_range = dict(id=2, do_not_use=True)
+        mac = dict(id=1, address=1, mac_address_range_id=mac_range["id"])
+        with self._stubs(mac=mac, mac_range=mac_range) as (mac_update,
+                                                           mac_delete):
+            self.ipam.deallocate_mac_address(self.context, mac["address"])
+            self.assertFalse(mac_update.called)
+            self.assertTrue(mac_delete.called)
+
     def test_deallocate_mac_mac_not_found_fails(self):
-        with self._stubs(mac=None) as mac_update:
+        with self._stubs(mac=None, mac_range=None) as (mac_update, mac_delete):
             self.assertRaises(exceptions.NotFound,
                               self.ipam.deallocate_mac_address, self.context,
                               0)