From d714249ca9e95297f5e53c9dcc0af9ba58d0ab6d Mon Sep 17 00:00:00 2001 From: Rajat Dhasmana Date: Mon, 29 Mar 2021 06:31:31 -0400 Subject: [PATCH] Make instance_uuid optional in attachment create Cinder and cinderclient assumes an attachment create request will always contain instance_uuid. This is not true when glance calls cinder for attachment in glance cinder configuration. This patch (along with the cinder patch) make the instance_uuid optional and allow glance to do attachments without passing instance_uuid. Change-Id: Ifbaca4aa87d890bc5130069638d42665b914b378 --- cinderclient/tests/unit/v3/fakes.py | 14 ++++++- .../tests/unit/v3/test_attachments.py | 11 +++++ cinderclient/tests/unit/v3/test_shell.py | 40 +++++++++++++++++++ cinderclient/v3/attachments.py | 5 ++- cinderclient/v3/shell.py | 2 + ...e-optional-server-id-9299d9da2b62b263.yaml | 10 +++++ 6 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/attachment-create-optional-server-id-9299d9da2b62b263.yaml diff --git a/cinderclient/tests/unit/v3/fakes.py b/cinderclient/tests/unit/v3/fakes.py index 7647fb39f..ffdfeb75e 100644 --- a/cinderclient/tests/unit/v3/fakes.py +++ b/cinderclient/tests/unit/v3/fakes.py @@ -29,6 +29,16 @@ fake_attachment = {'attachment': { 'instance': 'e84fda45-4de4-4ce4-8f39-fc9d3b0aa05e', 'volume_id': '557ad76c-ce54-40a3-9e91-c40d21665cc3', }} +fake_attachment_without_instance_id = {'attachment': { + 'status': 'reserved', + 'detached_at': '', + 'connection_info': {}, + 'attached_at': '', + 'attach_mode': None, + 'id': 'a232e9ae', + 'instance': None, + 'volume_id': '557ad76c-ce54-40a3-9e91-c40d21665cc3', }} + fake_attachment_list = {'attachments': [ {'instance': 'instance_1', 'name': 'attachment-1', @@ -297,7 +307,9 @@ class FakeHTTPClient(fake_v2.FakeHTTPClient): # def post_attachments(self, **kw): - return (200, {}, fake_attachment) + if kw['body']['attachment'].get('instance_uuid'): + return (200, {}, fake_attachment) + return (200, {}, fake_attachment_without_instance_id) def get_attachments(self, **kw): return (200, {}, fake_attachment_list) diff --git a/cinderclient/tests/unit/v3/test_attachments.py b/cinderclient/tests/unit/v3/test_attachments.py index 1802334ec..acf064639 100644 --- a/cinderclient/tests/unit/v3/test_attachments.py +++ b/cinderclient/tests/unit/v3/test_attachments.py @@ -31,6 +31,17 @@ class AttachmentsTest(utils.TestCase): cs.assert_called('POST', '/attachments') self.assertEqual(fakes.fake_attachment['attachment'], att) + def test_create_attachment_without_instance_uuid(self): + cs = fakes.FakeClient(api_versions.APIVersion('3.27')) + att = cs.attachments.create( + 'e84fda45-4de4-4ce4-8f39-fc9d3b0aa05e', + {}, + None, + 'null') + cs.assert_called('POST', '/attachments') + self.assertEqual( + fakes.fake_attachment_without_instance_id['attachment'], att) + def test_complete_attachment(self): cs = fakes.FakeClient(api_versions.APIVersion('3.44')) att = cs.attachments.complete('a232e9ae') diff --git a/cinderclient/tests/unit/v3/test_shell.py b/cinderclient/tests/unit/v3/test_shell.py index 756f51201..d0f1bfbe6 100644 --- a/cinderclient/tests/unit/v3/test_shell.py +++ b/cinderclient/tests/unit/v3/test_shell.py @@ -388,6 +388,25 @@ class ShellTest(utils.TestCase): {'cmd': 'abc 1233', 'body': {'instance_uuid': '1233', 'connector': {}, + 'volume_uuid': '1234'}}, + {'cmd': '1234', + 'body': {'connector': {}, + 'volume_uuid': '1234'}}, + {'cmd': '1234 ' + '--connect True ' + '--ip 10.23.12.23 --host server01 ' + '--platform x86_xx ' + '--ostype 123 ' + '--multipath true ' + '--mountpoint /123 ' + '--initiator aabbccdd', + 'body': {'connector': {'ip': '10.23.12.23', + 'host': 'server01', + 'os_type': '123', + 'multipath': 'true', + 'mountpoint': '/123', + 'initiator': 'aabbccdd', + 'platform': 'x86_xx'}, 'volume_uuid': '1234'}}) @mock.patch('cinderclient.utils.find_resource') @ddt.unpack @@ -429,6 +448,27 @@ class ShellTest(utils.TestCase): 'body': {'instance_uuid': '1233', 'connector': {}, 'volume_uuid': '1234', + 'mode': 'ro'}}, + {'cmd': '1234', + 'body': {'connector': {}, + 'volume_uuid': '1234', + 'mode': 'ro'}}, + {'cmd': '1234 ' + '--connect True ' + '--ip 10.23.12.23 --host server01 ' + '--platform x86_xx ' + '--ostype 123 ' + '--multipath true ' + '--mountpoint /123 ' + '--initiator aabbccdd', + 'body': {'connector': {'ip': '10.23.12.23', + 'host': 'server01', + 'os_type': '123', + 'multipath': 'true', + 'mountpoint': '/123', + 'initiator': 'aabbccdd', + 'platform': 'x86_xx'}, + 'volume_uuid': '1234', 'mode': 'ro'}}) @mock.patch('cinderclient.utils.find_resource') @ddt.unpack diff --git a/cinderclient/v3/attachments.py b/cinderclient/v3/attachments.py index e1e929003..506796270 100644 --- a/cinderclient/v3/attachments.py +++ b/cinderclient/v3/attachments.py @@ -27,11 +27,12 @@ class VolumeAttachmentManager(base.ManagerWithFind): resource_class = VolumeAttachment @api_versions.wraps('3.27') - def create(self, volume_id, connector, instance_id, mode='null'): + def create(self, volume_id, connector, instance_id=None, mode='null'): """Create a attachment for specified volume.""" body = {'attachment': {'volume_uuid': volume_id, - 'instance_uuid': instance_id, 'connector': connector}} + if instance_id: + body['attachment']['instance_uuid'] = instance_id if self.api_version >= api_versions.APIVersion("3.54"): if mode and mode != 'null': body['attachment']['mode'] = mode diff --git a/cinderclient/v3/shell.py b/cinderclient/v3/shell.py index 6d7d1eb3f..9865d38cf 100644 --- a/cinderclient/v3/shell.py +++ b/cinderclient/v3/shell.py @@ -2297,6 +2297,8 @@ def do_attachment_show(cs, args): help='Name or ID of volume or volumes to attach.') @utils.arg('server_id', metavar='', + nargs='?', + default=None, help='ID of server attaching to.') @utils.arg('--connect', metavar='', diff --git a/releasenotes/notes/attachment-create-optional-server-id-9299d9da2b62b263.yaml b/releasenotes/notes/attachment-create-optional-server-id-9299d9da2b62b263.yaml new file mode 100644 index 000000000..11935820b --- /dev/null +++ b/releasenotes/notes/attachment-create-optional-server-id-9299d9da2b62b263.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + When attaching to a host, we don't need a server id + so it shouldn't be mandatory to be supplied with + attachment-create operation. + The server_id parameter is made optional so we can + create attachments without passing it. + The backward compatibility is maintained so we can pass + it like how we currently do if required. \ No newline at end of file