From f00ffebea61f94f2334d1c1578bc23986bbcf58c Mon Sep 17 00:00:00 2001
From: Sean McGinnis <sean.mcginnis@gmail.com>
Date: Wed, 4 Apr 2018 14:56:27 -0500
Subject: [PATCH] Remove invalid 'unlock-volume' migration arg
There is an optional flag that can be passed in to a
volume migration to tell Cinder to 'lock' a volume so
no other process can abort the migration. This is
reflected correctly with the --lock-volume argument
flag to `openstack volume migrate`, but there is
another --unlock-volume flag that is shown in the help
text for this command that does not do anything and is
not used anywhere.
Since there is no action to "unlock" a volume, this
just causes confusion - including for Cinder developers
that know this API. To avoid confusion, this invalid
flag should just be removed from the command.
Change-Id: I5f111ed58803a1bf5d34e828341d735099247108
---
doc/source/cli/command-objects/volume.rst | 9 +------
.../tests/unit/volume/v2/test_volume.py | 24 -------------------
openstackclient/volume/v2/volume.py | 10 +-------
.../notes/unlock-volume-a6990fc3bf1f5f67.yaml | 5 ++++
4 files changed, 7 insertions(+), 41 deletions(-)
create mode 100644 releasenotes/notes/unlock-volume-a6990fc3bf1f5f67.yaml
diff --git a/doc/source/cli/command-objects/volume.rst b/doc/source/cli/command-objects/volume.rst
index a06a5d4007..fd704b04c2 100644
--- a/doc/source/cli/command-objects/volume.rst
+++ b/doc/source/cli/command-objects/volume.rst
@@ -226,7 +226,7 @@ Migrate volume to a new host
openstack volume migrate
--host <host>
[--force-host-copy]
- [--lock-volume | --unlock-volume]
+ [--lock-volume]
<volume>
.. option:: --host <host>
@@ -245,13 +245,6 @@ Migrate volume to a new host
*Volume version 2 only*
-.. option:: --unlock-volume
-
- If specified, the volume state will not be locked and the a
- migration can be aborted (default) (possibly by another operation)
-
- *Volume version 2 only*
-
.. _volume_migrate-volume:
.. describe:: <volume>
diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py
index 2fa924b8a5..f9a8b7c680 100644
--- a/openstackclient/tests/unit/volume/v2/test_volume.py
+++ b/openstackclient/tests/unit/volume/v2/test_volume.py
@@ -1320,7 +1320,6 @@ class TestVolumeMigrate(TestVolume):
verifylist = [
("force_host_copy", False),
("lock_volume", False),
- ("unlock_volume", False),
("host", "host@backend-name#pool"),
("volume", self._volume.id),
]
@@ -1342,7 +1341,6 @@ class TestVolumeMigrate(TestVolume):
verifylist = [
("force_host_copy", True),
("lock_volume", True),
- ("unlock_volume", False),
("host", "host@backend-name#pool"),
("volume", self._volume.id),
]
@@ -1354,27 +1352,6 @@ class TestVolumeMigrate(TestVolume):
self._volume.id, "host@backend-name#pool", True, True)
self.assertIsNone(result)
- def test_volume_migrate_with_unlock_volume(self):
- arglist = [
- "--unlock-volume",
- "--host", "host@backend-name#pool",
- self._volume.id,
- ]
- verifylist = [
- ("force_host_copy", False),
- ("lock_volume", False),
- ("unlock_volume", True),
- ("host", "host@backend-name#pool"),
- ("volume", self._volume.id),
- ]
- parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
- result = self.cmd.take_action(parsed_args)
- self.volumes_mock.get.assert_called_once_with(self._volume.id)
- self.volumes_mock.migrate_volume.assert_called_once_with(
- self._volume.id, "host@backend-name#pool", False, False)
- self.assertIsNone(result)
-
def test_volume_migrate_without_host(self):
arglist = [
self._volume.id,
@@ -1382,7 +1359,6 @@ class TestVolumeMigrate(TestVolume):
verifylist = [
("force_host_copy", False),
("lock_volume", False),
- ("unlock_volume", False),
("volume", self._volume.id),
]
diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py
index 61f846b02f..55b7fac300 100644
--- a/openstackclient/volume/v2/volume.py
+++ b/openstackclient/volume/v2/volume.py
@@ -472,21 +472,13 @@ class MigrateVolume(command.Command):
help=_("Enable generic host-based force-migration, "
"which bypasses driver optimizations")
)
- lock_group = parser.add_mutually_exclusive_group()
- lock_group.add_argument(
+ parser.add_argument(
'--lock-volume',
action="store_true",
help=_("If specified, the volume state will be locked "
"and will not allow a migration to be aborted "
"(possibly by another operation)")
)
- lock_group.add_argument(
- '--unlock-volume',
- action="store_true",
- help=_("If specified, the volume state will not be "
- "locked and the a migration can be aborted "
- "(default) (possibly by another operation)")
- )
return parser
def take_action(self, parsed_args):
diff --git a/releasenotes/notes/unlock-volume-a6990fc3bf1f5f67.yaml b/releasenotes/notes/unlock-volume-a6990fc3bf1f5f67.yaml
new file mode 100644
index 0000000000..60124bf890
--- /dev/null
+++ b/releasenotes/notes/unlock-volume-a6990fc3bf1f5f67.yaml
@@ -0,0 +1,5 @@
+---
+upgrade:
+ - |
+ The ``volume migrate --unlock`` argument did not actually do anything and
+ has now been removed.