From 476e50f4d7f7c7f3231bca8beb194776c28b2267 Mon Sep 17 00:00:00 2001
From: Yuuichi Fujioka <fujioka-yuuichi@zx.mxh.nes.nec.co.jp>
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: