openstack
/
manila
Code Issues Proposed changes

Make extra spec driver_handles_share_servers required 

Currently the driver_handles_share_servers is an optional extra_spec
on each share_type that allows the scheduler to filter. We want to
make this a required extra spec, so that every share type has this
value set to true or false. It's not be possible to remove that extra
spec. Modifying the extra spec is allowed as long as the value
remains either true or false.

- Add validation checks to types_manage and types_extra_specs API
controllers
- Add required_extra_specs to data returned by API methods
types_show() and types_index()
- Add db migration, which adds required extra_spec to all existing
 share_types
- Add get_required_extra_specs(), validate_required_extra_spec(),
 get_valid_required_extra_specs()
methods to share_types module
- Add appropriate unit tests

Partially implements bp share-type-require-driver-mode

Change-Id: I60a877b05a642ea2945b15248e885cf6f7f89aa0
This commit is contained in:
Igor Malinovskiy 2015-03-06 13:19:14 +02:00
parent 6ba1c1e6cb
commit 31a1be5c2a
21 changed files with 480 additions and 144 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
contrib/tempest/tempest/api/share/admin/test_multi_backend.py
View File

@ -44,8 +44,9 @@ class ShareMultiBackendTest(base.BaseSharesAdminTest):
extra_specs = {
"share_backend_name": CONF.share.backend_names[i],
}
__, st = cls.create_share_type(name=st_name,
extra_specs=extra_specs)
__, st = cls.create_share_type(
name=st_name,
extra_specs=cls.add_required_extra_specs_to_dict(extra_specs))
cls.sts.append(st["share_type"])
st_id = st["share_type"]["id"]
share_data_list.append({"kwargs": {"share_type_id": st_id}})

13
contrib/tempest/tempest/api/share/admin/test_share_types.py
View File

@ -28,9 +28,11 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke", ])
def test_share_type_create_delete(self):
name = data_utils.rand_name("tempest-manila")
extra_specs = self.add_required_extra_specs_to_dict()
# Create share type
resp, st_create = self.shares_client.create_share_type(name)
resp, st_create = self.shares_client.create_share_type(
name, extra_specs=extra_specs)
self.assertIn(int(resp["status"]), self.HTTP_SUCCESS)
self.assertEqual(name, st_create['share_type']['name'])
st_id = st_create['share_type']['id']
@ -48,7 +50,7 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke", ])
def test_share_type_create_get(self):
name = data_utils.rand_name("tempest-manila")
extra_specs = {"key": "value", }
extra_specs = self.add_required_extra_specs_to_dict({"key": "value", })
# Create share type
resp, st_create = self.create_share_type(name,
@ -70,9 +72,10 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke", ])
def test_share_type_create_list(self):
name = data_utils.rand_name("tempest-manila")
extra_specs = self.add_required_extra_specs_to_dict()
# Create share type
resp, st_create = self.create_share_type(name)
resp, st_create = self.create_share_type(name, extra_specs=extra_specs)
self.assertIn(int(resp["status"]), self.HTTP_SUCCESS)
st_id = st_create["share_type"]["id"]
@ -95,9 +98,9 @@ class ShareTypesAdminTest(base.BaseSharesAdminTest):
# Data
share_name = data_utils.rand_name("share")
shr_type_name = data_utils.rand_name("share-type")
extra_specs = {
extra_specs = self.add_required_extra_specs_to_dict({
"storage_protocol": CONF.share.storage_protocol,
}
})
# Create share type
resp, st_create = self.create_share_type(shr_type_name,

32
contrib/tempest/tempest/api/share/admin/test_share_types_extra_specs.py
View File

@ -24,15 +24,17 @@ class ExtraSpecsAdminTest(base.BaseSharesAdminTest):
def resource_setup(cls):
super(ExtraSpecsAdminTest, cls).resource_setup()
shr_type_name = data_utils.rand_name("share-type")
__, cls.share_type = cls.create_share_type(shr_type_name)
extra_specs = cls.add_required_extra_specs_to_dict()
__, cls.share_type = cls.create_share_type(shr_type_name,
extra_specs=extra_specs)
cls.share_type_id = cls.share_type["share_type"]["id"]
@test.attr(type=["gate", "smoke", ])
def test_share_type_extra_specs_list(self):
extra_specs = {
extra_specs = self.add_required_extra_specs_to_dict({
"key1": "value1",
"key2": "value2",
}
})
resp, es_create = self.shares_client.create_share_type_extra_specs(
self.share_type_id, extra_specs)
self.assertIn(int(resp["status"]), self.HTTP_SUCCESS)
@ -45,10 +47,10 @@ class ExtraSpecsAdminTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke", ])
def test_update_one_share_type_extra_spec(self):
extra_specs = {
extra_specs = self.add_required_extra_specs_to_dict({
"key1": "value1",
"key2": "value2",
}
})
# Create extra specs for share type
resp, es_create = self.shares_client.create_share_type_extra_specs(
@ -65,10 +67,10 @@ class ExtraSpecsAdminTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke", ])
def test_update_all_share_type_extra_specs(self):
extra_specs = {
extra_specs = self.add_required_extra_specs_to_dict({
"key1": "value1",
"key2": "value2",
}
})
# Create extra specs for share type
resp, es_create = self.shares_client.create_share_type_extra_specs(
@ -85,10 +87,10 @@ class ExtraSpecsAdminTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke", ])
def test_get_all_share_type_extra_specs(self):
extra_specs = {
extra_specs = self.add_required_extra_specs_to_dict({
"key1": "value1",
"key2": "value2",
}
})
# Create extra specs for share type
resp, es_create = self.shares_client.create_share_type_extra_specs(
@ -104,10 +106,10 @@ class ExtraSpecsAdminTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke", ])
def test_get_one_share_type_extra_spec(self):
extra_specs = {
extra_specs = self.add_required_extra_specs_to_dict({
"key1": "value1",
"key2": "value2",
}
})
# Create extra specs for share type
resp, es_create = self.shares_client.create_share_type_extra_specs(
@ -123,10 +125,10 @@ class ExtraSpecsAdminTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke", ])
def test_delete_one_share_type_extra_spec(self):
extra_specs = {
extra_specs = self.add_required_extra_specs_to_dict({
"key1": "value1",
"key2": "value2",
}
})
# Create extra specs for share type
resp, es_create = self.shares_client.create_share_type_extra_specs(
@ -143,4 +145,6 @@ class ExtraSpecsAdminTest(base.BaseSharesAdminTest):
resp, es_get_all = self.shares_client.get_share_type_extra_specs(
self.share_type_id)
self.assertIn(int(resp["status"]), self.HTTP_SUCCESS)
self.assertEqual({"key2": "value2", }, es_get_all)
self.assertEqual(
self.add_required_extra_specs_to_dict({"key2": "value2", }),
es_get_all)

35
contrib/tempest/tempest/api/share/admin/test_share_types_extra_specs_negative.py
View File

@ -26,7 +26,7 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesAdminTest):
def _create_share_type(self):
name = data_utils.rand_name("unique_st_name")
extra_specs = {"key": "value", }
extra_specs = self.add_required_extra_specs_to_dict({"key": "value"})
__, st = self.create_share_type(name, extra_specs=extra_specs)
return st
@ -41,7 +41,8 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesAdminTest):
self.assertRaises(
lib_exc.Unauthorized,
self.member_shares_client.create_share_type_extra_specs,
st["share_type"]["id"], {"key": "new_value"})
st["share_type"]["id"],
self.add_required_extra_specs_to_dict({"key": "new_value"}))
@test.attr(type=["gate", "smoke", ])
def test_try_list_extra_specs_with_user(self):
@ -95,35 +96,43 @@ class ExtraSpecsAdminNegativeTest(base.BaseSharesAdminTest):
def test_try_set_too_long_key(self):
too_big_key = "k" * 256
st = self._create_share_type()
self.assertRaises(exceptions.BadRequest,
self.shares_client.create_share_type_extra_specs,
st["share_type"]["id"], {too_big_key: "value"})
self.assertRaises(
exceptions.BadRequest,
self.shares_client.create_share_type_extra_specs,
st["share_type"]["id"],
self.add_required_extra_specs_to_dict({too_big_key: "value"}))
@test.attr(type=["gate", "smoke", ])
def test_try_set_too_long_value_with_creation(self):
too_big_value = "v" * 256
st = self._create_share_type()
self.assertRaises(exceptions.BadRequest,
self.shares_client.create_share_type_extra_specs,
st["share_type"]["id"], {"key": too_big_value})
self.assertRaises(
exceptions.BadRequest,
self.shares_client.create_share_type_extra_specs,
st["share_type"]["id"],
self.add_required_extra_specs_to_dict({"key": too_big_value}))
@test.attr(type=["gate", "smoke", ])
def test_try_set_too_long_value_with_update(self):
too_big_value = "v" * 256
st = self._create_share_type()
resp, body = self.shares_client.create_share_type_extra_specs(
st["share_type"]["id"], {"key": "value"})
st["share_type"]["id"],
self.add_required_extra_specs_to_dict({"key": "value"}))
self.assertIn(int(resp["status"]), self.HTTP_SUCCESS)
self.assertRaises(exceptions.BadRequest,
self.shares_client.update_share_type_extra_specs,
st["share_type"]["id"], {"key": too_big_value})
self.assertRaises(
exceptions.BadRequest,
self.shares_client.update_share_type_extra_specs,
st["share_type"]["id"],
self.add_required_extra_specs_to_dict({"key": too_big_value}))
@test.attr(type=["gate", "smoke", ])
def test_try_set_too_long_value_with_update_of_one_key(self):
too_big_value = "v" * 256
st = self._create_share_type()
resp, body = self.shares_client.create_share_type_extra_specs(
st["share_type"]["id"], {"key": "value"})
st["share_type"]["id"],
self.add_required_extra_specs_to_dict({"key": "value"}))
self.assertIn(int(resp["status"]), self.HTTP_SUCCESS)
self.assertRaises(exceptions.BadRequest,
self.shares_client.update_share_type_extra_spec,

5
contrib/tempest/tempest/api/share/admin/test_share_types_negative.py
View File

@ -26,7 +26,7 @@ class ShareTypesAdminNegativeTest(base.BaseSharesAdminTest):
def _create_share_type(self):
name = data_utils.rand_name("unique_st_name")
extra_specs = {"key": "value", }
extra_specs = self.add_required_extra_specs_to_dict({"key": "value"})
__, st = self.create_share_type(name, extra_specs=extra_specs)
return st
@ -82,4 +82,5 @@ class ShareTypesAdminNegativeTest(base.BaseSharesAdminTest):
st = self._create_share_type()
self.assertRaises(lib_exc.Conflict,
self.create_share_type,
st["share_type"]["name"])
st["share_type"]["name"],
extra_specs=self.add_required_extra_specs_to_dict())

7
contrib/tempest/tempest/api/share/admin/test_shares_actions.py
View File

@ -32,7 +32,8 @@ class SharesActionsAdminTest(base.BaseSharesAdminTest):
# create share type for share filtering purposes
cls.st_name = data_utils.rand_name("tempest-st-name")
cls.extra_specs = {'storage_protocol': CONF.share.storage_protocol}
cls.extra_specs = cls.add_required_extra_specs_to_dict(
{'storage_protocol': CONF.share.storage_protocol})
__, cls.st = cls.create_share_type(
name=cls.st_name,
cleanup_in_class=True,
@ -166,7 +167,9 @@ class SharesActionsAdminTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", ])
def test_list_shares_with_detail_filter_by_extra_specs(self):
filters = {"extra_specs": self.extra_specs}
filters = {
"extra_specs": {'storage_protocol': CONF.share.storage_protocol}
}
# list shares
__, shares = self.shares_client.list_shares_with_detail(params=filters)

9
contrib/tempest/tempest/api/share/base.py
View File

@ -19,6 +19,7 @@ import traceback
from tempest_lib import exceptions as lib_exc # noqa
import six
from tempest import clients_share as clients
from tempest.common import isolated_creds
from tempest.common.utils import data_utils
@ -461,6 +462,14 @@ class BaseSharesTest(test.BaseTestCase):
cls.method_resources.insert(0, resource)
return resp, st
@staticmethod
def add_required_extra_specs_to_dict(extra_specs=None):
value = six.text_type(CONF.share.multitenancy_enabled)
required = {"driver_handles_share_servers": value}
if extra_specs:
required.update(extra_specs)
return required
@classmethod
def clear_isolated_creds(cls, creds=None):
if creds is None:

5
devstack/plugin.sh
View File

@ -426,7 +426,10 @@ function create_default_share_type {
die $LINENO "Manila did not start"
fi
manila type-create $MANILA_DEFAULT_SHARE_TYPE
enabled_backends=(${MANILA_ENABLED_BACKENDS//,/ })
driver_handles_share_servers=$(iniget $MANILA_CONF ${enabled_backends[0]} driver_handles_share_servers)
manila type-create $MANILA_DEFAULT_SHARE_TYPE $driver_handles_share_servers
if [[ $MANILA_DEFAULT_SHARE_TYPE_EXTRA_SPECS ]]; then
manila type-key $MANILA_DEFAULT_SHARE_TYPE set $MANILA_DEFAULT_SHARE_TYPE_EXTRA_SPECS
fi

52
manila/api/contrib/types_extra_specs.py
View File

@ -46,27 +46,33 @@ class ShareTypeExtraSpecsController(wsgi.Controller):
except exception.NotFound as ex:
raise webob.exc.HTTPNotFound(explanation=ex.msg)
def _verify_extra_specs(self, extra_specs):
# keys and values in extra_specs can be only strings
# with length in range(1, 256)
is_valid = True
def _verify_extra_specs(self, extra_specs, verify_all_required=True):
if verify_all_required:
try:
share_types.get_valid_required_extra_specs(extra_specs)
except exception.InvalidExtraSpec as e:
raise webob.exc.HTTPBadRequest(explanation=six.text_type(e))
def is_valid_string(v):
return isinstance(v, six.string_types) and len(v) in range(1, 256)
def is_valid_extra_spec(k, v):
valid_extra_spec_key = is_valid_string(k)
valid_type = is_valid_string(v) or isinstance(v, bool)
valid_required_extra_spec = (
share_types.is_valid_required_extra_spec(k, v) in (None, True))
return (valid_extra_spec_key
and valid_type
and valid_required_extra_spec)
for k, v in six.iteritems(extra_specs):
if not (isinstance(k, six.string_types) and
len(k) in range(1, 256)):
is_valid = False
break
if isinstance(v, dict):
if is_valid_string(k) and isinstance(v, dict):
self._verify_extra_specs(v)
elif isinstance(v, six.string_types):
if len(v) not in range(1, 256):
is_valid = False
break
else:
is_valid = False
break
if not is_valid:
expl = _('Invalid request body')
raise webob.exc.HTTPBadRequest(explanation=expl)
elif not is_valid_extra_spec(k, v):
expl = _('Invalid extra_spec: %(key)s: %(value)s') % {
'key': k, 'value': v
}
raise webob.exc.HTTPBadRequest(explanation=expl)
def index(self, req, type_id):
"""Returns the list of extra specs for a given share type."""
@ -83,8 +89,8 @@ class ShareTypeExtraSpecsController(wsgi.Controller):
raise webob.exc.HTTPBadRequest()
self._check_type(context, type_id)
self._verify_extra_specs(body)
specs = body['extra_specs']
self._verify_extra_specs(specs)
self._check_key_names(specs.keys())
db.share_type_extra_specs_update_or_create(context, type_id, specs)
notifier_info = dict(type_id=type_id, specs=specs)
@ -105,7 +111,7 @@ class ShareTypeExtraSpecsController(wsgi.Controller):
if len(body) > 1:
expl = _('Request body contains too many items')
raise webob.exc.HTTPBadRequest(explanation=expl)
self._verify_extra_specs(body)
self._verify_extra_specs(body, False)
db.share_type_extra_specs_update_or_create(context, type_id, body)
notifier_info = dict(type_id=type_id, id=id)
notifier = rpc.get_notifier('shareTypeExtraSpecs')
@ -129,6 +135,10 @@ class ShareTypeExtraSpecsController(wsgi.Controller):
self._check_type(context, type_id)
authorize(context)
if id in share_types.get_required_extra_specs():
msg = _("Required extra specs can't be deleted.")
raise webob.exc.HTTPForbidden(explanation=msg)
try:
db.share_type_extra_specs_delete(context, type_id, id)
except exception.ShareTypeExtraSpecsNotFound as error:

12
manila/api/contrib/types_manage.py
View File

@ -21,6 +21,7 @@ from manila.api import extensions
from manila.api.openstack import wsgi
from manila.api.views import types as views_types
from manila import exception
from manila.i18n import _
from manila import rpc
from manila.share import share_types
@ -54,11 +55,20 @@ class ShareTypesManageController(wsgi.Controller):
specs = share_type.get('extra_specs', {})
if name is None or name == "" or len(name) > 255:
raise webob.exc.HTTPBadRequest()
msg = _("Type name is not valid.")
raise webob.exc.HTTPBadRequest(explanation=msg)
try:
required_extra_specs = (
share_types.get_valid_required_extra_specs(specs)
)
except exception.InvalidExtraSpec as e:
raise webob.exc.HTTPBadRequest(explanation=six.text_type(e))
try:
share_types.create(context, name, specs)
share_type = share_types.get_share_type_by_name(context, name)
share_type['required_extra_specs'] = required_extra_specs
notifier_info = dict(share_types=share_type)
rpc.get_notifier('shareType').info(
context, 'share_type.create', notifier_info)

4
manila/api/views/types.py
View File

@ -24,7 +24,9 @@ class ViewBuilder(common.ViewBuilder):
"""Trim away extraneous share type attributes."""
trimmed = dict(id=share_type.get('id'),
name=share_type.get('name'),
extra_specs=share_type.get('extra_specs'))
extra_specs=share_type.get('extra_specs'),
required_extra_specs=share_type.get(
'required_extra_specs'))
if brief:
return trimmed
else:

5
manila/common/constants.py
View File

@ -62,3 +62,8 @@ ACCESS_LEVELS = (
ACCESS_LEVEL_RW,
ACCESS_LEVEL_RO,
)
class ExtraSpecs(object):
DRIVER_HANDLES_SHARE_SERVERS = "driver_handles_share_servers"
REQUIRED = (DRIVER_HANDLES_SHARE_SERVERS, )

60
manila/db/migrations/alembic/versions/59eb64046740_add_required_extra_spec.py Normal file
View File

@ -0,0 +1,60 @@
# Copyright 2015 Mirantis, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
"""Add required extra spec
Revision ID: 59eb64046740
Revises: 162a3e673105
Create Date: 2015-01-29 15:33:25.348140
"""
# revision identifiers, used by Alembic.
revision = '59eb64046740'
down_revision = '4ee2cf4be19a'
from alembic import op
from sqlalchemy import sql
def upgrade():
connection = op.get_bind()
def escape_column_name(name):
return sql.column(name).compile(bind=connection)
insert_required_extra_spec = (
"INSERT INTO share_type_extra_specs "
" (%(deleted)s, %(share_type_id)s, %(key)s, %(value)s)"
" SELECT '0', st.id, 'driver_handles_share_servers', 'True'"
" FROM share_types as st "
" WHERE st.id NOT IN ( "
" SELECT es.share_type_id FROM share_type_extra_specs as es "
" WHERE es.spec_key = 'driver_handles_share_servers' )" % ({
'deleted': escape_column_name('deleted'),
'share_type_id': escape_column_name('share_type_id'),
'key': escape_column_name('spec_key'),
'value': escape_column_name('spec_value'),
}))
connection.execute(insert_required_extra_spec)
def downgrade():
"""Downgrade method.
We can't determine, which extra specs should be removed after insertion,
that's why do nothing here.
"""
pass

4
manila/db/sqlalchemy/api.py
View File

@ -1110,9 +1110,11 @@ def _metadata_refs(metadata_dict, meta_class):
metadata_refs = []
if metadata_dict:
for k, v in six.iteritems(metadata_dict):
value = six.text_type(v) if isinstance(v, bool) else v
metadata_ref = meta_class()
metadata_ref['key'] = k
metadata_ref['value'] = v
metadata_ref['value'] = value
metadata_refs.append(metadata_ref)
return metadata_refs

4
manila/exception.py
View File

@ -401,6 +401,10 @@ class InvalidShareType(Invalid):
message = _("Invalid share type: %(reason)s.")
class InvalidExtraSpec(Invalid):
message = _("Invalid extra_spec: %(reason)s.")
class VolumeNotFound(NotFound):
message = _("Volume %(volume_id)s could not be found.")

18
manila/scheduler/filter_scheduler.py
View File

@ -28,6 +28,7 @@ from manila.i18n import _
from manila.i18n import _LE
from manila.scheduler import driver
from manila.scheduler import scheduler_options
from manila.share import share_types
CONF = cfg.CONF
LOG = log.getLogger(__name__)
@ -120,6 +121,17 @@ class FilterScheduler(driver.Scheduler):
# 'volume_XX' to 'resource_XX' will make both filters happy.
resource_properties = share_properties.copy()
share_type = request_spec.get("share_type", {})
extra_specs = share_type.get('extra_specs', {})
if extra_specs:
for extra_spec_name in share_types.get_required_extra_specs():
extra_spec = extra_specs.get(extra_spec_name)
if extra_spec is not None:
share_type['extra_specs'][extra_spec_name] = (
"<is> %s" % extra_spec)
resource_type = request_spec.get("share_type") or {}
request_spec.update({'resource_properties': resource_properties})
@ -136,12 +148,6 @@ class FilterScheduler(driver.Scheduler):
'resource_type': resource_type
})
# NOTE(vponomaryov): skip key 'driver_handles_share_servers' temporary
# until server and client sides are both can handle it as required key.
if 'extra_specs' in filter_properties['share_type']:
filter_properties['share_type']['extra_specs'].pop(
'driver_handles_share_servers', None)
self.populate_filter_properties_share(request_spec, filter_properties)
# Find our local list of acceptable hosts by filtering and

73
manila/share/share_types.py
View File

@ -19,8 +19,10 @@
from oslo_config import cfg
from oslo_db import exception as db_exception
from oslo_log import log
from oslo_utils import strutils
import six
from manila.common import constants
from manila import context
from manila import db
from manila import exception
@ -31,8 +33,13 @@ CONF = cfg.CONF
LOG = log.getLogger(__name__)
def create(context, name, extra_specs={}):
def create(context, name, extra_specs):
"""Creates share types."""
try:
get_valid_required_extra_specs(extra_specs)
except exception.InvalidExtraSpec as e:
raise exception.InvalidShareType(reason=six.text_type(e))
try:
type_ref = db.share_type_create(context,
dict(name=name,
@ -60,6 +67,21 @@ def get_all_types(context, inactive=0, search_opts={}):
"""
share_types = db.share_type_get_all(context, inactive)
for type_name, type_args in six.iteritems(share_types):
required_extra_specs = {}
try:
required_extra_specs = get_valid_required_extra_specs(
type_args['extra_specs'])
except exception.InvalidExtraSpec as e:
values = {
'share_type': type_name,
'error': six.text_type(e)
}
LOG.exception(_LE('Share type %(share_type)s has invalid required'
' extra specs: %(error)s'), values)
type_args['required_extra_specs'] = required_extra_specs
if search_opts:
LOG.debug("Searching by: %s", search_opts)
@ -145,6 +167,55 @@ def get_share_type_extra_specs(share_type_id, key=False):
return extra_specs
def get_required_extra_specs():
return constants.ExtraSpecs.REQUIRED
def is_valid_required_extra_spec(key, value):
"""Validates required extra_spec value.
:param key: extra_spec name
:param value: extra_spec value
:return: None if provided extra_spec is not required
True/False if extra_spec is required and valid or not.
"""
if key not in get_required_extra_specs():
return
if key == constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS:
return strutils.bool_from_string(value, default=None) is not None
return False
def get_valid_required_extra_specs(extra_specs):
"""Returns required extra specs from dict.
Returns None if extra specs are not valid, or if
some required extras specs is missed.
"""
extra_specs = extra_specs or {}
missed_extra_specs = set(get_required_extra_specs()) - set(extra_specs)
if missed_extra_specs:
specs = ",".join(missed_extra_specs)
msg = _("Required extra specs '%s' not specified.") % specs
raise exception.InvalidExtraSpec(reason=msg)
required_extra_specs = {}
for k in get_required_extra_specs():
value = extra_specs.get(k, '')
if not is_valid_required_extra_spec(k, value):
msg = _("Value of required extra_spec %s is not valid") % k
raise exception.InvalidExtraSpec(reason=msg)
required_extra_specs[k] = value
return required_extra_specs
def share_types_diff(context, share_type_id1, share_type_id2):
"""Returns a 'diff' of two share types and whether they are equal.

103
manila/tests/api/contrib/test_types_extra_specs.py
View File

@ -15,10 +15,12 @@
# License for the specific language governing permissions and limitations
# under the License.
import ddt
import mock
import webob
from manila.api.contrib import types_extra_specs
from manila.common import constants
from manila import exception
from manila import test
from manila.tests.api import fakes
@ -59,6 +61,22 @@ def share_type_get(context, share_type_id):
pass
def get_large_string():
return "s" * 256
def get_extra_specs_dict(extra_specs, include_required=True):
if not extra_specs:
extra_specs = {}
if include_required:
extra_specs[constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS] = False
return {'extra_specs': extra_specs}
@ddt.ddt
class ShareTypesExtraSpecsTest(test.TestCase):
def setUp(self):
@ -124,18 +142,27 @@ class ShareTypesExtraSpecsTest(test.TestCase):
self.assertRaises(webob.exc.HTTPNotFound, self.controller.delete,
req, 1, 'key6')
def test_delete_forbidden(self):
key = constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
req = fakes.HTTPRequest.blank(self.api_path + '/' + key)
self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
req, 1, key)
def test_create(self):
self.mock_object(manila.db,
'share_type_extra_specs_update_or_create',
return_create_share_type_extra_specs)
body = {"extra_specs": {"key1": "value1"}}
body = get_extra_specs_dict({})
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 0)
req = fakes.HTTPRequest.blank(self.api_path)
res_dict = self.controller.create(req, 1, body)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 1)
self.assertEqual('value1', res_dict['extra_specs']['key1'])
self.assertTrue(
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
in res_dict['extra_specs'])
def test_create_with_too_small_key(self):
self.mock_object(manila.db,
@ -192,7 +219,7 @@ class ShareTypesExtraSpecsTest(test.TestCase):
share_type_extra_specs_update_or_create.\
return_value = mock_return_value
body = {"extra_specs": {"other_alphanum.-_:": "value1"}}
body = get_extra_specs_dict({"other_alphanum.-_:": "value1"})
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 0)
@ -213,9 +240,11 @@ class ShareTypesExtraSpecsTest(test.TestCase):
share_type_extra_specs_update_or_create.\
return_value = mock_return_value
body = {"extra_specs": {"other_alphanum.-_:": "value1",
"other2_alphanum.-_:": "value2",
"other3_alphanum.-_:": "value3"}}
body = get_extra_specs_dict({
"other_alphanum.-_:": "value1",
"other2_alphanum.-_:": "value2",
"other3_alphanum.-_:": "value3"
})
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 0)
@ -233,14 +262,15 @@ class ShareTypesExtraSpecsTest(test.TestCase):
self.mock_object(manila.db,
'share_type_extra_specs_update_or_create',
return_create_share_type_extra_specs)
body = {"key1": "value1"}
key = constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
body = {key: True}
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 0)
req = fakes.HTTPRequest.blank(self.api_path + '/key1')
res_dict = self.controller.update(req, 1, 'key1', body)
req = fakes.HTTPRequest.blank(self.api_path + '/' + key)
res_dict = self.controller.update(req, 1, key, body)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 1)
self.assertEqual('value1', res_dict['key1'])
self.assertEqual(True, res_dict[key])
def test_update_item_too_many_keys(self):
self.mock_object(manila.db,
@ -262,41 +292,40 @@ class ShareTypesExtraSpecsTest(test.TestCase):
self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
req, 1, 'bad', body)
def _extra_specs_empty_update(self, body):
@ddt.data(None,
{},
{"extra_specs": {
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: ""
}})
def test_update_invalid_body(self, body):
req = fakes.HTTPRequest.blank('/v2/fake/types/1/extra_specs')
req.method = 'POST'
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.update, req, '1', body)
def test_update_no_body(self):
self._extra_specs_empty_update(body=None)
def test_update_empty_body(self):
self._extra_specs_empty_update(body={})
def _extra_specs_create_bad_body(self, body):
@ddt.data(None,
{},
{'foo': {'a': 'b'}},
{'extra_specs': 'string'},
{"extra_specs": {"ke/y1": "value1"}},
{"key1": "value1", "ke/y2": "value2", "key3": "value3"},
{"extra_specs": {
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: ""}},
{"extra_specs": {
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: "111"}},
{"extra_specs": {
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS + "FAKE":
"fake"}},
{"extra_specs": {"": "value"}},
{"extra_specs": {"t": get_large_string()}},
{"extra_specs": {get_large_string(): get_large_string()}},
{"extra_specs": {get_large_string(): "v"}},
{"extra_specs": {"k": ""}},
)
def test_create_invalid_body(self, body):
req = fakes.HTTPRequest.blank('/v2/fake/types/1/extra_specs')
req.method = 'POST'
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create, req, '1', body)
def test_create_no_body(self):
self._extra_specs_create_bad_body(body=None)
def test_create_missing_volume(self):
body = {'foo': {'a': 'b'}}
self._extra_specs_create_bad_body(body=body)
def test_create_malformed_entity(self):
body = {'extra_specs': 'string'}
self._extra_specs_create_bad_body(body=body)
def test_create_invalid_key(self):
body = {"extra_specs": {"ke/y1": "value1"}}
self._extra_specs_create_bad_body(body=body)
def test_create_invalid_too_many_key(self):
body = {"key1": "value1", "ke/y2": "value2", "key3": "value3"}
self._extra_specs_create_bad_body(body=body)

