Merge "Require confirmation to reset server state."
This commit is contained in:
commit
db4739fc5c
@ -4481,15 +4481,27 @@ class SetServer(command.Command):
|
|||||||
'(repeat option to set multiple properties)'
|
'(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(
|
parser.add_argument(
|
||||||
'--state',
|
'--state',
|
||||||
metavar='<state>',
|
metavar='<state>',
|
||||||
choices=['active', 'error'],
|
choices=['active', 'error'],
|
||||||
help=_(
|
help=_(
|
||||||
'New server state '
|
'New server state.'
|
||||||
'**WARNING** This can result in instances that are no longer '
|
'**WARNING** Resetting the state is intended to work around '
|
||||||
'usable and should be used with caution '
|
'servers stuck in an intermediate state, such as deleting. '
|
||||||
'(admin only)'
|
'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(
|
parser.add_argument(
|
||||||
@ -4524,6 +4536,20 @@ class SetServer(command.Command):
|
|||||||
)
|
)
|
||||||
return parser
|
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):
|
def take_action(self, parsed_args):
|
||||||
compute_client = self.app.client_manager.compute
|
compute_client = self.app.client_manager.compute
|
||||||
server = compute_client.find_server(
|
server = compute_client.find_server(
|
||||||
@ -4574,6 +4600,17 @@ class SetServer(command.Command):
|
|||||||
)
|
)
|
||||||
|
|
||||||
if parsed_args.state:
|
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)
|
compute_client.reset_server_state(server, state=parsed_args.state)
|
||||||
|
|
||||||
if parsed_args.root_password:
|
if parsed_args.root_password:
|
||||||
|
@ -7943,6 +7943,7 @@ class TestServerSet(TestServer):
|
|||||||
arglist = [
|
arglist = [
|
||||||
'--state',
|
'--state',
|
||||||
'active',
|
'active',
|
||||||
|
'--auto-approve',
|
||||||
self.server.id,
|
self.server.id,
|
||||||
]
|
]
|
||||||
verifylist = [
|
verifylist = [
|
||||||
@ -7963,6 +7964,54 @@ class TestServerSet(TestServer):
|
|||||||
self.compute_client.add_tag_to_server.assert_not_called()
|
self.compute_client.add_tag_to_server.assert_not_called()
|
||||||
self.assertIsNone(result)
|
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):
|
def test_server_set_with_invalid_state(self):
|
||||||
arglist = [
|
arglist = [
|
||||||
'--state',
|
'--state',
|
||||||
|
@ -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.
|
Loading…
x
Reference in New Issue
Block a user