From d4eb9b04b65ebb8cfd746aaebd4d7c99f5604c66 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 20 Sep 2024 11:55:35 +0100 Subject: [PATCH] compute, volume: Tweak 'update_quota_set' compat shim The compatibility shim should modify the QuotaSet as a side-effect. Change-Id: Ia9546afe7762d119430b2a5ab1d7d00296245dde Closes-bug: #2081292 Signed-off-by: Stephen Finucane --- openstack/block_storage/v2/_proxy.py | 16 +++++--- openstack/block_storage/v3/_proxy.py | 16 +++++--- openstack/compute/v2/_proxy.py | 37 +++++++++++++------ .../tests/unit/block_storage/v2/test_proxy.py | 29 +++++++++++++++ .../tests/unit/block_storage/v3/test_proxy.py | 28 ++++++++++++++ openstack/tests/unit/compute/v2/test_proxy.py | 26 +++++++++++++ .../notes/bug-2081292-def552ed9c4e24a3.yaml | 10 +++++ 7 files changed, 138 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/bug-2081292-def552ed9c4e24a3.yaml diff --git a/openstack/block_storage/v2/_proxy.py b/openstack/block_storage/v2/_proxy.py index 0f322a459..f2d41fd11 100644 --- a/openstack/block_storage/v2/_proxy.py +++ b/openstack/block_storage/v2/_proxy.py @@ -824,18 +824,22 @@ class Proxy(_base_proxy.BaseBlockStorageProxy): "with the other quota set methods.", os_warnings.RemovedInSDK50Warning, ) - if isinstance(project, _quota_set.QuotaSet): - attrs['project_id'] = project.project_id - # cinder doesn't support any query parameters so we simply pop # these if 'query' in attrs: - attrs.pop('params') + warnings.warn( + "The query argument is no longer supported and should " + "be removed.", + os_warnings.RemovedInSDK50Warning, + ) + attrs.pop('query') + + res = self._get_resource(_quota_set.QuotaSet, project, **attrs) + return res.commit(self) else: project = self._get_resource(_project.Project, project) attrs['project_id'] = project.id - - return self._update(_quota_set.QuotaSet, None, **attrs) + return self._update(_quota_set.QuotaSet, None, **attrs) # ====== VOLUME METADATA ====== def get_volume_metadata(self, volume): diff --git a/openstack/block_storage/v3/_proxy.py b/openstack/block_storage/v3/_proxy.py index a648eb9c3..09ac39e5c 100644 --- a/openstack/block_storage/v3/_proxy.py +++ b/openstack/block_storage/v3/_proxy.py @@ -1820,18 +1820,22 @@ class Proxy(_base_proxy.BaseBlockStorageProxy): "with the other quota set methods.", os_warnings.RemovedInSDK50Warning, ) - if isinstance(project, _quota_set.QuotaSet): - attrs['project_id'] = project.project_id - # cinder doesn't support any query parameters so we simply pop # these if 'query' in attrs: - attrs.pop('params') + warnings.warn( + "The query argument is no longer supported and should " + "be removed.", + os_warnings.RemovedInSDK50Warning, + ) + attrs.pop('query') + + res = self._get_resource(_quota_set.QuotaSet, project, **attrs) + return res.commit(self) else: project = self._get_resource(_project.Project, project) attrs['project_id'] = project.id - - return self._update(_quota_set.QuotaSet, None, **attrs) + return self._update(_quota_set.QuotaSet, None, **attrs) # ====== SERVICES ====== @ty.overload diff --git a/openstack/compute/v2/_proxy.py b/openstack/compute/v2/_proxy.py index 675b47003..410c36265 100644 --- a/openstack/compute/v2/_proxy.py +++ b/openstack/compute/v2/_proxy.py @@ -2450,7 +2450,7 @@ class Proxy(proxy.Proxy): # ========== Quota sets ========== def get_quota_set(self, project, usage=False, **query): - """Show QuotaSet information for the project + """Show QuotaSet information for the project. :param project: ID or instance of :class:`~openstack.identity.project.Project` of the project for @@ -2473,7 +2473,7 @@ class Proxy(proxy.Proxy): return res.fetch(self, base_path=base_path, **query) def get_quota_set_defaults(self, project): - """Show QuotaSet defaults for the project + """Show QuotaSet defaults for the project. :param project: ID or instance of :class:`~openstack.identity.project.Project` of the project for @@ -2525,8 +2525,6 @@ class Proxy(proxy.Proxy): :returns: The updated QuotaSet :rtype: :class:`~openstack.compute.v2.quota_set.QuotaSet` """ - query = {} - if 'project_id' in attrs or isinstance(project, _quota_set.QuotaSet): warnings.warn( "The signature of 'update_quota_set' has changed and it " @@ -2534,23 +2532,38 @@ class Proxy(proxy.Proxy): "with the other quota set methods.", os_warnings.RemovedInSDK50Warning, ) - if isinstance(project, _quota_set.QuotaSet): - attrs['project_id'] = project.project_id + if user is not None: + raise exceptions.SDKException( + 'The user argument can only be provided once the entire ' + 'call has been updated.' + ) if 'query' in attrs: - query = attrs.pop('query') + warnings.warn( + "The query argument is no longer supported and should " + "be removed.", + os_warnings.RemovedInSDK50Warning, + ) + query = attrs.pop('query') or {} + else: + query = {} + + res = self._get_resource(_quota_set.QuotaSet, project, **attrs) + return res.commit(self, **query) else: project = self._get_resource(_project.Project, project) attrs['project_id'] = project.id if user: user = self._get_resource(_user.User, user) - query['user_id'] = user.id + query = {'user_id': user.id} + else: + query = {} - # we don't use Proxy._update since that doesn't allow passing arbitrary - # query string parameters - quota_set = self._get_resource(_quota_set.QuotaSet, None, **attrs) - return quota_set.commit(self, **query) + # we don't use Proxy._update since that doesn't allow passing + # arbitrary query string parameters + quota_set = self._get_resource(_quota_set.QuotaSet, None, **attrs) + return quota_set.commit(self, **query) # ========== Server actions ========== diff --git a/openstack/tests/unit/block_storage/v2/test_proxy.py b/openstack/tests/unit/block_storage/v2/test_proxy.py index 2831d06a5..c25d5d54a 100644 --- a/openstack/tests/unit/block_storage/v2/test_proxy.py +++ b/openstack/tests/unit/block_storage/v2/test_proxy.py @@ -9,7 +9,9 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. + from unittest import mock +import warnings from openstack.block_storage.v2 import _proxy from openstack.block_storage.v2 import backup @@ -24,6 +26,7 @@ from openstack.block_storage.v2 import volume from openstack.identity.v3 import project from openstack import proxy as proxy_base from openstack.tests.unit import test_proxy_base +from openstack import warnings as os_warnings class TestVolumeProxy(test_proxy_base.TestProxyBase): @@ -577,3 +580,29 @@ class TestQuotaSet(TestVolumeProxy): expected_kwargs={'project_id': 'prj', 'volumes': 123}, ) mock_get.assert_called_once_with(project.Project, 'prj') + + @mock.patch.object(proxy_base.Proxy, "_get_resource") + def test_quota_set_update__legacy(self, mock_get): + fake_quota_set = quota_set.QuotaSet(project_id='prj') + mock_get.side_effect = [fake_quota_set] + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + + self._verify( + 'openstack.resource.Resource.commit', + self.proxy.update_quota_set, + method_args=[fake_quota_set], + method_kwargs={'ram': 123}, + expected_args=[self.proxy], + expected_kwargs={}, + ) + + self.assertEqual(1, len(w)) + self.assertEqual( + os_warnings.RemovedInSDK50Warning, + w[-1].category, + ) + self.assertIn( + "The signature of 'update_quota_set' has changed ", + str(w[-1]), + ) diff --git a/openstack/tests/unit/block_storage/v3/test_proxy.py b/openstack/tests/unit/block_storage/v3/test_proxy.py index 27a9ee0e5..7a5514790 100644 --- a/openstack/tests/unit/block_storage/v3/test_proxy.py +++ b/openstack/tests/unit/block_storage/v3/test_proxy.py @@ -11,6 +11,7 @@ # under the License. from unittest import mock +import warnings from openstack.block_storage.v3 import _proxy from openstack.block_storage.v3 import backup @@ -30,6 +31,7 @@ from openstack.block_storage.v3 import volume from openstack.identity.v3 import project from openstack import proxy as proxy_base from openstack.tests.unit import test_proxy_base +from openstack import warnings as os_warnings class TestVolumeProxy(test_proxy_base.TestProxyBase): @@ -1050,3 +1052,29 @@ class TestQuotaSet(TestVolumeProxy): expected_kwargs={'project_id': 'prj', 'volumes': 123}, ) mock_get.assert_called_once_with(project.Project, 'prj') + + @mock.patch.object(proxy_base.Proxy, "_get_resource") + def test_quota_set_update__legacy(self, mock_get): + fake_quota_set = quota_set.QuotaSet(project_id='prj') + mock_get.side_effect = [fake_quota_set] + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + + self._verify( + 'openstack.resource.Resource.commit', + self.proxy.update_quota_set, + method_args=[fake_quota_set], + method_kwargs={'ram': 123}, + expected_args=[self.proxy], + expected_kwargs={}, + ) + + self.assertEqual(1, len(w)) + self.assertEqual( + os_warnings.RemovedInSDK50Warning, + w[-1].category, + ) + self.assertIn( + "The signature of 'update_quota_set' has changed ", + str(w[-1]), + ) diff --git a/openstack/tests/unit/compute/v2/test_proxy.py b/openstack/tests/unit/compute/v2/test_proxy.py index 05ac01a2f..c6a423189 100644 --- a/openstack/tests/unit/compute/v2/test_proxy.py +++ b/openstack/tests/unit/compute/v2/test_proxy.py @@ -1738,6 +1738,32 @@ class TestQuotaSet(TestComputeProxy): ] ) + @mock.patch.object(proxy_base.Proxy, "_get_resource") + def test_quota_set_update__legacy(self, mock_get): + fake_quota_set = quota_set.QuotaSet(project_id='prj') + mock_get.side_effect = [fake_quota_set] + with warnings.catch_warnings(record=True) as w: + warnings.simplefilter('always') + + self._verify( + 'openstack.resource.Resource.commit', + self.proxy.update_quota_set, + method_args=[fake_quota_set], + method_kwargs={'ram': 123}, + expected_args=[self.proxy], + expected_kwargs={}, + ) + + self.assertEqual(1, len(w)) + self.assertEqual( + os_warnings.RemovedInSDK50Warning, + w[-1].category, + ) + self.assertIn( + "The signature of 'update_quota_set' has changed ", + str(w[-1]), + ) + class TestServerAction(TestComputeProxy): def test_server_action_get(self): diff --git a/releasenotes/notes/bug-2081292-def552ed9c4e24a3.yaml b/releasenotes/notes/bug-2081292-def552ed9c4e24a3.yaml new file mode 100644 index 000000000..8acad24b0 --- /dev/null +++ b/releasenotes/notes/bug-2081292-def552ed9c4e24a3.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + The ``update_quota_set`` methods in the Compute and Block Storage (v2, v3) + proxy APIs were modified in v3.3.0 to accept ``Project`` objects as the + first argument. A compatibility shim was included to handle callers still + passing ``QuotaSet`` objects, but this shim did not modify the provided + ``QuotaSet`` object in place as the previous code did. This has now been + fixed. The shim is still expected to be removed in v5.0.0. + [`bug 2081292 `_]