Support tagging existing subnetpool

Change-Id: Iee68cbff7a491a8d0dda9aa05e677adc9ea30481
Closes-Bug: #1671222

kuryr_libnetwork/constants.py

@@ -34,6 +34,7 @@ NEUTRON_ID_UH_OPTION = 'kuryr.net.uuid.uh'
 
 DOCKER_EXPOSED_PORTS_OPTION = 'com.docker.network.endpoint.exposedports'
 KURYR_EXISTING_NEUTRON_NET = 'kuryr.net.existing'
+KURYR_EXISTING_NEUTRON_SUBNETPOOL = 'kuryr.subnetpool.existing'
 KURYR_EXISTING_NEUTRON_PORT = 'kuryr.port.existing'
 NETWORK_GATEWAY_OPTIONS = 'com.docker.network.gateway'
 NETWORK_GENERIC_OPTIONS = 'com.docker.network.generic'

kuryr_libnetwork/controllers.py

@@ -305,6 +305,14 @@ def _neutron_net_remove_tags(netid, tag):
         _neutron_net_remove_tag(netid, tag)
 
 
+def _neutron_subnetpool_add_tag(poolid, tag):
+    _neutron_add_tag('subnetpools', poolid, tag)
+
+
+def _neutron_subnetpool_remove_tag(poolid, tag):
+    _neutron_remove_tag('subnetpools', poolid, tag)
+
+
 def _neutron_subnet_add_tag(subnetid, tag):
     _neutron_add_tag('subnets', subnetid, tag)
 
@@ -1452,6 +1460,9 @@ def ipam_request_pool():
                     "exist.").format(pool_id or pool_name))
 
             pool_id = existing_pools[0]['id']
+            if app.tag_ext:
+                _neutron_subnetpool_add_tag(
+                    pool_id, const.KURYR_EXISTING_NEUTRON_SUBNETPOOL)
             prefixes = existing_pools[0]['prefixes']
             pool_cidr = ipaddress.ip_network(six.text_type(prefixes[0]))
             if pool_cidr == cidr:
@@ -1660,7 +1671,15 @@ def ipam_release_pool():
     pools = _get_subnetpools_by_attrs(id=pool_id)
     if pools:
         pool_name = pools[0]['name']
-        if not pool_name.startswith(cfg.CONF.subnetpool_name_prefix):
+        if app.tag_ext:
+            tags = pools[0].get('tags', [])
+            if const.KURYR_EXISTING_NEUTRON_SUBNETPOOL in tags:
+                _neutron_subnetpool_remove_tag(
+                    pool_id, const.KURYR_EXISTING_NEUTRON_SUBNETPOOL)
+                LOG.debug('Skip the cleanup since this is an existing Neutron '
+                          'subnetpool.')
+                return flask.jsonify(const.SCHEMA['SUCCESS'])
+        elif not pool_name.startswith(cfg.CONF.subnetpool_name_prefix):
             LOG.debug('Skip the cleanup since this is an existing Neutron '
                       'subnetpool.')
             return flask.jsonify(const.SCHEMA['SUCCESS'])

kuryr_libnetwork/tests/unit/base.py

@@ -76,9 +76,11 @@ class TestKuryrBase(TestCase):
 
     @staticmethod
     def _get_fake_v4_subnetpools(subnetpool_id, prefixes=["192.168.1.0/24"],
-                                 name="kuryr"):
+                                 name="kuryr", tags=None):
         # The following fake response is retrieved from the Neutron doc:
         #   http://developer.openstack.org/api-ref-networking-v2-ext.html#listSubnetPools  # noqa
+        if tags is None:
+            tags = []
         v4_subnetpools = {
             "subnetpools": [{
                 "min_prefixlen": "24",
@@ -91,7 +93,8 @@ class TestKuryrBase(TestCase):
                 "tenant_id": "9fadcee8aa7c40cdb2114fff7d569c08",
                 "prefixes": prefixes,
                 "ip_version": 4,
-                "shared": False
+                "shared": False,
+                "tags": tags
             }]
         }
 

kuryr_libnetwork/tests/unit/test_kuryr_ipam.py

@@ -112,11 +112,17 @@ class TestKuryrIpam(base.TestKuryrBase):
         decoded_json = jsonutils.loads(response.data)
         self.assertEqual(fake_kuryr_subnetpool_id, decoded_json['PoolID'])
 
+    @mock.patch('kuryr_libnetwork.controllers.app.neutron.add_tag')
     @mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnetpools')
     @mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
-    @ddt.data((FAKE_IP4_CIDR), (FAKE_IP6_CIDR))
+    @mock.patch('kuryr_libnetwork.controllers.app')
+    @ddt.data((True, FAKE_IP4_CIDR), (True, FAKE_IP6_CIDR),
+              (False, FAKE_IP4_CIDR), (False, FAKE_IP6_CIDR))
+    @ddt.unpack
     def test_ipam_driver_request_pool_with_pool_name_option(self,
-            pool_cidr, mock_list_subnets, mock_list_subnetpools):
+            use_tag_ext, pool_cidr, mock_app, mock_list_subnets,
+            mock_list_subnetpools, mock_add_tag):
+        mock_app.tag_ext = use_tag_ext
         fake_subnet = {"subnets": []}
         mock_list_subnets.return_value = fake_subnet
 
@@ -149,14 +155,26 @@ class TestKuryrIpam(base.TestKuryrBase):
 
         self.assertEqual(200, response.status_code)
         mock_list_subnets.assert_called_with(cidr=pool_cidr)
