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

Author: SeanMooney

openstackclient/compute/v2/server.py

@@ -4462,15 +4462,27 @@ def get_parser(self, prog_name):
                 '(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='<state>',
             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 @@ def get_parser(self, prog_name):
         )
         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 @@ def take_action(self, parsed_args):
             )
 
         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:

openstackclient/tests/unit/compute/v2/test_server.py

@@ -7935,6 +7935,7 @@ def test_server_set_with_state(self):
         arglist = [
             '--state',
             'active',
+            '--auto-approve',
             self.server.id,
         ]
         verifylist = [
@@ -7955,6 +7956,54 @@ def test_server_set_with_state(self):
         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',

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.