73
manila/tests/api/contrib/test_types_manage.py
View File

@ -13,9 +13,11 @@
# License for the specific language governing permissions and limitations
# under the License.
import ddt
import webob
from manila.api.contrib import types_manage
from manila.common import constants
from manila import exception
from manila.share import share_types
from manila import test
@ -60,17 +62,26 @@ def return_share_types_get_by_name(context, name):
return stub_share_type(int(name.split("_")[2]))
def make_create_body(name):
return {
def make_create_body(name="test_share_1", extra_specs=None,
spec_driver_handles_share_servers=True):
if not extra_specs:
extra_specs = {}
if spec_driver_handles_share_servers is not None:
extra_specs[constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS] =\
spec_driver_handles_share_servers
body = {
"share_type": {
"name": name,
"extra_specs": {
"key": "value",
}
"extra_specs": extra_specs
}
}
return body
@ddt.ddt
class ShareTypesManageApiTest(test.TestCase):
def setUp(self):
super(ShareTypesManageApiTest, self).setUp()
@ -108,41 +119,35 @@ class ShareTypesManageApiTest(test.TestCase):
self.controller._delete(req, 1)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 1)
def test_create(self):
body = make_create_body("share_type_1")
@ddt.data(make_create_body("share_type_1"),
make_create_body(spec_driver_handles_share_servers="false"),
make_create_body(spec_driver_handles_share_servers="true"),
make_create_body(spec_driver_handles_share_servers="1"),
make_create_body(spec_driver_handles_share_servers="0"),
make_create_body(spec_driver_handles_share_servers="True"),
make_create_body(spec_driver_handles_share_servers="False"),
make_create_body(spec_driver_handles_share_servers="FalsE"))
def test_create(self, body):
req = fakes.HTTPRequest.blank('/v2/fake/types')
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 0)
self.assertEqual(0, len(fake_notifier.NOTIFICATIONS))
res_dict = self.controller._create(req, body)
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 1)
self.assertEqual(1, len(fake_notifier.NOTIFICATIONS))
self.assertEqual(2, len(res_dict))
self.assertEqual('share_type_1', res_dict['share_type']['name'])
self.assertEqual('share_type_1', res_dict['volume_type']['name'])
def test_create_with_too_small_name(self):
@ddt.data(None,
make_create_body(""),
make_create_body("n" * 256),
{'foo': {'a': 'b'}},
{'share_type': 'string'},
make_create_body(spec_driver_handles_share_servers=None),
make_create_body(spec_driver_handles_share_servers=""),
make_create_body(spec_driver_handles_share_servers=[]),
)
def test_create_invalid_request(self, body):
req = fakes.HTTPRequest.blank('/v2/fake/types')
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 0)
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller._create, req, make_create_body(""))
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 0)
def test_create_with_too_big_name(self):
req = fakes.HTTPRequest.blank('/v2/fake/types')
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 0)
self.assertRaises(webob.exc.HTTPBadRequest, self.controller._create,
req, make_create_body("n" * 256))
self.assertEqual(len(fake_notifier.NOTIFICATIONS), 0)
def _create_share_type_bad_body(self, body):
req = fakes.HTTPRequest.blank('/v2/fake/types')
req.method = 'POST'
self.assertEqual(0, len(fake_notifier.NOTIFICATIONS))
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller._create, req, body)
def test_create_no_body(self):
self._create_share_type_bad_body(body=None)
def test_create_missing_volume(self):
self._create_share_type_bad_body(body={'foo': {'a': 'b'}})
def test_create_malformed_entity(self):
self._create_share_type_bad_body(body={'share_type': 'string'})
self.assertEqual(0, len(fake_notifier.NOTIFICATIONS))

15
manila/tests/api/v1/test_share_types.py
View File

@ -19,6 +19,7 @@ import webob
from manila.api.v1 import share_types as types
from manila.api.views import types as views_types
from manila.common import constants
from manila import exception
from manila.share import share_types
from manila import test
@ -31,12 +32,16 @@ def stub_share_type(id):
"key2": "value2",
"key3": "value3",
"key4": "value4",
"key5": "value5"
"key5": "value5",
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: "true",
}
return dict(
id=id,
name='share_type_%s' % str(id),
extra_specs=specs,
required_extra_specs={
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: "true",
}
)
@ -83,6 +88,10 @@ class ShareTypesApiTest(test.TestCase):
self.assertEqual(set(actual_names), set(expected_names))
for entry in res_dict['share_types']:
self.assertEqual('value1', entry['extra_specs']['key1'])
self.assertTrue('required_extra_specs' in entry)
required_extra_spec = entry['required_extra_specs'].get(
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS, '')
self.assertEqual('true', required_extra_spec)
def test_share_types_index_no_data(self):
self.mock_object(share_types, 'get_all_types',
@ -142,6 +151,7 @@ class ShareTypesApiTest(test.TestCase):
updated_at=now,
extra_specs={},
deleted_at=None,
required_extra_specs={},
id=42,
)
@ -152,6 +162,7 @@ class ShareTypesApiTest(test.TestCase):
expected_share_type = dict(
name='new_type',
extra_specs={},
required_extra_specs={},
id=42,
)
self.assertDictMatch(output['share_type'], expected_share_type)
@ -169,6 +180,7 @@ class ShareTypesApiTest(test.TestCase):
created_at=now,
updated_at=now,
extra_specs={},
required_extra_specs={},
deleted_at=None,
id=42 + i
)
@ -182,6 +194,7 @@ class ShareTypesApiTest(test.TestCase):
expected_share_type = dict(
name='new_type',
extra_specs={},
required_extra_specs={},
id=42 + i
)
self.assertDictMatch(output['share_types'][i],

90
manila/tests/share/test_share_types.py
View File

@ -1,5 +1,6 @@
# Copyright 2015 Deutsche Telekom AG. All rights reserved.
# Copyright 2015 Tom Barron. All rights reserved.
# Copyright 2015 Mirantis, Inc. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
@ -15,17 +16,29 @@
"""Test of Share Type methods for Manila."""
import copy
import datetime
import ddt
import mock
from manila.common import constants
from manila import context
from manila import db
from manila import exception
from manila.share import share_types
from manila import test
def create_share_type_dict(extra_specs=None):
return {
'fake_type': {
'name': 'fake1',
'extra_specs': extra_specs
}
}
@ddt.ddt
class ShareTypesTestCase(test.TestCase):
@ -35,6 +48,7 @@ class ShareTypesTestCase(test.TestCase):
'deleted': '0',
'deleted_at': None,
'extra_specs': {},
'required_extra_specs': {},
'id': u'fooid-1',
'name': u'test',
'updated_at': None
@ -48,13 +62,32 @@ class ShareTypesTestCase(test.TestCase):
'deleted': '0',
'deleted_at': None,
'extra_specs': fake_extra_specs,
'required_extra_specs': {},
'id': fake_share_type_id,
'name': u'test_with_extra',
'updated_at': None
}
}
fake_types = dict(fake_type.items() + fake_type_w_extra.items())
fake_type_w_valid_extra = {
'test_with_extra': {
'created_at': datetime.datetime(2015, 1, 22, 11, 45, 31),
'deleted': '0',
'deleted_at': None,
'extra_specs': {
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true'
},
'required_extra_specs': {
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true'
},
'id': u'fooid-2',
'name': u'test_with_extra',
'updated_at': None
}
}
fake_types = dict(fake_type.items() + fake_type_w_extra.items()
+ fake_type_w_valid_extra.items())
fake_share = {'id': u'fooid-1', 'share_type_id': fake_share_type_id}
@ -66,7 +99,7 @@ class ShareTypesTestCase(test.TestCase):
def test_get_all_types(self, share_type):
self.mock_object(db,
'share_type_get_all',
mock.Mock(return_value=share_type))
mock.Mock(return_value=copy.deepcopy(share_type)))
returned_type = share_types.get_all_types(self.context)
self.assertItemsEqual(share_type, returned_type)
@ -128,3 +161,56 @@ class ShareTypesTestCase(test.TestCase):
self.assertEqual(expected, spec_value)
share_types.get_share_type_extra_specs.assert_called_once_with(
self.fake_share_type_id)
@ddt.data({},
{"fake": "fake"},
{constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: None})
def test_create_without_required_extra_spec(self, extra_specs):
name = "fake_share_type"
self.assertRaises(exception.InvalidShareType, share_types.create,
self.context, name, extra_specs)
def test_get_share_type_required_extra_specs(self):
valid_required_extra_specs = (
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS,)
actual_result = share_types.get_required_extra_specs()
self.assertEqual(valid_required_extra_specs, actual_result)
def test_validate_required_extra_spec_other(self):
actual_result = share_types.is_valid_required_extra_spec(
'fake', 'fake')
self.assertEqual(None, actual_result)
@ddt.data('1', 'True', 'false', '0', True, False)
def test_validate_required_extra_spec_valid(self, value):
key = constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
actual_result = share_types.is_valid_required_extra_spec(key, value)
self.assertEqual(True, actual_result)
@ddt.data('invalid', {}, '0000000000')
def test_validate_required_extra_spec_invalid(self, value):
key = constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS
actual_result = share_types.is_valid_required_extra_spec(key, value)
self.assertEqual(False, actual_result)
@ddt.data({constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true'},
{constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true',
'another_key': True})
def test_get_valid_required_extra_specs_valid(self, specs):
actual_result = share_types.get_valid_required_extra_specs(specs)
valid_result = {
constants.ExtraSpecs.DRIVER_HANDLES_SHARE_SERVERS: 'true'
}
self.assertEqual(valid_result, actual_result)
@ddt.data(None, {})
def test_get_valid_required_extra_specs_invalid(self, specs):
self.assertRaises(exception.InvalidExtraSpec,
share_types.get_valid_required_extra_specs, specs)