openstack
/
manila
Code Issues Proposed changes

Merge "Fix available share servers to be reused"

This commit is contained in:
Zuul 2022-08-30 00:08:10 +00:00 committed by Gerrit Code Review
parent 8f2303046f 8dc6235b5f
commit d910514004
6 changed files with 103 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
manila/db/api.py
View File

@ -1095,13 +1095,20 @@ def share_server_search_by_identifier(context, identifier, session=None):
def share_server_get_all_by_host_and_share_subnet_valid(context, host,
share_subnet_id,
server_status=None,
session=None):
"""Get share server DB records by host and share net not error."""
return IMPL.share_server_get_all_by_host_and_share_subnet_valid(
context, host, share_subnet_id, session=session)
def share_server_get_all_by_host_and_share_subnet(context, host,
share_subnet_id,
session=None):
"""Get share server DB records by host and share net."""
return IMPL.share_server_get_all_by_host_and_share_subnet(
context, host, share_subnet_id, session=session)
def share_server_get_all(context):
"""Get all share server DB records."""
return IMPL.share_server_get_all(context)

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

@ -4445,24 +4445,21 @@ def share_server_search_by_identifier(context, identifier, session=None):
@require_context
def share_server_get_all_by_host_and_share_subnet_valid(context, host,
share_subnet_id,
server_status=None,
session=None):
query = (_server_get_query(context, session)
.filter_by(host=host)
.filter(models.ShareServer.share_network_subnets.any(
id=share_subnet_id)))
if server_status:
query.filter_by(status=server_status)
else:
query.filter(models.ShareServer.status.in_(
(constants.STATUS_CREATING, constants.STATUS_ACTIVE)))
result = (_server_get_query(context, session)
.filter_by(host=host)
.filter(models.ShareServer.share_network_subnets.any(
id=share_subnet_id))
.filter(models.ShareServer.status.in_(
(constants.STATUS_CREATING,
constants.STATUS_ACTIVE))).all())
result = query.all()
if not result:
filters_description = ('share_network_subnet_id is "%(share_net_id)s",'
' host is "%(host)s" and status in'
' "%(status_cr)s" or "%(status_act)s"') % {
'share_net_id': share_subnet_id,
filters_description = ('share_network_subnet_id is '
'"%(share_subnet_id)s", host is "%(host)s" and '
'status in "%(status_cr)s" or '
'"%(status_act)s"') % {
'share_subnet_id': share_subnet_id,
'host': host,
'status_cr': constants.STATUS_CREATING,
'status_act': constants.STATUS_ACTIVE,
@ -4472,6 +4469,27 @@ def share_server_get_all_by_host_and_share_subnet_valid(context, host,
return result
@require_context
def share_server_get_all_by_host_and_share_subnet(context, host,
share_subnet_id,
session=None):
result = (_server_get_query(context, session)
.filter_by(host=host)
.filter(models.ShareServer.share_network_subnets.any(
id=share_subnet_id)).all())
if not result:
filters_description = ('share_network_subnet_id is '
'"%(share_subnet_id)s" and host is '
'"%(host)s".') % {
'share_subnet_id': share_subnet_id,
'host': host,
}
raise exception.ShareServerNotFoundByFilters(
filters_description=filters_description)
return result
@require_context
def share_server_get_all(context):
return _server_get_query(context).all()

5
manila/share/manager.py
View File

@ -6155,9 +6155,8 @@ class ShareManager(manager.SchedulerDependentManager):
current_subnets = [subnet for subnet in current_subnets
if subnet['id'] != new_share_network_subnet_id]
share_servers = (
self.db.share_server_get_all_by_host_and_share_subnet_valid(
context, self.host, new_share_network_subnet_id,
server_status=constants.STATUS_SERVER_NETWORK_CHANGE))
self.db.share_server_get_all_by_host_and_share_subnet(
context, self.host, new_share_network_subnet_id))
for share_server in share_servers:
share_server_id = share_server['id']

63
manila/tests/db/sqlalchemy/test_api.py
View File

@ -3314,8 +3314,7 @@ class ShareServerDatabaseAPITestCase(test.TestCase):
db_api.share_server_update,
self.ctxt, fake_id, {})
@ddt.data(None, constants.STATUS_SERVER_NETWORK_CHANGE)
def test_get_all_by_host_and_share_net_valid(self, server_status):
def test_get_all_by_host_and_share_subnet_valid(self):
subnet_1 = {
'id': '1',
'share_network_id': '1',
@ -3326,16 +3325,11 @@ class ShareServerDatabaseAPITestCase(test.TestCase):
}
share_net_subnets1 = db_utils.create_share_network_subnet(**subnet_1)
share_net_subnets2 = db_utils.create_share_network_subnet(**subnet_2)
valid_no_status = {
valid = {
'share_network_subnets': [share_net_subnets1],
'host': 'host1',
'status': constants.STATUS_ACTIVE,
}
valid_with_status = {
'share_network_subnets': [share_net_subnets1],
'host': 'host1',
'status': constants.STATUS_SERVER_NETWORK_CHANGE,
}
invalid = {
'share_network_subnets': [share_net_subnets2],
'host': 'host1',
@ -3346,28 +3340,67 @@ class ShareServerDatabaseAPITestCase(test.TestCase):
'host': 'host2',
'status': constants.STATUS_ACTIVE,
}
if server_status:
valid = db_utils.create_share_server(**valid_with_status)
else:
valid = db_utils.create_share_server(**valid_no_status)
valid = db_utils.create_share_server(**valid)
db_utils.create_share_server(**invalid)
db_utils.create_share_server(**other)
servers = db_api.share_server_get_all_by_host_and_share_subnet_valid(
self.ctxt,
host='host1',
share_subnet_id='1',
server_status=server_status)
share_subnet_id='1')
self.assertEqual(valid['id'], servers[0]['id'])
def test_get_all_by_host_and_share_net_not_found(self):
def test_get_all_by_host_and_share_subnet_valid_not_found(self):
self.assertRaises(
exception.ShareServerNotFound,
db_api.share_server_get_all_by_host_and_share_subnet_valid,
self.ctxt, host='fake', share_subnet_id='fake'
)
def test_get_all_by_host_and_share_subnet(self):
subnet_1 = {
'id': '1',
'share_network_id': '1',
}
share_net_subnets1 = db_utils.create_share_network_subnet(**subnet_1)
valid = {
'share_network_subnets': [share_net_subnets1],
'host': 'host1',
'status': constants.STATUS_SERVER_NETWORK_CHANGE,
}
other = {
'share_network_subnets': [share_net_subnets1],
'host': 'host1',
'status': constants.STATUS_ERROR,
}
invalid = {
'share_network_subnets': [share_net_subnets1],
'host': 'host2',
'status': constants.STATUS_ACTIVE,
}
valid = db_utils.create_share_server(**valid)
invalid = db_utils.create_share_server(**invalid)
other = db_utils.create_share_server(**other)
servers = db_api.share_server_get_all_by_host_and_share_subnet(
self.ctxt,
host='host1',
share_subnet_id='1')
self.assertEqual(2, len(servers))
ids = [s['id'] for s in servers]
self.assertTrue(valid['id'] in ids)
self.assertTrue(other['id'] in ids)
self.assertFalse(invalid['id'] in ids)
def test_get_all_by_host_and_share_subnet_not_found(self):
self.assertRaises(
exception.ShareServerNotFound,
db_api.share_server_get_all_by_host_and_share_subnet,
self.ctxt, host='fake', share_subnet_id='fake'
)
def test_get_all(self):
srv1 = {
'host': 'host1',

10
manila/tests/share/test_manager.py
View File

@ -10018,7 +10018,7 @@ class ShareManagerTestCase(test.TestCase):
server = {'id': server_id}
mock_servers_get = self.mock_object(
self.share_manager.db,
'share_server_get_all_by_host_and_share_subnet_valid',
'share_server_get_all_by_host_and_share_subnet',
mock.Mock(return_value=[server]))
current_network_allocations = 'fake_current_net_allocations'
mock_form_net_allocations = self.mock_object(
@ -10054,8 +10054,7 @@ class ShareManagerTestCase(test.TestCase):
self.context, net_id, new_subnet['availability_zone_id'],
fallback_to_default=False)
mock_servers_get.assert_called_once_with(
self.context, self.share_manager.host, new_share_network_subnet_id,
server_status=constants.STATUS_SERVER_NETWORK_CHANGE)
self.context, self.share_manager.host, new_share_network_subnet_id)
mock_form_net_allocations.assert_called_once_with(
self.context, server['id'], subnets)
mock_instances_get.assert_called_once_with(
@ -10089,7 +10088,7 @@ class ShareManagerTestCase(test.TestCase):
server = {'id': server_id}
mock_servers_get = self.mock_object(
self.share_manager.db,
'share_server_get_all_by_host_and_share_subnet_valid',
'share_server_get_all_by_host_and_share_subnet',
mock.Mock(return_value=[server]))
current_network_allocations = 'fake_current_net_allocations'
mock_form_net_allocations = self.mock_object(
@ -10131,8 +10130,7 @@ class ShareManagerTestCase(test.TestCase):
self.context, net_id, new_subnet['availability_zone_id'],
fallback_to_default=False)
mock_servers_get.assert_called_once_with(
self.context, self.share_manager.host, new_share_network_subnet_id,
server_status=constants.STATUS_SERVER_NETWORK_CHANGE)
self.context, self.share_manager.host, new_share_network_subnet_id)
mock_form_net_allocations.assert_called_once_with(
self.context, server['id'], subnets)
mock_instances_get.assert_called_once_with(

8
releasenotes/notes/bug-1978962-fix-find-available-servers-2dec3a4f3f0ef7e4.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
Drivers using DHSS True mode has the server creation phase. This phase
tries to reuse one of available share servers, however, the Manila code
is considering all share servers states as available, rather than
considering only the active or creating ones. Now, only the correct share
servers are passed to drivers as available to be reused.