Fix task_state field shown on API < 2.5

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

manila/api/views/shares.py

@@ -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",
     ]
 
@@ -65,7 +66,6 @@ class ViewBuilder(common.ViewBuilder):
             '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'),
@@ -95,6 +95,10 @@ class ViewBuilder(common.ViewBuilder):
         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')

manila/tests/api/v1/test_shares.py

@@ -101,7 +101,6 @@ class ShareApiTest(test.TestCase):
             'snapshot_id': '2',
             'share_network_id': None,
             'status': 'fakestatus',
-            'task_state': None,
             'share_type': '1',
             'volume_type': '1',
             'is_public': False,
@@ -691,17 +690,11 @@ class ShareApiTest(test.TestCase):
     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):
+    def _list_detail_common_expected(self):
-        self.mock_object(share_api.API, 'get_all',
+        return {
-                         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 = {
             'shares': [
                 {
                     'status': 'fakestatus',
-                    'task_state': None,
                     'description': 'displaydesc',
                     'export_location': 'fake_location',
                     'export_locations': ['fake_location', 'fake_location2'],
@@ -732,57 +725,39 @@ class ShareApiTest(test.TestCase):
                 }
             ]
         }
+
+    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 = self._list_detail_common_expected()
-        expected = {
+        expected['shares'][0]['consistency_group_id'] = None
-            'shares': [
+        expected['shares'][0]['source_cgsnapshot_member_id'] = None
-                {
+        self._list_detail_test_common(req, expected)
-                    'status': 'fakestatus',
+
-                    'task_state': None,
+    def test_share_list_detail_with_task_state(self):
-                    'description': 'displaydesc',
+        env = {'QUERY_STRING': 'name=Share+Test+Name'}
-                    'export_location': 'fake_location',
+        req = fakes.HTTPRequest.blank('/shares/detail', environ=env,
-                    'export_locations': ['fake_location', 'fake_location2'],
+                                      version="2.5", experimental=True)
-                    'availability_zone': 'fakeaz',
+        expected = self._list_detail_common_expected()
-                    'name': 'displayname',
+        expected['shares'][0]['consistency_group_id'] = None
-                    'share_proto': 'FAKEPROTO',
+        expected['shares'][0]['source_cgsnapshot_member_id'] = None
-                    'metadata': {},
+        expected['shares'][0]['task_state'] = None
-                    'project_id': 'fakeproject',
+        self._list_detail_test_common(req, expected)
-                    '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'])
 
     def test_remove_invalid_options(self):
         ctx = context.RequestContext('fakeuser', 'fakeproject', is_admin=False)

manila_tempest_tests/services/share/v2/json/shares_client.py

@@ -220,8 +220,15 @@ class SharesV2Client(shares_client.SharesClient):
         """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):
+    def get_share(self, share_id, version=LATEST_MICROVERSION,
-        resp, body = self.get("shares/%s" % share_id, version=version)
+                  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)
 
@@ -493,12 +500,12 @@ class SharesV2Client(shares_client.SharesClient):
 
     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':