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

@@ -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')

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()

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)

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:

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

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: