Skip to content

Commit

Permalink
Fix task_state field shown on API < 2.5
Browse files Browse the repository at this point in the history 
After Share Migration was included, the task_state field was
added to be displayed on GET requests, but API versions prior to
2.5 should not see this field. This patch fixes that by using
microversions.

Closes-bug: #1494746
Change-Id: Ie755ae53fe8efdf1702a0ecbabf022a5fe4beb93
  • Loading branch information
Rodrigo Barbieri committed Sep 15, 2015
1 parent 1b9a801 commit 350dfb8
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 57 deletions.
6 changes: 5 additions & 1 deletion manila/api/views/shares.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class ViewBuilder(common.ViewBuilder):
_collection_name = 'shares'
_detail_version_modifiers = [
"add_consistency_group_fields",
"add_task_state_field",
"modify_share_type_field",
]

Expand Down Expand Up @@ -65,7 +66,6 @@ def detail(self, request, share):
'availability_zone': share.get('availability_zone'),
'created_at': share.get('created_at'),
'status': share.get('status'),
'task_state': share.get('task_state'),
'name': share.get('display_name'),
'description': share.get('display_description'),
'project_id': share.get('project_id'),
Expand Down Expand Up @@ -95,6 +95,10 @@ def add_consistency_group_fields(self, share_dict, share):
share_dict['source_cgsnapshot_member_id'] = share.get(
'source_cgsnapshot_member_id')

@common.ViewBuilder.versioned_method("2.5", None, True)
def add_task_state_field(self, share_dict, share):
share_dict['task_state'] = share.get('task_state')

@common.ViewBuilder.versioned_method("2.6")
def modify_share_type_field(self, share_dict, share):
share_type = share.get('share_type_id')
Expand Down
79 changes: 27 additions & 52 deletions manila/tests/api/v1/test_shares.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ def _get_expected_share_detailed_response(self, values=None, admin=False):
'snapshot_id': '2',
'share_network_id': None,
'status': 'fakestatus',
'task_state': None,
'share_type': '1',
'volume_type': '1',
'is_public': False,
Expand Down Expand Up @@ -691,17 +690,11 @@ def test_share_list_detail_with_search_opts_by_non_admin(self):
def test_share_list_detail_with_search_opts_by_admin(self):
self._share_list_detail_with_search_opts(use_admin_context=True)

def test_share_list_detail(self):
self.mock_object(share_api.API, 'get_all',
stubs.stub_share_get_all_by_project)
env = {'QUERY_STRING': 'name=Share+Test+Name'}
req = fakes.HTTPRequest.blank('/shares/detail', environ=env)
res_dict = self.controller.detail(req)
expected = {
def _list_detail_common_expected(self):
return {
'shares': [
{
'status': 'fakestatus',
'task_state': None,
'description': 'displaydesc',
'export_location': 'fake_location',
'export_locations': ['fake_location', 'fake_location2'],
Expand Down Expand Up @@ -732,57 +725,39 @@ def test_share_list_detail(self):
}
]
}

def _list_detail_test_common(self, req, expected):
self.mock_object(share_api.API, 'get_all',
stubs.stub_share_get_all_by_project)
res_dict = self.controller.detail(req)
self.assertEqual(expected, res_dict)
self.assertEqual(res_dict['shares'][0]['volume_type'],
res_dict['shares'][0]['share_type'])

def test_share_list_detail(self):
env = {'QUERY_STRING': 'name=Share+Test+Name'}
req = fakes.HTTPRequest.blank('/shares/detail', environ=env)
expected = self._list_detail_common_expected()
self._list_detail_test_common(req, expected)

def test_share_list_detail_with_consistency_group(self):
self.mock_object(share_api.API, 'get_all',
stubs.stub_share_get_all_by_project)
env = {'QUERY_STRING': 'name=Share+Test+Name'}
req = fakes.HTTPRequest.blank('/shares/detail', environ=env,
version="2.4")
res_dict = self.controller.detail(req)
expected = {
'shares': [
{
'status': 'fakestatus',
'task_state': None,
'description': 'displaydesc',
'export_location': 'fake_location',
'export_locations': ['fake_location', 'fake_location2'],
'availability_zone': 'fakeaz',
'name': 'displayname',
'share_proto': 'FAKEPROTO',
'metadata': {},
'project_id': 'fakeproject',
'host': 'fakehost',
'id': '1',
'snapshot_id': '2',
'share_network_id': None,
'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
'size': 1,
'share_type': '1',
'volume_type': '1',
'is_public': False,
'consistency_group_id': None,
'source_cgsnapshot_member_id': None,
'links': [
{
'href': 'http://localhost/v1/fake/shares/1',
'rel': 'self'
},
{
'href': 'http://localhost/fake/shares/1',
'rel': 'bookmark'
}
],
}
]
}
self.assertEqual(expected, res_dict)
self.assertEqual(res_dict['shares'][0]['volume_type'],
res_dict['shares'][0]['share_type'])
expected = self._list_detail_common_expected()
expected['shares'][0]['consistency_group_id'] = None
expected['shares'][0]['source_cgsnapshot_member_id'] = None
self._list_detail_test_common(req, expected)

def test_share_list_detail_with_task_state(self):
env = {'QUERY_STRING': 'name=Share+Test+Name'}
req = fakes.HTTPRequest.blank('/shares/detail', environ=env,
version="2.5", experimental=True)
expected = self._list_detail_common_expected()
expected['shares'][0]['consistency_group_id'] = None
expected['shares'][0]['source_cgsnapshot_member_id'] = None
expected['shares'][0]['task_state'] = None
self._list_detail_test_common(req, expected)

def test_remove_invalid_options(self):
ctx = context.RequestContext('fakeuser', 'fakeproject', is_admin=False)
Expand Down
15 changes: 11 additions & 4 deletions manila_tempest_tests/services/share/v2/json/shares_client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,15 @@ def list_shares_with_detail(self, params=None,
"""Get detailed list of shares w/o filters."""
return self.list_shares(detailed=True, params=params, version=version)

def get_share(self, share_id, version=LATEST_MICROVERSION):
resp, body = self.get("shares/%s" % share_id, version=version)
def get_share(self, share_id, version=LATEST_MICROVERSION,
experimental=False):
headers = None
extra_headers = False
if experimental:
headers = EXPERIMENTAL
extra_headers = True
resp, body = self.get("shares/%s" % share_id, version=version,
headers=headers, extra_headers=extra_headers)
self.expected_success(200, resp.status)
return self._parse_resp(body)

Expand Down Expand Up @@ -493,12 +500,12 @@ def migrate_share(self, share_id, host, version=LATEST_MICROVERSION):

def wait_for_migration_completed(self, share_id, dest_host):
"""Waits for a share to migrate to a certain host."""
share = self.get_share(share_id)
share = self.get_share(share_id, "2.5", True)
migration_timeout = CONF.share.migration_timeout
start = int(time.time())
while share['task_state'] != 'migration_success':
time.sleep(self.build_interval)
share = self.get_share(share_id)
share = self.get_share(share_id, "2.5", True)
if share['task_state'] == 'migration_success':
return share
elif share['task_state'] == 'migration_error':
Expand Down

0 comments on commit 350dfb8

Please sign in to comment.