Support --no-property in "volume set" command

Add "--no-property" option to "volume set" command in v1 and v2
and update the test cases.

Change-Id: Id5660f23b3b2d9aa72f4c16b19ce83f3f7ed2fa4

6 files changed, 82 insertions, 10 deletions

diff --git a/openstackclient/tests/functional/volume/v1/test_volume.py b/openstackclient/tests/functional/volume/v1/test_volume.py
index 992dfd66..3f04e071 100644
--- a/openstackclient/tests/functional/volume/v1/test_volume.py
+++ b/openstackclient/tests/functional/volume/v1/test_volume.py
@@ -141,8 +141,9 @@ class VolumeTests(common.BaseVolumeTests):
             '--name ' + new_name +
             ' --size 2 ' +
             '--description bbbb ' +
-            '--property Alpha=c ' +
+            '--no-property ' +
             '--property Beta=b ' +
+            '--property Gamma=c ' +
             '--bootable ' +
             name,
         )
@@ -165,7 +166,7 @@ class VolumeTests(common.BaseVolumeTests):
             cmd_output["display_description"],
         )
         self.assertEqual(
-            "Alpha='c', Beta='b'",
+            "Beta='b', Gamma='c'",
             cmd_output["properties"],
         )
         self.assertEqual(
@@ -176,7 +177,7 @@ class VolumeTests(common.BaseVolumeTests):
         # Test volume unset
         raw_output = self.openstack(
             'volume unset ' +
-            '--property Alpha ' +
+            '--property Beta ' +
             new_name,
         )
         self.assertOutput('', raw_output)
@@ -186,7 +187,7 @@ class VolumeTests(common.BaseVolumeTests):
             new_name
         ))
         self.assertEqual(
-            "Beta='b'",
+            "Gamma='c'",
             cmd_output["properties"],
         )
 
diff --git a/openstackclient/tests/functional/volume/v2/test_volume.py b/openstackclient/tests/functional/volume/v2/test_volume.py
index 203ca819..ce98236f 100644
--- a/openstackclient/tests/functional/volume/v2/test_volume.py
+++ b/openstackclient/tests/functional/volume/v2/test_volume.py
@@ -104,7 +104,7 @@ class VolumeTests(common.BaseVolumeTests):
         # TODO(qiangjiahui): Add project option to filter tests when we can
         # specify volume with project
 
-    def test_volume_set(self):
+    def test_volume_set_and_unset(self):
         """Tests create volume, set, unset, show, delete"""
         name = uuid.uuid4().hex
         new_name = name + "_"
@@ -144,8 +144,11 @@ class VolumeTests(common.BaseVolumeTests):
             '--name ' + new_name +
             ' --size 2 ' +
             '--description bbbb ' +
-            '--property Alpha=c ' +
+            '--no-property ' +
             '--property Beta=b ' +
+            '--property Gamma=c ' +
+            '--image-property a=b ' +
+            '--image-property c=d ' +
             '--bootable ' +
             name,
         )
@@ -168,10 +171,14 @@ class VolumeTests(common.BaseVolumeTests):
             cmd_output["description"],
         )
         self.assertEqual(
-            "Alpha='c', Beta='b'",
+            "Beta='b', Gamma='c'",
             cmd_output["properties"],
         )
         self.assertEqual(
+            {'a': 'b', 'c': 'd'},
+            cmd_output["volume_image_metadata"],
+        )
+        self.assertEqual(
             'true',
             cmd_output["bootable"],
         )
@@ -179,7 +186,8 @@ class VolumeTests(common.BaseVolumeTests):
         # Test volume unset
         raw_output = self.openstack(
             'volume unset ' +
-            '--property Alpha ' +
+            '--property Beta ' +
+            '--image-property a ' +
             new_name,
         )
         self.assertOutput('', raw_output)
@@ -189,9 +197,13 @@ class VolumeTests(common.BaseVolumeTests):
             new_name
         ))
         self.assertEqual(
-            "Beta='b'",
+            "Gamma='c'",
             cmd_output["properties"],
         )
+        self.assertEqual(
+            {'c': 'd'},
+            cmd_output["volume_image_metadata"],
+        )
 
     def test_volume_snapshot(self):
         """Tests volume create from snapshot"""
diff --git a/openstackclient/tests/unit/volume/v1/test_volume.py b/openstackclient/tests/unit/volume/v1/test_volume.py
index 6c6d9a1d..d46a7ba9 100644
--- a/openstackclient/tests/unit/volume/v1/test_volume.py
+++ b/openstackclient/tests/unit/volume/v1/test_volume.py
@@ -1071,6 +1071,7 @@ class TestVolumeSet(TestVolume):
 
     def test_volume_set_property(self):
         arglist = [
+            '--no-property',
             '--property', 'myprop=myvalue',
             self._volume.display_name,
         ]
@@ -1080,6 +1081,7 @@ class TestVolumeSet(TestVolume):
             ('name', None),
             ('description', None),
             ('size', None),
+            ('no_property', True),
             ('property', {'myprop': 'myvalue'}),
             ('volume', self._volume.display_name),
             ('bootable', False),
@@ -1097,6 +1099,10 @@ class TestVolumeSet(TestVolume):
             self._volume.id,
             metadata
         )
+        self.volumes_mock.delete_metadata.assert_called_with(
+            self._volume.id,
+            self._volume.metadata.keys()
+        )
         self.volumes_mock.update_readonly_flag.assert_not_called()
         self.assertIsNone(result)
 
diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py
index 4fef9dd9..fbe719f3 100644
--- a/openstackclient/tests/unit/volume/v2/test_volume.py
+++ b/openstackclient/tests/unit/volume/v2/test_volume.py
@@ -1368,6 +1368,24 @@ class TestVolumeSet(TestVolume):
         # Get the command object to test
         self.cmd = volume.SetVolume(self.app, None)
 
+    def test_volume_set_property(self):
+        arglist = [
+            '--property', 'a=b',
+            '--property', 'c=d',
+            self.new_volume.id,
+        ]
+        verifylist = [
+            ('property', {'a': 'b', 'c': 'd'}),
+            ('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)
+
     def test_volume_set_image_property(self):
         arglist = [
             '--image-property', 'Alpha=a',
diff --git a/openstackclient/volume/v1/volume.py b/openstackclient/volume/v1/volume.py
index 739484df..8e1097f5 100644
--- a/openstackclient/volume/v1/volume.py
+++ b/openstackclient/volume/v1/volume.py
@@ -440,6 +440,15 @@ class SetVolume(command.Command):
             help=_('Extend volume size in GB'),
         )
         parser.add_argument(
+            "--no-property",
+            dest="no_property",
+            action="store_true",
+            help=_("Remove all properties from <volume> "
+                   "(specify both --no-property and --property to "
+                   "remove the current properties before setting "
+                   "new properties.)"),
+        )
+        parser.add_argument(
             '--property',
             metavar='<key=value>',
             action=parseractions.KeyValueAction,
@@ -489,6 +498,15 @@ class SetVolume(command.Command):
             except Exception as e:
                 LOG.error(_("Failed to set volume size: %s"), e)
                 result += 1
+
+        if parsed_args.no_property:
+            try:
+                volume_client.volumes.delete_metadata(
+                    volume.id, volume.metadata.keys())
+            except Exception as e:
+                LOG.error(_("Failed to clean volume properties: %s"), e)
+                result += 1
+
         if parsed_args.property:
             try:
                 volume_client.volumes.set_metadata(
diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py
index 78db261b..c361d700 100644
--- a/openstackclient/volume/v2/volume.py
+++ b/openstackclient/volume/v2/volume.py
@@ -524,6 +524,15 @@ class SetVolume(command.Command):
             help=_('New volume description'),
         )
         parser.add_argument(
+            "--no-property",
+            dest="no_property",
+            action="store_true",
+            help=_("Remove all properties from <volume> "
+                   "(specify both --no-property and --property to "
+                   "remove the current properties before setting "
+                   "new properties.)"),
+        )
+        parser.add_argument(
             '--property',
             metavar='<key=value>',
             action=parseractions.KeyValueAction,
@@ -561,7 +570,7 @@ class SetVolume(command.Command):
             choices=['never', 'on-demand'],
             help=_('Migration policy while re-typing volume '
                    '("never" or "on-demand", default is "never" ) '
-                   '(available only when "--type" option is specified)'),
+                   '(available only when --type option is specified)'),
         )
         bootable_group = parser.add_mutually_exclusive_group()
         bootable_group.add_argument(
@@ -607,6 +616,14 @@ class SetVolume(command.Command):
                 LOG.error(_("Failed to set volume size: %s"), e)
                 result += 1
 
+        if parsed_args.no_property:
+            try:
+                volume_client.volumes.delete_metadata(
+                    volume.id, volume.metadata.keys())
+            except Exception as e:
+                LOG.error(_("Failed to clean volume properties: %s"), e)
+                result += 1
+
         if parsed_args.property:
             try:
                 volume_client.volumes.set_metadata(