Merge "Add user_id echo in manila show/create/manage API"

manila/api/openstack/api_version_request.py

@@ -65,13 +65,14 @@ REST_API_VERSION_HISTORY = """
             'migration_get_progress', 'migration_complete' APIs, renamed
             'migrate_share' to 'migration_start' and added notify parameter
              to 'migration_start'.
+    * 2.16 - Add user_id in share show/create/manage API.
 """
 
 # The minimum and maximum versions of the API supported
 # The default api version request is defined to be the
 # the minimum version of the API supported.
 _MIN_API_VERSION = "2.0"
-_MAX_API_VERSION = "2.15"
+_MAX_API_VERSION = "2.16"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 

manila/api/openstack/rest_api_version_history.rst

@@ -106,3 +106,7 @@ ____
   Added Share migration 'migration_cancel', 'migration_get_progress',
   'migration_complete' APIs, renamed 'migrate_share' to 'migration_start' and
   added notify parameter to 'migration_start'.
+
+2.16
+----
+  Add user_id in share show/create/manage API.

manila/api/views/shares.py

@@ -28,6 +28,7 @@ class ViewBuilder(common.ViewBuilder):
         "remove_export_locations",
         "add_access_rules_status_field",
         "add_replication_fields",
+        "add_user_id",
     ]
 
     def summary_list(self, request, shares):
@@ -144,6 +145,10 @@ class ViewBuilder(common.ViewBuilder):
         share_dict['replication_type'] = share.get('replication_type')
         share_dict['has_replicas'] = share['has_replicas']
 
+    @common.ViewBuilder.versioned_method("2.16")
+    def add_user_id(self, share_dict, share):
+        share_dict['user_id'] = share.get('user_id')
+
     def _list_view(self, func, request, shares):
         """Provide a view for a list of shares."""
         shares_list = [func(request, share)['share'] for share in shares]

manila/tests/api/v2/test_shares.py

@@ -258,6 +258,32 @@ class ShareAPITest(test.TestCase):
         self.assertEqual("fakenetid",
                          create_mock.call_args[1]['share_network_id'])
 
+    @ddt.data("2.15", "2.16")
+    def test_share_create_original_with_user_id(self, microversion):
+        self.mock_object(share_api.API, 'create', self.create_mock)
+        body = {"share": copy.deepcopy(self.share)}
+        req = fakes.HTTPRequest.blank('/shares', version=microversion)
+
+        res_dict = self.controller.create(req, body)
+
+        expected = self._get_expected_share_detailed_response(self.share)
+        if api_version.APIVersionRequest(microversion) >= (
+                api_version.APIVersionRequest("2.16")):
+            expected['share']['user_id'] = 'fakeuser'
+        else:
+            self.assertNotIn('user_id', expected['share'])
+        expected['share']['task_state'] = None
+        expected['share']['consistency_group_id'] = None
+        expected['share']['source_cgsnapshot_member_id'] = None
+        expected['share']['replication_type'] = None
+        expected['share']['share_type_name'] = None
+        expected['share']['has_replicas'] = False
+        expected['share']['access_rules_status'] = 'active'
+        expected['share'].pop('export_location')
+        expected['share'].pop('export_locations')
+
+        self.assertEqual(expected, res_dict)
+
     @ddt.data('2.6', '2.7', '2.14', '2.15')
     def test_migration_start(self, version):
         share = db_utils.create_share()
@@ -789,6 +815,30 @@ class ShareAPITest(test.TestCase):
         expected['share']['task_state'] = None
         self.assertEqual(expected, res_dict)
 
+    @ddt.data("2.15", "2.16")
+    def test_share_show_with_user_id(self, microversion):
+        req = fakes.HTTPRequest.blank('/shares/1', version=microversion)
+
+        res_dict = self.controller.show(req, '1')
+
+        expected = self._get_expected_share_detailed_response()
+        if api_version.APIVersionRequest(microversion) >= (
+                api_version.APIVersionRequest("2.16")):
+            expected['share']['user_id'] = 'fakeuser'
+        else:
+            self.assertNotIn('user_id', expected['share'])
+        expected['share']['consistency_group_id'] = None
+        expected['share']['source_cgsnapshot_member_id'] = None
+        expected['share']['share_type_name'] = None
+        expected['share']['task_state'] = None
+        expected['share']['access_rules_status'] = 'active'
+        expected['share'].pop('export_location')
+        expected['share'].pop('export_locations')
+        expected['share']['replication_type'] = None
+        expected['share']['has_replicas'] = False
+
+        self.assertEqual(expected, res_dict)
+
     def test_share_show_admin(self):
         req = fakes.HTTPRequest.blank('/shares/1', use_admin_context=True)
         expected = self._get_expected_share_detailed_response(admin=True)
