From c9e0c01f67a00e63bb5d8b5781d7e8e87b39136c Mon Sep 17 00:00:00 2001
From: Huanxuan Ao <huanxuan.ao@easystack.cn>
Date: Tue, 27 Sep 2016 12:27:05 +0800
Subject: [PATCH] Add and modify options for "volume create" command

1.Add mutually exclusive options into a mutually
exclusive group.
2.Add "--source-replicated", "--consistency-group",
"--hint" and "multi-attach" options
3.Make --size option to be optional under some cases

Closes-Bug: #1568005
Closes-Bug: #1627913
Implements: bp implement-cinder-features
Co-Authored-By: Roman Vasilets <rvasilets@mirantis.com>
Change-Id: I2c4c3073195d33774e477f4d7f22e383b14b41dd
---
 doc/source/command-objects/volume.rst         |  31 ++++-
 .../tests/unit/volume/v1/test_volume.py       |  62 +++++++++
 .../tests/unit/volume/v2/test_volume.py       | 131 ++++++++++++++++--
 openstackclient/volume/v1/volume.py           |  30 ++--
 openstackclient/volume/v2/volume.py           |  67 ++++++++-
 .../notes/bug-1627913-2adf4182977e5926.yaml   |   5 +
 6 files changed, 296 insertions(+), 30 deletions(-)
 create mode 100644 releasenotes/notes/bug-1627913-2adf4182977e5926.yaml

diff --git a/doc/source/command-objects/volume.rst b/doc/source/command-objects/volume.rst
index f772557fc6..8f1233614c 100644
--- a/doc/source/command-objects/volume.rst
+++ b/doc/source/command-objects/volume.rst
@@ -13,21 +13,23 @@ Create new volume
 .. code:: bash
 
     os volume create
-        --size <size>
+        [--size <size>]
         [--type <volume-type>]
-        [--image <image>]
-        [--snapshot <snapshot>]
-        [--source <volume>]
+        [--image <image> | --snapshot <snapshot> | --source <volume> | --source-replicated <replicated-volume>]
         [--description <description>]
         [--user <user>]
         [--project <project>]
         [--availability-zone <availability-zone>]
+        [--consistency-group <consistency-group>]
         [--property <key=value> [...] ]
+        [--hint <key=value> [...] ]
+        [--multi-attach]
         <name>
 
-.. option:: --size <size> (required)
+.. option:: --size <size>
 
     Volume size in GB
+    (Required unless --snapshot or --source or --source-replicated is specified)
 
 .. option:: --type <volume-type>
 
@@ -46,10 +48,14 @@ Create new volume
 
     Use :option:`\<snapshot\>` as source of volume (name or ID)
 
-.. option:: --source <source>
+.. option:: --source <volume>
 
     Volume to clone (name or ID)
 
+.. option:: --source-replicated <replicated-volume>
+
+    Replicated volume to clone (name or ID)
+
 .. option:: --description <description>
 
     Volume description
@@ -66,10 +72,23 @@ Create new volume
 
     Create volume in :option:`\<availability-zone\>`
 
+.. option:: --consistency-group <consistency-group>
+
+    Consistency group where the new volume belongs to
+
 .. option:: --property <key=value>
 
     Set a property on this volume (repeat option to set multiple properties)
 
+.. option:: --hint <key=value>
+
+    Arbitrary scheduler hint key-value pairs to help boot an instance
+    (repeat option to set multiple hints)
+
+.. option:: --multi-attach
+
+    Allow volume to be attached more than once (default to False)
+
 .. _volume_create-name:
 .. describe:: <name>
 
diff --git a/openstackclient/tests/unit/volume/v1/test_volume.py b/openstackclient/tests/unit/volume/v1/test_volume.py
index 895f1f876b..73c00844e8 100644
--- a/openstackclient/tests/unit/volume/v1/test_volume.py
+++ b/openstackclient/tests/unit/volume/v1/test_volume.py
@@ -23,6 +23,7 @@ from osc_lib import utils
 
 from openstackclient.tests.unit import fakes
 from openstackclient.tests.unit.identity.v2_0 import fakes as identity_fakes
+from openstackclient.tests.unit import utils as tests_utils
 from openstackclient.tests.unit.volume.v1 import fakes as volume_fakes
 from openstackclient.volume.v1 import volume
 
@@ -411,6 +412,67 @@ class TestVolumeCreate(TestVolume):
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.datalist, data)
 
+    def test_volume_create_with_source(self):
+        self.volumes_mock.get.return_value = self.new_volume
+        arglist = [
+            '--source', self.new_volume.id,
+            self.new_volume.display_name,
+        ]
+        verifylist = [
+            ('source', self.new_volume.id),
+            ('name', self.new_volume.display_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.volumes_mock.create.assert_called_with(
+            None,
+            None,
+            self.new_volume.id,
+            self.new_volume.display_name,
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+            None,
+        )
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.datalist, data)
+
+    def test_volume_create_without_size(self):
+        arglist = [
+            self.new_volume.display_name,
+        ]
+        verifylist = [
+            ('name', self.new_volume.display_name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+                          parsed_args)
+
+    def test_volume_create_with_multi_source(self):
+        arglist = [
+            '--image', 'source_image',
+            '--source', 'source_volume',
+            '--snapshot', 'source_snapshot',
+            '--size', str(self.new_volume.size),
+            self.new_volume.display_name,
+        ]
+        verifylist = [
+            ('image', 'source_image'),
+            ('source', 'source_volume'),
+            ('snapshot', 'source_snapshot'),
+            ('size', self.new_volume.size),
+            ('name', self.new_volume.display_name),
+        ]
+
+        self.assertRaises(tests_utils.ParserException, self.check_parser,
+                          self.cmd, arglist, verifylist)
+
 
 class TestVolumeDelete(TestVolume):
 
diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py
index 5bdde9deba..f4a7c14283 100644
--- a/openstackclient/tests/unit/volume/v2/test_volume.py
+++ b/openstackclient/tests/unit/volume/v2/test_volume.py
@@ -21,6 +21,7 @@ from osc_lib import utils
 
 from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes
 from openstackclient.tests.unit.image.v2 import fakes as image_fakes
+from openstackclient.tests.unit import utils as tests_utils
 from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes
 from openstackclient.volume.v2 import volume
 
@@ -45,6 +46,10 @@ class TestVolume(volume_fakes.TestVolume):
         self.snapshots_mock = self.app.client_manager.volume.volume_snapshots
         self.snapshots_mock.reset_mock()
 
+        self.consistencygroups_mock = (
+            self.app.client_manager.volume.consistencygroups)
+        self.consistencygroups_mock.reset_mock()
+
     def setup_volumes_mock(self, count):
         volumes = volume_fakes.FakeVolume.create_volumes(count=count)
 
@@ -123,18 +128,28 @@ class TestVolumeCreate(TestVolume):
             availability_zone=None,
             metadata=None,
             imageRef=None,
-            source_volid=None
+            source_volid=None,
+            consistencygroup_id=None,
+            source_replica=None,
+            multiattach=False,
+            scheduler_hints=None,
         )
 
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.datalist, data)
 
     def test_volume_create_options(self):
+        consistency_group = (
+            volume_fakes.FakeConsistencyGroup.create_one_consistency_group())
+        self.consistencygroups_mock.get.return_value = consistency_group
         arglist = [
             '--size', str(self.new_volume.size),
             '--description', self.new_volume.description,
             '--type', self.new_volume.volume_type,
             '--availability-zone', self.new_volume.availability_zone,
+            '--consistency-group', consistency_group.id,
+            '--hint', 'k=v',
+            '--multi-attach',
             self.new_volume.name,
         ]
         verifylist = [
@@ -142,6 +157,9 @@ class TestVolumeCreate(TestVolume):
             ('description', self.new_volume.description),
             ('type', self.new_volume.volume_type),
             ('availability_zone', self.new_volume.availability_zone),
+            ('consistency_group', consistency_group.id),
+            ('hint', {'k': 'v'}),
+            ('multi_attach', True),
             ('name', self.new_volume.name),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -162,7 +180,11 @@ class TestVolumeCreate(TestVolume):
             availability_zone=self.new_volume.availability_zone,
             metadata=None,
             imageRef=None,
-            source_volid=None
+            source_volid=None,
+            consistencygroup_id=consistency_group.id,
+            source_replica=None,
+            multiattach=True,
+            scheduler_hints={'k': 'v'},
         )
 
         self.assertEqual(self.columns, columns)
@@ -204,7 +226,11 @@ class TestVolumeCreate(TestVolume):
             availability_zone=None,
             metadata=None,
             imageRef=None,
-            source_volid=None
+            source_volid=None,
+            consistencygroup_id=None,
+            source_replica=None,
+            multiattach=False,
+            scheduler_hints=None,
         )
 
         self.assertEqual(self.columns, columns)
@@ -246,7 +272,11 @@ class TestVolumeCreate(TestVolume):
             availability_zone=None,
             metadata=None,
             imageRef=None,
-            source_volid=None
+            source_volid=None,
+            consistencygroup_id=None,
+            source_replica=None,
+            multiattach=False,
+            scheduler_hints=None,
         )
 
         self.assertEqual(self.columns, columns)
@@ -282,7 +312,11 @@ class TestVolumeCreate(TestVolume):
             availability_zone=None,
             metadata={'Alpha': 'a', 'Beta': 'b'},
             imageRef=None,
-            source_volid=None
+            source_volid=None,
+            consistencygroup_id=None,
+            source_replica=None,
+            multiattach=False,
+            scheduler_hints=None,
         )
 
         self.assertEqual(self.columns, columns)
@@ -321,6 +355,10 @@ class TestVolumeCreate(TestVolume):
             metadata=None,
             imageRef=image.id,
             source_volid=None,
+            consistencygroup_id=None,
+            source_replica=None,
+            multiattach=False,
+            scheduler_hints=None,
         )
 
         self.assertEqual(self.columns, columns)
@@ -358,7 +396,11 @@ class TestVolumeCreate(TestVolume):
             availability_zone=None,
             metadata=None,
             imageRef=image.id,
-            source_volid=None
+            source_volid=None,
+            consistencygroup_id=None,
+            source_replica=None,
+            multiattach=False,
+            scheduler_hints=None,
         )
 
         self.assertEqual(self.columns, columns)
@@ -368,12 +410,10 @@ class TestVolumeCreate(TestVolume):
         snapshot = volume_fakes.FakeSnapshot.create_one_snapshot()
         self.new_volume.snapshot_id = snapshot.id
         arglist = [
-            '--size', str(self.new_volume.size),
             '--snapshot', self.new_volume.snapshot_id,
             self.new_volume.name,
         ]
         verifylist = [
-            ('size', self.new_volume.size),
             ('snapshot', self.new_volume.snapshot_id),
             ('name', self.new_volume.name),
         ]
@@ -387,7 +427,7 @@ class TestVolumeCreate(TestVolume):
         columns, data = self.cmd.take_action(parsed_args)
 
         self.volumes_mock.create.assert_called_once_with(
-            size=self.new_volume.size,
+            size=None,
             snapshot_id=snapshot.id,
             name=self.new_volume.name,
             description=None,
@@ -397,12 +437,83 @@ class TestVolumeCreate(TestVolume):
             availability_zone=None,
             metadata=None,
             imageRef=None,
-            source_volid=None
+            source_volid=None,
+            consistencygroup_id=None,
+            source_replica=None,
+            multiattach=False,
+            scheduler_hints=None,
         )
 
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.datalist, data)
 
+    def test_volume_create_with_source_replicated(self):
+        self.volumes_mock.get.return_value = self.new_volume
+        arglist = [
+            '--source-replicated', self.new_volume.id,
+            self.new_volume.name,
+        ]
+        verifylist = [
+            ('source_replicated', self.new_volume.id),
+            ('name', self.new_volume.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+        self.volumes_mock.create.assert_called_once_with(
+            size=None,
+            snapshot_id=None,
+            name=self.new_volume.name,
+            description=None,
+            volume_type=None,
+            user_id=None,
+            project_id=None,
+            availability_zone=None,
+            metadata=None,
+            imageRef=None,
+            source_volid=None,
+            consistencygroup_id=None,
+            source_replica=self.new_volume.id,
+            multiattach=False,
+            scheduler_hints=None,
+        )
+
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.datalist, data)
+
+    def test_volume_create_without_size(self):
+        arglist = [
+            self.new_volume.name,
+        ]
+        verifylist = [
+            ('name', self.new_volume.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(exceptions.CommandError, self.cmd.take_action,
+                          parsed_args)
+
+    def test_volume_create_with_multi_source(self):
+        arglist = [
+            '--image', 'source_image',
+            '--source', 'source_volume',
+            '--snapshot', 'source_snapshot',
+            '--source-replicated', 'source_replicated_volume',
+            '--size', str(self.new_volume.size),
+            self.new_volume.name,
+        ]
+        verifylist = [
+            ('image', 'source_image'),
+            ('source', 'source_volume'),
+            ('snapshot', 'source_snapshot'),
+            ('source-replicated', 'source_replicated_volume'),
+            ('size', self.new_volume.size),
+            ('name', self.new_volume.name),
+        ]
+
+        self.assertRaises(tests_utils.ParserException, self.check_parser,
+                          self.cmd, arglist, verifylist)
+
 
 class TestVolumeDelete(TestVolume):
 
diff --git a/openstackclient/volume/v1/volume.py b/openstackclient/volume/v1/volume.py
index 89fa2014ae..cafe8ce6ab 100644
--- a/openstackclient/volume/v1/volume.py
+++ b/openstackclient/volume/v1/volume.py
@@ -30,6 +30,20 @@ from openstackclient.i18n import _
 LOG = logging.getLogger(__name__)
 
 
+def _check_size_arg(args):
+    """Check whether --size option is required or not.
+
+    Require size parameter only in case when snapshot or source
+    volume is not specified.
+    """
+
+    if ((args.snapshot or args.source)
+            is None and args.size is None):
+        msg = _("--size is a required option if snapshot "
+                "or source volume is not specified.")
+        raise exceptions.CommandError(msg)
+
+
 class CreateVolume(command.ShowOne):
     """Create new volume"""
 
@@ -43,32 +57,32 @@ class CreateVolume(command.ShowOne):
         parser.add_argument(
             '--size',
             metavar='<size>',
-            required=True,
             type=int,
-            help=_('Volume size in GB'),
+            help=_("Volume size in GB (Required unless --snapshot or "
+                   "--source is specified)"),
         )
         parser.add_argument(
             '--type',
             metavar='<volume-type>',
             help=_("Set the type of volume"),
         )
-        parser.add_argument(
+        source_group = parser.add_mutually_exclusive_group()
+        source_group.add_argument(
             '--image',
             metavar='<image>',
             help=_('Use <image> as source of volume (name or ID)'),
         )
-        snapshot_group = parser.add_mutually_exclusive_group()
-        snapshot_group.add_argument(
+        source_group.add_argument(
             '--snapshot',
             metavar='<snapshot>',
             help=_('Use <snapshot> as source of volume (name or ID)'),
         )
-        snapshot_group.add_argument(
+        source_group.add_argument(
             '--snapshot-id',
             metavar='<snapshot-id>',
             help=argparse.SUPPRESS,
         )
-        parser.add_argument(
+        source_group.add_argument(
             '--source',
             metavar='<volume>',
             help=_('Volume to clone (name or ID)'),
@@ -104,7 +118,7 @@ class CreateVolume(command.ShowOne):
         return parser
 
     def take_action(self, parsed_args):
-
+        _check_size_arg(parsed_args)
         identity_client = self.app.client_manager.identity
         image_client = self.app.client_manager.image
         volume_client = self.app.client_manager.volume
diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py
index b006718996..e74051145b 100644
--- a/openstackclient/volume/v2/volume.py
+++ b/openstackclient/volume/v2/volume.py
@@ -30,6 +30,20 @@ from openstackclient.identity import common as identity_common
 LOG = logging.getLogger(__name__)
 
 
+def _check_size_arg(args):
+    """Check whether --size option is required or not.
+
+    Require size parameter only in case when snapshot or source
+    volume is not specified.
+    """
+
+    if ((args.snapshot or args.source or args.source_replicated)
+            is None and args.size is None):
+        msg = _("--size is a required option if snapshot "
+                "or source volume is not specified.")
+        raise exceptions.CommandError(msg)
+
+
 class CreateVolume(command.ShowOne):
     """Create new volume"""
 
@@ -44,29 +58,35 @@ class CreateVolume(command.ShowOne):
             "--size",
             metavar="<size>",
             type=int,
-            required=True,
-            help=_("Volume size in GB"),
+            help=_("Volume size in GB (Required unless --snapshot or "
+                   "--source or --source-replicated is specified)"),
         )
         parser.add_argument(
             "--type",
             metavar="<volume-type>",
             help=_("Set the type of volume"),
         )
-        parser.add_argument(
+        source_group = parser.add_mutually_exclusive_group()
+        source_group.add_argument(
             "--image",
             metavar="<image>",
             help=_("Use <image> as source of volume (name or ID)"),
         )
-        parser.add_argument(
+        source_group.add_argument(
             "--snapshot",
             metavar="<snapshot>",
             help=_("Use <snapshot> as source of volume (name or ID)"),
         )
-        parser.add_argument(
+        source_group.add_argument(
             "--source",
             metavar="<volume>",
             help=_("Volume to clone (name or ID)"),
         )
+        source_group.add_argument(
+            "--source-replicated",
+            metavar="<replicated-volume>",
+            help=_("Replicated volume to clone (name or ID)"),
+        )
         parser.add_argument(
             "--description",
             metavar="<description>",
@@ -87,6 +107,11 @@ class CreateVolume(command.ShowOne):
             metavar="<availability-zone>",
             help=_("Create volume in <availability-zone>"),
         )
+        parser.add_argument(
+            "--consistency-group",
+            metavar="consistency-group>",
+            help=_("Consistency group where the new volume belongs to"),
+        )
         parser.add_argument(
             "--property",
             metavar="<key=value>",
@@ -94,9 +119,23 @@ class CreateVolume(command.ShowOne):
             help=_("Set a property to this volume "
                    "(repeat option to set multiple properties)"),
         )
+        parser.add_argument(
+            "--hint",
+            metavar="<key=value>",
+            action=parseractions.KeyValueAction,
+            help=_("Arbitrary scheduler hint key-value pairs to help boot "
+                   "an instance (repeat option to set multiple hints)"),
+        )
+        parser.add_argument(
+            "--multi-attach",
+            action="store_true",
+            help=_("Allow volume to be attached more than once "
+                   "(default to False)")
+        )
         return parser
 
     def take_action(self, parsed_args):
+        _check_size_arg(parsed_args)
         identity_client = self.app.client_manager.identity
         volume_client = self.app.client_manager.volume
         image_client = self.app.client_manager.image
@@ -107,6 +146,18 @@ class CreateVolume(command.ShowOne):
                 volume_client.volumes,
                 parsed_args.source).id
 
+        replicated_source_volume = None
+        if parsed_args.source_replicated:
+            replicated_source_volume = utils.find_resource(
+                volume_client.volumes,
+                parsed_args.source_replicated).id
+
+        consistency_group = None
+        if parsed_args.consistency_group:
+            consistency_group = utils.find_resource(
+                volume_client.consistencygroups,
+                parsed_args.consistency_group).id
+
         image = None
         if parsed_args.image:
             image = utils.find_resource(
@@ -142,7 +193,11 @@ class CreateVolume(command.ShowOne):
             availability_zone=parsed_args.availability_zone,
             metadata=parsed_args.property,
             imageRef=image,
-            source_volid=source_volume
+            source_volid=source_volume,
+            consistencygroup_id=consistency_group,
+            source_replica=replicated_source_volume,
+            multiattach=parsed_args.multi_attach,
+            scheduler_hints=parsed_args.hint,
         )
         # Remove key links from being displayed
         volume._info.update(
diff --git a/releasenotes/notes/bug-1627913-2adf4182977e5926.yaml b/releasenotes/notes/bug-1627913-2adf4182977e5926.yaml
new file mode 100644
index 0000000000..454d4c8958
--- /dev/null
+++ b/releasenotes/notes/bug-1627913-2adf4182977e5926.yaml
@@ -0,0 +1,5 @@
+---
+features:
+  - Add ``--source-replicated``, ``--consistency-group``, ``--hint`` and
+    ``--multi-attach`` options to ``volume create`` command in volume v2.
+    [Bug `1627913 <https://bugs.launchpad.net/python-openstackclient/+bug/1627913>`_]