From 729b8f8474e6653cdd9eb34c6c2b558024c1ddcb Mon Sep 17 00:00:00 2001 From: Mitsuhiro Tanino Date: Fri, 4 Mar 2016 15:47:31 -0500 Subject: [PATCH] Don't reset volume status when resetting migration status In case of failed volume migration, status of the volume is still in-use and the migration status is set to error. Current reset-migration-status command resets not only migration status but also volume status. However the volume status should not reset because the volume is still attached. Closes-Bug #1552058 Change-Id: I9a8a5ed6a00bdcffecbf98862fe60aee373f5e9b --- cinderclient/tests/unit/v2/fakes.py | 3 ++- cinderclient/tests/unit/v2/test_shell.py | 6 ++---- cinderclient/tests/unit/v2/test_volumes.py | 7 +++++++ cinderclient/v3/shell.py | 8 ++++++-- cinderclient/v3/volumes.py | 2 +- .../do-not-reset-volume-status-ae8e28132d7bfacd.yaml | 5 +++++ 6 files changed, 23 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/do-not-reset-volume-status-ae8e28132d7bfacd.yaml diff --git a/cinderclient/tests/unit/v2/fakes.py b/cinderclient/tests/unit/v2/fakes.py index 5e4ff3a78..fe69b8414 100644 --- a/cinderclient/tests/unit/v2/fakes.py +++ b/cinderclient/tests/unit/v2/fakes.py @@ -439,7 +439,8 @@ class FakeHTTPClient(base_client.HTTPClient): elif action == 'os-roll_detaching': assert body[action] is None elif action == 'os-reset_status': - assert 'status' in body[action] + assert ('status' or 'attach_status' or 'migration_status' + in body[action]) elif action == 'os-extend': assert list(body[action]) == ['new_size'] elif action == 'os-migrate_volume': diff --git a/cinderclient/tests/unit/v2/test_shell.py b/cinderclient/tests/unit/v2/test_shell.py index 07f27c176..d94c302a1 100644 --- a/cinderclient/tests/unit/v2/test_shell.py +++ b/cinderclient/tests/unit/v2/test_shell.py @@ -595,8 +595,7 @@ class ShellTest(utils.TestCase): def test_reset_state_with_attach_status(self): self.run_command('reset-state --attach-status detached 1234') - expected = {'os-reset_status': {'status': 'available', - 'attach_status': 'detached'}} + expected = {'os-reset_status': {'attach_status': 'detached'}} self.assert_called('POST', '/volumes/1234/action', body=expected) def test_reset_state_with_attach_status_with_flag(self): @@ -608,8 +607,7 @@ class ShellTest(utils.TestCase): def test_reset_state_with_reset_migration_status(self): self.run_command('reset-state --reset-migration-status 1234') - expected = {'os-reset_status': {'status': 'available', - 'migration_status': 'none'}} + expected = {'os-reset_status': {'migration_status': 'none'}} self.assert_called('POST', '/volumes/1234/action', body=expected) def test_reset_state_multiple(self): diff --git a/cinderclient/tests/unit/v2/test_volumes.py b/cinderclient/tests/unit/v2/test_volumes.py index 11fc40994..ab92490ea 100644 --- a/cinderclient/tests/unit/v2/test_volumes.py +++ b/cinderclient/tests/unit/v2/test_volumes.py @@ -190,6 +190,13 @@ class VolumesTest(utils.TestCase): self._assert_request_id(vol) def test_reset_state(self): + v = cs.volumes.get('1234') + self._assert_request_id(v) + vol = cs.volumes.reset_state(v, 'in-use', attach_status='detached') + cs.assert_called('POST', '/volumes/1234/action') + self._assert_request_id(vol) + + def test_reset_state_migration_status(self): v = cs.volumes.get('1234') self._assert_request_id(v) vol = cs.volumes.reset_state(v, 'in-use', attach_status='detached', diff --git a/cinderclient/v3/shell.py b/cinderclient/v3/shell.py index dbb43cfde..96d27481f 100644 --- a/cinderclient/v3/shell.py +++ b/cinderclient/v3/shell.py @@ -491,14 +491,15 @@ def do_force_delete(cs, args): @utils.arg('volume', metavar='', nargs='+', help='Name or ID of volume to modify.') -@utils.arg('--state', metavar='', default='available', +@utils.arg('--state', metavar='', default=None, help=('The state to assign to the volume. Valid values are ' '"available", "error", "creating", "deleting", "in-use", ' '"attaching", "detaching", "error_deleting" and ' '"maintenance". ' 'NOTE: This command simply changes the state of the ' 'Volume in the DataBase with no regard to actual status, ' - 'exercise caution when using. Default=available.')) + 'exercise caution when using. Default=None, that means the ' + 'state is unchanged.')) @utils.arg('--attach-status', metavar='', default=None, help=('The attach status to assign to the volume in the DataBase, ' 'with no regard to the actual status. Valid values are ' @@ -521,6 +522,9 @@ def do_reset_state(cs, args): """ failure_flag = False migration_status = 'none' if args.reset_migration_status else None + if not (args.state or args.attach_status or migration_status): + # Nothing specified, default to resetting state + args.state = 'available' for volume in args.volume: try: diff --git a/cinderclient/v3/volumes.py b/cinderclient/v3/volumes.py index eb9c3bc89..8d1bc2570 100644 --- a/cinderclient/v3/volumes.py +++ b/cinderclient/v3/volumes.py @@ -501,7 +501,7 @@ class VolumeManager(base.ManagerWithFind): :param migration_status: The migration_status of the volume to be set, or None to keep the current status. """ - body = {'status': state} + body = {'status': state} if state else {} if attach_status: body.update({'attach_status': attach_status}) if migration_status: diff --git a/releasenotes/notes/do-not-reset-volume-status-ae8e28132d7bfacd.yaml b/releasenotes/notes/do-not-reset-volume-status-ae8e28132d7bfacd.yaml new file mode 100644 index 000000000..f45bd069f --- /dev/null +++ b/releasenotes/notes/do-not-reset-volume-status-ae8e28132d7bfacd.yaml @@ -0,0 +1,5 @@ +--- +fixes: +- Default value of reset-state ``state`` option is changed + from ``available`` to ``None`` because unexpected ``state`` + reset happens when resetting migration status.