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
This commit is contained in:
		| @@ -29,6 +29,16 @@ fake_attachment = {'attachment': { | |||||||
|     'instance': 'e84fda45-4de4-4ce4-8f39-fc9d3b0aa05e', |     'instance': 'e84fda45-4de4-4ce4-8f39-fc9d3b0aa05e', | ||||||
|     'volume_id': '557ad76c-ce54-40a3-9e91-c40d21665cc3', }} |     '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': [ | fake_attachment_list = {'attachments': [ | ||||||
|     {'instance': 'instance_1', |     {'instance': 'instance_1', | ||||||
|      'name': 'attachment-1', |      'name': 'attachment-1', | ||||||
| @@ -297,7 +307,9 @@ class FakeHTTPClient(fake_v2.FakeHTTPClient): | |||||||
|     # |     # | ||||||
|  |  | ||||||
|     def post_attachments(self, **kw): |     def post_attachments(self, **kw): | ||||||
|  |         if kw['body']['attachment'].get('instance_uuid'): | ||||||
|             return (200, {}, fake_attachment) |             return (200, {}, fake_attachment) | ||||||
|  |         return (200, {}, fake_attachment_without_instance_id) | ||||||
|  |  | ||||||
|     def get_attachments(self, **kw): |     def get_attachments(self, **kw): | ||||||
|         return (200, {}, fake_attachment_list) |         return (200, {}, fake_attachment_list) | ||||||
|   | |||||||
| @@ -31,6 +31,17 @@ class AttachmentsTest(utils.TestCase): | |||||||
|         cs.assert_called('POST', '/attachments') |         cs.assert_called('POST', '/attachments') | ||||||
|         self.assertEqual(fakes.fake_attachment['attachment'], att) |         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): |     def test_complete_attachment(self): | ||||||
|         cs = fakes.FakeClient(api_versions.APIVersion('3.44')) |         cs = fakes.FakeClient(api_versions.APIVersion('3.44')) | ||||||
|         att = cs.attachments.complete('a232e9ae') |         att = cs.attachments.complete('a232e9ae') | ||||||
|   | |||||||
| @@ -388,6 +388,25 @@ class ShellTest(utils.TestCase): | |||||||
|               {'cmd': 'abc 1233', |               {'cmd': 'abc 1233', | ||||||
|                'body': {'instance_uuid': '1233', |                'body': {'instance_uuid': '1233', | ||||||
|                         'connector': {}, |                         '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'}}) |                         'volume_uuid': '1234'}}) | ||||||
|     @mock.patch('cinderclient.utils.find_resource') |     @mock.patch('cinderclient.utils.find_resource') | ||||||
|     @ddt.unpack |     @ddt.unpack | ||||||
| @@ -429,6 +448,27 @@ class ShellTest(utils.TestCase): | |||||||
|                'body': {'instance_uuid': '1233', |                'body': {'instance_uuid': '1233', | ||||||
|                         'connector': {}, |                         'connector': {}, | ||||||
|                         'volume_uuid': '1234', |                         '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'}}) |                         'mode': 'ro'}}) | ||||||
|     @mock.patch('cinderclient.utils.find_resource') |     @mock.patch('cinderclient.utils.find_resource') | ||||||
|     @ddt.unpack |     @ddt.unpack | ||||||
|   | |||||||
| @@ -27,11 +27,12 @@ class VolumeAttachmentManager(base.ManagerWithFind): | |||||||
|     resource_class = VolumeAttachment |     resource_class = VolumeAttachment | ||||||
|  |  | ||||||
|     @api_versions.wraps('3.27') |     @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.""" |         """Create a attachment for specified volume.""" | ||||||
|         body = {'attachment': {'volume_uuid': volume_id, |         body = {'attachment': {'volume_uuid': volume_id, | ||||||
|                                'instance_uuid': instance_id, |  | ||||||
|                                'connector': connector}} |                                'connector': connector}} | ||||||
|  |         if instance_id: | ||||||
|  |             body['attachment']['instance_uuid'] = instance_id | ||||||
|         if self.api_version >= api_versions.APIVersion("3.54"): |         if self.api_version >= api_versions.APIVersion("3.54"): | ||||||
|             if mode and mode != 'null': |             if mode and mode != 'null': | ||||||
|                 body['attachment']['mode'] = mode |                 body['attachment']['mode'] = mode | ||||||
|   | |||||||
| @@ -2297,6 +2297,8 @@ def do_attachment_show(cs, args): | |||||||
|            help='Name or ID of volume or volumes to attach.') |            help='Name or ID of volume or volumes to attach.') | ||||||
| @utils.arg('server_id', | @utils.arg('server_id', | ||||||
|            metavar='<server_id>', |            metavar='<server_id>', | ||||||
|  |            nargs='?', | ||||||
|  |            default=None, | ||||||
|            help='ID of server attaching to.') |            help='ID of server attaching to.') | ||||||
| @utils.arg('--connect', | @utils.arg('--connect', | ||||||
|            metavar='<connect>', |            metavar='<connect>', | ||||||
|   | |||||||
| @@ -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. | ||||||
		Reference in New Issue
	
	Block a user
	 Rajat Dhasmana
					Rajat Dhasmana