openstack
/
openstacksdk
Code Issues Proposed changes

Allow server-side filtering of Neutron floating IPs 

Neutron API allows to filter floating IPs based on some fields. This
enables server-side (efficient) filtering that Shade should leverage
when it can.

Unfortunately Nova-net (which is deprecated) doesn't support server-side
filtering when it comes to FIP, so we have to distinguish the two at
several places.

This patch also updates the `search_floating_ips` method to use
server-side filtering when appropriate.

Change-Id: I28c960091c1679ca588c0cfb3bed1881cd6b03d2
This commit is contained in:
Jordan Pittier 2016-11-04 18:59:33 +01:00
parent 5296d06ae1
commit 958f972552
4 changed files with 117 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
shade/openstackcloud.py
View File

@ -1383,14 +1383,18 @@ class OpenStackCloud(_normalize.Normalizer):
pools = self.list_floating_ip_pools()
return _utils._filter_list(pools, name, filters)
# Note (dguerri): when using Neutron, this can be optimized using
# server-side search.
# There are some cases in which such optimization is not possible (e.g.
# nested attributes or list of objects) so we need to use the client-side
# filtering when we can't do otherwise.
# With Neutron, there are some cases in which full server side filtering is
# not possible (e.g. nested attributes or list of objects) so we also need
# to use the client-side filtering
# The same goes for all neutron-related search/get methods!
def search_floating_ips(self, id=None, filters=None):
floating_ips = self.list_floating_ips()
# `filters` could be a jmespath expression which Neutron server doesn't
# understand, obviously.
if self._use_neutron_floating() and isinstance(filters, dict):
kwargs = {'filters': filters}
else:
kwargs = {}
floating_ips = self.list_floating_ips(**kwargs)
return _utils._filter_list(floating_ips, id, filters)
def search_stacks(self, name_or_id=None, filters=None):
@ -1700,26 +1704,47 @@ class OpenStackCloud(_normalize.Normalizer):
with _utils.shade_exceptions("Error fetching floating IP pool list"):
return self.manager.submit_task(_tasks.FloatingIPPoolList())
def _list_floating_ips(self):
def _list_floating_ips(self, filters=None):
if self._use_neutron_floating():
try:
return self._normalize_floating_ips(
self._neutron_list_floating_ips())
self._neutron_list_floating_ips(filters))
except OpenStackCloudURINotFound as e:
# Nova-network don't support server-side floating ips
# filtering, so it's safer to die hard than to fallback to Nova
# which may return more results that expected.
if filters:
self.log.error(
"Something went wrong talking to neutron API. Can't "
"fallback to Nova since it doesn't support server-side"
" filtering when listing floating ips."
)
raise
self.log.debug(
"Something went wrong talking to neutron API: "
"'%(msg)s'. Trying with Nova.", {'msg': str(e)})
# Fall-through, trying with Nova
else:
if filters:
raise ValueError(
"Nova-network don't support server-side floating ips "
"filtering. Use the search_floatting_ips method instead"
)
floating_ips = self._nova_list_floating_ips()
return self._normalize_floating_ips(floating_ips)
def list_floating_ips(self):
def list_floating_ips(self, filters=None):
"""List all available floating IPs.
:param filters: (optional) dict of filter conditions to push down
:returns: A list of floating IP ``munch.Munch``.
"""
# If pushdown filters are specified, bypass local caching.
if filters:
return self._list_floating_ips(filters)
if (time.time() - self._floating_ips_time) >= self._FLOAT_AGE:
# Since we're using cached data anyway, we don't need to
# have more than one thread actually submit the list
@ -1738,10 +1763,12 @@ class OpenStackCloud(_normalize.Normalizer):
self._floating_ips_lock.release()
return self._floating_ips
def _neutron_list_floating_ips(self):
def _neutron_list_floating_ips(self, filters=None):
if not filters:
filters = {}
with _utils.neutron_exceptions("error fetching floating IPs list"):
return self.manager.submit_task(
_tasks.NeutronFloatingIPList())['floatingips']
_tasks.NeutronFloatingIPList(**filters))['floatingips']
def _nova_list_floating_ips(self):
with _utils.shade_exceptions("Error fetching floating IPs list"):

49
shade/tests/functional/test_floating_ip.py
View File

