volume: Make better use of argparse

The latest in a series.

Change-Id: I8273c817e38120ba0b25aebdbfa1c2872222765e
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

openstackclient/tests/unit/volume/v2/test_volume.py

@@ -188,7 +188,7 @@ class TestVolumeCreate(TestVolume):
             self.new_volume.name,
         ]
         verifylist = [
-            ('property', {'Alpha': 'a', 'Beta': 'b'}),
+            ('properties', {'Alpha': 'a', 'Beta': 'b'}),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -382,9 +382,7 @@ class TestVolumeCreate(TestVolume):
         ]
         verifylist = [
             ('bootable', True),
-            ('non_bootable', False),
             ('read_only', True),
-            ('read_write', False),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -427,9 +425,7 @@ class TestVolumeCreate(TestVolume):
         ]
         verifylist = [
             ('bootable', False),
-            ('non_bootable', True),
             ('read_only', False),
-            ('read_write', True),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -481,9 +477,7 @@ class TestVolumeCreate(TestVolume):
         ]
         verifylist = [
             ('bootable', True),
-            ('non_bootable', False),
             ('read_only', True),
-            ('read_write', False),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -532,9 +526,7 @@ class TestVolumeCreate(TestVolume):
         ]
         verifylist = [
             ('bootable', False),
-            ('non_bootable', True),
             ('read_only', True),
-            ('read_write', False),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -1407,16 +1399,16 @@ class TestVolumeSet(TestVolume):
             self.new_volume.id,
         ]
         verifylist = [
-            ('property', {'a': 'b', 'c': 'd'}),
+            ('properties', {'a': 'b', 'c': 'd'}),
             ('volume', self.new_volume.id),
-            ('bootable', False),
-            ('non_bootable', False),
+            ('bootable', None),
+            ('read_only', None),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         self.cmd.take_action(parsed_args)
         self.volumes_mock.set_metadata.assert_called_with(
-            self.new_volume.id, parsed_args.property
+            self.new_volume.id, parsed_args.properties
         )
 
     def test_volume_set_image_property(self):
@@ -1428,10 +1420,10 @@ class TestVolumeSet(TestVolume):
             self.new_volume.id,
         ]
         verifylist = [
-            ('image_property', {'Alpha': 'a', 'Beta': 'b'}),
+            ('image_properties', {'Alpha': 'a', 'Beta': 'b'}),
             ('volume', self.new_volume.id),
-            ('bootable', False),
-            ('non_bootable', False),
+            ('bootable', None),
+            ('read_only', None),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -1439,14 +1431,13 @@ class TestVolumeSet(TestVolume):
         # returns nothing
         self.cmd.take_action(parsed_args)
         self.volumes_mock.set_image_metadata.assert_called_with(
-            self.new_volume.id, parsed_args.image_property
+            self.new_volume.id, parsed_args.image_properties
         )
 
     def test_volume_set_state(self):
         arglist = ['--state', 'error', self.new_volume.id]
         verifylist = [
-            ('read_only', False),
-            ('read_write', False),
+            ('read_only', None),
             ('state', 'error'),
             ('volume', self.new_volume.id),
         ]
@@ -1511,36 +1502,40 @@ class TestVolumeSet(TestVolume):
 
     def test_volume_set_bootable(self):
         arglist = [
-            ['--bootable', self.new_volume.id],
-            ['--non-bootable', self.new_volume.id],
+            '--bootable',
+            self.new_volume.id,
         ]
         verifylist = [
-            [
-                ('bootable', True),
-                ('non_bootable', False),
-                ('volume', self.new_volume.id),
-            ],
-            [
-                ('bootable', False),
-                ('non_bootable', True),
-                ('volume', self.new_volume.id),
-            ],
+            ('bootable', True),
+            ('volume', self.new_volume.id),
         ]
-        for index in range(len(arglist)):
-            parsed_args = self.check_parser(
-                self.cmd, arglist[index], verifylist[index]
-            )
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-            self.cmd.take_action(parsed_args)
-            self.volumes_mock.set_bootable.assert_called_with(
-                self.new_volume.id, verifylist[index][0][1]
-            )
+        self.cmd.take_action(parsed_args)
+        self.volumes_mock.set_bootable.assert_called_with(
+            self.new_volume.id, verifylist[0][1]
+        )
 
-    def test_volume_set_readonly(self):
+    def test_volume_set_non_bootable(self):
+        arglist = [
+            '--non-bootable',
+            self.new_volume.id,
+        ]
+        verifylist = [
+            ('bootable', False),
+            ('volume', self.new_volume.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+        self.volumes_mock.set_bootable.assert_called_with(
+            self.new_volume.id, verifylist[0][1]
+        )
+
+    def test_volume_set_read_only(self):
         arglist = ['--read-only', self.new_volume.id]
         verifylist = [
             ('read_only', True),
-            ('read_write', False),
             ('volume', self.new_volume.id),
         ]
 
@@ -1556,7 +1551,6 @@ class TestVolumeSet(TestVolume):
         arglist = ['--read-write', self.new_volume.id]
         verifylist = [
             ('read_only', False),
-            ('read_write', True),
             ('volume', self.new_volume.id),
         ]
 
@@ -1686,7 +1680,7 @@ class TestVolumeUnset(TestVolume):
             self.new_volume.id,
         ]
         verifylist = [
-            ('image_property', {'Alpha': 'a', 'Beta': 'b'}),
+            ('image_properties', {'Alpha': 'a', 'Beta': 'b'}),
             ('volume', self.new_volume.id),
         ]
         parsed_args = self.check_parser(self.cmd_set, arglist, verifylist)
@@ -1702,7 +1696,7 @@ class TestVolumeUnset(TestVolume):
             self.new_volume.id,
         ]
         verifylist_unset = [
-            ('image_property', ['Alpha']),
+            ('image_properties', ['Alpha']),
             ('volume', self.new_volume.id),
         ]
         parsed_args_unset = self.check_parser(
@@ -1714,7 +1708,7 @@ class TestVolumeUnset(TestVolume):
         self.cmd_unset.take_action(parsed_args_unset)
 
         self.volumes_mock.delete_image_metadata.assert_called_with(
-            self.new_volume.id, parsed_args_unset.image_property
+            self.new_volume.id, parsed_args_unset.image_properties
         )
 
     def test_volume_unset_image_property_fail(self):
@@ -1729,8 +1723,8 @@ class TestVolumeUnset(TestVolume):
             self.new_volume.id,
         ]
         verifylist = [
-            ('image_property', ['Alpha']),
-            ('property', ['Beta']),
+            ('image_properties', ['Alpha']),
+            ('properties', ['Beta']),
             ('volume', self.new_volume.id),
         ]
         parsed_args = self.check_parser(self.cmd_unset, arglist, verifylist)
@@ -1743,10 +1737,10 @@ class TestVolumeUnset(TestVolume):
                 'One or more of the unset operations failed', str(e)
             )
         self.volumes_mock.delete_image_metadata.assert_called_with(
-            self.new_volume.id, parsed_args.image_property
+            self.new_volume.id, parsed_args.image_properties
         )
         self.volumes_mock.delete_metadata.assert_called_with(
-            self.new_volume.id, parsed_args.property
+            self.new_volume.id, parsed_args.properties
         )
 
 

openstackclient/tests/unit/volume/v3/test_volume.py

@@ -179,7 +179,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume):
             self.new_volume.name,
         ]
         verifylist = [
-            ('property', {'Alpha': 'a', 'Beta': 'b'}),
+            ('properties', {'Alpha': 'a', 'Beta': 'b'}),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -440,9 +440,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume):
         ]
         verifylist = [
             ('bootable', True),
-            ('non_bootable', False),
             ('read_only', True),
-            ('read_write', False),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -486,9 +484,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume):
         ]
         verifylist = [
             ('bootable', False),
-            ('non_bootable', True),
             ('read_only', False),
-            ('read_write', True),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -541,9 +537,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume):
         ]
         verifylist = [
             ('bootable', True),
-            ('non_bootable', False),
             ('read_only', True),
-            ('read_write', False),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -593,9 +587,7 @@ class TestVolumeCreateLegacy(volume_fakes.TestVolume):
         ]
         verifylist = [
             ('bootable', False),
-            ('non_bootable', True),
             ('read_only', True),
-            ('read_write', False),
             ('size', self.new_volume.size),
             ('name', self.new_volume.name),
         ]
@@ -838,7 +830,7 @@ class TestVolumeCreate(volume_fakes.TestVolume):
             description=parsed_args.description,
             volume_type=parsed_args.type,
             availability_zone=parsed_args.availability_zone,
-            metadata=parsed_args.property,
+            metadata=parsed_args.properties,
             bootable=parsed_args.bootable,
             cluster=getattr(parsed_args, 'cluster', None),
         )
@@ -1815,16 +1807,16 @@ class TestVolumeSet(volume_fakes.TestVolume):
             self.new_volume.id,
         ]
         verifylist = [
-            ('property', {'a': 'b', 'c': 'd'}),
+            ('properties', {'a': 'b', 'c': 'd'}),
+            ('read_only', None),
+            ('bootable', None),
             ('volume', self.new_volume.id),
-            ('bootable', False),
-            ('non_bootable', False),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         self.cmd.take_action(parsed_args)
         self.volumes_mock.set_metadata.assert_called_with(
-            self.new_volume.id, parsed_args.property
+            self.new_volume.id, parsed_args.properties
         )
 
     def test_volume_set_image_property(self):
@@ -1836,10 +1828,10 @@ class TestVolumeSet(volume_fakes.TestVolume):
             self.new_volume.id,
         ]
         verifylist = [
-            ('image_property', {'Alpha': 'a', 'Beta': 'b'}),
+            ('image_properties', {'Alpha': 'a', 'Beta': 'b'}),
+            ('read_only', None),
+            ('bootable', None),
             ('volume', self.new_volume.id),
-            ('bootable', False),
-            ('non_bootable', False),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
@@ -1847,15 +1839,15 @@ class TestVolumeSet(volume_fakes.TestVolume):
         # returns nothing
         self.cmd.take_action(parsed_args)
         self.volumes_mock.set_image_metadata.assert_called_with(
-            self.new_volume.id, parsed_args.image_property
+            self.new_volume.id, parsed_args.image_properties
         )
 
     def test_volume_set_state(self):
         arglist = ['--state', 'error', self.new_volume.id]
         verifylist = [
-            ('read_only', False),
-            ('read_write', False),
             ('state', 'error'),
+            ('read_only', None),
+            ('bootable', None),
             ('volume', self.new_volume.id),
         ]
 
@@ -1919,36 +1911,40 @@ class TestVolumeSet(volume_fakes.TestVolume):
 
     def test_volume_set_bootable(self):
         arglist = [
-            ['--bootable', self.new_volume.id],
-            ['--non-bootable', self.new_volume.id],
+            '--bootable',
+            self.new_volume.id,
         ]
         verifylist = [
-            [
-                ('bootable', True),
-                ('non_bootable', False),
-                ('volume', self.new_volume.id),
-            ],
-            [
-                ('bootable', False),
-                ('non_bootable', True),
-                ('volume', self.new_volume.id),
-            ],
+            ('bootable', True),
+            ('volume', self.new_volume.id),
         ]
-        for index in range(len(arglist)):
-            parsed_args = self.check_parser(
-                self.cmd, arglist[index], verifylist[index]
-            )
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-            self.cmd.take_action(parsed_args)
-            self.volumes_mock.set_bootable.assert_called_with(
-                self.new_volume.id, verifylist[index][0][1]
-            )
+        self.cmd.take_action(parsed_args)
+        self.volumes_mock.set_bootable.assert_called_with(
+            self.new_volume.id, verifylist[0][1]
+        )
+
+    def test_volume_set_non_bootable(self):
+        arglist = [
+            '--non-bootable',
+            self.new_volume.id,
+        ]
+        verifylist = [
+            ('bootable', False),
+            ('volume', self.new_volume.id),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+        self.volumes_mock.set_bootable.assert_called_with(
+            self.new_volume.id, verifylist[0][1]
+        )
 
     def test_volume_set_readonly(self):
         arglist = ['--read-only', self.new_volume.id]
         verifylist = [
             ('read_only', True),
-            ('read_write', False),
             ('volume', self.new_volume.id),
         ]
 
@@ -1964,7 +1960,6 @@ class TestVolumeSet(volume_fakes.TestVolume):
         arglist = ['--read-write', self.new_volume.id]
         verifylist = [
             ('read_only', False),
-            ('read_write', True),
             ('volume', self.new_volume.id),
         ]
 
@@ -2100,7 +2095,7 @@ class TestVolumeUnset(volume_fakes.TestVolume):
             self.new_volume.id,
         ]
         verifylist = [
-            ('image_property', {'Alpha': 'a', 'Beta': 'b'}),
+            ('image_properties', {'Alpha': 'a', 'Beta': 'b'}),
             ('volume', self.new_volume.id),
         ]
         parsed_args = self.check_parser(self.cmd_set, arglist, verifylist)
@@ -2116,7 +2111,7 @@ class TestVolumeUnset(volume_fakes.TestVolume):
             self.new_volume.id,
         ]
         verifylist_unset = [
-            ('image_property', ['Alpha']),
+            ('image_properties', ['Alpha']),
             ('volume', self.new_volume.id),
         ]
         parsed_args_unset = self.check_parser(
@@ -2128,7 +2123,7 @@ class TestVolumeUnset(volume_fakes.TestVolume):
         self.cmd_unset.take_action(parsed_args_unset)
 
         self.volumes_mock.delete_image_metadata.assert_called_with(
-            self.new_volume.id, parsed_args_unset.image_property
+            self.new_volume.id, parsed_args_unset.image_properties
         )
 
     def test_volume_unset_image_property_fail(self):
@@ -2143,8 +2138,8 @@ class TestVolumeUnset(volume_fakes.TestVolume):
             self.new_volume.id,
         ]
         verifylist = [
-            ('image_property', ['Alpha']),
-            ('property', ['Beta']),
+            ('image_properties', ['Alpha']),
+            ('properties', ['Beta']),
             ('volume', self.new_volume.id),
         ]
         parsed_args = self.check_parser(self.cmd_unset, arglist, verifylist)
@@ -2157,10 +2152,10 @@ class TestVolumeUnset(volume_fakes.TestVolume):
                 'One or more of the unset operations failed', str(e)
             )
         self.volumes_mock.delete_image_metadata.assert_called_with(
-            self.new_volume.id, parsed_args.image_property
+            self.new_volume.id, parsed_args.image_properties
         )
         self.volumes_mock.delete_metadata.assert_called_with(
-            self.new_volume.id, parsed_args.property
+            self.new_volume.id, parsed_args.properties
         )
 
 

openstackclient/volume/v2/volume.py

@@ -169,6 +169,7 @@ class CreateVolume(command.ShowOne):
             "--property",
             metavar="<key=value>",
             action=parseractions.KeyValueAction,
+            dest="properties",
             help=_(
                 "Set a property to this volume "
                 "(repeat option to set multiple properties)"
@@ -189,22 +190,30 @@ class CreateVolume(command.ShowOne):
         bootable_group.add_argument(
             "--bootable",
             action="store_true",
+            dest="bootable",
+            default=None,
             help=_("Mark volume as bootable"),
         )
         bootable_group.add_argument(
             "--non-bootable",
-            action="store_true",
+            action="store_false",
+            dest="bootable",
+            default=None,
             help=_("Mark volume as non-bootable (default)"),
         )
         readonly_group = parser.add_mutually_exclusive_group()
         readonly_group.add_argument(
             "--read-only",
             action="store_true",
+            dest="read_only",
+            default=None,
             help=_("Set volume to read-only access mode"),
         )
         readonly_group.add_argument(
             "--read-write",
-            action="store_true",
+            action="store_false",
+            dest="read_only",
+            default=None,
             help=_("Set volume to read-write access mode (default)"),
         )
         return parser, source_group
@@ -265,14 +274,14 @@ class CreateVolume(command.ShowOne):
             description=parsed_args.description,
             volume_type=parsed_args.type,
             availability_zone=parsed_args.availability_zone,
-            metadata=parsed_args.property,
+            metadata=parsed_args.properties,
             imageRef=image,
             source_volid=source_volume,
             consistencygroup_id=consistency_group,
             scheduler_hints=parsed_args.hint,
         )
 
-        if parsed_args.bootable or parsed_args.non_bootable:
+        if parsed_args.bootable is not None:
             try:
                 if utils.wait_for_status(
                     volume_client.volumes.get,
@@ -291,7 +300,8 @@ class CreateVolume(command.ShowOne):
                     raise exceptions.CommandError(msg)
             except Exception as e:
                 LOG.error(_("Failed to set volume bootable property: %s"), e)
-        if parsed_args.read_only or parsed_args.read_write:
+
+        if parsed_args.read_only is not None:
             try:
                 if utils.wait_for_status(
                     volume_client.volumes.get,
@@ -622,6 +632,7 @@ class SetVolume(command.Command):
             '--property',
             metavar='<key=value>',
             action=parseractions.KeyValueAction,
+            dest="properties",
             help=_(
                 'Set a property on this volume '
                 '(repeat option to set multiple properties)'
@@ -631,6 +642,7 @@ class SetVolume(command.Command):
             '--image-property',
             metavar='<key=value>',
             action=parseractions.KeyValueAction,
+            dest="image_properties",
             help=_(
                 'Set an image property on this volume '
                 '(repeat option to set multiple image properties)'
@@ -707,22 +719,30 @@ class SetVolume(command.Command):
         bootable_group.add_argument(
             "--bootable",
             action="store_true",
+            dest="bootable",
+            default=None,
             help=_("Mark volume as bootable"),
         )
         bootable_group.add_argument(
             "--non-bootable",
-            action="store_true",
+            action="store_false",
+            dest="bootable",
+            default=None,
             help=_("Mark volume as non-bootable"),
         )
         readonly_group = parser.add_mutually_exclusive_group()
         readonly_group.add_argument(
             "--read-only",
             action="store_true",
+            dest="read_only",
+            default=None,
             help=_("Set volume to read-only access mode"),
         )
         readonly_group.add_argument(
             "--read-write",
-            action="store_true",
+            action="store_false",
+            dest="read_only",
+            default=None,
             help=_("Set volume to read-write access mode"),
         )
         return parser
@@ -771,28 +791,31 @@ class SetVolume(command.Command):
                 LOG.error(_("Failed to clean volume properties: %s"), e)
                 result += 1
 
-        if parsed_args.property:
+        if parsed_args.properties:
             try:
                 volume_client.volumes.set_metadata(
-                    volume.id, parsed_args.property
+                    volume.id, parsed_args.properties
                 )
             except Exception as e:
-                LOG.error(_("Failed to set volume property: %s"), e)
+                LOG.error(_("Failed to set volume properties: %s"), e)
                 result += 1
-        if parsed_args.image_property:
+
+        if parsed_args.image_properties:
             try:
                 volume_client.volumes.set_image_metadata(
-                    volume.id, parsed_args.image_property
+                    volume.id, parsed_args.image_properties
                 )
             except Exception as e:
-                LOG.error(_("Failed to set image property: %s"), e)
+                LOG.error(_("Failed to set image properties: %s"), e)
                 result += 1
+
         if parsed_args.state:
             try:
                 volume_client.volumes.reset_state(volume.id, parsed_args.state)
             except Exception as e:
                 LOG.error(_("Failed to set volume state: %s"), e)
                 result += 1
+
         if parsed_args.attached:
             try:
                 volume_client.volumes.reset_state(
@@ -801,6 +824,7 @@ class SetVolume(command.Command):
             except Exception as e:
                 LOG.error(_("Failed to set volume attach-status: %s"), e)
                 result += 1
+
         if parsed_args.detached:
             try:
                 volume_client.volumes.reset_state(
@@ -809,7 +833,8 @@ class SetVolume(command.Command):
             except Exception as e:
                 LOG.error(_("Failed to set volume attach-status: %s"), e)
                 result += 1
-        if parsed_args.bootable or parsed_args.non_bootable:
+
+        if parsed_args.bootable is not None:
             try:
                 volume_client.volumes.set_bootable(
                     volume.id, parsed_args.bootable
@@ -817,7 +842,8 @@ class SetVolume(command.Command):
             except Exception as e:
                 LOG.error(_("Failed to set volume bootable property: %s"), e)
                 result += 1
-        if parsed_args.read_only or parsed_args.read_write:
+
+        if parsed_args.read_only is not None:
             try:
                 volume_client.volumes.update_readonly_flag(
                     volume.id, parsed_args.read_only
@@ -828,6 +854,7 @@ class SetVolume(command.Command):
                     e,
                 )
                 result += 1
+
         policy = parsed_args.migration_policy or parsed_args.retype_policy
         if parsed_args.type:
             # get the migration policy
@@ -928,6 +955,7 @@ class UnsetVolume(command.Command):
             '--property',
             metavar='<key>',
             action='append',
+            dest='properties',
             help=_(
                 'Remove a property from volume '
                 '(repeat option to remove multiple properties)'
@@ -937,6 +965,7 @@ class UnsetVolume(command.Command):
             '--image-property',
             metavar='<key>',
             action='append',
+            dest='image_properties',
             help=_(
                 'Remove an image property from volume '
                 '(repeat option to remove multiple image properties)'
@@ -949,22 +978,22 @@ class UnsetVolume(command.Command):
         volume = utils.find_resource(volume_client.volumes, parsed_args.volume)
 
         result = 0
-        if parsed_args.property:
+        if parsed_args.properties:
             try:
                 volume_client.volumes.delete_metadata(
-                    volume.id, parsed_args.property
+                    volume.id, parsed_args.properties
                 )
             except Exception as e:
-                LOG.error(_("Failed to unset volume property: %s"), e)
+                LOG.error(_("Failed to unset volume properties: %s"), e)
                 result += 1
 
-        if parsed_args.image_property:
+        if parsed_args.image_properties:
             try:
                 volume_client.volumes.delete_image_metadata(
-                    volume.id, parsed_args.image_property
+                    volume.id, parsed_args.image_properties
                 )
             except Exception as e:
-                LOG.error(_("Failed to unset image property: %s"), e)
+                LOG.error(_("Failed to unset image properties: %s"), e)
                 result += 1
 
         if result > 0:

openstackclient/volume/v3/volume.py

@@ -194,8 +194,7 @@ class CreateVolume(volume_v2.CreateVolume):
                 parsed_args.size
                 or parsed_args.consistency_group
                 or parsed_args.hint
-                or parsed_args.read_only
-                or parsed_args.read_write
+                or parsed_args.read_only is not None
             ):
                 msg = _(
                     "The --size, --consistency-group, --hint, --read-only "
@@ -232,7 +231,7 @@ class CreateVolume(volume_v2.CreateVolume):
                 description=parsed_args.description,
                 volume_type=parsed_args.type,
                 availability_zone=parsed_args.availability_zone,
-                metadata=parsed_args.property,
+                metadata=parsed_args.properties,
                 bootable=parsed_args.bootable,
             )
             return zip(*sorted(volume.items()))
@@ -287,7 +286,7 @@ class CreateVolume(volume_v2.CreateVolume):
             description=parsed_args.description,
             volume_type=parsed_args.type,
             availability_zone=parsed_args.availability_zone,
-            metadata=parsed_args.property,
+            metadata=parsed_args.properties,
             imageRef=image,
             source_volid=source_volume,
             consistencygroup_id=consistency_group,
@@ -295,7 +294,7 @@ class CreateVolume(volume_v2.CreateVolume):
             backup_id=backup,
         )
 
-        if parsed_args.bootable or parsed_args.non_bootable:
+        if parsed_args.bootable is not None:
             try:
                 if utils.wait_for_status(
                     volume_client.volumes.get,
@@ -314,7 +313,8 @@ class CreateVolume(volume_v2.CreateVolume):
                     raise exceptions.CommandError(msg)
             except Exception as e:
                 LOG.error(_("Failed to set volume bootable property: %s"), e)
-        if parsed_args.read_only or parsed_args.read_write:
+
+        if parsed_args.read_only is not None:
             try:
                 if utils.wait_for_status(
                     volume_client.volumes.get,
@@ -638,6 +638,7 @@ class SetVolume(command.Command):
             '--property',
             metavar='<key=value>',
             action=parseractions.KeyValueAction,
+            dest='properties',
             help=_(
                 'Set a property on this volume '
                 '(repeat option to set multiple properties)'
@@ -647,6 +648,7 @@ class SetVolume(command.Command):
             '--image-property',
             metavar='<key=value>',
             action=parseractions.KeyValueAction,
+            dest='image_properties',
             help=_(
                 'Set an image property on this volume '
                 '(repeat option to set multiple image properties)'
@@ -723,22 +725,30 @@ class SetVolume(command.Command):
         bootable_group.add_argument(
             "--bootable",
             action="store_true",
+            dest="bootable",
+            default=None,
             help=_("Mark volume as bootable"),
         )
         bootable_group.add_argument(
             "--non-bootable",
-            action="store_true",
+            action="store_false",
+            dest="bootable",
+            default=None,
             help=_("Mark volume as non-bootable"),
         )
         readonly_group = parser.add_mutually_exclusive_group()
         readonly_group.add_argument(
             "--read-only",
             action="store_true",
+            dest="read_only",
+            default=None,
             help=_("Set volume to read-only access mode"),
         )
         readonly_group.add_argument(
             "--read-write",
-            action="store_true",
+            action="store_false",
+            dest="read_only",
+            default=None,
             help=_("Set volume to read-write access mode"),
         )
         return parser
@@ -796,28 +806,31 @@ class SetVolume(command.Command):
                 LOG.error(_("Failed to clean volume properties: %s"), e)
                 result += 1
 
-        if parsed_args.property:
+        if parsed_args.properties:
             try:
                 volume_client.volumes.set_metadata(
-                    volume.id, parsed_args.property
+                    volume.id, parsed_args.properties
                 )
             except Exception as e:
-                LOG.error(_("Failed to set volume property: %s"), e)
+                LOG.error(_("Failed to set volume properties: %s"), e)
                 result += 1
-        if parsed_args.image_property:
+
+        if parsed_args.image_properties:
             try:
                 volume_client.volumes.set_image_metadata(
-                    volume.id, parsed_args.image_property
+                    volume.id, parsed_args.image_properties
                 )
             except Exception as e:
-                LOG.error(_("Failed to set image property: %s"), e)
+                LOG.error(_("Failed to set image properties: %s"), e)
                 result += 1
+
         if parsed_args.state:
             try:
                 volume_client.volumes.reset_state(volume.id, parsed_args.state)
             except Exception as e:
                 LOG.error(_("Failed to set volume state: %s"), e)
                 result += 1
+
         if parsed_args.attached:
             try:
                 volume_client.volumes.reset_state(
@@ -826,6 +839,7 @@ class SetVolume(command.Command):
             except Exception as e:
                 LOG.error(_("Failed to set volume attach-status: %s"), e)
                 result += 1
+
         if parsed_args.detached:
             try:
                 volume_client.volumes.reset_state(
@@ -834,7 +848,8 @@ class SetVolume(command.Command):
             except Exception as e:
                 LOG.error(_("Failed to set volume attach-status: %s"), e)
                 result += 1
-        if parsed_args.bootable or parsed_args.non_bootable:
+
+        if parsed_args.bootable is not None:
             try:
                 volume_client.volumes.set_bootable(
                     volume.id, parsed_args.bootable
@@ -842,7 +857,8 @@ class SetVolume(command.Command):
             except Exception as e:
                 LOG.error(_("Failed to set volume bootable property: %s"), e)
                 result += 1
-        if parsed_args.read_only or parsed_args.read_write:
+
+        if parsed_args.read_only is not None:
             try:
                 volume_client.volumes.update_readonly_flag(
                     volume.id, parsed_args.read_only
@@ -853,6 +869,7 @@ class SetVolume(command.Command):
                     e,
                 )
                 result += 1
+
         policy = parsed_args.migration_policy or parsed_args.retype_policy
         if parsed_args.type:
             # get the migration policy
@@ -953,6 +970,7 @@ class UnsetVolume(command.Command):
             '--property',
             metavar='<key>',
             action='append',
+            dest='properties',
             help=_(
                 'Remove a property from volume '
                 '(repeat option to remove multiple properties)'
@@ -962,6 +980,7 @@ class UnsetVolume(command.Command):
             '--image-property',
             metavar='<key>',
             action='append',
+            dest='image_properties',
             help=_(
                 'Remove an image property from volume '
                 '(repeat option to remove multiple image properties)'
@@ -974,22 +993,22 @@ class UnsetVolume(command.Command):
         volume = utils.find_resource(volume_client.volumes, parsed_args.volume)
 
         result = 0
-        if parsed_args.property:
+        if parsed_args.properties:
             try:
                 volume_client.volumes.delete_metadata(
-                    volume.id, parsed_args.property
+                    volume.id, parsed_args.properties
                 )
             except Exception as e:
-                LOG.error(_("Failed to unset volume property: %s"), e)
+                LOG.error(_("Failed to unset volume properties: %s"), e)
                 result += 1
 
-        if parsed_args.image_property:
+        if parsed_args.image_properties:
             try:
                 volume_client.volumes.delete_image_metadata(
-                    volume.id, parsed_args.image_property
+                    volume.id, parsed_args.image_properties
                 )
             except Exception as e:
-                LOG.error(_("Failed to unset image property: %s"), e)
+                LOG.error(_("Failed to unset image properties: %s"), e)
                 result += 1
 
         if result > 0: