From 87982a56771cf66f6ae4beec92e4de9d27388fc9 Mon Sep 17 00:00:00 2001 From: TommyLike Date: Fri, 7 Apr 2017 20:15:08 +0800 Subject: [PATCH] [BugFix] Add method policy in attachment APIs There are some issues around new attach/detach API/CLI, fix them step by step. This patch adds policy in attachment APIs, also add related testcases in API unit testcases. Closes-Bug: #1680836 Change-Id: I310fec39719ead39f26d97ee4ba95187e1fb2069 --- cinder/api/v3/attachments.py | 4 +- cinder/tests/unit/api/v3/test_attachments.py | 72 ++++++++++++++++++++ cinder/tests/unit/policy.json | 4 ++ etc/cinder/policy.json | 4 ++ 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/cinder/api/v3/attachments.py b/cinder/api/v3/attachments.py index 093443adff0..f1bc29c81ee 100644 --- a/cinder/api/v3/attachments.py +++ b/cinder/api/v3/attachments.py @@ -227,8 +227,8 @@ class AttachmentsController(wsgi.Controller): err_msg = ( _("Unable to create attachment for volume (%s).") % ex.msg) LOG.exception(err_msg) - except Exception as ex: - err_msg = _("Unable to create attachment for volume.") + except Exception: + err_msg = _("Unable to update the attachment.") LOG.exception(err_msg) finally: if err_msg: diff --git a/cinder/tests/unit/api/v3/test_attachments.py b/cinder/tests/unit/api/v3/test_attachments.py index c3f06fd4e6a..1599cfaa920 100644 --- a/cinder/tests/unit/api/v3/test_attachments.py +++ b/cinder/tests/unit/api/v3/test_attachments.py @@ -18,15 +18,18 @@ Tests for attachments Api. """ import ddt +import mock import webob from cinder.api.v3 import attachments as v3_attachments from cinder import context +from cinder import exception from cinder import objects from cinder import test from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_constants as fake from cinder.volume import api as volume_api +from cinder.volume import rpcapi as volume_rpcapi ATTACHMENTS_MICRO_VERSION = '3.27' @@ -70,9 +73,78 @@ class AttachmentsAPITestCase(test.TestCase): volume = objects.Volume(ctxt) volume.display_name = display_name volume.project_id = project_id + volume.status = 'available' + volume.attach_status = 'attached' volume.create() return volume + def test_create_attachment(self): + req = fakes.HTTPRequest.blank('/v3/%s/attachments' % + fake.PROJECT_ID, + version=ATTACHMENTS_MICRO_VERSION) + body = { + "attachment": + { + "connector": None, + "instance_uuid": fake.UUID1, + "volume_uuid": self.volume1.id + }, + } + + attachment = self.controller.create(req, body) + + self.assertEqual(self.volume1.id, + attachment['attachment']['volume_id']) + self.assertEqual(fake.UUID1, + attachment['attachment']['instance']) + + @mock.patch.object(volume_rpcapi.VolumeAPI, 'attachment_update') + def test_update_attachment(self, mock_update): + fake_connector = {'fake_key': 'fake_value'} + mock_update.return_value = fake_connector + req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' % + (fake.PROJECT_ID, self.attachment1.id), + version=ATTACHMENTS_MICRO_VERSION, + use_admin_context=True) + body = { + "attachment": + { + "connector": {'fake_key': 'fake_value'}, + }, + } + + attachment = self.controller.update(req, self.attachment1.id, body) + + self.assertEqual(fake_connector, + attachment['attachment']['connection_info']) + self.assertEqual(fake.UUID1, attachment['attachment']['instance']) + + @ddt.data('reserved', 'attached') + @mock.patch.object(volume_rpcapi.VolumeAPI, 'attachment_delete') + def test_delete_attachment(self, status, mock_delete): + volume1 = self._create_volume(display_name='fake_volume_1', + project_id=fake.PROJECT_ID) + attachment = self._create_attachement( + volume_uuid=volume1.id, instance_uuid=fake.UUID1, + attach_status=status) + req = fakes.HTTPRequest.blank('/v3/%s/attachments/%s' % + (fake.PROJECT_ID, attachment.id), + version=ATTACHMENTS_MICRO_VERSION, + use_admin_context=True) + + self.controller.delete(req, attachment.id) + + volume2 = objects.Volume.get_by_id(self.ctxt, volume1.id) + if status == 'reserved': + self.assertEqual('detached', volume2.attach_status) + self.assertRaises( + exception.VolumeAttachmentNotFound, + objects.VolumeAttachment.get_by_id, self.ctxt, attachment.id) + else: + self.assertEqual('attached', volume2.attach_status) + mock_delete.assert_called_once_with(req.environ['cinder.context'], + attachment.id, mock.ANY) + def _create_attachement(self, ctxt=None, volume_uuid=None, instance_uuid=None, mountpoint=None, attach_time=None, detach_time=None, diff --git a/cinder/tests/unit/policy.json b/cinder/tests/unit/policy.json index 8cb8da2059d..d944799b4c3 100644 --- a/cinder/tests/unit/policy.json +++ b/cinder/tests/unit/policy.json @@ -103,6 +103,10 @@ "backup:update": "rule:admin_or_owner", "backup:backup_project_attribute": "rule:admin_api", + "volume:attachment_create": "", + "volume:attachment_update": "rule:admin_or_owner", + "volume:attachment_delete": "rule:admin_or_owner", + "consistencygroup:create" : "", "consistencygroup:delete": "", "consistencygroup:update": "", diff --git a/etc/cinder/policy.json b/etc/cinder/policy.json index 6a651220e89..a32ad1ccdda 100644 --- a/etc/cinder/policy.json +++ b/etc/cinder/policy.json @@ -93,6 +93,10 @@ "backup:update": "rule:admin_or_owner", "backup:backup_project_attribute": "rule:admin_api", + "volume:attachment_create": "", + "volume:attachment_update": "rule:admin_or_owner", + "volume:attachment_delete": "rule:admin_or_owner", + "snapshot_extension:snapshot_actions:update_snapshot_status": "", "snapshot_extension:snapshot_manage": "rule:admin_api", "snapshot_extension:snapshot_unmanage": "rule:admin_api",