From c0b0a5aa6770028777e19e0e7ac833f8cb5575ae Mon Sep 17 00:00:00 2001 From: Alex Schultz Date: Mon, 11 May 2020 10:07:51 -0600 Subject: [PATCH] Fix upgrade prompt We need to prompt the user if we should upgrade earlier in the code so we can handle additional things like package upgrades in the undercloud upgrade after the prompt. This change pulls the confirmation logic out of tripleo_deploy and reuses the prompt_user_for_confirmation code in the tripleo upgrade and undercloud upgrade actions. Change-Id: I8dbcae39e6f6d966c1337bad5fb5ba673f7be874 Closes-Bug: #1877825 --- tripleoclient/constants.py | 10 +++++ .../tests/v1/tripleo/test_tripleo_upgrade.py | 44 ++++--------------- .../v1/undercloud/test_install_upgrade.py | 20 ++++++--- tripleoclient/v1/tripleo_deploy.py | 21 --------- tripleoclient/v1/tripleo_upgrade.py | 7 +++ tripleoclient/v1/undercloud.py | 6 +++ 6 files changed, 47 insertions(+), 61 deletions(-) diff --git a/tripleoclient/constants.py b/tripleoclient/constants.py index a01235ad8..871670b8c 100644 --- a/tripleoclient/constants.py +++ b/tripleoclient/constants.py @@ -15,6 +15,8 @@ import os +from osc_lib.i18n import _ + # NOTE(cloudnull): Condition imports and exceptions to support PY2, When we # drop py2 this should be simplified. try: @@ -188,3 +190,11 @@ UNDERCLOUD_EXTRA_PACKAGES = [ "openstack-tripleo-validations", "tripleo-ansible" ] + +# UPGRADE_PROMPT +UPGRADE_PROMPT = _('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 ' + 'prefer to skip this warning in the future') diff --git a/tripleoclient/tests/v1/tripleo/test_tripleo_upgrade.py b/tripleoclient/tests/v1/tripleo/test_tripleo_upgrade.py index 388fa6bf4..01d119e13 100644 --- a/tripleoclient/tests/v1/tripleo/test_tripleo_upgrade.py +++ b/tripleoclient/tests/v1/tripleo/test_tripleo_upgrade.py @@ -16,7 +16,6 @@ import mock from osc_lib.tests import utils -import six # Load the plugin init module for the plugin list and show commands from tripleoclient import exceptions @@ -33,9 +32,11 @@ class TestUpgrade(utils.TestCommand): self.cmd.ansible_dir = '/tmp' self.ansible_playbook_cmd = "ansible-playbook" + @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', + return_value=True) @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.take_action', autospec=True) - def test_take_action(self, mock_deploy): + def test_take_action(self, mock_deploy, mock_confirm): verifylist = [ ('local_ip', '127.0.0.1'), ('templates', '/tmp/thtroot'), @@ -61,12 +62,11 @@ class TestUpgrade(utils.TestCommand): parsed_args.upgrade = True mock_deploy.assert_called_with(self.cmd, parsed_args) + @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', + return_value=True) @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy.take_action', autospec=True) - @mock.patch('sys.stdin', spec=six.StringIO) - def test_take_action_prompt(self, mock_stdin, mock_deploy): - mock_stdin.isatty.return_value = True - mock_stdin.readline.return_value = 'y' + def test_take_action_prompt(self, mock_deploy, mock_confirm): parsed_args = self.check_parser(self.cmd, ['--local-ip', '127.0.0.1', '--templates', '/tmp/thtroot', @@ -83,12 +83,11 @@ class TestUpgrade(utils.TestCommand): parsed_args.upgrade = True mock_deploy.assert_called_with(self.cmd, parsed_args) + @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', + return_value=False) @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy', autospec=True) - @mock.patch('sys.stdin', spec=six.StringIO) - def test_take_action_prompt_no(self, mock_stdin, mock_deploy): - mock_stdin.isatty.return_value = True - mock_stdin.readline.return_value = 'n' + def test_take_action_prompt_no(self, mock_deploy, mock_confirm): parsed_args = self.check_parser(self.cmd, ['--local-ip', '127.0.0.1', '--templates', '/tmp/thtroot', @@ -104,29 +103,4 @@ class TestUpgrade(utils.TestCommand): parsed_args.upgrade = True self.assertRaises(exceptions.UndercloudUpgradeNotConfirmed, self.cmd.take_action, parsed_args) - mock_stdin.readline.assert_called_with() - mock_deploy.assert_not_called() - - @mock.patch('tripleoclient.v1.tripleo_deploy.Deploy', - autospec=True) - @mock.patch('sys.stdin', spec=six.StringIO) - def test_take_action_prompt_invalid_option(self, mock_stdin, mock_deploy): - mock_stdin.isatty.return_value = True - mock_stdin.readline.return_value = 'Dontwant' - parsed_args = self.check_parser(self.cmd, - ['--local-ip', '127.0.0.1', - '--templates', '/tmp/thtroot', - '--stack', 'undercloud', - '--output-dir', '/my', - '-e', '/tmp/thtroot/puppet/foo.yaml', - '-e', '/tmp/thtroot//docker/bar.yaml', - '-e', '/tmp/thtroot42/notouch.yaml', - '-e', '~/custom.yaml', - '-e', 'something.yaml', - '-e', '../../../outside.yaml'], []) - parsed_args.standlone = True - parsed_args.upgrade = True - self.assertRaises(exceptions.UndercloudUpgradeNotConfirmed, - self.cmd.take_action, parsed_args) - mock_stdin.readline.assert_called_with() mock_deploy.assert_not_called() diff --git a/tripleoclient/tests/v1/undercloud/test_install_upgrade.py b/tripleoclient/tests/v1/undercloud/test_install_upgrade.py index d5ac44fe1..7e6f0020a 100644 --- a/tripleoclient/tests/v1/undercloud/test_install_upgrade.py +++ b/tripleoclient/tests/v1/undercloud/test_install_upgrade.py @@ -575,6 +575,8 @@ class TestUndercloudUpgrade(TestPluginV1): app_args.verbose_level = 1 self.cmd = undercloud.UpgradeUndercloud(self.app, app_args) + @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', + return_value=True) @mock.patch.object(sys, 'executable', 'python2') # TODO(cjeanner) drop once we have proper oslo.privsep @mock.patch('os.geteuid', return_value=1001) @@ -588,7 +590,7 @@ class TestUndercloudUpgrade(TestPluginV1): mock_subprocess, mock_wr, mock_os, mock_copy, mock_user, - mock_getuid): + mock_getuid, mock_confirm): arglist = ['--no-validations'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -636,6 +638,8 @@ class TestUndercloudUpgrade(TestPluginV1): '--force-stack-update', '--no-validations', '--inflight-validations', '--dry-run', '--yes', '--debug']) + @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', + return_value=True) # TODO(cjeanner) drop once we have proper oslo.privsep @mock.patch('os.geteuid', return_value=1001) @mock.patch('getpass.getuser', return_value='stack') @@ -648,7 +652,7 @@ class TestUndercloudUpgrade(TestPluginV1): mock_subprocess, mock_wr, mock_os, mock_copy, mock_user, - mock_getuid): + mock_getuid, mock_confirm): arglist = ['--no-validations', '--skip-package-updates'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -704,6 +708,8 @@ class TestUndercloudUpgrade(TestPluginV1): '/usr/share/openstack-tripleo-heat-templates/' 'undercloud-stack-vstate-dropin.yaml']) + @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', + return_value=True) # TODO(cjeanner) drop once we have proper oslo.privsep @mock.patch('os.geteuid', return_value=1001) @mock.patch('getpass.getuser', return_value='stack') @@ -716,7 +722,7 @@ class TestUndercloudUpgrade(TestPluginV1): mock_subprocess, mock_wr, mock_os, mock_copy, mock_user, - mock_getuid): + mock_getuid, mock_confirm): arglist = ['--no-validations', '--skip-package-updates'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -772,6 +778,8 @@ class TestUndercloudUpgrade(TestPluginV1): '/usr/share/openstack-tripleo-heat-templates/' 'undercloud-stack-vstate-dropin.yaml']) + @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', + return_value=True) # TODO(cjeanner) drop once we have proper oslo.privsep @mock.patch('os.geteuid', return_value=1001) @mock.patch('getpass.getuser', return_value='stack') @@ -784,7 +792,7 @@ class TestUndercloudUpgrade(TestPluginV1): mock_subprocess, mock_wr, mock_os, mock_copy, mock_user, - mock_getuid): + mock_getuid, mock_confirm): arglist = ['--no-validations', '--skip-package-updates'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -909,6 +917,8 @@ class TestUndercloudUpgrade(TestPluginV1): '/usr/share/openstack-tripleo-heat-templates/' 'undercloud-stack-vstate-dropin.yaml']) + @mock.patch('tripleoclient.utils.prompt_user_for_confirmation', + return_value=True) # TODO(cjeanner) drop once we have proper oslo.privsep @mock.patch('os.geteuid', return_value=1001) @mock.patch('getpass.getuser', return_value='stack') @@ -921,7 +931,7 @@ class TestUndercloudUpgrade(TestPluginV1): mock_subprocess, mock_wr, mock_os, mock_copy, mock_user, - mock_getuid): + mock_getuid, mock_confirm): arglist = ['--no-validations', '--skip-package-updates'] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) diff --git a/tripleoclient/v1/tripleo_deploy.py b/tripleoclient/v1/tripleo_deploy.py index f620a0b29..75468496c 100644 --- a/tripleoclient/v1/tripleo_deploy.py +++ b/tripleoclient/v1/tripleo_deploy.py @@ -1406,27 +1406,6 @@ class Deploy(command.Command): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) - unconf_msg = _('User did not confirm upgrade, so exiting. ' - 'Consider using the --yes parameter if you ' - 'prefer to skip this warning in the future') - try: - if parsed_args.upgrade and ( - not parsed_args.yes and sys.stdin.isatty()): - prompt_response = six.moves.input( - ('It is strongly recommended to perform a backup ' - 'before the upgrade. Are you sure you want to ' - 'upgrade [y/N]?') - ).lower() - if not prompt_response.startswith('y'): - raise exceptions.UndercloudUpgradeNotConfirmed(unconf_msg) - except KeyboardInterrupt: - # ctrl-c - raise exceptions.UndercloudUpgradeNotConfirmed("(ctrl-c) %s" % - unconf_msg) - except EOFError: - # ctrl-d - raise exceptions.UndercloudUpgradeNotConfirmed("(ctrl-d) %s" % - unconf_msg) try: if parsed_args.standalone: diff --git a/tripleoclient/v1/tripleo_upgrade.py b/tripleoclient/v1/tripleo_upgrade.py index dc92cc234..c053e5405 100644 --- a/tripleoclient/v1/tripleo_upgrade.py +++ b/tripleoclient/v1/tripleo_upgrade.py @@ -15,6 +15,9 @@ from oslo_config import cfg from oslo_log import log as logging +from tripleoclient import constants +from tripleoclient.exceptions import UndercloudUpgradeNotConfirmed +from tripleoclient import utils from tripleoclient.v1.tripleo_deploy import Deploy CONF = cfg.CONF @@ -29,6 +32,10 @@ class Upgrade(Deploy): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) + if (not parsed_args.yes + and not utils.prompt_user_for_confirmation( + constants.UPGRADE_PROMPT, self.log)): + raise UndercloudUpgradeNotConfirmed(constants.UPGRADE_NO) parsed_args.standalone = True parsed_args.upgrade = True diff --git a/tripleoclient/v1/undercloud.py b/tripleoclient/v1/undercloud.py index fa60bf085..2f6078ba0 100644 --- a/tripleoclient/v1/undercloud.py +++ b/tripleoclient/v1/undercloud.py @@ -247,6 +247,12 @@ class UpgradeUndercloud(InstallUndercloud): self.osloconfig['undercloud_log_file']) self.log.debug("take action(%s)" % parsed_args) + if (not parsed_args.yes + and not utils.prompt_user_for_confirmation( + constants.UPGRADE_PROMPT, self.log)): + raise exceptions.UndercloudUpgradeNotConfirmed( + constants.UPGRADE_NO) + utils.ensure_run_as_normal_user() if not parsed_args.skip_package_updates: