Add "yes" prompt for update/upgrades commands

Update/Upgrade commands have now a prompt by default that ask for
confirmation before proceeding. It'll prevent an user to run the
command that may cause the problems to infrastructure.
This prompt can be skipped with --yes/-y argument.

Note: putting "UPDATE" and "UPGRADE" in uppercase to make sure this is
visible and clear. We have seen many users running the wrong command and
ending up doing an upgrade instead of an update.

Note2: this prompt will be ported to the upgrade and FFWD workflows to
prevent unexpected execution to prevent potential harm to
infrastructures.

Depends-On: https://review.opendev.org/741480
Change-Id: I838e6748879c668dd004ca2243b7b00b857c2a7b
This commit is contained in:
Emilien Macchi 2020-05-22 09:30:00 -04:00 committed by Bogdan Dobrelya
parent 9c53cb3ef5
commit 7a044ca251
9 changed files with 150 additions and 14 deletions

View File

@ -0,0 +1,7 @@
---
features:
- |
The upgrade/update commands have a prompt by default now that ask for
confirmation before proceeding. It'll prevent an operator to run the
command and cause the problems to infrastructure.
This prompt can be skipped with --yes/-y argument.

View File

@ -196,10 +196,17 @@ UNDERCLOUD_EXTRA_PACKAGES = [
"tripleo-ansible"
]
# UPGRADE_PROMPT
UPGRADE_PROMPT = _('It is strongly recommended to perform a backup '
UPGRADE_PROMPT = _('You are about to run a UPGRADE command. '
'It is strongly recommended to perform a backup '
'before the upgrade. Are you sure you want to '
'upgrade [y/N]?')
UPGRADE_NO = _('User did not confirm upgrade, so exiting. '
'Consider using the --yes parameter if you '
'Consider using the --yes/-y parameter if you '
'prefer to skip this warning in the future')
UPDATE_PROMPT = _('You are about to run a UPDATE command. '
'It is strongly recommended to perform a backup '
'before the update. Are you sure you want to '
'update [y/N]?')
UPDATE_NO = _('User did not confirm update, so exiting. '
'Consider using the --yes/-y parameter if you '
'prefer to skip this warning in the future')

View File

@ -121,6 +121,14 @@ class UndercloudUpgradeNotConfirmed(Base):
"""Undercloud upgrade security question not confirmed."""
class OvercloudUpdateNotConfirmed(Base):
"""Overcloud Update security question not confirmed."""
class OvercloudUpgradeNotConfirmed(Base):
"""Overcloud Update security question not confirmed."""
class CellExportError(Base):
"""Cell export failed"""

View File

@ -36,6 +36,8 @@ class TestOvercloudUpdatePrepare(fakes.TestOvercloudUpdatePrepare):
self.mock_uuid4 = uuid4_patcher.start()
self.addCleanup(self.mock_uuid4.stop)
@mock.patch('tripleoclient.utils.prompt_user_for_confirmation',
return_value=True)
@mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
'_get_undercloud_host_entry', autospec=True,
return_value='192.168.0.1 uc.ctlplane.localhost uc.ctlplane')
@ -53,7 +55,8 @@ class TestOvercloudUpdatePrepare(fakes.TestOvercloudUpdatePrepare):
'_deploy_tripleo_heat_templates_tmpdir', autospec=True)
def test_update_out(self, mock_deploy, mock_open, mock_copy, mock_yaml,
mock_abspath, mock_update, mock_logger,
mock_get_stack, mock_get_undercloud_host_entry):
mock_get_stack, mock_get_undercloud_host_entry,
mock_confirm):
mock_stack = mock.Mock(parameters={'DeployIdentifier': ''})
mock_stack.stack_name = 'overcloud'
mock_get_stack.return_value = mock_stack
@ -77,6 +80,8 @@ class TestOvercloudUpdatePrepare(fakes.TestOvercloudUpdatePrepare):
container='overcloud',
)
@mock.patch('tripleoclient.utils.prompt_user_for_confirmation',
return_value=True)
@mock.patch('tripleoclient.utils.get_stack',
autospec=True)
@mock.patch('tripleoclient.workflows.package_update.update',
@ -89,7 +94,7 @@ class TestOvercloudUpdatePrepare(fakes.TestOvercloudUpdatePrepare):
'_deploy_tripleo_heat_templates', autospec=True)
def test_update_failed(self, mock_deploy, mock_copy, mock_yaml,
mock_abspath, mock_open, mock_update,
mock_get_stack):
mock_get_stack, mock_confirm):
mock_stack = mock.Mock(parameters={'DeployIdentifier': ''})
mock_stack.stack_name = 'overcloud'
mock_get_stack.return_value = mock_stack
@ -124,13 +129,16 @@ class TestOvercloudUpdateRun(fakes.TestOvercloudUpdateRun):
self.mock_uuid4 = uuid4_patcher.start()
self.addCleanup(self.mock_uuid4.stop)
@mock.patch('tripleoclient.utils.prompt_user_for_confirmation',
return_value=True)
@mock.patch('tripleoclient.utils.run_ansible_playbook',
autospec=True)
@mock.patch('os.path.expanduser')
@mock.patch('oslo_concurrency.processutils.execute')
@mock.patch('six.moves.builtins.open')
def test_update_with_no_limit(
self, mock_open, mock_execute, mock_expanduser, update_ansible):
self, mock_open, mock_execute, mock_expanduser, update_ansible,
mock_confirm):
mock_expanduser.return_value = '/home/fake/'
argslist = []
verifylist = [
@ -151,9 +159,11 @@ class TestOvercloudUpdateConverge(fakes.TestOvercloudUpdateConverge):
app_args.verbose_level = 1
self.cmd = overcloud_update.UpdateConverge(self.app, app_args)
@mock.patch('tripleoclient.utils.prompt_user_for_confirmation',
return_value=True)
@mock.patch(
'tripleoclient.v1.overcloud_deploy.DeployOvercloud.take_action')
def test_update_converge(self, deploy_action):
def test_update_converge(self, deploy_action, mock_confirm):
argslist = ['--templates', '--stack', 'cloud']
verifylist = [
('stack', 'cloud')

View File

@ -37,7 +37,8 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare):
uuid4_patcher = mock.patch('uuid.uuid4', return_value="UUID4")
self.mock_uuid4 = uuid4_patcher.start()
self.addCleanup(self.mock_uuid4.stop)
@mock.patch('tripleoclient.utils.prompt_user_for_confirmation',
return_value=True)
@mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
'take_action')
@mock.patch('tripleoclient.workflows.deployment.'
@ -56,7 +57,8 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare):
mock_get_stack,
add_env,
mock_enable_ssh_admin,
mock_overcloud_deploy):
mock_overcloud_deploy,
mock_confirm):
mock_stack = mock.Mock(parameters={'DeployIdentifier': ''})
mock_stack.stack_name = 'overcloud'
@ -90,6 +92,8 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare):
mock.ANY
)
@mock.patch('tripleoclient.utils.prompt_user_for_confirmation',
return_value=True)
@mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
'take_action')
@mock.patch('tripleoclient.utils.get_stack',
@ -98,7 +102,8 @@ class TestOvercloudUpgradePrepare(fakes.TestOvercloudUpgradePrepare):
@mock.patch('six.moves.builtins.open')
@mock.patch('yaml.safe_load')
def test_upgrade_failed(self, mock_yaml, mock_open,
add_env, mock_get_stack, mock_overcloud_deploy):
add_env, mock_get_stack, mock_overcloud_deploy,
mock_confirm):
mock_overcloud_deploy.side_effect = exceptions.DeploymentError()
mock_yaml.return_value = {'fake_container': 'fake_value'}
mock_stack = mock.Mock(parameters={'DeployIdentifier': ''})
@ -142,6 +147,8 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun):
self.mock_uuid4 = uuid4_patcher.start()
self.addCleanup(self.mock_uuid4.stop)
@mock.patch('tripleoclient.utils.prompt_user_for_confirmation',
return_value=True)
@mock.patch(
'ansible_runner.runner_config.RunnerConfig',
autospec=True,
@ -158,7 +165,7 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun):
@mock.patch('six.moves.builtins.open')
def test_upgrade_limit_with_playbook_and_user(
self, mock_open, mock_execute, mock_expanduser, upgrade_ansible,
mock_run, mock_run_prepare):
mock_run, mock_run_prepare, mock_confirm):
mock_expanduser.return_value = '/home/fake/'
argslist = ['--limit', 'Compute, Controller',
'--playbook', 'fake-playbook1.yaml',
@ -171,6 +178,8 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun):
self.check_parser(self.cmd, argslist, verifylist)
@mock.patch('tripleoclient.utils.prompt_user_for_confirmation',
return_value=True)
@mock.patch(
'ansible_runner.runner_config.RunnerConfig',
autospec=True,
@ -187,7 +196,7 @@ class TestOvercloudUpgradeRun(fakes.TestOvercloudUpgradeRun):
@mock.patch('six.moves.builtins.open')
def test_upgrade_nodes_with_playbook_no_skip_tags(
self, mock_open, mock_execute, mock_expanduser, upgrade_ansible,
mock_run, mock_run_prepare):
mock_run, mock_run_prepare, mock_confirm):
mock_expanduser.return_value = '/home/fake/'
argslist = ['--limit', 'compute-0,compute-1',
'--playbook', 'fake-playbook.yaml', ]

