[BugFix] 'Mountpoint' is missing in attachment CLIs.

There are some issues around new attach/detach APIs/CLIs,
fix them step by step. This patch also adds related
testcases and fix some errors in help message.

Closes-Bug: #1675973

Change-Id: I769ea6267403919220c515d471e7bbb8d2d95463

cinderclient/tests/unit/v3/fakes.py

@@ -222,6 +222,44 @@ class FakeHTTPClient(fake_v2.FakeHTTPClient):
         return (200, {},
                 {'backups': backup})
 
+    #
+    # Attachments
+    #
+    def post_attachments(self, **kw):
+        return (202, {}, {
+            'attachment': {'instance': 1234,
+                           'name': 'attachment-1',
+                           'volume_id': 'fake_volume_1',
+                           'status': 'reserved'}})
+
+    def get_attachments(self, **kw):
+        return (200, {}, {
+            'attachments': [{'instance': 1,
+                             'name': 'attachment-1',
+                             'volume_id': 'fake_volume_1',
+                             'status': 'reserved'},
+                            {'instance': 2,
+                             'name': 'attachment-2',
+                             'volume_id': 'fake_volume_2',
+                             'status': 'reserverd'}]})
+
+    def get_attachments_1234(self, **kw):
+        return (200, {}, {
+            'attachment': {'instance': 1234,
+                           'name': 'attachment-1',
+                           'volume_id': 'fake_volume_1',
+                           'status': 'reserved'}})
+
+    def put_attachments_1234(self, **kw):
+        return (200, {}, {
+            'attachment': {'instance': 1234,
+                           'name': 'attachment-1',
+                           'volume_id': 'fake_volume_1',
+                           'status': 'reserved'}})
+
+    def delete_attachments_1234(self, **kw):
+        return 204, {}, None
+
     #
     # GroupTypes
     #

cinderclient/tests/unit/v3/test_shell.py

@@ -95,6 +95,84 @@ class ShellTest(utils.TestCase):
         self.run_command('availability-zone-list')
         self.assert_called('GET', '/os-availability-zone')
 
+    @ddt.data({'cmd': '1234 --instance 1233',
+               'body': {'instance_uuid': '1233',
+                        'connector': {},
+                        'volume_uuid': '1234'}},
+              {'cmd': '1234 --instance 1233 '
+                      '--connect True '
+                      '--ip 10.23.12.23 --host server01 '
+                      '--platform x86_xx '
+                      '--ostype 123 '
+                      '--multipath true '
+                      '--mountpoint /123 '
+                      '--initiator aabbccdd',
+               'body': {'instance_uuid': '1233',
+                        'connector': {'ip': '10.23.12.23',
+                                      'host': 'server01',
+                                      'os_type': '123',
+                                      'multipath': 'true',
+                                      'mountpoint': '/123',
+                                      'initiator': 'aabbccdd',
+                                      'platform': 'x86_xx'},
+                        'volume_uuid': '1234'}})
+    @ddt.unpack
+    def test_attachment_create(self, cmd, body):
+        command = '--os-volume-api-version 3.27 attachment-create '
+        command += cmd
+        self.run_command(command)
+        expected = {'attachment': body}
+        self.assert_called('POST', '/attachments', body=expected)
+
+    @ddt.data({'cmd': '',
+               'expected': ''},
+              {'cmd': '--volume-id 1234',
+               'expected': '?volume_id=1234'},
+              {'cmd': '--status error',
+               'expected': '?status=error'},
+              {'cmd': '--all-tenants 1',
+               'expected': '?all_tenants=1'},
+              {'cmd': '--all-tenants 1 --volume-id 12345',
+               'expected': '?all_tenants=1&volume_id=12345'}
+              )
+    @ddt.unpack
+    def test_attachment_list(self, cmd, expected):
+        command = '--os-volume-api-version 3.27 attachment-list '
+        command += cmd
+        self.run_command(command)
+        self.assert_called('GET', '/attachments%s' % expected)
+
+    def test_attachment_show(self):
+        self.run_command('--os-volume-api-version 3.27 attachment-show 1234')
+        self.assert_called('GET', '/attachments/1234')
+
+    @ddt.data({'cmd': '1234 '
+                      '--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'}}})
+    @ddt.unpack
+    def test_attachment_update(self, cmd, body):
+        command = '--os-volume-api-version 3.27 attachment-update '
+        command += cmd
+        self.run_command(command)
+        self.assert_called('PUT', '/attachments/1234', body={'attachment':
+                                                             body})
+
+    def test_attachment_delete(self):
+        self.run_command('--os-volume-api-version 3.27 '
+                         'attachment-delete 1234')
+        self.assert_called('DELETE', '/attachments/1234')
+
     def test_upload_to_image(self):
         expected = {'os-volume_upload_image': {'force': False,
                                                'container_format': 'bare',

cinderclient/v3/shell.py

@@ -1417,7 +1417,7 @@ def do_attachment_show(cs, args):
 @utils.arg('--multipath',
            metavar='<multipath>',
            default=False,
-           help='OS type. Default=False.')
+           help='Use multipath. Default=False.')
 @utils.arg('--mountpoint',
            metavar='<mountpoint>',
            default=None,
@@ -1433,7 +1433,8 @@ def do_attachment_create(cs, args):
                      'platform': args.platform,
                      'host': args.host,
                      'os_type': args.ostype,
-                     'multipath': args.multipath}
+                     'multipath': args.multipath,
+                     'mountpoint': args.mountpoint}
     attachment = cs.attachments.create(args.volume,
                                        connector,
                                        args.instance)
@@ -1470,7 +1471,7 @@ def do_attachment_create(cs, args):
 @utils.arg('--multipath',
            metavar='<multipath>',
            default=False,
-           help='OS type. Default=False.')
+           help='Use multipath. Default=False.')
 @utils.arg('--mountpoint',
            metavar='<mountpoint>',
            default=None,
@@ -1486,7 +1487,8 @@ def do_attachment_update(cs, args):
                  'platform': args.platform,
                  'host': args.host,
                  'os_type': args.ostype,
-                 'multipath': args.multipath}
+                 'multipath': args.multipath,
+                 'mountpoint': args.mountpoint}
     attachment = cs.attachments.update(args.attachment,
                                        connector)
     attachment_dict = attachment.to_dict()
@@ -1505,6 +1507,7 @@ def do_attachment_delete(cs, args):
     for attachment in args.attachment:
         cs.attachments.delete(attachment)
 
+
 @api_versions.wraps('3.0')
 def do_version_list(cs, args):
     """List all API versions."""

releasenotes/notes/bug-1675973-ad91a7a9f50e658a.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - The mountpoint argument was ignored when creating an attachment
+    and now has been fixed.
+    [Bug `1675973 <https://bugs.launchpad.net/bugs/1675973>`_]