+        if mock_app.tag_ext:
+            mock_add_tag.assert_called_once_with(
+                'subnetpools', fake_kuryr_subnetpool_id,
+                const.KURYR_EXISTING_NEUTRON_SUBNETPOOL)
+        else:
+            mock_add_tag.assert_not_called()
         decoded_json = jsonutils.loads(response.data)
         self.assertEqual(fake_kuryr_subnetpool_id, decoded_json['PoolID'])
 
+    @mock.patch('kuryr_libnetwork.controllers.app.neutron.add_tag')
     @mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnetpools')
     @mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
-    @ddt.data((FAKE_IP4_CIDR), (FAKE_IP6_CIDR))
+    @mock.patch('kuryr_libnetwork.controllers.app')
+    @ddt.data((True, FAKE_IP4_CIDR), (True, FAKE_IP6_CIDR),
+              (False, FAKE_IP4_CIDR), (False, FAKE_IP6_CIDR))
+    @ddt.unpack
     def test_ipam_driver_request_pool_with_pool_id_option(self,
-            pool_cidr, mock_list_subnets, mock_list_subnetpools):
+            use_tag_ext, pool_cidr, mock_app, mock_list_subnets,
+            mock_list_subnetpools, mock_add_tag):
+        mock_app.tag_ext = use_tag_ext
         fake_subnet = {"subnets": []}
         mock_list_subnets.return_value = fake_subnet
 
@@ -186,16 +204,27 @@ class TestKuryrIpam(base.TestKuryrBase):
 
         self.assertEqual(200, response.status_code)
         mock_list_subnets.assert_called_with(cidr=pool_cidr)
+        if mock_app.tag_ext:
+            mock_add_tag.assert_called_once_with(
+                'subnetpools', fake_kuryr_subnetpool_id,
+                const.KURYR_EXISTING_NEUTRON_SUBNETPOOL)
+        else:
+            mock_add_tag.assert_not_called()
         decoded_json = jsonutils.loads(response.data)
         self.assertEqual(fake_kuryr_subnetpool_id, decoded_json['PoolID'])
 
+    @mock.patch('kuryr_libnetwork.controllers.app.neutron.add_tag')
     @mock.patch('kuryr_libnetwork.controllers.app.neutron.create_subnetpool')
     @mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnetpools')
     @mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
-    @ddt.data((FAKE_IP4_CIDR), (FAKE_IP6_CIDR))
+    @mock.patch('kuryr_libnetwork.controllers.app')
+    @ddt.data((True, FAKE_IP4_CIDR), (True, FAKE_IP6_CIDR),
+              (False, FAKE_IP4_CIDR), (False, FAKE_IP6_CIDR))
+    @ddt.unpack
     def test_ipam_driver_request_pool_with_unmatched_cidr(self,
-            pool_cidr, mock_list_subnets, mock_list_subnetpools,
+            use_tag_ext, pool_cidr, mock_app, mock_list_subnets,
-            mock_create_subnetpool):
+            mock_list_subnetpools, mock_create_subnetpool, mock_add_tag):
+        mock_app.tag_ext = use_tag_ext
         fake_subnet = {"subnets": []}
         mock_list_subnets.return_value = fake_subnet
         subnet_ip4_cidr = '10.0.0.0/24'
@@ -263,6 +292,12 @@ class TestKuryrIpam(base.TestKuryrBase):
         mock_list_subnets.assert_called_with(cidr=subnet_cidr)
         mock_create_subnetpool.assert_called_with(
             {'subnetpool': new_subnetpool})
+        if mock_app.tag_ext:
+            mock_add_tag.assert_called_once_with(
+                'subnetpools', fake_existing_subnetpool_id,
+                const.KURYR_EXISTING_NEUTRON_SUBNETPOOL)
+        else:
+            mock_add_tag.assert_not_called()
         decoded_json = jsonutils.loads(response.data)
         self.assertEqual(fake_kuryr_subnetpool_id, decoded_json['PoolID'])
 
@@ -361,9 +396,12 @@ class TestKuryrIpam(base.TestKuryrBase):
 
         fake_kuryr_subnetpool_id = uuidutils.generate_uuid()
         fake_subnetpool_name = 'fake_pool_name'
+        fake_tags = []
+        if mock_app.tag_ext:
+            fake_tags.append(const.KURYR_EXISTING_NEUTRON_SUBNETPOOL)
         kuryr_subnetpools = self._get_fake_v4_subnetpools(
             fake_kuryr_subnetpool_id, prefixes=[FAKE_IP4_CIDR],
-            name=fake_subnetpool_name)
+            name=fake_subnetpool_name, tags=fake_tags)
         mock_list_subnetpools.return_value = kuryr_subnetpools
 
         docker_endpoint_id = lib_utils.get_hash()
@@ -393,9 +431,12 @@ class TestKuryrIpam(base.TestKuryrBase):
                 id=fake_kuryr_subnetpool_id)
             mock_list_subnets.assert_called_with(
                 cidr=FAKE_IP4_CIDR)
-            mock_remove_tag.assert_called_with('subnets',
+            mock_remove_tag.assert_any_call('subnets',
                                             subnet_v4_id,
                                             fake_kuryr_subnetpool_id)
+            mock_remove_tag.assert_any_call(
+                'subnetpools', fake_kuryr_subnetpool_id,
+                const.KURYR_EXISTING_NEUTRON_SUBNETPOOL)
 
     @mock.patch('kuryr_libnetwork.controllers._neutron_port_add_tag')
     @mock.patch('kuryr_libnetwork.controllers.app.neutron.create_port')

releasenotes/notes/bug-1671222-9ea0cf3ab39f0abc.yaml

@@ -0,0 +1,5 @@
+---
+other:
+  - |
+    Tag existing subnetpool if tag extension is enabled in Neutron.
+    This will ensure the subnetpool to deleted is the one kuryr created.