From 25cd1178b31a3d5582ab4ad77518fb63a856fd88 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Thu, 10 Apr 2025 13:01:56 +0100 Subject: [PATCH] Require confirmation to reset server state. This change updates the server set state command to require confirmation before it is applied. The same pattern as project clean is used and a new --auto-approve flag is added to allow skipping the prompt. Operators often use reset state in cases that are incorrect this change updates the warning to be more explicit about when and when not to use it. Change-Id: Iab14739cf6043ad45ad49edff0580e81d75af2fd --- openstackclient/compute/v2/server.py | 45 +++++++++++++++-- .../tests/unit/compute/v2/test_server.py | 49 +++++++++++++++++++ .../confirm-reset-state-24497c8b24990aa7.yaml | 6 +++ 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/confirm-reset-state-24497c8b24990aa7.yaml diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index a3ca4d07a3..3345ecab5c 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -4462,15 +4462,27 @@ class SetServer(command.Command): '(repeat option to set multiple properties)' ), ) + parser.add_argument( + '--auto-approve', + action='store_true', + help=_( + "Allow server state override without asking for confirmation" + ), + ) parser.add_argument( '--state', metavar='', choices=['active', 'error'], help=_( - 'New server state ' - '**WARNING** This can result in instances that are no longer ' - 'usable and should be used with caution ' - '(admin only)' + 'New server state.' + '**WARNING** Resetting the state is intended to work around ' + 'servers stuck in an intermediate state, such as deleting. ' + 'If the server is in an error state then this is almost ' + 'never the correct command to run and you should prefer hard ' + 'reboot where possible. In particular, if the server is in ' + 'an error state due to a move operation, setting the state ' + 'can result in instances that are no longer usable. Proceed ' + 'with caution. (admin only)' ), ) parser.add_argument( @@ -4505,6 +4517,20 @@ class SetServer(command.Command): ) return parser + @staticmethod + def ask_user_yesno(msg): + """Ask user Y/N question + + :param str msg: question text + :return bool: User choice + """ + while True: + answer = getpass.getpass('{} [{}]: '.format(msg, 'y/n')) + if answer in ('y', 'Y', 'yes'): + return True + elif answer in ('n', 'N', 'no'): + return False + def take_action(self, parsed_args): compute_client = self.app.client_manager.compute server = compute_client.find_server( @@ -4555,6 +4581,17 @@ class SetServer(command.Command): ) if parsed_args.state: + if not parsed_args.auto_approve: + if not self.ask_user_yesno( + _( + "Resetting the server state can make it much harder " + "to recover a server from an error state. If the " + "server is in error status due to a failed move " + "operation then this is likely not the correct " + "approach to fix the problem. Do you wish to continue?" + ) + ): + return compute_client.reset_server_state(server, state=parsed_args.state) if parsed_args.root_password: diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py index 39af6af7a2..25ee1d65b4 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -7935,6 +7935,7 @@ class TestServerSet(TestServer): arglist = [ '--state', 'active', + '--auto-approve', self.server.id, ] verifylist = [ @@ -7955,6 +7956,54 @@ class TestServerSet(TestServer): self.compute_client.add_tag_to_server.assert_not_called() self.assertIsNone(result) + def test_server_set_with_state_prompt_y(self): + arglist = [ + '--state', + 'active', + self.server.id, + ] + verifylist = [ + ('state', 'active'), + ('server', self.server.id), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + with mock.patch('getpass.getpass', return_value='y'): + result = self.cmd.take_action(parsed_args) + + self.compute_client.reset_server_state.assert_called_once_with( + self.server, state='active' + ) + self.compute_client.update_server.assert_not_called() + self.compute_client.set_server_metadata.assert_not_called() + self.compute_client.change_server_password.assert_not_called() + self.compute_client.clear_server_password.assert_not_called() + self.compute_client.add_tag_to_server.assert_not_called() + self.assertIsNone(result) + + def test_server_set_with_state_prompt_n(self): + arglist = [ + '--state', + 'active', + self.server.id, + ] + verifylist = [ + ('state', 'active'), + ('server', self.server.id), + ] + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + with mock.patch('getpass.getpass', return_value='n'): + result = self.cmd.take_action(parsed_args) + + self.compute_client.reset_server_state.assert_not_called() + self.compute_client.update_server.assert_not_called() + self.compute_client.set_server_metadata.assert_not_called() + self.compute_client.change_server_password.assert_not_called() + self.compute_client.clear_server_password.assert_not_called() + self.compute_client.add_tag_to_server.assert_not_called() + self.assertIsNone(result) + def test_server_set_with_invalid_state(self): arglist = [ '--state', diff --git a/releasenotes/notes/confirm-reset-state-24497c8b24990aa7.yaml b/releasenotes/notes/confirm-reset-state-24497c8b24990aa7.yaml new file mode 100644 index 0000000000..b381e5a821 --- /dev/null +++ b/releasenotes/notes/confirm-reset-state-24497c8b24990aa7.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + The ``openstack server set`` command has been extended with a new + parameter ``--auto-approve`` and the existing ``--state`` parameter + has been modified to require confirmation before resetting the state.