diff --git a/releasenotes/notes/upgrade_update_prompt-f6ace53f02b62fa0.yaml b/releasenotes/notes/upgrade_update_prompt-f6ace53f02b62fa0.yaml new file mode 100644 index 000000000..3fa17f5d8 --- /dev/null +++ b/releasenotes/notes/upgrade_update_prompt-f6ace53f02b62fa0.yaml @@ -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. diff --git a/tripleoclient/constants.py b/tripleoclient/constants.py index 7a83af1a7..51ac2fcf9 100644 --- a/tripleoclient/constants.py +++ b/tripleoclient/constants.py @@ -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') diff --git a/tripleoclient/exceptions.py b/tripleoclient/exceptions.py index b6840a117..9141c3e11 100644 --- a/tripleoclient/exceptions.py +++ b/tripleoclient/exceptions.py @@ -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""" diff --git a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py index c435fe574..1ef4231e6 100644 --- a/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py +++ b/tripleoclient/tests/v1/overcloud_update/test_overcloud_update.py @@ -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') diff --git a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py b/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py index 260e5ae61..4418ff470 100644 --- a/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py +++ b/tripleoclient/tests/v1/overcloud_upgrade/test_overcloud_upgrade.py @@ -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', ] diff --git a/tripleoclient/v1/overcloud_external_update.py b/tripleoclient/v1/overcloud_external_update.py index 73cd38712..0445f6ba3 100644 --- a/tripleoclient/v1/overcloud_external_update.py +++ b/tripleoclient/v1/overcloud_external_update.py @@ -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, diff --git a/tripleoclient/v1/overcloud_external_upgrade.py b/tripleoclient/v1/overcloud_external_upgrade.py index c22241965..c17867138 100644 --- a/tripleoclient/v1/overcloud_external_upgrade.py +++ b/tripleoclient/v1/overcloud_external_upgrade.py @@ -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, diff --git a/tripleoclient/v1/overcloud_update.py b/tripleoclient/v1/overcloud_update.py index 668471f0e..02605a1ba 100644 --- a/tripleoclient/v1/overcloud_update.py +++ b/tripleoclient/v1/overcloud_update.py @@ -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) diff --git a/tripleoclient/v1/overcloud_upgrade.py b/tripleoclient/v1/overcloud_upgrade.py index a7914814f..dd713bfcb 100644 --- a/tripleoclient/v1/overcloud_upgrade.py +++ b/tripleoclient/v1/overcloud_upgrade.py @@ -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.