@@ -1865,14 +1915,52 @@ class ShareManageTest(test.TestCase):
     def test_share_manage_with_is_public(self, data):
         self._test_share_manage(data, "2.8")
 
+    def test_share_manage_with_user_id(self):
+        self._test_share_manage(get_fake_manage_body(
+            name='foo', description='bar', is_public=True), "2.16")
+
     def _test_share_manage(self, data, version):
-        self._setup_manage_mocks()
+        expected = {
-        return_share = {
+            'share': {
-            'share_type_id': '', 'id': 'fake',
+                'status': 'fakestatus',
-            'instance': {},
+                'description': 'displaydesc',
+                'availability_zone': 'fakeaz',
+                'name': 'displayname',
+                'share_proto': 'FAKEPROTO',
+                'metadata': {},
+                'project_id': 'fakeproject',
+                'host': 'fakehost',
+                'id': 'fake',
+                'snapshot_id': '2',
+                'share_network_id': None,
+                'created_at': datetime.datetime(1, 1, 1, 1, 1, 1),
+                'size': 1,
+                'share_type_name': None,
+                'share_server_id': 'fake_share_server_id',
+                'share_type': '1',
+                'volume_type': '1',
+                'is_public': False,
+                'consistency_group_id': None,
+                'source_cgsnapshot_member_id': None,
+                'snapshot_support': True,
+                'task_state': None,
+                'links': [
+                    {
+                        'href': 'http://localhost/v1/fake/shares/fake',
+                        'rel': 'self'
+                    },
+                    {
+                        'href': 'http://localhost/fake/shares/fake',
+                        'rel': 'bookmark'
                     }
+                ],
+            }
+        }
+        self._setup_manage_mocks()
+        return_share = mock.Mock(
+            return_value=stubs.stub_share('fake', instance={}))
         self.mock_object(
-            share_api.API, 'manage', mock.Mock(return_value=return_share))
+            share_api.API, 'manage', return_share)
         share = {
             'host': data['share']['service_host'],
             'export_location': data['share']['export_path'],
@@ -1883,6 +1971,25 @@ class ShareManageTest(test.TestCase):
         }
         driver_options = data['share'].get('driver_options', {})
 
+        if (api_version.APIVersionRequest(version) <=
+                api_version.APIVersionRequest('2.8')):
+            expected['share']['export_location'] = 'fake_location'
+            expected['share']['export_locations'] = (
+                ['fake_location', 'fake_location2'])
+
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.10')):
+            expected['share']['access_rules_status'] = (
+                constants.STATUS_ACTIVE)
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.11')):
+            expected['share']['has_replicas'] = False
+            expected['share']['replication_type'] = None
+
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.16')):
+            expected['share']['user_id'] = 'fakeuser'
+
         if (api_version.APIVersionRequest(version) >=
                 api_version.APIVersionRequest('2.8')):
             share['is_public'] = data['share']['is_public']
@@ -1894,7 +2001,9 @@ class ShareManageTest(test.TestCase):
 
         share_api.API.manage.assert_called_once_with(
             mock.ANY, share, driver_options)
+
         self.assertIsNotNone(actual_result)
+        self.assertEqual(expected, actual_result)
         self.mock_policy_check.assert_called_once_with(
             req.environ['manila.context'], self.resource_name, 'manage')
 

manila_tempest_tests/config.py

@@ -36,7 +36,7 @@ ShareGroup = [
                help="The minimum api microversion is configured to be the "
                     "value of the minimum microversion supported by Manila."),
     cfg.StrOpt("max_api_microversion",
-               default="2.15",
+               default="2.16",
                help="The maximum api microversion is configured to be the "
                     "value of the latest microversion supported by Manila."),
     cfg.StrOpt("region",

manila_tempest_tests/tests/api/admin/test_share_manage.py

@@ -83,13 +83,22 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
             data.append(creation_data)
         if utils.is_microversion_ge(CONF.share.max_api_microversion, "2.8"):
             data.append(creation_data)
+        if utils.is_microversion_ge(CONF.share.max_api_microversion, "2.16"):
+            data.append(creation_data)
         shares_created = cls.create_shares(data)
 
         cls.shares = []
         # Load all share data (host, etc.)
         for share in shares_created:
             # Unmanage shares from manila
-            cls.shares.append(cls.shares_client.get_share(share['id']))
+            get_share = cls.shares_v2_client.get_share(share['id'])
+            if utils.is_microversion_ge(
+                    CONF.share.max_api_microversion, "2.9"):
+                get_share["export_locations"] = (
+                    cls.shares_v2_client.list_share_export_locations(
+                        share["id"])
+                    )
+            cls.shares.append(get_share)
             cls.shares_client.unmanage_share(share['id'])
             cls.shares_client.wait_for_resource_deletion(
                 share_id=share['id'])
@@ -138,6 +147,11 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
         else:
             self.assertFalse(managed_share['is_public'])
 
+        if utils.is_microversion_ge(version, "2.16"):
+            self.assertEqual(share['user_id'], managed_share['user_id'])
+        else:
+            self.assertNotIn('user_id', managed_share)
+
         # Delete share
         self.shares_v2_client.delete_share(managed_share['id'])
         self.shares_v2_client.wait_for_resource_deletion(
@@ -156,6 +170,11 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
     def test_manage_with_is_public_True(self):
         self._test_manage(share=self.shares[3], is_public=True, version="2.8")
 
+    @test.attr(type=["gate", "smoke"])
+    @base.skip_if_microversion_not_supported("2.16")
+    def test_manage_show_user_id(self):
+        self._test_manage(share=self.shares[4], version="2.16")
+
     @test.attr(type=["gate", "smoke"])
     def test_manage(self):
         # After 'unmanage' operation, share instance should be deleted.

manila_tempest_tests/tests/api/test_shares.py

@@ -84,6 +84,12 @@ class SharesNFSTest(base.BaseSharesTest):
             detailed_elements.add('replication_type')
             self.assertTrue(detailed_elements.issubset(share.keys()), msg)
 
+        # In v 2.16 and beyond, we add user_id in show/create/manage
+        # share echo.
+        if utils.is_microversion_supported('2.16'):
+            detailed_elements.add('user_id')
+            self.assertTrue(detailed_elements.issubset(share.keys()), msg)
+
         # Delete share
         self.shares_v2_client.delete_share(share['id'])
         self.shares_v2_client.wait_for_resource_deletion(share_id=share['id'])

manila_tempest_tests/tests/api/test_shares_actions.py

@@ -99,6 +99,8 @@ class SharesActionsTest(base.BaseSharesTest):
             expected_keys.append("access_rules_status")
         if utils.is_microversion_ge(version, '2.11'):
             expected_keys.append("replication_type")
+        if utils.is_microversion_ge(version, '2.16'):
+            expected_keys.append("user_id")
         actual_keys = list(share.keys())
         [self.assertIn(key, actual_keys) for key in expected_keys]
 
@@ -150,6 +152,11 @@ class SharesActionsTest(base.BaseSharesTest):
     def test_get_share_with_replication_type_key(self):
         self._get_share('2.11')
 
+    @test.attr(type=["gate", ])
+    @utils.skip_if_microversion_not_supported('2.16')
+    def test_get_share_with_user_id(self):
+        self._get_share('2.16')
+
     @test.attr(type=["gate", ])
     def test_list_shares(self):
 
@@ -192,7 +199,8 @@ class SharesActionsTest(base.BaseSharesTest):
             keys.append("access_rules_status")
         if utils.is_microversion_ge(version, '2.11'):
             keys.append("replication_type")
-
+        if utils.is_microversion_ge(version, '2.16'):
+            keys.append("user_id")
         [self.assertIn(key, sh.keys()) for sh in shares for key in keys]
 
         # our shares in list and have no duplicates
@@ -234,6 +242,11 @@ class SharesActionsTest(base.BaseSharesTest):
     def test_list_shares_with_detail_replication_type_key(self):
         self._list_shares_with_detail('2.11')
 
+    @test.attr(type=["gate", ])
+    @utils.skip_if_microversion_not_supported('2.16')
+    def test_list_shares_with_user_id(self):
+        self._list_shares_with_detail('2.16')
+
     @test.attr(type=["gate", ])
     def test_list_shares_with_detail_filter_by_metadata(self):
         filters = {'metadata': self.metadata}

releasenotes/notes/add-user-id-echo-8f42db469b27ff14.yaml

@@ -0,0 +1,3 @@
+---
+features:
+  - User ID is added to the JSON response of the /shares APIs.