openstack/cinder
Code Issues Proposed changes

Merge "Respond with HTTP 409 on resource conflict"

Browse Source
This commit is contained in:
Zuul
2025-09-10 17:26:16 +00:00
committed by Gerrit Code Review
parent e3625c2eb6 67ac0fca89
commit cd32292649
7 changed files with 138 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
api-ref/source/v3/attachments.inc
View File

@@ -326,6 +326,7 @@ Response codes
- 400
- 404
- 409
Request
-------

4
cinder/api/v3/attachments.py
View File

@@ -250,11 +250,11 @@ class AttachmentsController(wsgi.Controller):
self.volume_api.attachment_update(context,
attachment_ref,
connector))
except exception.NotAuthorized:
except (exception.NotAuthorized, exception.Invalid):
raise
except exception.CinderException as ex:
err_msg = (
_("Unable to update attachment.(%s).") % ex.msg)
_("Unable to update attachment (%s).") % ex.msg)
LOG.exception(err_msg)
except Exception:
err_msg = _("Unable to update the attachment.")

5
cinder/exception.py
View File

@@ -220,6 +220,11 @@ class InvalidVolume(Invalid):
message = _("Invalid volume: %(reason)s")
class ResourceConflict(Invalid):
message = _("Resource conflict: %(reason)s")
code = 409
class InvalidContentType(Invalid):
message = _("Invalid content type %(content_type)s.")

108
cinder/tests/unit/api/v3/test_attachments.py
View File

@@ -18,6 +18,7 @@
from unittest import mock
import ddt
import webob
from cinder.api import microversions as mv
from cinder.api.v3 import attachments as v3_attachments
@@ -140,6 +141,113 @@ class AttachmentsAPITestCase(test.TestCase):
self.controller.update, req,
self.attachment1.id, body=body)
@mock.patch.object(volume_api.API, 'attachment_update')
def test_update_attachment_not_authorized(self, mock_update):
exc = exception.NotAuthorized(reason='Operation is not authorized.')
mock_update.side_effect = exc
req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' %
(fake.PROJECT_ID, self.attachment1.id),
version=mv.NEW_ATTACH,
use_admin_context=True)
body = {
"attachment":
{
"connector": {'fake_key': 'fake_value',
'host': 'somehost',
'connection_info': 'a'},
},
}
self.assertRaises(exception.NotAuthorized,
self.controller.update, req,
self.attachment1.id, body=body)
@mock.patch('cinder.volume.api.API.attachment_update')
def test_update_attachment_invalid_volume_conflict(self, mock_update):
exc = exception.ResourceConflict(
reason='Duplicate connectors or improper volume status')
mock_update.side_effect = exc
req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' %
(fake.PROJECT_ID, self.attachment1.id),
version=mv.NEW_ATTACH,
use_admin_context=True)
body = {
"attachment":
{
"connector": {'fake_key': 'fake_value',
'host': 'somehost',
'connection_info': 'a'},
},
}
self.assertRaises(exception.ResourceConflict,
self.controller.update, req,
self.attachment1.id, body=body)
@mock.patch.object(volume_api.API, 'attachment_update')
def test_update_attachment_generic_exception_invalid(self, mock_update):
exc = exception.Invalid(reason='Invalid class generic Exception')
mock_update.side_effect = exc
req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' %
(fake.PROJECT_ID, self.attachment1.id),
version=mv.NEW_ATTACH,
use_admin_context=True)
body = {
"attachment":
{
"connector": {'fake_key': 'fake_value',
'host': 'somehost',
'connection_info': 'a'},
},
}
self.assertRaises(exception.Invalid,
self.controller.update, req,
self.attachment1.id, body=body)
@mock.patch.object(volume_api.API, 'attachment_update')
def test_update_attachment_cinder_exception(self, mock_update):
exc = exception.CinderException(reason='Generic Cinder Exception')
mock_update.side_effect = exc
req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' %
(fake.PROJECT_ID, self.attachment1.id),
version=mv.NEW_ATTACH,
use_admin_context=True)
body = {
"attachment":
{
"connector": {'fake_key': 'fake_value',
'host': 'somehost',
'connection_info': 'a'},
},
}
self.assertRaises(webob.exc.HTTPInternalServerError,
self.controller.update, req,
self.attachment1.id, body=body)
@mock.patch.object(volume_api.API, 'attachment_update')
def test_update_attachment_all_other_exceptions(self, mock_update):
exc = Exception('The most generic Exception')
mock_update.side_effect = exc
req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' %
(fake.PROJECT_ID, self.attachment1.id),
version=mv.NEW_ATTACH,
use_admin_context=True)
body = {
"attachment":
{
"connector": {'fake_key': 'fake_value',
'host': 'somehost',
'connection_info': 'a'},
},
}
self.assertRaises(webob.exc.HTTPInternalServerError,
self.controller.update, req,
self.attachment1.id, body=body)
@ddt.data(mv.get_prior_version(mv.RESOURCE_FILTER),
mv.RESOURCE_FILTER, mv.LIKE_FILTER)
@mock.patch('cinder.api.common.reject_invalid_filters')

22
cinder/tests/unit/attachments/test_attachments_api.py
View File

@@ -302,11 +302,11 @@ class AttachmentManagerTestCase(test.TestCase):
vref.save()
connector = {'fake': 'connector',
'host': 'somehost'}
self.assertRaises(exception.InvalidVolume,
self.volume_api.attachment_update,
self.context,
aref,
connector)
caught_exc = self.assertRaises(
exception.ResourceConflict,
self.volume_api.attachment_update,
self.context, aref, connector)
self.assertEqual(409, caught_exc.code)
@mock.patch('cinder.db.sqlalchemy.api.volume_attachment_update',
return_value={})
@@ -342,11 +342,13 @@ class AttachmentManagerTestCase(test.TestCase):
with mock.patch('cinder.objects.Volume.get_by_id', return_value=vref):
with mock.patch.object(self.volume_api.volume_rpcapi,
'attachment_update') as m_au:
self.assertRaises(exception.InvalidVolume,
self.volume_api.attachment_update,
self.context,
vref.volume_attachment[1],
connector)
caught_exc = self.assertRaises(
exception.ResourceConflict,
self.volume_api.attachment_update,
self.context,
vref.volume_attachment[1],
connector)
self.assertEqual(409, caught_exc.code)
m_au.assert_not_called()
mock_va_update.assert_not_called()
mock_db_upd.assert_not_called()

4
cinder/volume/api.py
View File

@@ -2548,7 +2548,7 @@ class API(base.Base):
'volume_id': volume_ref.id,
'volume_status': volume_ref.status}
LOG.error(msg)
raise exception.InvalidVolume(reason=msg)
raise exception.ResourceConflict(reason=msg)
if (len(volume_ref.volume_attachment) > 1 and
not (volume_ref.multiattach or
@@ -2570,7 +2570,7 @@ class API(base.Base):
msg = _('duplicate connectors detected on volume '
'%(vol)s') % {'vol': volume_ref.id}
raise exception.InvalidVolume(reason=msg)
raise exception.ResourceConflict(reason=msg)
connection_info = (
self.volume_rpcapi.attachment_update(ctxt,

8
releasenotes/notes/fix-500-http-error-on-resource-conflict.yaml Normal file
View File

@@ -0,0 +1,8 @@
---
fixes:
- |
`Bug #1907295 <https://bugs.launchpad.net/cinder/+bug/1907295>`_: Fixed
When a volume was not in the correct status to accept an attachment update
(e.g.: volume in error or duplicate connectors), the REST API was returning
a 500 (Internal Server Error). It now correctly returns the response code
409 (Conflict) in this situation.