Fix an issue on release_address
On ipam_release_address, kuryr unset all the neutron ports with the released IP address. This is incorrect because IP address is not unique across an OpenStack deployment. This patch adds a check for verifying the subnet of a port before reseting the port. If the port's subnet doesn't match the subnet of the pool, the port is not the target and won't be reset. In order to identify the subnet given a pool ID. This patch tags each kuryr created subnet with the pool ID. Change-Id: I8f85ae53c924aa1f4ff877f52a56d267f984dbf5 Closes-Bug: #1782947 Related-Bug: #1782942
This commit is contained in:
parent
a65ae5590c
commit
3299f82199
@ -598,7 +598,10 @@ def _create_kuryr_subnet(pool_cidr, subnet_cidr, pool_id, network_id, gateway):
|
||||
if gateway:
|
||||
new_kuryr_subnet[0]['gateway_ip'] = gateway
|
||||
|
||||
app.neutron.create_subnet({'subnets': new_kuryr_subnet})
|
||||
subnets = app.neutron.create_subnet({'subnets': new_kuryr_subnet})
|
||||
if app.tag_ext:
|
||||
for subnet in subnets['subnets']:
|
||||
_neutron_subnet_add_tag(subnet['id'], pool_id)
|
||||
LOG.debug("Created kuryr subnet %s", new_kuryr_subnet)
|
||||
|
||||
|
||||
@ -1885,6 +1888,10 @@ def ipam_release_address():
|
||||
if not len(subnets):
|
||||
LOG.info("Subnet already deleted.")
|
||||
return flask.jsonify(const.SCHEMA['SUCCESS'])
|
||||
if app.tag_ext:
|
||||
subnets = [subnet
|
||||
for subnet in subnets
|
||||
if pool_id in subnet.get('tags') or []]
|
||||
|
||||
iface = ipaddress.ip_interface(six.text_type(rel_address))
|
||||
rel_ip_address = six.text_type(iface.ip)
|
||||
@ -1904,9 +1911,12 @@ def ipam_release_address():
|
||||
'binding:host_id': ''}
|
||||
if port['name'].startswith(port['device_id']):
|
||||
updated_port["device_id"] = ''
|
||||
app.neutron.update_port(port['id'], {'port': updated_port})
|
||||
_neutron_port_remove_tag(port['id'],
|
||||
const.KURYR_EXISTING_NEUTRON_PORT)
|
||||
for subnet in subnets:
|
||||
if (port['fixed_ips'][0]['subnet_id'] == subnet['id']):
|
||||
app.neutron.update_port(
|
||||
port['id'], {'port': updated_port})
|
||||
_neutron_port_remove_tag(
|
||||
port['id'], const.KURYR_EXISTING_NEUTRON_PORT)
|
||||
except n_exceptions.NeutronClientException as ex:
|
||||
LOG.error("Error happened while fetching "
|
||||
"and deleting port, %s", ex)
|
||||
|
@ -19,7 +19,6 @@ from oslotest import base
|
||||
from kuryr.lib import constants as lib_const
|
||||
from kuryr.lib import utils as lib_utils
|
||||
from kuryr_libnetwork import app
|
||||
from kuryr_libnetwork import constants as const
|
||||
from kuryr_libnetwork import controllers
|
||||
from kuryr_libnetwork.port_driver import driver
|
||||
from kuryr_libnetwork import utils
|
||||
@ -267,8 +266,7 @@ class TestKuryrBase(TestCase):
|
||||
}
|
||||
if subnetpool_id:
|
||||
fake_v4_subnet['subnet'].update(subnetpool_id=subnetpool_id)
|
||||
if not str(name).startswith(const.SUBNET_NAME_PREFIX):
|
||||
fake_v4_subnet['subnet'].get('tags').append(subnetpool_id)
|
||||
fake_v4_subnet['subnet'].get('tags').append(subnetpool_id)
|
||||
|
||||
return fake_v4_subnet
|
||||
|
||||
|
@ -603,7 +603,7 @@ class TestKuryr(base.TestKuryrBase):
|
||||
for tag in tags:
|
||||
mock_add_tag.assert_any_call('networks',
|
||||
fake_neutron_net_id, tag)
|
||||
mock_add_tag.assert_called_with('networks', fake_neutron_net_id,
|
||||
mock_add_tag.assert_any_call('networks', fake_neutron_net_id,
|
||||
utils.existing_net_tag(docker_network_id))
|
||||
mock_create_subnet.assert_any_call(fake_v4_subnet_request)
|
||||
mock_create_subnet.assert_any_call(fake_v6_subnet_request)
|
||||
|
@ -1576,7 +1576,7 @@ class TestKuryrIpam(base.TestKuryrBase):
|
||||
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
|
||||
@mock.patch('kuryr_libnetwork.controllers.app')
|
||||
@ddt.data((False), (True))
|
||||
def test_ipam_driver_release_address_w_existing_port_w_same_ip_and_cidr(
|
||||
def test_ipam_driver_release_address_w_same_ip_and_cidr(
|
||||
self, use_tag_ext, mock_app, mock_list_subnets, mock_list_ports,
|
||||
mock_delete_port):
|
||||
# It checks only the kuryr port is removed even if the other port
|
||||
@ -1646,3 +1646,93 @@ class TestKuryrIpam(base.TestKuryrBase):
|
||||
mock_list_ports.assert_called()
|
||||
mock_delete_port.assert_called_once_with(
|
||||
fake_port['port']['id'])
|
||||
|
||||
@mock.patch('kuryr_libnetwork.controllers.app.neutron.remove_tag')
|
||||
@mock.patch('kuryr_libnetwork.controllers.app.neutron.update_port')
|
||||
@mock.patch('kuryr_libnetwork.controllers.app.neutron.delete_port')
|
||||
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_ports')
|
||||
@mock.patch('kuryr_libnetwork.controllers.app.neutron.list_subnets')
|
||||
@mock.patch('kuryr_libnetwork.controllers.app')
|
||||
def test_ipam_driver_release_address_w_existing_port_w_same_ip_and_cidr(
|
||||
self, mock_app, mock_list_subnets, mock_list_ports,
|
||||
mock_delete_port, mock_update_port, mock_remove_tag):
|
||||
# TODO(hongbin): Current implementation still delete existing ports
|
||||
# if tag extension is not enabled. This needs to be fixed and test
|
||||
# case needs to be added after.
|
||||
mock_app.tag_ext = True
|
||||
fake_kuryr_subnetpool_id = uuidutils.generate_uuid()
|
||||
fake_kuryr_subnetpool_id2 = uuidutils.generate_uuid()
|
||||
# faking list_subnets
|
||||
docker_network_id = lib_utils.get_hash()
|
||||
docker_network_id2 = lib_utils.get_hash()
|
||||
docker_endpoint_id = lib_utils.get_hash()
|
||||
docker_endpoint_id2 = lib_utils.get_hash()
|
||||
subnet_v4_id = uuidutils.generate_uuid()
|
||||
subnet_v4_id2 = uuidutils.generate_uuid()
|
||||
fake_v4_subnet = self._get_fake_v4_subnet(
|
||||
docker_network_id, docker_endpoint_id, subnet_v4_id,
|
||||
subnetpool_id=fake_kuryr_subnetpool_id,
|
||||
cidr=FAKE_IP4_CIDR)
|
||||
fake_v4_subnet2 = self._get_fake_v4_subnet(
|
||||
docker_network_id2, docker_endpoint_id2, subnet_v4_id2,
|
||||
subnetpool_id=fake_kuryr_subnetpool_id2,
|
||||
cidr=FAKE_IP4_CIDR)
|
||||
fake_subnet_response = {
|
||||
'subnets': [
|
||||
fake_v4_subnet['subnet'], fake_v4_subnet2['subnet']
|
||||
]
|
||||
}
|
||||
mock_list_subnets.return_value = fake_subnet_response
|
||||
|
||||
fake_ip4 = '10.0.0.5'
|
||||
# faking list_ports and delete_port
|
||||
neutron_network_id = uuidutils.generate_uuid()
|
||||
fake_neutron_port_id = uuidutils.generate_uuid()
|
||||
fake_port = base.TestKuryrBase._get_fake_port(
|
||||
docker_endpoint_id, neutron_network_id,
|
||||
fake_neutron_port_id, lib_const.PORT_STATUS_ACTIVE,
|
||||
subnet_v4_id,
|
||||
neutron_subnet_v4_address=fake_ip4,
|
||||
device_owner=lib_const.DEVICE_OWNER,
|
||||
tags=[const.KURYR_EXISTING_NEUTRON_PORT])
|
||||
fake_port2 = base.TestKuryrBase._get_fake_port(
|
||||
docker_endpoint_id2, neutron_network_id,
|
||||
fake_neutron_port_id, lib_const.PORT_STATUS_ACTIVE,
|
||||
subnet_v4_id2,
|
||||
neutron_subnet_v4_address=fake_ip4,
|
||||
device_owner=lib_const.DEVICE_OWNER,
|
||||
tags=[const.KURYR_EXISTING_NEUTRON_PORT])
|
||||
|
||||
fake_port['port']['fixed_ips'] = [
|
||||
{'subnet_id': subnet_v4_id, 'ip_address': fake_ip4}
|
||||
]
|
||||
fake_port2['port']['fixed_ips'] = [
|
||||
{'subnet_id': subnet_v4_id2, 'ip_address': fake_ip4}
|
||||
]
|
||||
|
||||
list_port_response = {'ports': [fake_port['port'],
|
||||
fake_port2['port']]}
|
||||
mock_list_ports.return_value = list_port_response
|
||||
|
||||
mock_delete_port.return_value = {}
|
||||
|
||||
fake_request = {
|
||||
'PoolID': fake_kuryr_subnetpool_id,
|
||||
'Address': fake_ip4
|
||||
}
|
||||
response = self.app.post('/IpamDriver.ReleaseAddress',
|
||||
content_type='application/json',
|
||||
data=jsonutils.dumps(fake_request))
|
||||
|
||||
self.assertEqual(200, response.status_code)
|
||||
mock_list_subnets.assert_called_with(
|
||||
subnetpool_id=fake_kuryr_subnetpool_id)
|
||||
mock_list_ports.assert_called()
|
||||
expect_updated_port = {'name': '', 'device_owner': '',
|
||||
'device_id': '', 'binding:host_id': ''}
|
||||
mock_update_port.assert_called_once_with(fake_port['port']['id'],
|
||||
{'port': expect_updated_port})
|
||||
mock_delete_port.assert_not_called()
|
||||
mock_remove_tag.assert_called_once_with('ports',
|
||||
fake_port['port']['id'],
|
||||
const.KURYR_EXISTING_NEUTRON_PORT)
|
||||
|
Loading…
Reference in New Issue
Block a user