openstack/cinder
Code Issues Proposed changes

Merge "api: Simplify enable/disable APIs (clusters)"

Browse Source
This commit is contained in:
Zuul
2025-12-01 21:52:04 +00:00
committed by Gerrit Code Review
parent 5bc8ed7d63 91b5501585
commit 82ae28c270
4 changed files with 139 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
cinder/api/openstack/wsgi.py
View File

@@ -21,6 +21,7 @@ import inspect
import math
import time
from oslo_config import cfg
from oslo_log import log as logging
from oslo_serialization import jsonutils
from oslo_utils import encodeutils
@@ -39,6 +40,7 @@ from cinder.i18n import _
from cinder import utils
from cinder.wsgi import common as wsgi
CONF = cfg.CONF
LOG = logging.getLogger(__name__)
SUPPORTED_CONTENT_TYPES = (
@@ -1292,6 +1294,7 @@ class Controller(object, metaclass=ControllerMetaclass):
class APIMapper(routes.Mapper):
def routematch(self, url=None, environ=None):
if url == "":
result = self._match("", environ)
@@ -1306,6 +1309,26 @@ class APIMapper(routes.Mapper):
kwargs['requirements']['format'] = 'json'
return routes.Mapper.connect(self, *args, **kwargs)
def create_route(self, path, method, controller, action):
# NOTE: project_id parameter is only valid if its hex or hex + dashes
# (note, integers are a subset of this). This is required to handle
# our overlapping routes issues.
project_id_regex = CONF.project_id_regex
project_id_token = '{project_id:%s}' % project_id_regex
self.connect(
'/%s%s' % (project_id_token, path),
conditions={"method": [method]},
controller=controller,
action=action,
)
self.connect(
path,
conditions={"method": [method]},
controller=controller,
action=action,
)
class Fault(webob.exc.HTTPException):
"""Wrap webob.exc.HTTPException to provide API friendly response."""

81
cinder/api/v3/clusters.py
View File

@@ -15,7 +15,7 @@
from cinder.api import microversions as mv
from cinder.api.openstack import wsgi
from cinder.api.schemas import clusters as cluster
from cinder.api.schemas import clusters as schema
from cinder.api.v3.views import clusters as clusters_view
from cinder.api import validation
from cinder.common import constants
@@ -90,28 +90,14 @@ class ClusterController(wsgi.Controller):
return clusters_view.ViewBuilder.list(clusters, detail,
replication_data)
@wsgi.Controller.api_version(mv.CLUSTER_SUPPORT)
def update(self, req, id, body):
def _update(
self, context, name, binary, disabled, disabled_reason=None,
show_replication_data=False,
):
"""Enable/Disable scheduling for a cluster."""
# NOTE(geguileo): This method tries to be consistent with services
# update endpoint API.
# Let the wsgi middleware convert NotAuthorized exceptions
context = req.environ['cinder.context']
context.authorize(policy.UPDATE_POLICY)
if id not in ('enable', 'disable'):
raise exception.NotFound(message=_("Unknown action"))
disabled = id != 'enable'
disabled_reason = self._disable_cluster(
req, body=body) if disabled else self._enable_cluster(
req, body=body)
name = body['name']
binary = body.get('binary', constants.VOLUME_BINARY)
# Let wsgi handle NotFound exception
cluster = objects.Cluster.get_by_id(context, None, binary=binary,
name=name)
@@ -120,23 +106,56 @@ class ClusterController(wsgi.Controller):
cluster.save()
# We return summary data plus the disabled reason
replication_data = req.api_version_request.matches(
mv.REPLICATION_CLUSTER)
ret_val = clusters_view.ViewBuilder.summary(cluster, replication_data)
ret_val = clusters_view.ViewBuilder.summary(
cluster, show_replication_data,
)
ret_val['cluster']['disabled_reason'] = disabled_reason
return ret_val
@validation.schema(cluster.disable_cluster)
def _disable_cluster(self, req, body):
reason = body.get('disabled_reason')
if reason:
reason = reason.strip()
return reason
@wsgi.Controller.api_version(mv.CLUSTER_SUPPORT)
@validation.schema(schema.disable_cluster)
def disable(self, req, body):
# NOTE(geguileo): This method tries to be consistent with services
# disable endpoint API.
@validation.schema(cluster.enable_cluster)
def _enable_cluster(self, req, body):
pass
# Let the wsgi middleware convert NotAuthorized exceptions
context = req.environ['cinder.context']
context.authorize(policy.UPDATE_POLICY)
name = body['name']
binary = body.get('binary', constants.VOLUME_BINARY)
disabled_reason = body.get('disabled_reason')
if disabled_reason:
disabled_reason = disabled_reason.strip()
show_replication_data = req.api_version_request.matches(
mv.REPLICATION_CLUSTER)
return self._update(
context, name=name, binary=binary, disabled=True,
disabled_reason=disabled_reason,
show_replication_data=show_replication_data)
@wsgi.Controller.api_version(mv.CLUSTER_SUPPORT)
@validation.schema(schema.enable_cluster)
def enable(self, req, body):
# NOTE(geguileo): This method tries to be consistent with services
# enable endpoint API.
# Let the wsgi middleware convert NotAuthorized exceptions
context = req.environ['cinder.context']
context.authorize(policy.UPDATE_POLICY)
name = body['name']
binary = body.get('binary', constants.VOLUME_BINARY)
show_replication_data = req.api_version_request.matches(
mv.REPLICATION_CLUSTER)
return self._update(
context, name=name, binary=binary, disabled=False,
show_replication_data=show_replication_data)
def create_resource():

13
cinder/api/v3/router.py
View File

@@ -181,9 +181,16 @@ class APIRouter(base_wsgi.Router):
controller=self.resources['messages'])
self.resources['clusters'] = clusters.create_resource()
mapper.resource('cluster', 'clusters',
controller=self.resources['clusters'],
collection={'detail': 'GET'})
mapper.create_route(
'/clusters', 'GET', self.resources['clusters'], 'index')
mapper.create_route(
'/clusters/detail', 'GET', self.resources['clusters'], 'detail')
mapper.create_route(
'/clusters/{id}', 'GET', self.resources['clusters'], 'show')
mapper.create_route(
'/clusters/enable', 'PUT', self.resources['clusters'], 'enable')
mapper.create_route(
'/clusters/disable', 'PUT', self.resources['clusters'], 'disable')
self.resources['types'] = types.create_resource()
mapper.resource("type", "types",

94
cinder/tests/unit/api/v3/test_cluster.py
View File

@@ -305,16 +305,15 @@ class ClustersTestCase(test.TestCase):
side_effect=fake_db_api_cluster_update)
@mock.patch('cinder.db.sqlalchemy.api.cluster_get',
side_effect=fake_db_api_cluster_get)
def test_update_enable(self, get_mock, update_mock):
def test_enable(self, get_mock, update_mock):
req = FakeRequest()
expected = {'cluster': {'name': 'cluster_name',
'binary': 'cinder-volume',
'state': 'up',
'status': 'enabled',
'disabled_reason': None}}
res = self.controller.update(req, 'enable',
body={'name': 'cluster_name',
'binary': 'cinder-volume'})
res = self.controller.enable(
req, body={'name': 'cluster_name', 'binary': 'cinder-volume'})
self.assertEqual(expected, res)
ctxt = req.environ['cinder.context']
get_mock.assert_called_once_with(ctxt,
@@ -328,7 +327,7 @@ class ClustersTestCase(test.TestCase):
side_effect=fake_db_api_cluster_update)
@mock.patch('cinder.db.sqlalchemy.api.cluster_get',
side_effect=fake_db_api_cluster_get)
def test_update_disable(self, get_mock, update_mock):
def test_disable(self, get_mock, update_mock):
req = FakeRequest()
disabled_reason = 'For testing'
expected = {'cluster': {'name': 'cluster_name',
@@ -336,10 +335,9 @@ class ClustersTestCase(test.TestCase):
'binary': 'cinder-volume',
'status': 'disabled',
'disabled_reason': disabled_reason}}
res = self.controller.update(req, 'disable',
body={'name': 'cluster_name',
'binary': 'cinder-volume',
'disabled_reason': disabled_reason})
res = self.controller.disable(
req, body={'name': 'cluster_name', 'binary': 'cinder-volume',
'disabled_reason': disabled_reason})
self.assertEqual(expected, res)
ctxt = req.environ['cinder.context']
get_mock.assert_called_once_with(ctxt,
@@ -349,46 +347,66 @@ class ClustersTestCase(test.TestCase):
ctxt, 1,
{'disabled': True, 'disabled_reason': disabled_reason})
def test_update_wrong_action(self):
def test_enable_missing_name(self):
req = FakeRequest()
self.assertRaises(exception.NotFound, self.controller.update,
req, 'action', body={'name': 'cluster_name'})
self.assertRaises(exception.ValidationError, self.controller.enable,
req, body={'binary': 'cinder-volume'})
@ddt.data('enable', 'disable')
def test_update_missing_name(self, action):
def test_disable_missing_name(self):
req = FakeRequest()
self.assertRaises(exception.ValidationError, self.controller.update,
req, action, body={'binary': 'cinder-volume'})
self.assertRaises(
exception.ValidationError,
self.controller.disable,
req,
body={'binary': 'cinder-volume'})
def test_update_with_binary_more_than_255_characters(self):
def test_enable_with_binary_more_than_255_characters(self):
req = FakeRequest()
self.assertRaises(exception.ValidationError, self.controller.update,
req, 'enable', body={'name': 'cluster_name',
'binary': 'a' * 256})
self.assertRaises(
exception.ValidationError,
self.controller.enable,
req,
body={'name': 'cluster_name', 'binary': 'a' * 256})
def test_update_with_name_more_than_255_characters(self):
def test_enable_with_name_more_than_255_characters(self):
req = FakeRequest()
self.assertRaises(exception.ValidationError, self.controller.update,
req, 'enable', body={'name': 'a' * 256,
'binary': 'cinder-volume'})
self.assertRaises(
exception.ValidationError,
self.controller.enable,
req,
body={'name': 'a' * 256, 'binary': 'cinder-volume'})
@ddt.data('a' * 256, ' ')
def test_update_wrong_disabled_reason(self, disabled_reason):
def test_enable_wrong_disabled_reason(self, disabled_reason):
req = FakeRequest()
self.assertRaises(exception.ValidationError, self.controller.update,
req, 'disable',
body={'name': 'cluster_name',
'disabled_reason': disabled_reason})
self.assertRaises(
exception.ValidationError,
self.controller.enable,
req,
body={'name': 'cluster_name', 'disabled_reason': disabled_reason})
@ddt.data('enable', 'disable')
def test_update_unauthorized(self, action):
def test_enable_unauthorized(self):
req = FakeRequest(is_admin=False)
self.assertRaises(exception.PolicyNotAuthorized,
self.controller.update, req, action,
body={'name': 'fake_name'})
self.assertRaises(
exception.PolicyNotAuthorized,
self.controller.enable,
req, body={'name': 'fake_name'})
@ddt.data('enable', 'disable')
def test_update_wrong_version(self, action):
def test_disable_unauthorized(self):
req = FakeRequest(is_admin=False)
self.assertRaises(
exception.PolicyNotAuthorized,
self.controller.disable,
req, body={'name': 'fake_name'})
def test_enable_wrong_version(self):
req = FakeRequest(version=mv.get_prior_version(mv.CLUSTER_SUPPORT))
self.assertRaises(exception.VersionNotFoundForAPIMethod,
self.controller.update, req, action, {})
self.assertRaises(
exception.VersionNotFoundForAPIMethod,
self.controller.enable, req, {})
def test_disable_wrong_version(self):
req = FakeRequest(version=mv.get_prior_version(mv.CLUSTER_SUPPORT))
self.assertRaises(
exception.VersionNotFoundForAPIMethod,
self.controller.disable, req, {})