Fix multiple export locations during migration
During a migration, the destination share instance export location was being shown, thus the user could get confused as to which export location to choose when mounting his share, since only the source could be mounted. This patch addresses this by filtering the destination export location out. APIImpact Change-Id: I20cf1a76399693d751fa87fc4df375162e7c5f1d Closes-bug: #1660726
This commit is contained in:
parent
595a2bd73c
commit
53b91e513e
@ -45,7 +45,8 @@ class ShareExportLocationController(wsgi.Controller):
|
||||
context = req.environ['manila.context']
|
||||
self._verify_share(context, share_id)
|
||||
export_locations = db_api.share_export_locations_get_by_share_id(
|
||||
context, share_id, include_admin_only=context.is_admin)
|
||||
context, share_id, include_admin_only=context.is_admin,
|
||||
ignore_migration_destination=True)
|
||||
return self._view_builder.summary_list(req, export_locations)
|
||||
|
||||
@wsgi.Controller.api_version('2.9')
|
||||
|
@ -676,10 +676,12 @@ def share_export_locations_get(context, share_id):
|
||||
|
||||
|
||||
def share_export_locations_get_by_share_id(context, share_id,
|
||||
include_admin_only=True):
|
||||
include_admin_only=True,
|
||||
ignore_migration_destination=False):
|
||||
"""Get all export locations of a share by its ID."""
|
||||
return IMPL.share_export_locations_get_by_share_id(
|
||||
context, share_id, include_admin_only=include_admin_only)
|
||||
context, share_id, include_admin_only=include_admin_only,
|
||||
ignore_migration_destination=ignore_migration_destination)
|
||||
|
||||
|
||||
def share_export_locations_get_by_share_instance_id(context,
|
||||
|
@ -2643,9 +2643,14 @@ def _share_export_locations_get(context, share_instance_ids,
|
||||
@require_context
|
||||
@require_share_exists
|
||||
def share_export_locations_get_by_share_id(context, share_id,
|
||||
include_admin_only=True):
|
||||
include_admin_only=True,
|
||||
ignore_migration_destination=False):
|
||||
share = share_get(context, share_id)
|
||||
ids = [instance.id for instance in share.instances]
|
||||
if ignore_migration_destination:
|
||||
ids = [instance.id for instance in share.instances
|
||||
if instance['status'] != constants.STATUS_MIGRATING_TO]
|
||||
else:
|
||||
ids = [instance.id for instance in share.instances]
|
||||
rows = _share_export_locations_get(
|
||||
context, ids, include_admin_only=include_admin_only)
|
||||
return rows
|
||||
|
@ -1396,11 +1396,27 @@ class ShareInstanceExportLocationsMetadataDatabaseAPITestCase(test.TestCase):
|
||||
def setUp(self):
|
||||
super(self.__class__, self).setUp()
|
||||
self.ctxt = context.get_admin_context()
|
||||
self.share = db_utils.create_share()
|
||||
share_id = 'fake_share_id'
|
||||
instances = [
|
||||
db_utils.create_share_instance(
|
||||
share_id=share_id,
|
||||
status=constants.STATUS_AVAILABLE),
|
||||
db_utils.create_share_instance(
|
||||
share_id=share_id,
|
||||
status=constants.STATUS_MIGRATING),
|
||||
db_utils.create_share_instance(
|
||||
share_id=share_id,
|
||||
status=constants.STATUS_MIGRATING_TO),
|
||||
]
|
||||
self.share = db_utils.create_share(
|
||||
id=share_id,
|
||||
instances=instances)
|
||||
self.initial_locations = ['/fake/foo/', '/fake/bar', '/fake/quuz']
|
||||
db_api.share_export_locations_update(
|
||||
self.ctxt, self.share.instance['id'], self.initial_locations,
|
||||
delete=False)
|
||||
self.shown_locations = ['/fake/foo/', '/fake/bar']
|
||||
for i in range(0, 3):
|
||||
db_api.share_export_locations_update(
|
||||
self.ctxt, instances[i]['id'], self.initial_locations[i],
|
||||
delete=False)
|
||||
|
||||
def _get_export_location_uuid_by_path(self, path):
|
||||
els = db_api.share_export_locations_get_by_share_id(
|
||||
@ -1416,14 +1432,21 @@ class ShareInstanceExportLocationsMetadataDatabaseAPITestCase(test.TestCase):
|
||||
els = db_api.share_export_locations_get_by_share_id(
|
||||
self.ctxt, self.share.id)
|
||||
self.assertEqual(3, len(els))
|
||||
for path in self.initial_locations:
|
||||
for path in self.shown_locations:
|
||||
self.assertTrue(any([path in el.path for el in els]))
|
||||
|
||||
def test_get_export_locations_by_share_id_ignore_migration_dest(self):
|
||||
els = db_api.share_export_locations_get_by_share_id(
|
||||
self.ctxt, self.share.id, ignore_migration_destination=True)
|
||||
self.assertEqual(2, len(els))
|
||||
for path in self.shown_locations:
|
||||
self.assertTrue(any([path in el.path for el in els]))
|
||||
|
||||
def test_get_export_locations_by_share_instance_id(self):
|
||||
els = db_api.share_export_locations_get_by_share_instance_id(
|
||||
self.ctxt, self.share.instance.id)
|
||||
self.assertEqual(3, len(els))
|
||||
for path in self.initial_locations:
|
||||
self.assertEqual(1, len(els))
|
||||
for path in [self.shown_locations[1]]:
|
||||
self.assertTrue(any([path in el.path for el in els]))
|
||||
|
||||
def test_export_location_metadata_update_delete(self):
|
||||
|
@ -0,0 +1,4 @@
|
||||
---
|
||||
fixes:
|
||||
- Export locations pertaining to migration destinations
|
||||
are no longer shown until migration is complete.
|
Loading…
Reference in New Issue
Block a user