Skip to content

Commit f00ffeb

Browse files
committed
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
1 parent 06263bd commit f00ffeb

4 files changed

Lines changed: 7 additions & 41 deletions

File tree

doc/source/cli/command-objects/volume.rst

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ Migrate volume to a new host
226226
openstack volume migrate
227227
--host <host>
228228
[--force-host-copy]
229-
[--lock-volume | --unlock-volume]
229+
[--lock-volume]
230230
<volume>
231231
232232
.. option:: --host <host>
@@ -245,13 +245,6 @@ Migrate volume to a new host
245245
246246
*Volume version 2 only*
247247
248-
.. option:: --unlock-volume
249-
250-
If specified, the volume state will not be locked and the a
251-
migration can be aborted (default) (possibly by another operation)
252-
253-
*Volume version 2 only*
254-
255248
.. _volume_migrate-volume:
256249
.. describe:: <volume>
257250

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

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,7 +1320,6 @@ def test_volume_migrate(self):
13201320
verifylist = [
13211321
("force_host_copy", False),
13221322
("lock_volume", False),
1323-
("unlock_volume", False),
13241323
("host", "host@backend-name#pool"),
13251324
("volume", self._volume.id),
13261325
]
@@ -1342,7 +1341,6 @@ def test_volume_migrate_with_option(self):
13421341
verifylist = [
13431342
("force_host_copy", True),
13441343
("lock_volume", True),
1345-
("unlock_volume", False),
13461344
("host", "host@backend-name#pool"),
13471345
("volume", self._volume.id),
13481346
]
@@ -1354,35 +1352,13 @@ def test_volume_migrate_with_option(self):
13541352
self._volume.id, "host@backend-name#pool", True, True)
13551353
self.assertIsNone(result)
13561354

1357-
def test_volume_migrate_with_unlock_volume(self):
1358-
arglist = [
1359-
"--unlock-volume",
1360-
"--host", "host@backend-name#pool",
1361-
self._volume.id,
1362-
]
1363-
verifylist = [
1364-
("force_host_copy", False),
1365-
("lock_volume", False),
1366-
("unlock_volume", True),
1367-
("host", "host@backend-name#pool"),
1368-
("volume", self._volume.id),
1369-
]
1370-
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
1371-
1372-
result = self.cmd.take_action(parsed_args)
1373-
self.volumes_mock.get.assert_called_once_with(self._volume.id)
1374-
self.volumes_mock.migrate_volume.assert_called_once_with(
1375-
self._volume.id, "host@backend-name#pool", False, False)
1376-
self.assertIsNone(result)
1377-
13781355
def test_volume_migrate_without_host(self):
13791356
arglist = [
13801357
self._volume.id,
13811358
]
13821359
verifylist = [
13831360
("force_host_copy", False),
13841361
("lock_volume", False),
1385-
("unlock_volume", False),
13861362
("volume", self._volume.id),
13871363
]
13881364

openstackclient/volume/v2/volume.py

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -472,21 +472,13 @@ def get_parser(self, prog_name):
472472
help=_("Enable generic host-based force-migration, "
473473
"which bypasses driver optimizations")
474474
)
475-
lock_group = parser.add_mutually_exclusive_group()
476-
lock_group.add_argument(
475+
parser.add_argument(
477476
'--lock-volume',
478477
action="store_true",
479478
help=_("If specified, the volume state will be locked "
480479
"and will not allow a migration to be aborted "
481480
"(possibly by another operation)")
482481
)
483-
lock_group.add_argument(
484-
'--unlock-volume',
485-
action="store_true",
486-
help=_("If specified, the volume state will not be "
487-
"locked and the a migration can be aborted "
488-
"(default) (possibly by another operation)")
489-
)
490482
return parser
491483

492484
def take_action(self, parsed_args):
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
upgrade:
3+
- |
4+
The ``volume migrate --unlock`` argument did not actually do anything and
5+
has now been removed.

0 commit comments

Comments
 (0)