diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index ffe237595..20b326b08 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -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"): diff --git a/shade/tests/functional/test_floating_ip.py b/shade/tests/functional/test_floating_ip.py index 17e70d5a6..608aff5a0 100644 --- a/shade/tests/functional/test_floating_ip.py +++ b/shade/tests/functional/test_floating_ip.py @@ -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})] + ) diff --git a/shade/tests/unit/test_floating_ip_neutron.py b/shade/tests/unit/test_floating_ip_neutron.py index cb2a4409f..e0ed99679 100644 --- a/shade/tests/unit/test_floating_ip_neutron.py +++ b/shade/tests/unit/test_floating_ip_neutron.py @@ -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, diff --git a/shade/tests/unit/test_floating_ip_nova.py b/shade/tests/unit/test_floating_ip_nova.py index e05cea7be..adf6ed32d 100644 --- a/shade/tests/unit/test_floating_ip_nova.py +++ b/shade/tests/unit/test_floating_ip_nova.py @@ -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):