@ -243,3 +243,52 @@ class TestFloatingIP(base.BaseFunctionalTestCase):
id=None, filters={'floating_ip_address': ip})
self.demo_cloud.detach_ip_from_server(
server_id=new_server.id, floating_ip_id=f_ip['id'])
def test_list_floating_ips(self):
fip_admin = self.operator_cloud.create_floating_ip()
self.addCleanup(self.operator_cloud.delete_floating_ip, fip_admin.id)
fip_user = self.demo_cloud.create_floating_ip()
self.addCleanup(self.demo_cloud.delete_floating_ip, fip_user.id)
# Get all the floating ips.
fip_id_list = [
fip.id for fip in self.operator_cloud.list_floating_ips()
]
if self.demo_cloud.has_service('network'):
# Neutron returns all FIP for all projects by default
self.assertIn(fip_admin.id, fip_id_list)
self.assertIn(fip_user.id, fip_id_list)
# Ask Neutron for only a subset of all the FIPs.
filtered_fip_id_list = [
fip.id for fip in self.operator_cloud.list_floating_ips(
{'tenant_id': self.demo_cloud.current_project_id}
)
]
self.assertNotIn(fip_admin.id, filtered_fip_id_list)
self.assertIn(fip_user.id, filtered_fip_id_list)
else:
self.assertIn(fip_admin.id, fip_id_list)
# By default, Nova returns only the FIPs that belong to the
# project which made the listing request.
self.assertNotIn(fip_user.id, fip_id_list)
self.assertRaisesRegex(
ValueError, "Nova-network don't support server-side.*",
self.operator_cloud.list_floating_ips, filters={'foo': 'bar'}
)
def test_search_floating_ips(self):
fip_user = self.demo_cloud.create_floating_ip()
self.addCleanup(self.demo_cloud.delete_floating_ip, fip_user.id)
self.assertIn(
fip_user['id'],
[fip.id for fip in self.demo_cloud.search_floating_ips(
filters={"attached": False})]
)
self.assertNotIn(
fip_user['id'],
[fip.id for fip in self.demo_cloud.search_floating_ips(
filters={"attached": True})]
)

24
shade/tests/unit/test_floating_ip_neutron.py
View File

@ -166,9 +166,8 @@ class TestFloatingIP(base.TestCase):
self.assertEqual('UNKNOWN', normalized[0]['status'])
@patch.object(OpenStackCloud, 'neutron_client')
@patch.object(OpenStackCloud, 'has_service')
@patch.object(OpenStackCloud, 'has_service', return_value=True)
def test_list_floating_ips(self, mock_has_service, mock_neutron_client):
mock_has_service.return_value = True
mock_neutron_client.list_floatingips.return_value = \
self.mock_floating_ip_list_rep
@ -179,6 +178,21 @@ class TestFloatingIP(base.TestCase):
self.assertAreInstances(floating_ips, dict)
self.assertEqual(2, len(floating_ips))
@patch.object(OpenStackCloud, 'neutron_client')
@patch.object(OpenStackCloud, 'has_service', return_value=True)
def test_list_floating_ips_with_filters(self, mock_has_service,
mock_neutron_client):
mock_neutron_client.list_floatingips.side_effect = \
exc.OpenStackCloudURINotFound("")
try:
self.cloud.list_floating_ips(filters={'Foo': 42})
except exc.OpenStackCloudException as e:
self.assertIsInstance(
e.inner_exception[1], exc.OpenStackCloudURINotFound)
mock_neutron_client.list_floatingips.assert_called_once_with(Foo=42)
@patch.object(OpenStackCloud, 'neutron_client')
@patch.object(OpenStackCloud, 'has_service')
def test_search_floating_ips(self, mock_has_service, mock_neutron_client):
@ -189,7 +203,7 @@ class TestFloatingIP(base.TestCase):
floating_ips = self.cloud.search_floating_ips(
filters={'attached': False})
mock_neutron_client.list_floatingips.assert_called_with()
mock_neutron_client.list_floatingips.assert_called_with(attached=False)
self.assertIsInstance(floating_ips, list)
self.assertAreInstances(floating_ips, dict)
self.assertEqual(1, len(floating_ips))
@ -311,7 +325,7 @@ class TestFloatingIP(base.TestCase):
mock_keystone_session.get_project_id.assert_called_once_with()
mock_get_ext_nets.assert_called_once_with()
mock__neutron_list_fips.assert_called_once_with()
mock__neutron_list_fips.assert_called_once_with(None)
mock__filter_list.assert_called_once_with(
[], name_or_id=None,
filters={'port_id': None,
@ -349,7 +363,7 @@ class TestFloatingIP(base.TestCase):
mock_keystone_session.get_project_id.assert_called_once_with()
mock_get_ext_nets.assert_called_once_with()
mock__neutron_list_fips.assert_called_once_with()
mock__neutron_list_fips.assert_called_once_with(None)
mock__filter_list.assert_called_once_with(
[], name_or_id=None,
filters={'port_id': None,

11
shade/tests/unit/test_floating_ip_nova.py
View File

@ -99,6 +99,17 @@ class TestFloatingIP(base.TestCase):
self.assertEqual(3, len(floating_ips))
self.assertAreInstances(floating_ips, dict)
@patch.object(OpenStackCloud, 'nova_client')
@patch.object(OpenStackCloud, 'has_service')
def test_list_floating_ips_with_filters(self, mock_has_service,
mock_nova_client):
mock_has_service.side_effect = has_service_side_effect
self.assertRaisesRegex(
ValueError, "Nova-network don't support server-side",
self.cloud.list_floating_ips, filters={'Foo': 42}
)
@patch.object(OpenStackCloud, 'nova_client')
@patch.object(OpenStackCloud, 'has_service')
def test_search_floating_ips(self, mock_has_service, mock_nova_client):