Merge "Require confirmation to reset server state."

Zuul · openstack-gerrit · commit db4739fc5c80 · 2025-04-11T13:23:47.000Z
diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
@@ -4481,15 +4481,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(
@@ -4524,6 +4536,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(
@@ -4574,6 +4600,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:
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -7943,6 +7943,7 @@ def test_server_set_with_state(self):
         arglist = [
             '--state',
             'active',
+            '--auto-approve',
             self.server.id,
         ]
         verifylist = [
@@ -7963,6 +7964,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',
diff --git a/releasenotes/notes/confirm-reset-state-24497c8b24990aa7.yaml 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.
