From 3bc383a6a8ab15a4b10aea8b03720b6597c0708c Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Sat, 3 Jun 2017 17:18:30 +0000 Subject: [PATCH] Support tagging existing subnetpool Change-Id: Iee68cbff7a491a8d0dda9aa05e677adc9ea30481 Closes-Bug: #1671222 --- kuryr_libnetwork/constants.py | 1 + kuryr_libnetwork/controllers.py | 21 ++++++- kuryr_libnetwork/tests/unit/base.py | 7 ++- .../tests/unit/test_kuryr_ipam.py | 63 +++++++++++++++---- .../notes/bug-1671222-9ea0cf3ab39f0abc.yaml | 5 ++ 5 files changed, 83 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/bug-1671222-9ea0cf3ab39f0abc.yaml diff --git a/kuryr_libnetwork/constants.py b/kuryr_libnetwork/constants.py index c2258b4d..4af8b6cb 100644 --- a/kuryr_libnetwork/constants.py +++ b/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' diff --git a/kuryr_libnetwork/controllers.py b/kuryr_libnetwork/controllers.py index 7a22c4f8..15b96faf 100644 --- a/kuryr_libnetwork/controllers.py +++ b/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']) diff --git a/kuryr_libnetwork/tests/unit/base.py b/kuryr_libnetwork/tests/unit/base.py index 33f224fd..5465bf3c 100644 --- a/kuryr_libnetwork/tests/unit/base.py +++ b/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 }] } diff --git a/kuryr_libnetwork/tests/unit/test_kuryr_ipam.py b/kuryr_libnetwork/tests/unit/test_kuryr_ipam.py index ff2d9d63..63f1913d 100644 --- a/kuryr_libnetwork/tests/unit/test_kuryr_ipam.py +++ b/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, - mock_create_subnetpool): + use_tag_ext, pool_cidr, mock_app, mock_list_subnets, + 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', - subnet_v4_id, - fake_kuryr_subnetpool_id) + 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') diff --git a/releasenotes/notes/bug-1671222-9ea0cf3ab39f0abc.yaml b/releasenotes/notes/bug-1671222-9ea0cf3ab39f0abc.yaml new file mode 100644 index 00000000..cd3fa918 --- /dev/null +++ b/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.