View File

@ -19,6 +19,8 @@ from oslo_log import log as logging
from osc_lib.i18n import _
from osc_lib import utils
from tripleoclient.exceptions import OvercloudUpdateNotConfirmed
from tripleoclient import command
from tripleoclient import constants
from tripleoclient import utils as oooutils
@ -91,6 +93,12 @@ class ExternalUpdateRun(command.Command):
default=True,
help=_('This option no longer has any effect.')
)
parser.add_argument('-y', '--yes', default=False,
action='store_true',
help=_("Use -y or --yes to skip the confirmation "
"required before any upgrade "
"operation. Use this with caution! "),
)
parser.add_argument(
'--limit',
action='store',
@ -105,6 +113,12 @@ class ExternalUpdateRun(command.Command):
def take_action(self, parsed_args):
self.log.debug("take_action(%s)" % parsed_args)
if (not parsed_args.yes
and not oooutils.prompt_user_for_confirmation(
constants.UPDATE_PROMPT, self.log)):
raise OvercloudUpdateNotConfirmed(constants.UPDATE_NO)
_, ansible_dir = self.get_ansible_key_and_dir(
no_workflow=True,
stack=parsed_args.stack,

View File

@ -18,6 +18,8 @@ from oslo_log import log as logging
from osc_lib.i18n import _
from osc_lib import utils
from tripleoclient.exceptions import OvercloudUpgradeNotConfirmed
from tripleoclient import command
from tripleoclient import constants
from tripleoclient import utils as oooutils
@ -90,6 +92,13 @@ class ExternalUpgradeRun(command.Command):
default=True,
help=_('This option no longer has any effect.')
)
parser.add_argument('-y', '--yes', default=False,
action='store_true',
help=_("Use -y or --yes to skip the confirmation "
"required before any upgrade "
"operation. Use this with caution! "),
)
parser.add_argument(
'--limit',
action='store',
@ -104,6 +113,12 @@ class ExternalUpgradeRun(command.Command):
def take_action(self, parsed_args):
self.log.debug("take_action(%s)" % parsed_args)
if (not parsed_args.yes
and not oooutils.prompt_user_for_confirmation(
constants.UPGRADE_PROMPT, self.log)):
raise OvercloudUpgradeNotConfirmed(constants.UPGRADE_NO)
_, ansible_dir = self.get_ansible_key_and_dir(
no_workflow=True,
stack=parsed_args.stack,

View File

@ -18,6 +18,8 @@ from oslo_log import log as logging
from osc_lib.i18n import _
from osc_lib import utils
from tripleoclient.exceptions import OvercloudUpdateNotConfirmed
from tripleoclient import command
from tripleoclient import constants
from tripleoclient import utils as oooutils
@ -43,10 +45,22 @@ class UpdatePrepare(DeployOvercloud):
def get_parser(self, prog_name):
parser = super(UpdatePrepare, self).get_parser(prog_name)
parser.add_argument('-y', '--yes', default=False,
action='store_true',
help=_("Use -y or --yes to skip the confirmation "
"required before any update operation. "
"Use this with caution! "),
)
return parser
def take_action(self, parsed_args):
self.log.debug("take_action(%s)" % parsed_args)
if (not parsed_args.yes
and not oooutils.prompt_user_for_confirmation(
constants.UPDATE_PROMPT, self.log)):
raise OvercloudUpdateNotConfirmed(constants.UPDATE_NO)
clients = self.app.client_manager
stack = oooutils.get_stack(clients.orchestration,
@ -147,10 +161,22 @@ class UpdateRun(command.Command):
help=_('A list of tags to skip when running the the'
' config-download ansible-playbook command.')
)
parser.add_argument(
'-y', '--yes',
default=False,
action='store_true',
help=_("Use -y or --yes to skip the confirmation required before "
"any update operation. Use this with caution! "),
)
return parser
def take_action(self, parsed_args):
self.log.debug("take_action(%s)" % parsed_args)
if (not parsed_args.yes
and not oooutils.prompt_user_for_confirmation(
constants.UPDATE_PROMPT, self.log)):
raise OvercloudUpdateNotConfirmed(constants.UPDATE_NO)
# NOTE(cloudnull): The string option "all" was a special default
# that is no longer relevant. To retain compatibility
# this condition has been put in place.
@ -198,9 +224,24 @@ class UpdateConverge(DeployOvercloud):
log = logging.getLogger(__name__ + ".UpdateConverge")
def get_parser(self, prog_name):
parser = super(UpdateConverge, self).get_parser(prog_name)
parser.add_argument('-y', '--yes', default=False,
action='store_true',
help=_("Use -y or --yes to skip the confirmation "
"required before any update operation. "
"Use this with caution! "),
)
return parser
def take_action(self, parsed_args):
self.log.debug("take_action(%s)" % parsed_args)
if (not parsed_args.yes
and not oooutils.prompt_user_for_confirmation(
constants.UPDATE_PROMPT, self.log)):
raise OvercloudUpdateNotConfirmed(constants.UPDATE_NO)
# Add the update-converge.yaml environment to unset noops
templates_dir = (parsed_args.templates or
constants.TRIPLEO_HEAT_TEMPLATES)

View File

@ -18,6 +18,8 @@ from oslo_log import log as logging
from osc_lib.i18n import _
from osc_lib import utils
from tripleoclient.exceptions import OvercloudUpgradeNotConfirmed
from tripleoclient import command
from tripleoclient import constants
from tripleoclient import exceptions
@ -49,11 +51,22 @@ class UpgradePrepare(DeployOvercloud):
def get_parser(self, prog_name):
parser = super(UpgradePrepare, self).get_parser(prog_name)
parser.add_argument('-y', '--yes', default=False,
action='store_true',
help=_("Use -y or --yes to skip the confirmation "
"required before any upgrade "
"operation. Use this with caution! "),
)
return parser
def take_action(self, parsed_args):
self.log.debug("take_action(%s)" % parsed_args)
if (not parsed_args.yes
and not oooutils.prompt_user_for_confirmation(
constants.UPGRADE_PROMPT, self.log)):
raise OvercloudUpgradeNotConfirmed(constants.UPGRADE_NO)
# Throw deprecation warning if service is enabled and
# ask user if upgrade should still be continued.
if parsed_args.environment_files:
@ -178,13 +191,19 @@ class UpgradeRun(command.Command):
help=_('Name or ID of heat stack '
'(default=Env: OVERCLOUD_STACK_NAME)'),
default=utils.env('OVERCLOUD_STACK_NAME',
default='overcloud'))
default='overcloud')
)
parser.add_argument('--no-workflow', dest='no_workflow',
action='store_true',
default=True,
help=_('This option no longer has any effect.')
)
parser.add_argument('-y', '--yes', default=False,
action='store_true',
help=_("Use -y or --yes to skip the confirmation "
"required before any upgrade "
"operation. Use this with caution! ")
)
return parser
def _validate_skip_tags(self, skip_tags):
@ -199,6 +218,12 @@ class UpgradeRun(command.Command):
def take_action(self, parsed_args):
self.log.debug("take_action(%s)" % parsed_args)
if (not parsed_args.yes
and not oooutils.prompt_user_for_confirmation(
constants.UPGRADE_PROMPT, self.log)):
raise OvercloudUpgradeNotConfirmed(constants.UPGRADE_NO)
# NOTE(cloudnull): The string option "all" was a special default
# that is no longer relevant. To retain compatibility
# this condition has been put in place.