Add share state verification for API 'unmanage'
Shares with transitional states like 'creating' or 'deleting' should not be allowed to be unmanaged. For the moment "manage/unmanage" operation is allowed only for driver mode without handling of share servers. So, API operations against shares created on top of share servers should be restricted on API level. Change-Id: I73b554448eadcb96ef00f44535014e14dea91472 Closes-Bug: #1434642 Closes-Bug: #1434511
This commit is contained in:
parent
449eef25df
commit
a0703047d5
@ -19,7 +19,9 @@ from webob import exc
|
||||
|
||||
from manila.api import extensions
|
||||
from manila.api.openstack import wsgi
|
||||
from manila.common import constants
|
||||
from manila import exception
|
||||
from manila.i18n import _
|
||||
from manila.i18n import _LI
|
||||
from manila import share
|
||||
|
||||
@ -43,6 +45,18 @@ class ShareUnmanageController(wsgi.Controller):
|
||||
|
||||
try:
|
||||
share = self.share_api.get(context, id)
|
||||
if share.get('share_server_id'):
|
||||
msg = _("Operation 'unmanage' is not supported for shares "
|
||||
"that are created on top of share servers "
|
||||
"(created with share-networks).")
|
||||
raise exc.HTTPForbidden(explanation=msg)
|
||||
# NOTE(vponomaryov): use 'upper' translation because share
|
||||
# statuses not always used from common place yet.
|
||||
elif share['status'].upper() in constants.TRANSITIONAL_STATUSES:
|
||||
msg = _("Share with transitional state can not be unmanaged. "
|
||||
"Share '%(s_id)s' is in '%(state)s' state.") % dict(
|
||||
state=share['status'], s_id=share['id'])
|
||||
raise exc.HTTPForbidden(explanation=msg)
|
||||
self.share_api.unmanage(context, share)
|
||||
except exception.NotFound as e:
|
||||
raise exc.HTTPNotFound(explanation=six.text_type(e))
|
||||
|
@ -18,6 +18,8 @@ STATUS_CREATING = 'CREATING'
|
||||
STATUS_DELETING = 'DELETING'
|
||||
STATUS_DELETED = 'DELETED'
|
||||
STATUS_ERROR = 'ERROR'
|
||||
STATUS_ERROR_DELETING = 'ERROR_DELETING'
|
||||
STATUS_AVAILABLE = 'AVAILABLE'
|
||||
STATUS_ACTIVE = 'ACTIVE'
|
||||
STATUS_INACTIVE = 'INACTIVE'
|
||||
STATUS_ACTIVATING = 'ACTIVATING'
|
||||
@ -28,6 +30,12 @@ STATUS_UNMANAGING = 'UNMANAGE_STARTING'
|
||||
STATUS_UNMANAGE_ERROR = 'UNMANAGE_ERROR'
|
||||
STATUS_UNMANAGED = 'UNMANAGED'
|
||||
|
||||
TRANSITIONAL_STATUSES = (
|
||||
STATUS_CREATING, STATUS_DELETING,
|
||||
STATUS_ACTIVATING, STATUS_DEACTIVATING,
|
||||
STATUS_MANAGING, STATUS_UNMANAGING,
|
||||
)
|
||||
|
||||
SUPPORTED_SHARE_PROTOCOLS = (
|
||||
'NFS', 'CIFS', 'GLUSTERFS', 'HDFS')
|
||||
|
||||
|
@ -18,6 +18,7 @@ import mock
|
||||
import webob
|
||||
|
||||
from manila.api.contrib import share_unmanage
|
||||
from manila.common import constants
|
||||
from manila import exception
|
||||
from manila.share import api as share_api
|
||||
from manila import test
|
||||
@ -46,13 +47,41 @@ class ShareUnmanageTest(test.TestCase):
|
||||
)
|
||||
|
||||
def test_unmanage_share(self):
|
||||
self.mock_object(share_api.API, 'get', mock.Mock(return_value={}))
|
||||
share = dict(status=constants.STATUS_AVAILABLE, id='foo_id')
|
||||
self.mock_object(share_api.API, 'get', mock.Mock(return_value=share))
|
||||
self.mock_object(share_api.API, 'unmanage', mock.Mock())
|
||||
|
||||
actual_result = self.controller.unmanage(self.request, self.share_id)
|
||||
|
||||
self.assertEqual(202, actual_result.status_int)
|
||||
|
||||
def test_unmanage_share_based_on_share_server(self):
|
||||
share = dict(share_server_id='foo_id', id='bar_id')
|
||||
self.mock_object(
|
||||
self.controller.share_api, 'get',
|
||||
mock.Mock(return_value=share))
|
||||
|
||||
self.assertRaises(
|
||||
webob.exc.HTTPForbidden,
|
||||
self.controller.unmanage, self.request, share['id'])
|
||||
|
||||
self.controller.share_api.get.assert_called_once_with(
|
||||
self.request.environ['manila.context'], share['id'])
|
||||
|
||||
@ddt.data(*constants.TRANSITIONAL_STATUSES)
|
||||
def test_unmanage_share_with_transitional_state(self, share_status):
|
||||
share = dict(status=share_status, id='foo_id')
|
||||
self.mock_object(
|
||||
self.controller.share_api, 'get',
|
||||
mock.Mock(return_value=share))
|
||||
|
||||
self.assertRaises(
|
||||
webob.exc.HTTPForbidden,
|
||||
self.controller.unmanage, self.request, share['id'])
|
||||
|
||||
self.controller.share_api.get.assert_called_once_with(
|
||||
self.request.environ['manila.context'], share['id'])
|
||||
|
||||
def test_unmanage_share_not_found(self):
|
||||
self.mock_object(share_api.API, 'get', mock.Mock(
|
||||
side_effect=exception.NotFound))
|
||||
@ -65,7 +94,8 @@ class ShareUnmanageTest(test.TestCase):
|
||||
@ddt.data(exception.InvalidShare(reason="fake"),
|
||||
exception.PolicyNotAuthorized(action="fake"),)
|
||||
def test_unmanage_share_invalid(self, side_effect):
|
||||
self.mock_object(share_api.API, 'get', mock.Mock(return_value={}))
|
||||
share = dict(status=constants.STATUS_AVAILABLE, id='foo_id')
|
||||
self.mock_object(share_api.API, 'get', mock.Mock(return_value=share))
|
||||
self.mock_object(share_api.API, 'unmanage', mock.Mock(
|
||||
side_effect=side_effect))
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user