Browse Source

Merge "fix unmange share with manage_error status will lead to quota error" into stable/victoria

changes/37/793437/1
Zuul 3 weeks ago
committed by Gerrit Code Review
parent
commit
696e039b76
8 changed files with 59 additions and 41 deletions
  1. +1
    -0
      manila/common/constants.py
  2. +5
    -4
      manila/scheduler/manager.py
  3. +6
    -2
      manila/share/api.py
  4. +34
    -28
      manila/share/manager.py
  5. +3
    -3
      manila/tests/api/v1/test_share_unmanage.py
  6. +1
    -1
      manila/tests/scheduler/test_manager.py
  7. +3
    -3
      manila/tests/share/test_manager.py
  8. +6
    -0
      releasenotes/notes/bug-1883506-fix-delete-manage-error-share-will-lead-to-quota-error-085fd3b7d15ae109.yaml

+ 1
- 0
manila/common/constants.py View File

@ -28,6 +28,7 @@ STATUS_INACTIVE = 'inactive'
STATUS_MANAGING = 'manage_starting'
STATUS_MANAGE_ERROR = 'manage_error'
STATUS_UNMANAGING = 'unmanage_starting'
STATUS_MANAGE_ERROR_UNMANAGING = 'manage_error_unmanage_starting'
STATUS_UNMANAGE_ERROR = 'unmanage_error'
STATUS_UNMANAGED = 'unmanaged'
STATUS_EXTENDING = 'extending'


+ 5
- 4
manila/scheduler/manager.py View File

@ -129,12 +129,13 @@ class SchedulerManager(manager.Manager):
"""Ensure that the host exists and can accept the share."""
def _manage_share_set_error(self, context, ex, request_spec):
# NOTE(nidhimittalhada): set size as 1 because design expects size
# to be set, it also will allow us to handle delete/unmanage
# operations properly with this errored share according to quotas.
# NOTE(haixin) if failed to scheduler backend for manage share,
# and we do not commit quota usages here, so we should set size 0
# because we don't know the real size of the size, and we will
# skip quota cuts when unmanage share with manage_error status.
self._set_share_state_and_notify(
'manage_share',
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1},
{'status': constants.STATUS_MANAGE_ERROR, 'size': 0},
context, ex, request_spec)
share_ref = db.share_get(context, share_id)


+ 6
- 2
manila/share/api.py View File

@ -916,7 +916,12 @@ class API(base.Base):
self._check_is_share_busy(share)
update_data = {'status': constants.STATUS_UNMANAGING,
if share['status'] == constants.STATUS_MANAGE_ERROR:
update_status = constants.STATUS_MANAGE_ERROR_UNMANAGING
else:
update_status = constants.STATUS_UNMANAGING
update_data = {'status': update_status,
'terminated_at': timeutils.utcnow()}
share_ref = self.db.share_update(context, share['id'], update_data)
@ -1128,7 +1133,6 @@ class API(base.Base):
_("Share still has %d dependent share group snapshot "
"members.") % share_group_snapshot_members_count)
raise exception.InvalidShare(reason=msg)
self._check_is_share_busy(share)
for share_instance in share.instances:
if share_instance['host']:


+ 34
- 28
manila/share/manager.py View File

@ -2589,6 +2589,8 @@ class ShareManager(manager.SchedulerDependentManager):
)
if not share_update.get('size'):
# NOTE(haixin)if failed to get real size of share, will not
# commit quota usages.
msg = _("Driver cannot calculate share size.")
raise exception.InvalidShare(reason=msg)
@ -2639,12 +2641,12 @@ class ShareManager(manager.SchedulerDependentManager):
self.db.share_update(context, share_id, share_update)
except Exception:
# NOTE(vponomaryov): set size as 1 because design expects size
# to be set, it also will allow us to handle delete/unmanage
# operations properly with this errored share according to quotas.
# NOTE(haixin) we should set size 0 because we don't know the real
# size of the size, and we will skip quota cuts when
# delete/unmanage share.
self.db.share_update(
context, share_id,
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
{'status': constants.STATUS_MANAGE_ERROR, 'size': 0})
raise
@add_hooks
@ -2767,30 +2769,34 @@ class ShareManager(manager.SchedulerDependentManager):
("Share can not be unmanaged: %s."), e)
return
deltas = {
'project_id': project_id,
'shares': -1,
'gigabytes': -share_ref['size'],
'share_type_id': share_instance['share_type_id'],
}
# NOTE(carloss): while unmanaging a share, a share will not contain
# replicas other than the active one. So there is no need to
# recalculate the amount of share replicas to be deallocated.
if supports_replication:
deltas.update({'share_replicas': -1,
'replica_gigabytes': -share_ref['size']})
try:
reservations = QUOTAS.reserve(context, **deltas)
QUOTAS.commit(
context, reservations, project_id=project_id,
share_type_id=share_instance['share_type_id'],
)
except Exception as e:
# Note(imalinovskiy):
# Quota reservation errors here are not fatal, because
# unmanage is administrator API and he/she could update user
# quota usages later if it's required.
LOG.warning("Failed to update quota usages: %s.", e)
# NOTE(haixin) we will skip quota cuts when unmanag share with
# 'error_manage' status, because we have not commit quota usages when
# we failed to manage the share.
if share_ref['status'] != constants.STATUS_MANAGE_ERROR_UNMANAGING:
deltas = {
'project_id': project_id,
'shares': -1,
'gigabytes': -share_ref['size'],
'share_type_id': share_instance['share_type_id'],
}
# NOTE(carloss): while unmanaging a share, a share will not
# contain replicas other than the active one. So there is no need
# to recalculate the amount of share replicas to be deallocated.
if supports_replication:
deltas.update({'share_replicas': -1,
'replica_gigabytes': -share_ref['size']})
try:
reservations = QUOTAS.reserve(context, **deltas)
QUOTAS.commit(
context, reservations, project_id=project_id,
share_type_id=share_instance['share_type_id'],
)
except Exception as e:
# Note(imalinovskiy):
# Quota reservation errors here are not fatal, because
# unmanage is administrator API and he/she could update user
# quota usages later if it's required.
LOG.warning("Failed to update quota usages: %s.", e)
if self.configuration.safe_get('unmanage_remove_access_rules'):
try:


+ 3
- 3
manila/tests/api/v1/test_share_unmanage.py View File

@ -52,9 +52,9 @@ class ShareUnmanageTest(test.TestCase):
self.mock_policy_check = self.mock_object(
policy, 'check_policy', mock.Mock(return_value=True))
def test_unmanage_share(self):
share = dict(status=constants.STATUS_AVAILABLE, id='foo_id',
instance={})
@ddt.data(constants.STATUS_AVAILABLE, constants.STATUS_MANAGE_ERROR)
def test_unmanage_share(self, status):
share = dict(status=status, id='foo_id', instance={})
self.mock_object(share_api.API, 'get', mock.Mock(return_value=share))
self.mock_object(share_api.API, 'unmanage', mock.Mock())
self.mock_object(


+ 1
- 1
manila/tests/scheduler/test_manager.py View File

@ -353,7 +353,7 @@ class SchedulerManagerTestCase(test.TestCase):
{'share_id': share_id}, None)
db_update.assert_called_once_with(
self.context, share_id,
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
{'status': constants.STATUS_MANAGE_ERROR, 'size': 0})
def test_create_share_replica_exception_path(self):
"""Test 'raisable' exceptions for create_share_replica."""


+ 3
- 3
manila/tests/share/test_manager.py View File

@ -2631,7 +2631,7 @@ class ShareManagerTestCase(test.TestCase):
self.share_manager.db.share_update.assert_called_once_with(
utils.IsAMatcher(context.RequestContext), share_id,
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
{'status': constants.STATUS_MANAGE_ERROR, 'size': 0})
(self.share_manager._get_extra_specs_from_share_type.
assert_called_once_with(
mock.ANY, share['instance']['share_type_id']))
@ -2739,7 +2739,7 @@ class ShareManagerTestCase(test.TestCase):
assert_called_once_with(mock.ANY, driver_options))
self.share_manager.db.share_update.assert_called_once_with(
utils.IsAMatcher(context.RequestContext), share_id,
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
{'status': constants.STATUS_MANAGE_ERROR, 'size': 0})
(self.share_manager._get_extra_specs_from_share_type.
assert_called_once_with(
mock.ANY, share['instance']['share_type_id']))
@ -2772,7 +2772,7 @@ class ShareManagerTestCase(test.TestCase):
assert_called_once_with(mock.ANY, driver_options))
self.share_manager.db.share_update.assert_called_once_with(
mock.ANY, share_id,
{'status': constants.STATUS_MANAGE_ERROR, 'size': 1})
{'status': constants.STATUS_MANAGE_ERROR, 'size': 0})
(self.share_manager._get_extra_specs_from_share_type.
assert_called_once_with(
mock.ANY, share['instance']['share_type_id']))


+ 6
- 0
releasenotes/notes/bug-1883506-fix-delete-manage-error-share-will-lead-to-quota-error-085fd3b7d15ae109.yaml View File

@ -0,0 +1,6 @@
---
fixes:
- |
Fixed `bug #1883506 <https://bugs.launchpad.net/manila/+bug/18835060>`_
that caused a quota error when delete or unmanage a share that failed to
manage.

Loading…
Cancel
Save