diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py index 6d7fc4aa63..34d569beca 100644 --- a/openstackclient/compute/v2/server.py +++ b/openstackclient/compute/v2/server.py @@ -4481,15 +4481,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( @@ -4524,6 +4536,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( @@ -4574,6 +4600,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 0ae9725375..37e0091638 100644 --- a/openstackclient/tests/unit/compute/v2/test_server.py +++ b/openstackclient/tests/unit/compute/v2/test_server.py @@ -7943,6 +7943,7 @@ class TestServerSet(TestServer): arglist = [ '--state', 'active', + '--auto-approve', self.server.id, ] verifylist = [ @@ -7963,6 +7964,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.