From 476e50f4d7f7c7f3231bca8beb194776c28b2267 Mon Sep 17 00:00:00 2001 From: Yuuichi Fujioka Date: Mon, 29 Jul 2013 05:32:01 +0900 Subject: [PATCH] Allow name argument to flavor-access-add Administrator cannot use the flavor name in arguments of "nova flavor-access-add" and "nova flavor-access-remove" in following conditions. 1. The flavor is non-public. 2. The falvor doesn't belong administrator's tenant. This patch fixes this problem. Change-Id: Ic5e3222d0ff62667e003a61e6a94e85833da5d11 Fixes: bug #1205298 --- novaclient/base.py | 14 +++++--- novaclient/tests/v1_1/fakes.py | 47 +++++++++++++++++++++++---- novaclient/tests/v1_1/test_flavors.py | 18 ++++++++-- novaclient/tests/v1_1/test_shell.py | 19 +++++++++-- novaclient/utils.py | 4 +-- novaclient/v1_1/shell.py | 12 +++++-- 6 files changed, 93 insertions(+), 21 deletions(-) diff --git a/novaclient/base.py b/novaclient/base.py index adcf29d5a..cbe29eb51 100644 --- a/novaclient/base.py +++ b/novaclient/base.py @@ -199,13 +199,19 @@ class ManagerWithFind(Manager): searches = kwargs.items() detailed = True - if 'detailed' in inspect.getargspec(self.list).args: + list_kwargs = {} + + list_argspec = inspect.getargspec(self.list) + if 'detailed' in list_argspec.args: detailed = ("human_id" not in kwargs and "name" not in kwargs and "display_name" not in kwargs) - listing = self.list(detailed=detailed) - else: - listing = self.list() + list_kwargs['detailed'] = detailed + + if 'is_public' in list_argspec.args and 'is_public' in kwargs: + list_kwargs['is_public'] = kwargs['is_public'] + + listing = self.list(**list_kwargs) for obj in listing: try: diff --git a/novaclient/tests/v1_1/fakes.py b/novaclient/tests/v1_1/fakes.py index 7673317fc..bb6f866af 100644 --- a/novaclient/tests/v1_1/fakes.py +++ b/novaclient/tests/v1_1/fakes.py @@ -20,9 +20,11 @@ import urlparse import six from novaclient import client as base_client -from novaclient.v1_1 import client +from novaclient import exceptions +from novaclient.openstack.common import strutils from novaclient.tests import fakes from novaclient.tests import utils +from novaclient.v1_1 import client class FakeClient(fakes.FakeClient, client.Client): @@ -67,6 +69,7 @@ class FakeHTTPClient(base_client.HTTPClient): munged_url = url.rsplit('?', 1)[0] munged_url = munged_url.strip('/').replace('/', '_').replace('.', '_') munged_url = munged_url.replace('-', '_') + munged_url = munged_url.replace(' ', '_') callback = "%s_%s" % (method.lower(), munged_url) @@ -594,7 +597,7 @@ class FakeHTTPClient(base_client.HTTPClient): ]}) def get_flavors_detail(self, **kw): - return (200, {}, {'flavors': [ + flavors = {'flavors': [ {'id': 1, 'name': '256 MB Server', 'ram': 256, 'disk': 10, 'OS-FLV-EXT-DATA:ephemeral': 10, 'os-flavor-access:is_public': True, @@ -607,20 +610,45 @@ class FakeHTTPClient(base_client.HTTPClient): 'OS-FLV-EXT-DATA:ephemeral': 0, 'os-flavor-access:is_public': True, 'links': {}} - ]}) + ]} + + if 'is_public' not in kw: + filter_is_public = True + else: + if kw['is_public'].lower() == 'none': + filter_is_public = None + else: + filter_is_public = strutils.bool_from_string(kw['is_public'], + True) + + if filter_is_public is not None: + if filter_is_public: + flavors['flavors'] = [ + v for v in flavors['flavors'] + if v['os-flavor-access:is_public'] + ] + else: + flavors['flavors'] = [ + v for v in flavors['flavors'] + if not v['os-flavor-access:is_public'] + ] + + return (200, {}, flavors) def get_flavors_1(self, **kw): return ( 200, {}, - {'flavor': self.get_flavors_detail()[2]['flavors'][0]} + {'flavor': + self.get_flavors_detail(is_public='None')[2]['flavors'][0]} ) def get_flavors_2(self, **kw): return ( 200, {}, - {'flavor': self.get_flavors_detail()[2]['flavors'][1]} + {'flavor': + self.get_flavors_detail(is_public='None')[2]['flavors'][1]} ) def get_flavors_3(self, **kw): @@ -636,12 +664,16 @@ class FakeHTTPClient(base_client.HTTPClient): }}, ) + def get_flavors_512_MB_Server(self, **kw): + raise exceptions.NotFound('404') + def get_flavors_aa1(self, **kw): # Aplhanumeric flavor id are allowed. return ( 200, {}, - {'flavor': self.get_flavors_detail()[2]['flavors'][2]} + {'flavor': + self.get_flavors_detail(is_public='None')[2]['flavors'][2]} ) def delete_flavors_flavordelete(self, **kw): @@ -654,7 +686,8 @@ class FakeHTTPClient(base_client.HTTPClient): return ( 202, {}, - {'flavor': self.get_flavors_detail()[2]['flavors'][0]} + {'flavor': + self.get_flavors_detail(is_public='None')[2]['flavors'][0]} ) def get_flavors_1_os_extra_specs(self, **kw): diff --git a/novaclient/tests/v1_1/test_flavors.py b/novaclient/tests/v1_1/test_flavors.py index eb7be2a4c..398e6df13 100644 --- a/novaclient/tests/v1_1/test_flavors.py +++ b/novaclient/tests/v1_1/test_flavors.py @@ -21,12 +21,24 @@ class FlavorsTest(utils.TestCase): for flavor in fl: self.assertTrue(isinstance(flavor, flavors.Flavor)) - def test_list_flavors_is_public(self): + def test_list_flavors_is_public_none(self): fl = cs.flavors.list(is_public=None) cs.assert_called('GET', '/flavors/detail?is_public=None') for flavor in fl: self.assertTrue(isinstance(flavor, flavors.Flavor)) + def test_list_flavors_is_public_false(self): + fl = cs.flavors.list(is_public=False) + cs.assert_called('GET', '/flavors/detail?is_public=False') + for flavor in fl: + self.assertTrue(isinstance(flavor, flavors.Flavor)) + + def test_list_flavors_is_public_true(self): + fl = cs.flavors.list(is_public=True) + cs.assert_called('GET', '/flavors/detail') + for flavor in fl: + self.assertTrue(isinstance(flavor, flavors.Flavor)) + def test_get_flavor_details(self): f = cs.flavors.get(1) cs.assert_called('GET', '/flavors/1') @@ -59,8 +71,8 @@ class FlavorsTest(utils.TestCase): cs.assert_called('GET', '/flavors/detail') self.assertEqual(f.name, '256 MB Server') - f = cs.flavors.find(disk=20) - self.assertEqual(f.name, '512 MB Server') + f = cs.flavors.find(disk=0) + self.assertEqual(f.name, '128 MB Server') self.assertRaises(exceptions.NotFound, cs.flavors.find, disk=12345) diff --git a/novaclient/tests/v1_1/test_shell.py b/novaclient/tests/v1_1/test_shell.py index 1ca22c62f..69f9b69ba 100644 --- a/novaclient/tests/v1_1/test_shell.py +++ b/novaclient/tests/v1_1/test_shell.py @@ -73,7 +73,10 @@ class ShellTest(utils.TestCase): @mock.patch('sys.stdout', StringIO.StringIO()) def run_command(self, cmd): - self.shell.main(cmd.split()) + if isinstance(cmd, list): + self.shell.main(cmd) + else: + self.shell.main(cmd.split()) return sys.stdout.getvalue() def assert_called(self, method, url, body=None, **kwargs): @@ -449,16 +452,26 @@ class ShellTest(utils.TestCase): cmd = 'flavor-access-list' self.assertRaises(exceptions.CommandError, self.run_command, cmd) - def test_flavor_access_add(self): + def test_flavor_access_add_by_id(self): self.run_command('flavor-access-add 2 proj2') self.assert_called('POST', '/flavors/2/action', {'addTenantAccess': {'tenant': 'proj2'}}) - def test_flavor_access_remove(self): + def test_flavor_access_add_by_name(self): + self.run_command(['flavor-access-add', '512 MB Server', 'proj2']) + self.assert_called('POST', '/flavors/2/action', + {'addTenantAccess': {'tenant': 'proj2'}}) + + def test_flavor_access_remove_by_id(self): self.run_command('flavor-access-remove 2 proj2') self.assert_called('POST', '/flavors/2/action', {'removeTenantAccess': {'tenant': 'proj2'}}) + def test_flavor_access_remove_by_name(self): + self.run_command(['flavor-access-remove', '512 MB Server', 'proj2']) + self.assert_called('POST', '/flavors/2/action', + {'removeTenantAccess': {'tenant': 'proj2'}}) + def test_image_show(self): self.run_command('image-show 1') self.assert_called('GET', '/images/1') diff --git a/novaclient/utils.py b/novaclient/utils.py index 532ea976c..2167dc0bb 100644 --- a/novaclient/utils.py +++ b/novaclient/utils.py @@ -192,7 +192,7 @@ def print_dict(d, dict_property="Property", dict_value="Value", wrap=0): print(strutils.safe_encode(pt.get_string())) -def find_resource(manager, name_or_id): +def find_resource(manager, name_or_id, **find_args): """Helper for the _find_* methods.""" # first try to get entity as integer id try: @@ -216,7 +216,7 @@ def find_resource(manager, name_or_id): try: try: - return manager.find(human_id=name_or_id) + return manager.find(human_id=name_or_id, **find_args) except exceptions.NotFound: pass diff --git a/novaclient/v1_1/shell.py b/novaclient/v1_1/shell.py index 7358a87a9..927e1f029 100644 --- a/novaclient/v1_1/shell.py +++ b/novaclient/v1_1/shell.py @@ -605,7 +605,7 @@ def do_flavor_access_list(cs, args): help='Filter results by tenant ID.') def do_flavor_access_add(cs, args): """Add flavor access for the given tenant.""" - flavor = _find_flavor(cs, args.flavor) + flavor = _find_flavor_for_admin(cs, args.flavor) access_list = cs.flavor_access.add_tenant_access(flavor, args.tenant) columns = ['Flavor_ID', 'Tenant_ID'] utils.print_list(access_list, columns) @@ -618,7 +618,7 @@ def do_flavor_access_add(cs, args): help='Filter results by tenant ID.') def do_flavor_access_remove(cs, args): """Remove flavor access for the given tenant.""" - flavor = _find_flavor(cs, args.flavor) + flavor = _find_flavor_for_admin(cs, args.flavor) access_list = cs.flavor_access.remove_tenant_access(flavor, args.tenant) columns = ['Flavor_ID', 'Tenant_ID'] utils.print_list(access_list, columns) @@ -1376,6 +1376,14 @@ def _find_image(cs, image): return utils.find_resource(cs.images, image) +def _find_flavor_for_admin(cs, flavor): + """Get a flavor for administrator by name, ID, or RAM size.""" + try: + return utils.find_resource(cs.flavors, flavor, is_public='None') + except exceptions.NotFound: + return cs.flavors.find(ram=flavor) + + def _find_flavor(cs, flavor): """Get a flavor by name, ID, or RAM size.""" try: