Merge "Add "--read-only" and "--read-write" options in "volume set""

doc/source/command-objects/volume.rst

@@ -255,6 +255,7 @@ Set volume properties
         [--image-property <key=value> [...] ]
         [--state <state>]
         [--bootable | --non-bootable]
+        [--read-only | --read-write]
         <volume>
 
 .. option:: --name <name>
@@ -281,6 +282,14 @@ Set volume properties
 
     Mark volume as non-bootable
 
+.. option:: --read-only
+
+    Set volume to read-only access mode
+
+.. option:: --read-write
+
+    Set volume to read-write access mode
+
 .. option:: --image-property <key=value>
 
     Set an image property on this volume

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

@@ -906,8 +906,7 @@ class TestVolumeSet(TestVolume):
         )
         self.assertIsNone(result)
 
-    @mock.patch.object(volume.LOG, 'error')
-    def test_volume_set_size_smaller(self, mock_log_error):
+    def test_volume_set_size_smaller(self):
         self._volume.status = 'available'
         arglist = [
             '--size', '1',
@@ -922,15 +921,11 @@ class TestVolumeSet(TestVolume):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        result = self.cmd.take_action(parsed_args)
+        self.assertRaises(exceptions.CommandError,
+                          self.cmd.take_action,
+                          parsed_args)
 
-        mock_log_error.assert_called_with("New size must be greater "
-                                          "than %s GB",
-                                          self._volume.size)
-        self.assertIsNone(result)
-
-    @mock.patch.object(volume.LOG, 'error')
-    def test_volume_set_size_not_available(self, mock_log_error):
+    def test_volume_set_size_not_available(self):
         self._volume.status = 'error'
         arglist = [
             '--size', '130',
@@ -945,12 +940,9 @@ class TestVolumeSet(TestVolume):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        result = self.cmd.take_action(parsed_args)
-
-        mock_log_error.assert_called_with("Volume is in %s state, it must be "
-                                          "available before size can be "
-                                          "extended", 'error')
-        self.assertIsNone(result)
+        self.assertRaises(exceptions.CommandError,
+                          self.cmd.take_action,
+                          parsed_args)
 
     def test_volume_set_property(self):
         arglist = [
@@ -958,6 +950,8 @@ class TestVolumeSet(TestVolume):
             self._volume.display_name,
         ]
         verifylist = [
+            ('read_only', False),
+            ('read_write', False),
             ('name', None),
             ('description', None),
             ('size', None),
@@ -978,6 +972,7 @@ class TestVolumeSet(TestVolume):
             self._volume.id,
             metadata
         )
+        self.volumes_mock.update_readonly_flag.assert_not_called()
         self.assertIsNone(result)
 
     def test_volume_set_bootable(self):
@@ -1005,6 +1000,44 @@ class TestVolumeSet(TestVolume):
             self.volumes_mock.set_bootable.assert_called_with(
                 self._volume.id, verifylist[index][0][1])
 
+    def test_volume_set_readonly(self):
+        arglist = [
+            '--read-only',
+            self._volume.id
+        ]
+        verifylist = [
+            ('read_only', True),
+            ('read_write', False),
+            ('volume', self._volume.id)
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+        self.volumes_mock.update_readonly_flag.assert_called_once_with(
+            self._volume.id,
+            True)
+        self.assertIsNone(result)
+
+    def test_volume_set_read_write(self):
+        arglist = [
+            '--read-write',
+            self._volume.id
+        ]
+        verifylist = [
+            ('read_only', False),
+            ('read_write', True),
+            ('volume', self._volume.id)
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+        self.volumes_mock.update_readonly_flag.assert_called_once_with(
+            self._volume.id,
+            False)
+        self.assertIsNone(result)
+
 
 class TestVolumeShow(TestVolume):
 

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

@@ -1123,6 +1123,8 @@ class TestVolumeSet(TestVolume):
             self.new_volume.id
         ]
         verifylist = [
+            ('read_only', False),
+            ('read_write', False),
             ('state', 'error'),
             ('volume', self.new_volume.id)
         ]
@@ -1132,6 +1134,7 @@ class TestVolumeSet(TestVolume):
         result = self.cmd.take_action(parsed_args)
         self.volumes_mock.reset_state.assert_called_with(
             self.new_volume.id, 'error')
+        self.volumes_mock.update_readonly_flag.assert_not_called()
         self.assertIsNone(result)
 
     def test_volume_set_state_failed(self):
@@ -1180,6 +1183,44 @@ class TestVolumeSet(TestVolume):
             self.volumes_mock.set_bootable.assert_called_with(
                 self.new_volume.id, verifylist[index][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)
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+        self.volumes_mock.update_readonly_flag.assert_called_once_with(
+            self.new_volume.id,
+            True)
+        self.assertIsNone(result)
+
+    def test_volume_set_read_write(self):
+        arglist = [
+            '--read-write',
+            self.new_volume.id
+        ]
+        verifylist = [
+            ('read_only', False),
+            ('read_write', True),
+            ('volume', self.new_volume.id)
+        ]
+
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+        self.volumes_mock.update_readonly_flag.assert_called_once_with(
+            self.new_volume.id,
+            False)
+        self.assertIsNone(result)
+
 
 class TestVolumeShow(TestVolume):
 

openstackclient/volume/v1/volume.py

@@ -419,38 +419,78 @@ class SetVolume(command.Command):
             action="store_true",
             help=_("Mark volume as non-bootable")
         )
+        readonly_group = parser.add_mutually_exclusive_group()
+        readonly_group.add_argument(
+            "--read-only",
+            action="store_true",
+            help=_("Set volume to read-only access mode")
+        )
+        readonly_group.add_argument(
+            "--read-write",
+            action="store_true",
+            help=_("Set volume to read-write access mode")
+        )
         return parser
 
     def take_action(self, parsed_args):
         volume_client = self.app.client_manager.volume
         volume = utils.find_resource(volume_client.volumes, parsed_args.volume)
 
+        result = 0
         if parsed_args.size:
-            if volume.status != 'available':
-                LOG.error(_("Volume is in %s state, it must be available "
-                            "before size can be extended"), volume.status)
-                return
-            if parsed_args.size <= volume.size:
-                LOG.error(_("New size must be greater than %s GB"),
-                          volume.size)
-                return
-            volume_client.volumes.extend(volume.id, parsed_args.size)
-
+            try:
+                if volume.status != 'available':
+                    msg = (_("Volume is in %s state, it must be available "
+                           "before size can be extended") % volume.status)
+                    raise exceptions.CommandError(msg)
+                if parsed_args.size <= volume.size:
+                    msg = (_("New size must be greater than %s GB")
+                           % volume.size)
+                    raise exceptions.CommandError(msg)
+                volume_client.volumes.extend(volume.id, parsed_args.size)
+            except Exception as e:
+                LOG.error(_("Failed to set volume size: %s"), e)
+                result += 1
         if parsed_args.property:
-            volume_client.volumes.set_metadata(volume.id, parsed_args.property)
+            try:
+                volume_client.volumes.set_metadata(
+                    volume.id,
+                    parsed_args.property)
+            except Exception as e:
+                LOG.error(_("Failed to set volume property: %s"), e)
+                result += 1
         if parsed_args.bootable or parsed_args.non_bootable:
             try:
                 volume_client.volumes.set_bootable(
                     volume.id, parsed_args.bootable)
             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:
+            try:
+                volume_client.volumes.update_readonly_flag(
+                    volume.id,
+                    parsed_args.read_only)
+            except Exception as e:
+                LOG.error(_("Failed to set volume read-only access "
+                            "mode flag: %s"), e)
+                result += 1
         kwargs = {}
         if parsed_args.name:
             kwargs['display_name'] = parsed_args.name
         if parsed_args.description:
             kwargs['display_description'] = parsed_args.description
         if kwargs:
-            volume_client.volumes.update(volume.id, **kwargs)
+            try:
+                volume_client.volumes.update(volume.id, **kwargs)
+            except Exception as e:
+                LOG.error(_("Failed to update volume display name "
+                          "or display description: %s"), e)
+                result += 1
+
+        if result > 0:
+            raise exceptions.CommandError(_("One or more of the "
+                                          "set operations failed"))
 
 
 class ShowVolume(command.ShowOne):

openstackclient/volume/v2/volume.py

@@ -520,6 +520,17 @@ class SetVolume(command.Command):
             action="store_true",
             help=_("Mark volume as non-bootable")
         )
+        readonly_group = parser.add_mutually_exclusive_group()
+        readonly_group.add_argument(
+            "--read-only",
+            action="store_true",
+            help=_("Set volume to read-only access mode")
+        )
+        readonly_group.add_argument(
+            "--read-write",
+            action="store_true",
+            help=_("Set volume to read-write access mode")
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -570,6 +581,15 @@ 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:
+            try:
+                volume_client.volumes.update_readonly_flag(
+                    volume.id,
+                    parsed_args.read_only)
+            except Exception as e:
+                LOG.error(_("Failed to set volume read-only access "
+                            "mode flag: %s"), e)
+                result += 1
 
         kwargs = {}
         if parsed_args.name:

releasenotes/notes/bp-cinder-command-support-3db775ba331e113d.yaml

@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    Add ``--read-only`` and ``--read-write`` options to ``volume set`` command.
+    [Blueprint `cinder-command-support <https://blueprints.launchpad.net/python-openstackclient/+spec/cinder-command-support>`_]