Fix server delete attempt along with share net deletion

This bug was introduced when manage/unmange servers with DHSS=True
was merged. This behavior is expected except in two scenarios:

1. If the share server was managed;
2. If any share belonging to this share server was unmanaged;

In both cases, the `is_auto_deletable` field is set to False. This
field was added to prevent the share server from being deleted by
the periodic cleanup job.

This patch fixes the bug by checking the `is_auto_deletable` field
when the share network deletion triggers the share server deletion.

APIImpact

Change-Id: Ib83b8222100fd5a3d718ad2cb7887b1d79ebd546
Depends-On: I7d330b8b5fb3f08d3cbdae2c1735e266e75a70d3
Closes-bug: #1820118
This commit is contained in:
silvacarloss 2019-03-18 14:14:51 -03:00
parent 4b6cfcf690
commit f4fc7aa08a
3 changed files with 62 additions and 0 deletions

View File

@ -61,6 +61,10 @@ class ShareNetworkController(wsgi.Controller):
return self._view_builder.build_share_network(req, share_network)
def _all_share_servers_are_auto_deletable(self, share_network):
return all([ss['is_auto_deletable'] for ss
in share_network['share_servers']])
def delete(self, req, id):
"""Delete specified share network."""
context = req.environ['manila.context']
@ -90,6 +94,16 @@ class ShareNetworkController(wsgi.Controller):
LOG.error(msg)
raise exc.HTTPConflict(explanation=msg)
# NOTE(silvacarlose): Do not allow the deletion of any share server
# if one of them has the flag is_auto_deletable = False
if not self._all_share_servers_are_auto_deletable(share_network):
msg = _("The service cannot determine if there are any "
"non-managed shares on the share network %(id)s, so it "
"cannot be deleted. Please contact the cloud "
"administrator to rectify.") % {'id': id}
LOG.error(msg)
raise exc.HTTPConflict(explanation=msg)
for share_server in share_network['share_servers']:
self.share_rpcapi.delete_share_server(context, share_server)
db_api.share_network_delete(context, id)

View File

@ -2605,6 +2605,17 @@ class ShareManager(manager.SchedulerDependentManager):
if share_server and share_server['is_auto_deletable']:
self.db.share_server_update(context, share_server['id'],
{'is_auto_deletable': False})
msg = ("Since share %(share)s has been un-managed from share "
"server %(server)s. This share server must be removed "
"manually, either by un-managing or by deleting it. The "
"share network %(network)s cannot be deleted unless this "
"share server has been removed.")
msg_args = {
'share': share_id,
'server': share_server['id'],
'network': share_server['share_network_id']
}
LOG.warning(msg, msg_args)
LOG.info("Share %s: unmanaged successfully.", share_id)

View File

@ -211,6 +211,9 @@ class ShareNetworkAPITest(test.TestCase):
self.mock_object(db_api, 'share_instances_get_all_by_share_network',
mock.Mock(return_value=[]))
self.mock_object(self.controller.share_rpcapi, 'delete_share_server')
self.mock_object(self.controller,
'_all_share_servers_are_auto_deletable',
mock.Mock(return_value=True))
self.mock_object(db_api, 'share_network_delete')
self.controller.delete(self.req, share_nw['id'])
@ -246,6 +249,9 @@ class ShareNetworkAPITest(test.TestCase):
mock.Mock(return_value=share_nw))
self.mock_object(db_api, 'share_instances_get_all_by_share_network',
mock.Mock(return_value=[]))
self.mock_object(self.controller,
'_all_share_servers_are_auto_deletable',
mock.Mock(return_value=True))
self.mock_object(self.controller.share_rpcapi, 'delete_share_server')
self.mock_object(db_api, 'share_network_delete')
self.mock_object(share_networks.QUOTAS, 'reserve',
@ -307,6 +313,37 @@ class ShareNetworkAPITest(test.TestCase):
db_api.share_network_get.assert_called_once_with(
self.req.environ['manila.context'], share_nw['id'])
def test_delete_contains_is_auto_deletable_false_servers(self):
share_nw = fake_share_network.copy()
self.mock_object(db_api, 'share_network_get',
mock.Mock(return_value=share_nw))
self.mock_object(db_api, 'count_share_groups_in_share_network')
self.mock_object(share_networks.ShareNetworkController,
'_all_share_servers_are_auto_deletable',
mock.Mock(return_value=False))
self.assertRaises(webob_exc.HTTPConflict,
self.controller.delete,
self.req,
share_nw['id'])
db_api.share_network_get.assert_called_once_with(
self.req.environ['manila.context'], share_nw['id'])
@ddt.data(
({'share_servers': [{'is_auto_deletable': True},
{'is_auto_deletable': True}]}, True),
({'share_servers': [{'is_auto_deletable': True},
{'is_auto_deletable': False}]}, False),
)
@ddt.unpack
def test__share_servers_are_auto_deletable(self, fake_share_network,
expected_result):
self.assertEqual(
expected_result,
self.controller._all_share_servers_are_auto_deletable(
fake_share_network))
def test_show_nominal(self):
share_nw = 'fake network id'
with mock.patch.object(db_api,